From f2ab40d9f266494145978534715a16fdae3b18b3 Mon Sep 17 00:00:00 2001 From: Nathan Ridge Date: Wed, 8 Nov 2017 17:28:58 -0500 Subject: [PATCH] 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 --- .../ast2/cxx14/ReturnTypeDeductionTests.java | 50 +++++++++-- .../core/dom/parser/cpp/CPPClosureType.java | 2 +- .../dom/parser/cpp/semantics/CPPVisitor.java | 87 +++++++++++-------- 3 files changed, 93 insertions(+), 46 deletions(-) diff --git a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ast2/cxx14/ReturnTypeDeductionTests.java b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ast2/cxx14/ReturnTypeDeductionTests.java index 800ae1308b7..5bc59adfeeb 100644 --- a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ast2/cxx14/ReturnTypeDeductionTests.java +++ b/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(); - ICPPFunction f = bh.assertNonProblem(functionName); - assertInstance(f.getType().getReturnType(), IProblemType.class); + assertInstance(getReturnType(functionName), 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; } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPClosureType.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPClosureType.java index 45460e1feaa..99d967081ef 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPClosureType.java +++ b/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; } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/CPPVisitor.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/CPPVisitor.java index 70329995609..c24d6402b15 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/CPPVisitor.java +++ b/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 - // and getReturnEvaluation() will construct an EvalFixed.INCOMPLETE as desired. + // If all return statements are recursive, fReturnEvals will remain empty + // and deduceReturnType() will error out as desired. return; } - if (fReturnEval == null) { - fReturnEval = returnEval; - } else if (!fReturnEval.getType().isSameType(returnType)) { - fReturnEval = EvalFixed.INCOMPLETE; - } + fReturnEvals = ArrayUtil.append(fReturnEvals, returnEval); } - public ICPPEvaluation getReturnEvaluation() { - if (fReturnEval == null) { - if (!fEncounteredReturnStatement) { - return voidEval; - } - fReturnEval = EvalFixed.INCOMPLETE; + public ICPPEvaluation[] getReturnEvaluations() { + if (fReturnEvals.length == 0 && !fEncounteredReturnStatement) { + fReturnEvals = ArrayUtil.append(fReturnEvals, voidEval); } - return fReturnEval; + return ArrayUtil.trim(fReturnEvals); } } - public static IType deduceReturnType(IASTStatement functionBody, IASTDeclSpecifier autoDeclSpec, - IASTDeclarator autoDeclarator, PlaceholderKind placeholder, IASTNode point) { - ICPPEvaluation returnEval = null; - 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: + private static IType deduceTypeFromReturnEvaluation(ICPPEvaluation returnEval, + IASTDeclSpecifier autoDeclSpec, IASTDeclarator autoDeclarator, PlaceholderKind placeholder) { + // [dcl.type.auto.deduct] p3: // 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 returnEval.getType(); - } else { - return createAutoType(returnEval, autoDeclSpec, autoDeclarator); + 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); } }