Index: include/clang/Analysis/Analyses/ExprMutationAnalyzer.h =================================================================== --- include/clang/Analysis/Analyses/ExprMutationAnalyzer.h +++ include/clang/Analysis/Analyses/ExprMutationAnalyzer.h @@ -17,6 +17,8 @@ namespace clang { +class FunctionParmMutationAnalyzer; + /// Analyzes whether any mutative operations are applied to an expression within /// a given statement. class ExprMutationAnalyzer { @@ -27,13 +29,13 @@ bool isMutated(const Decl *Dec) { return findDeclMutation(Dec) != nullptr; } bool isMutated(const Expr *Exp) { return findMutation(Exp) != nullptr; } const Stmt *findMutation(const Expr *Exp); + const Stmt *findDeclMutation(const Decl *Dec); private: bool isUnevaluated(const Expr *Exp); const Stmt *findExprMutation(ArrayRef Matches); const Stmt *findDeclMutation(ArrayRef Matches); - const Stmt *findDeclMutation(const Decl *Dec); const Stmt *findDirectMutation(const Expr *Exp); const Stmt *findMemberMutation(const Expr *Exp); @@ -41,12 +43,32 @@ const Stmt *findCastMutation(const Expr *Exp); const Stmt *findRangeLoopMutation(const Expr *Exp); const Stmt *findReferenceMutation(const Expr *Exp); + const Stmt *findFunctionArgMutation(const Expr *Exp); const Stmt &Stm; ASTContext &Context; + llvm::DenseMap> + FuncParmAnalyzer; llvm::DenseMap Results; }; +// A convenient wrapper around ExprMutationAnalyzer for analyzing function +// params. +class FunctionParmMutationAnalyzer { +public: + FunctionParmMutationAnalyzer(const FunctionDecl &Func, ASTContext &Context); + + bool isMutated(const ParmVarDecl *Parm) { + return findMutation(Parm) != nullptr; + } + const Stmt *findMutation(const ParmVarDecl *Parm); + +private: + ExprMutationAnalyzer BodyAnalyzer; + llvm::DenseMap Results; +}; + } // namespace clang #endif // LLVM_CLANG_ANALYSIS_ANALYSES_EXPRMUTATIONANALYZER_H Index: lib/Analysis/ExprMutationAnalyzer.cpp =================================================================== --- lib/Analysis/ExprMutationAnalyzer.cpp +++ lib/Analysis/ExprMutationAnalyzer.cpp @@ -79,7 +79,8 @@ &ExprMutationAnalyzer::findArrayElementMutation, &ExprMutationAnalyzer::findCastMutation, &ExprMutationAnalyzer::findRangeLoopMutation, - &ExprMutationAnalyzer::findReferenceMutation}) { + &ExprMutationAnalyzer::findReferenceMutation, + &ExprMutationAnalyzer::findFunctionArgMutation}) { if (const Stmt *S = (this->*Finder)(Exp)) return Results[Exp] = S; } @@ -192,10 +193,15 @@ // Used as non-const-ref argument when calling a function. // An argument is assumed to be non-const-ref when the function is unresolved. + // Instantiated template functions are not handled here but in + // findFunctionArgMutation which has additional smarts for handling forwarding + // references. const auto NonConstRefParam = forEachArgumentWithParam( equalsNode(Exp), parmVarDecl(hasType(nonConstReferenceType()))); + const auto NotInstantiated = unless(hasDeclaration(isInstantiated())); const auto AsNonConstRefArg = anyOf( - callExpr(NonConstRefParam), cxxConstructExpr(NonConstRefParam), + callExpr(NonConstRefParam, NotInstantiated), + cxxConstructExpr(NonConstRefParam, NotInstantiated), callExpr(callee(expr(anyOf(unresolvedLookupExpr(), unresolvedMemberExpr(), cxxDependentScopeMemberExpr(), hasType(templateTypeParmType())))), @@ -305,4 +311,81 @@ return findDeclMutation(Refs); } +const Stmt *ExprMutationAnalyzer::findFunctionArgMutation(const Expr *Exp) { + const auto NonConstRefParam = forEachArgumentWithParam( + equalsNode(Exp), + parmVarDecl(hasType(nonConstReferenceType())).bind("parm")); + const auto IsInstantiated = hasDeclaration(isInstantiated()); + const auto FuncDecl = hasDeclaration(functionDecl().bind("func")); + const auto Matches = match( + findAll(expr(anyOf(callExpr(NonConstRefParam, IsInstantiated, FuncDecl), + cxxConstructExpr(NonConstRefParam, IsInstantiated, + FuncDecl))) + .bind("expr")), + Stm, Context); + for (const auto &Nodes : Matches) { + const auto *Exp = Nodes.getNodeAs("expr"); + const auto *Func = Nodes.getNodeAs("func"); + if (!Func->getBody()) + return Exp; + + const auto *Parm = Nodes.getNodeAs("parm"); + const auto AllParams = + Func->getPrimaryTemplate()->getTemplatedDecl()->parameters(); + QualType ParmType = + AllParams[std::min(Parm->getFunctionScopeIndex(), + AllParams.size() - 1)] + ->getType(); + if (auto *T = ParmType->getAs()) + ParmType = T->getPattern(); + + // If param type is forwarding reference, follow into the function + // definition and see whether the param is mutated inside. + if (auto *RefType = ParmType->getAs()) { + if (!RefType->getPointeeType().getQualifiers() && + RefType->getPointeeType()->getAs()) { + auto &Analyzer = FuncParmAnalyzer[Func]; + if (!Analyzer) + Analyzer.reset(new FunctionParmMutationAnalyzer(*Func, Context)); + if (Analyzer->findMutation(Parm)) + return Exp; + continue; + } + } + // Not forwarding reference. + return Exp; + } + return nullptr; +} + +FunctionParmMutationAnalyzer::FunctionParmMutationAnalyzer( + const FunctionDecl &Func, ASTContext &Context) + : BodyAnalyzer(*Func.getBody(), Context) { + if (const auto *Ctor = dyn_cast(&Func)) { + // CXXCtorInitializer might also mutate Param but they're not part of + // function body, check them eagerly here since they're typically trivial. + for (const CXXCtorInitializer *Init : Ctor->inits()) { + ExprMutationAnalyzer InitAnalyzer(*Init->getInit(), Context); + for (const ParmVarDecl *Parm : Ctor->parameters()) { + if (Results.find(Parm) != Results.end()) + continue; + if (const Stmt *S = InitAnalyzer.findDeclMutation(Parm)) + Results[Parm] = S; + } + } + } +} + +const Stmt * +FunctionParmMutationAnalyzer::findMutation(const ParmVarDecl *Parm) { + const auto Memoized = Results.find(Parm); + if (Memoized != Results.end()) + return Memoized->second; + + if (const Stmt *S = BodyAnalyzer.findDeclMutation(Parm)) + return Results[Parm] = S; + + return Results[Parm] = nullptr; +} + } // namespace clang Index: unittests/Analysis/ExprMutationAnalyzerTest.cpp =================================================================== --- unittests/Analysis/ExprMutationAnalyzerTest.cpp +++ unittests/Analysis/ExprMutationAnalyzerTest.cpp @@ -595,6 +595,68 @@ EXPECT_FALSE(isMutated(Results, AST.get())); } +TEST(ExprMutationAnalyzerTest, FollowFuncArgModified) { + auto AST = + tooling::buildASTFromCode("template void g(T&& t) { t = 10; }" + "void f() { int x; g(x); }"); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(x)")); + + AST = tooling::buildASTFromCode( + "void h(int&);" + "template void g(Args&&... args) { h(args...); }" + "void f() { int x; g(x); }"); + Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(x)")); + + AST = tooling::buildASTFromCode( + "void h(int&, int);" + "template void g(Args&&... args) { h(args...); }" + "void f() { int x, y; g(x, y); }"); + Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(x, y)")); + + AST = tooling::buildASTFromCode( + "void h(int, int&);" + "template void g(Args&&... args) { h(args...); }" + "void f() { int x, y; g(y, x); }"); + Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(y, x)")); +} + +TEST(ExprMutationAnalyzerTest, FollowFuncArgNotModified) { + auto AST = tooling::buildASTFromCode("template void g(T&&) {}" + "void f() { int x; g(x); }"); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isMutated(Results, AST.get())); + + AST = tooling::buildASTFromCode("template void g(T&& t) { t; }" + "void f() { int x; g(x); }"); + Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isMutated(Results, AST.get())); + + AST = + tooling::buildASTFromCode("template void g(Args&&...) {}" + "void f() { int x; g(x); }"); + Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isMutated(Results, AST.get())); + + AST = + tooling::buildASTFromCode("template void g(Args&&...) {}" + "void f() { int y, x; g(y, x); }"); + Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isMutated(Results, AST.get())); + + AST = tooling::buildASTFromCode( + "void h(int, int&);" + "template void g(Args&&... args) { h(args...); }" + "void f() { int x, y; g(x, y); }"); + Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isMutated(Results, AST.get())); +} + TEST(ExprMutationAnalyzerTest, ArrayElementModified) { const auto AST = tooling::buildASTFromCode("void f() { int x[2]; x[0] = 10; }");