Bug 526975 - Deduce return type correctly in the presence of multiple return statements

The previous implementation deviated from the C++ standard by checking
that the types of the return expressions are the same, rather than the
return types after deduction against the placeholder type.

There was also a bug in the return type deduction code for lambdas,
where for a lambda without an explicit placeholder in the trailing-
return-type, the deduction process wouldn't be performed.

Change-Id: I2f0b9f1c7778aef60e4cd7ada9386b99be52669a

core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ast2/cxx14/ReturnTypeDeductionTests.java

@@ -18,25 +18,39 @@ import org.eclipse.cdt.core.dom.ast.cpp.ICPPTemplateInstance;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPVariable;
 
 public class ReturnTypeDeductionTests extends AST2CPPTestBase {
-	private void assertReturnType(String functionName, IType returnType) throws Exception {
+	private IType getReturnType(String functionName) throws Exception {
 		BindingAssertionHelper bh = getAssertionHelper();
 		ICPPFunction f = bh.assertNonProblem(functionName);
-		assertSameType(f.getType().getReturnType(), returnType);
+		return f.getType().getReturnType();
+	}
+	
+	private void assertReturnType(String functionName, IType returnType) throws Exception {
+		assertSameType(getReturnType(functionName), returnType);
 	}
 	
 	private void assertReturnTypeProblem(String functionName) throws Exception {
-		BindingAssertionHelper bh = getAssertionHelper();
+		assertInstance(getReturnType(functionName), IProblemType.class);
-		ICPPFunction f = bh.assertNonProblem(functionName);
-		assertInstance(f.getType().getReturnType(), IProblemType.class);
 	}
 	
-	private void assertLambdaReturnType(String lambdaName, IType returnType) throws Exception {
+	private void assertReturnTypeValid(String functionName) throws Exception {
+		assertFalse(getReturnType(functionName) instanceof IProblemType);
+	}
+	
+	private IType getLambdaReturnType(String lambdaName) throws Exception {
 		BindingAssertionHelper bh = getAssertionHelper();
 		ICPPVariable lambda = bh.assertNonProblem(lambdaName);
 		IType lambdaType = lambda.getType();
 		assertInstance(lambdaType, CPPClosureType.class);
 		ICPPFunction f = ((CPPClosureType) lambdaType).getFunctionCallOperator();
-		assertSameType(f.getType().getReturnType(), returnType);
+		return f.getType().getReturnType();
+	}
+	
+	private void assertLambdaReturnType(String lambdaName, IType returnType) throws Exception {
+		assertSameType(getLambdaReturnType(lambdaName), returnType);
+	}
+	
+	private void assertLambdaReturnTypeValid(String lambdaName) throws Exception {
+		assertFalse(getLambdaReturnType(lambdaName) instanceof IProblemType);
 	}
 	
 	//	auto f() { return 42; }
@@ -54,6 +68,17 @@ public class ReturnTypeDeductionTests extends AST2CPPTestBase {
 		assertReturnType("f", CommonCPPTypes.int_);
 	}
 	
+	//	struct S {};
+	//	auto f(const S& s, bool c) {
+	//		if (c)
+	//			return S();
+	//		else
+	//			return s;
+	//	}
+	public void testMultipleReturnsDifferingByConst() throws Exception {
+		assertReturnTypeValid("f");
+	}
+	
 	//	auto f(int x) {
 	//		if (x < 10)
 	//			return 42;
@@ -165,6 +190,17 @@ public class ReturnTypeDeductionTests extends AST2CPPTestBase {
 		assertLambdaReturnType("f4", CommonCPPTypes.rvalueReferenceToInt);
 	}
 	
+	//	struct S {};
+	//	auto f = [](const S& s, bool c) {
+	//		if (c)
+	//			return S();
+	//		else
+	//			return s;
+	//	};
+	public void testLambdaWithMultipleReturnsDifferingByConst() throws Exception {
+		assertLambdaReturnTypeValid("f");
+	}
+	
 	//	struct A {
 	//		virtual auto f() { return 42; }
 	//		virtual decltype(auto) g() { return 42; }

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPClosureType.java

@@ -196,7 +196,7 @@ public class CPPClosureType extends PlatformObject implements ICPPClassType, ICP
 		IASTCompoundStatement body = fLambdaExpression.getBody();
 		if (body != null) {
 			return CPPVisitor.deduceReturnType(body, declSpecForDeduction, declaratorForDeduction, 
-					placeholder, body);
+					placeholder);
 		}
 		return ProblemType.CANNOT_DEDUCE_AUTO_TYPE;
 	}

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/CPPVisitor.java

@@ -2304,7 +2304,9 @@ public class CPPVisitor extends ASTQueries {
 				return ProblemType.CANNOT_DEDUCE_AUTO_TYPE;
 			}
 		}
