shuaiwang updated this revision to Diff 165420.
shuaiwang marked 10 inline comments as done.
shuaiwang added a comment.
More test cases addressing review comments
Repository:
rC Clang
https://reviews.llvm.org/D52008
Files:
include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
lib/Analysis/ExprMutationAnalyzer.cpp
unittests/Analysis/ExprMutationAnalyzerTest.cpp
Index: unittests/Analysis/ExprMutationAnalyzerTest.cpp
===================================================================
--- unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -595,6 +595,96 @@
EXPECT_FALSE(isMutated(Results, AST.get()));
}
+TEST(ExprMutationAnalyzerTest, FollowFuncArgModified) {
+ auto AST =
+ tooling::buildASTFromCode("template <class T> 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 <class... Args> 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 <class... Args> 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)"));
+ Results = match(withEnclosingCompound(declRefTo("y")), AST->getASTContext());
+ EXPECT_FALSE(isMutated(Results, AST.get()));
+
+ AST = tooling::buildASTFromCode(
+ "void h(int, int&);"
+ "template <class... Args> 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)"));
+ Results = match(withEnclosingCompound(declRefTo("y")), AST->getASTContext());
+ EXPECT_FALSE(isMutated(Results, AST.get()));
+
+ AST = tooling::buildASTFromCode(
+ "struct S { template <class T> S(T&& t) { t = 10; } };"
+ "void f() { int x; S s(x); }");
+ Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x"));
+
+ AST = tooling::buildASTFromCode(
+ "struct S { template <class T> S(T&& t) : m(++t) { } int m; };"
+ "void f() { int x; S s(x); }");
+ Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x"));
+}
+
+TEST(ExprMutationAnalyzerTest, FollowFuncArgNotModified) {
+ auto AST = tooling::buildASTFromCode("template <class T> 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 <class T> 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 <class... Args> 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 <class... Args> 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 <class... Args> 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()));
+
+ AST = tooling::buildASTFromCode(
+ "struct S { template <class T> S(T&& t) { t; } };"
+ "void f() { int x; S s(x); }");
+ Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_FALSE(isMutated(Results, AST.get()));
+
+ AST = tooling::buildASTFromCode(
+ "struct S { template <class T> S(T&& t) : m(t) { } int m; };"
+ "void f() { int x; S s(x); }");
+ 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; }");
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>("expr");
+ const auto *Func = Nodes.getNodeAs<FunctionDecl>("func");
+ if (!Func->getBody())
+ return Exp;
+
+ const auto *Parm = Nodes.getNodeAs<ParmVarDecl>("parm");
+ const auto AllParams =
+ Func->getPrimaryTemplate()->getTemplatedDecl()->parameters();
+ QualType ParmType =
+ AllParams[std::min<size_t>(Parm->getFunctionScopeIndex(),
+ AllParams.size() - 1)]
+ ->getType();
+ if (auto *T = ParmType->getAs<PackExpansionType>())
+ 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<RValueReferenceType>()) {
+ if (!RefType->getPointeeType().getQualifiers() &&
+ RefType->getPointeeType()->getAs<TemplateTypeParmType>()) {
+ 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<CXXConstructorDecl>(&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: 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,26 +29,46 @@
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<ast_matchers::BoundNodes> Matches);
const Stmt *findDeclMutation(ArrayRef<ast_matchers::BoundNodes> Matches);
- const Stmt *findDeclMutation(const Decl *Dec);
const Stmt *findDirectMutation(const Expr *Exp);
const Stmt *findMemberMutation(const Expr *Exp);
const Stmt *findArrayElementMutation(const Expr *Exp);
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<const FunctionDecl *,
+ std::unique_ptr<FunctionParmMutationAnalyzer>>
+ FuncParmAnalyzer;
llvm::DenseMap<const Expr *, const Stmt *> 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<const ParmVarDecl *, const Stmt *> Results;
+};
+
} // namespace clang
#endif // LLVM_CLANG_ANALYSIS_ANALYSES_EXPRMUTATIONANALYZER_H
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
