From d8c2330efe8078aea2de20b5a4524ed12f7c08af Mon Sep 17 00:00:00 2001 From: Bassem Girgis Date: Mon, 8 Oct 2018 19:55:12 -0500 Subject: [PATCH] Bug 509751 - Process function template with non-dependent return type in ReturnChecker Change-Id: I8274affff8152dba35233a06cd8cdaef39cf00bb Signed-off-by: Bassem Girgis --- .../META-INF/MANIFEST.MF | 2 +- .../internal/checkers/ReturnChecker.java | 36 ++++++++++++------- .../internal/checkers/ReturnCheckerTest.java | 23 ++++++++++++ 3 files changed, 48 insertions(+), 13 deletions(-) diff --git a/codan/org.eclipse.cdt.codan.checkers/META-INF/MANIFEST.MF b/codan/org.eclipse.cdt.codan.checkers/META-INF/MANIFEST.MF index 5b4c3a1aadb..153a218bf87 100644 --- a/codan/org.eclipse.cdt.codan.checkers/META-INF/MANIFEST.MF +++ b/codan/org.eclipse.cdt.codan.checkers/META-INF/MANIFEST.MF @@ -2,7 +2,7 @@ Manifest-Version: 1.0 Bundle-ManifestVersion: 2 Bundle-Name: %Bundle-Name Bundle-SymbolicName: org.eclipse.cdt.codan.checkers;singleton:=true -Bundle-Version: 3.2.1.qualifier +Bundle-Version: 3.2.100.qualifier Bundle-Activator: org.eclipse.cdt.codan.checkers.CodanCheckersActivator Require-Bundle: org.eclipse.core.runtime, org.eclipse.core.resources, diff --git a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/ReturnChecker.java b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/ReturnChecker.java index d3f44231951..e12ea7bbf8c 100644 --- a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/ReturnChecker.java +++ b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/ReturnChecker.java @@ -48,11 +48,11 @@ import org.eclipse.cdt.core.dom.ast.IBinding; import org.eclipse.cdt.core.dom.ast.IType; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTFunctionDefinition; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTLambdaExpression; -import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTTemplateDeclaration; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTTryBlockStatement; import org.eclipse.cdt.core.dom.ast.cpp.ICPPConstructor; import org.eclipse.cdt.core.dom.ast.cpp.ICPPMethod; import org.eclipse.cdt.core.parser.util.CharArrayUtils; +import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.CPPTemplates; /** * The checker suppose to find issue related to mismatched return value/function @@ -104,12 +104,13 @@ public class ReturnChecker extends AbstractAstFunctionChecker { if (returnValue != null) { hasret = true; } - if (isNonVoid(func) && !isConstructorDestructor(func)) { + ReturnTypeKind returnKind = getReturnTypeKind(func); + if (returnKind == ReturnTypeKind.NonVoid && !isConstructorDestructor(func)) { if (checkImplicitReturn(RET_NO_VALUE_ID) || isExplicitReturn(func)) { if (returnValue == null) reportProblem(RET_NO_VALUE_ID, ret); } - } else { + } else if (returnKind == ReturnTypeKind.Void) { if (returnValue instanceof IASTExpression) { IType type = ((IASTExpression) returnValue).getExpressionType(); if (isVoid(type)) @@ -123,7 +124,7 @@ public class ReturnChecker extends AbstractAstFunctionChecker { } } - public boolean isConstructorDestructor(IASTFunctionDefinition func) { + private static boolean isConstructorDestructor(IASTFunctionDefinition func) { if (func instanceof ICPPASTFunctionDefinition) { IBinding method = func.getDeclarator().getName().resolveBinding(); if (method instanceof ICPPConstructor @@ -149,12 +150,10 @@ public class ReturnChecker extends AbstractAstFunctionChecker { @Override protected void processFunction(IASTFunctionDefinition func) { cachedReturnType = null; - if (func.getParent() instanceof ICPPASTTemplateDeclaration) - return; // If it is template get out of here. ReturnStmpVisitor visitor = new ReturnStmpVisitor(func); func.accept(visitor); - boolean nonVoid = isNonVoid(func); - if (nonVoid && !isMain(func)) { + ReturnTypeKind returnKind = getReturnTypeKind(func); + if (returnKind == ReturnTypeKind.NonVoid && !isMain(func)) { // There a return but maybe it is only on one branch. IASTStatement body = func.getBody(); if (body instanceof IASTCompoundStatement) { @@ -251,6 +250,10 @@ public class ReturnChecker extends AbstractAstFunctionChecker { && ((IASTSimpleDeclSpecifier) declSpecifier).getType() == IASTSimpleDeclSpecifier.t_unspecified); } + enum ReturnTypeKind { + Void, NonVoid, Unknown + } + /** * Checks if the function has a return type other than void. Constructors and destructors * don't have return type. @@ -258,10 +261,19 @@ public class ReturnChecker extends AbstractAstFunctionChecker { * @param func the function to check * @return {@code true} if the function has a non void return type */ - private boolean isNonVoid(IASTFunctionDefinition func) { + @SuppressWarnings("restriction") + private ReturnTypeKind getReturnTypeKind(IASTFunctionDefinition func) { if (isConstructorDestructor(func)) - return false; - return !isVoid(getReturnType(func)); + return ReturnTypeKind.Void; + IType returnType = getReturnType(func); + if (CPPTemplates.isDependentType(returnType)) { + // Could instantiate to void or not. + // If we care to, we could do some more heuristic analysis. + // For example, if C is a class template, `C` will always be non-void, + // but `typename C::type` is still unknown. + return ReturnTypeKind.Unknown; + } + return isVoid(returnType) ? ReturnTypeKind.Void : ReturnTypeKind.NonVoid; } private IType getReturnType(IASTFunctionDefinition func) { @@ -271,7 +283,7 @@ public class ReturnChecker extends AbstractAstFunctionChecker { return cachedReturnType; } - public boolean isVoid(IType type) { + private static boolean isVoid(IType type) { return type instanceof IBasicType && ((IBasicType) type).getKind() == IBasicType.Kind.eVoid; } diff --git a/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/internal/checkers/ReturnCheckerTest.java b/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/internal/checkers/ReturnCheckerTest.java index cb9ce2f339a..4965121c295 100644 --- a/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/internal/checkers/ReturnCheckerTest.java +++ b/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/internal/checkers/ReturnCheckerTest.java @@ -464,4 +464,27 @@ public class ReturnCheckerTest extends CheckerTestCase { public void testReturnTypeDeduction_540112() throws Exception { checkSampleAboveCpp(); } + + // void waldo() { + // return 5; // error here on line 2 + // } + public void testNonTemplateFunctionReturn_509751() throws Exception { + checkSampleAboveCpp(); + } + + // template + // void waldoT() { + // return 5; // error here on line 3 + // } + public void testTemplateFunctionReturn_509751a() throws Exception { + checkSampleAboveCpp(); + } + + // template + // T waldoT() { + // return 5; + // } + public void testTemplateFunctionReturn_509751b() throws Exception { + checkSampleAboveCpp(); + } }