-		type = decorateType(type, declSpec, declarator);
+		if (declSpec != null && declarator != null) {
+			type = decorateType(type, declSpec, declarator);
+		}
 		ICPPFunctionTemplate template = new AutoTypeResolver(type);
 		CPPTemplateParameterMap paramMap = new CPPTemplateParameterMap(1);
 		TemplateArgumentDeduction.deduceFromFunctionArgs(template, Collections.singletonList(initType),
@@ -2327,14 +2329,17 @@ public class CPPVisitor extends ASTQueries {
 			type = (IType) CPPTemplates.instantiate(initializer_list_template,
 					new ICPPTemplateArgument[] { new CPPTemplateTypeArgument(type) });
 		}
-		return decorateType(type, declSpec, declarator);
+		if (declSpec != null && declarator != null) {
+			type = decorateType(type, declSpec, declarator);
+		}
+		return type;
 	}
 	
 	private static class ReturnTypeDeducer extends ReturnStatementVisitor {
 		private static final ICPPEvaluation voidEval = new EvalFixed(
 				CPPSemantics.VOID_TYPE, ValueCategory.PRVALUE, IntegralValue.UNKNOWN);
 		
-		private ICPPEvaluation fReturnEval = null;
+		private ICPPEvaluation[] fReturnEvals = ICPPEvaluation.EMPTY_ARRAY;
 		private boolean fEncounteredReturnStatement = false;
 		
 		protected ReturnTypeDeducer(IASTFunctionDefinition func) {
@@ -2344,12 +2349,11 @@ public class CPPVisitor extends ASTQueries {
 		@Override
 		protected void onReturnStatement(IASTReturnStatement stmt) {
 			fEncounteredReturnStatement = true;
-			ICPPEvaluation returnEval = null;
 			IASTInitializerClause returnExpression = stmt.getReturnArgument();
+			ICPPEvaluation returnEval;
 			if (returnExpression == null) {
 				returnEval = voidEval;
 			} else {
-				
 				returnEval = ((ICPPASTInitializerClause) returnExpression).getEvaluation();
 			}
 			IType returnType = returnEval.getType();
@@ -2358,42 +2362,25 @@ public class CPPVisitor extends ASTQueries {
 				// the type those return expressions will be a problem type. We ignore 
 				// these, because we can still successfully deduce from another return 
 				// statement that is not recursive.
-				// If all return statements are recursive, fReturnEval will remain null
+				// If all return statements are recursive, fReturnEvals will remain empty
-				// and getReturnEvaluation() will construct an EvalFixed.INCOMPLETE as desired.
+				// and deduceReturnType() will error out as desired.
 				return;
 			}
 			
-			if (fReturnEval == null) {
+			fReturnEvals = ArrayUtil.append(fReturnEvals, returnEval);
-				fReturnEval = returnEval;
-			} else if (!fReturnEval.getType().isSameType(returnType)) {
-				fReturnEval = EvalFixed.INCOMPLETE;
-			}
 		}
 		
-		public ICPPEvaluation getReturnEvaluation() {
+		public ICPPEvaluation[] getReturnEvaluations() {
-			if (fReturnEval == null) {
+			if (fReturnEvals.length == 0 && !fEncounteredReturnStatement) {
-				if (!fEncounteredReturnStatement) {
+				fReturnEvals = ArrayUtil.append(fReturnEvals, voidEval);
-					return voidEval;
-				}
-				fReturnEval = EvalFixed.INCOMPLETE;
 			}
-			return fReturnEval;
+			return ArrayUtil.trim(fReturnEvals);
 		}
 	}
 
-	public static IType deduceReturnType(IASTStatement functionBody, IASTDeclSpecifier autoDeclSpec, 
+	private static IType deduceTypeFromReturnEvaluation(ICPPEvaluation returnEval, 
-			IASTDeclarator autoDeclarator, PlaceholderKind placeholder, IASTNode point) {
+			IASTDeclSpecifier autoDeclSpec, IASTDeclarator autoDeclarator, PlaceholderKind placeholder) {
-		ICPPEvaluation returnEval = null;
+		// [dcl.type.auto.deduct] p3:
-		if (functionBody != null) {
-			ReturnTypeDeducer deducer = new ReturnTypeDeducer(null);
-			functionBody.accept(deducer);
-			returnEval = deducer.getReturnEvaluation();
-		}
-		if (returnEval == null || returnEval == EvalFixed.INCOMPLETE) {
-			return ProblemType.CANNOT_DEDUCE_AUTO_TYPE;
-		}
-		
-		// [dcl.spec.auto] p7:
 		//   If the deduction is for a return statement and the initializer is a
 		//   braced-init-list, the proram is ill-formed.
 		if (returnEval instanceof EvalInitList) {
@@ -2407,12 +2394,36 @@ public class CPPVisitor extends ASTQueries {
 			}
 			return CPPSemantics.getDeclTypeForEvaluation(returnEval);
 		} else /* auto */ {
-			if (autoDeclSpec == null || autoDeclarator == null) {
+			return createAutoType(returnEval, autoDeclSpec, autoDeclarator);
-				return returnEval.getType();
+		}
-			} else {
+	}
-				return createAutoType(returnEval, autoDeclSpec, autoDeclarator);
+	
+	public static IType deduceReturnType(IASTStatement functionBody, IASTDeclSpecifier autoDeclSpec, 
+			IASTDeclarator autoDeclarator, PlaceholderKind placeholder) {
+		ICPPEvaluation[] returnEvals = ICPPEvaluation.EMPTY_ARRAY;
+		if (functionBody != null) {
+			ReturnTypeDeducer deducer = new ReturnTypeDeducer(null);
+			functionBody.accept(deducer);
+			returnEvals = deducer.getReturnEvaluations();
+		}
+		if (returnEvals.length == 0) {
+			return ProblemType.CANNOT_DEDUCE_AUTO_TYPE;
+		}
+		
+		// [dcl.spec.auto] p7: 
+		//   If a function with a declared return type that contains a placeholder type has multiple 
+		//   return statements, the return type is deduced for each such return statement.
+		//   If the type deduced is not the same in each deduction, the program is ill-formed.
+		IType returnType = deduceTypeFromReturnEvaluation(returnEvals[0], autoDeclSpec, autoDeclarator, 
+				placeholder);
+		for (int i = 1; i < returnEvals.length; ++i) {
+			IType otherType = deduceTypeFromReturnEvaluation(returnEvals[i], autoDeclSpec, autoDeclarator, 
+					placeholder);
+			if (!returnType.isSameType(otherType)) {
+				return ProblemType.CANNOT_DEDUCE_AUTO_TYPE;
 			}
 		}
+		return returnType;
 	}
 	
 	/**
@@ -2453,7 +2464,7 @@ public class CPPVisitor extends ASTQueries {
 		if (returnType == null) {
 			// Try to deduce return type from return statement.
 			
-			// [dcl.spec.auto] p14:
+			// [dcl.spec.auto] p12:
 			//   A function declared with a return type that uses a placeholder type
 			//   shall not be virtual.
 			if (((ICPPASTDeclSpecifier) declSpec).isVirtual())
@@ -2462,7 +2473,7 @@ public class CPPVisitor extends ASTQueries {
 			ICPPASTFunctionDefinition definition= CPPFunction.getFunctionDefinition(declarator);
 			if (definition != null) {
 				returnType = deduceReturnType(definition.getBody(), declSpecForDeduction, 
-						declaratorForDeduction, placeholder, declaratorForDeduction);
+						declaratorForDeduction, placeholder);
 			}
 		}