[clang-tidy] ExprMutationAnalyzer: construct from references. Fixes PR38888

Summary:
I have hit this the rough way, while trying to use this in D51870.

There is no particular point in storing the pointers, and moreover
the pointers are assumed to be non-null, and that assumption is not
enforced. If they are null, it won't be able to do anything good
with them anyway.

Initially i thought about simply adding asserts() that they are
not null, but taking/storing references looks like even cleaner solution?

Fixes [[ https://bugs.llvm.org/show_bug.cgi?id=38888 | PR38888 ]]

Reviewers: JonasToth, shuaiwang, alexfh, george.karpenkov

Reviewed By: shuaiwang

Subscribers: xazax.hun, a.sidorin, Szelethus, cfe-commits

Tags: #clang-tools-extra

Differential Revision: https://reviews.llvm.org/D51884

llvm-svn: 341854

diff --git a/clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp b/clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
index 867d95d..0a3cc4f 100644 (file)
--- a/clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
@@ -88,8 +88,8 @@ bool ForRangeCopyCheck::handleCopyIsOnlyConstReferenced(
   // Because the fix (changing to `const auto &`) will introduce an unused
   // compiler warning which can't be suppressed.
   // Since this case is very rare, it is safe to ignore it.
-  if (!utils::ExprMutationAnalyzer(ForRange.getBody(), &Context)
-          .isMutated(&LoopVar) &&
+  if (!utils::ExprMutationAnalyzer(*ForRange.getBody(), Context)
+           .isMutated(&LoopVar) &&
       !utils::decl_ref_expr::allDeclRefExprs(LoopVar, *ForRange.getBody(),
                                              Context)
            .empty()) {

diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
index 8c9259c..63b853d 100644 (file)
--- a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
@@ -95,14 +95,14 @@ void UnnecessaryValueParamCheck::check(const MatchFinder::MatchResult &Result) {
   // Do not trigger on non-const value parameters when they are mutated either
   // within the function body or within init expression(s) when the function is
   // a ctor.
-  if (utils::ExprMutationAnalyzer(Function->getBody(), Result.Context)
+  if (utils::ExprMutationAnalyzer(*Function->getBody(), *Result.Context)
           .isMutated(Param))
     return;
   // CXXCtorInitializer might also mutate Param but they're not part of function
   // body, so check them separately here.
   if (const auto *Ctor = dyn_cast<CXXConstructorDecl>(Function)) {
     for (const auto *Init : Ctor->inits()) {
-      if (utils::ExprMutationAnalyzer(Init->getInit(), Result.Context)
+      if (utils::ExprMutationAnalyzer(*Init->getInit(), *Result.Context)
               .isMutated(Param))
         return;
     }

diff --git a/clang-tools-extra/clang-tidy/utils/ExprMutationAnalyzer.cpp b/clang-tools-extra/clang-tidy/utils/ExprMutationAnalyzer.cpp
index f1cd1da..f979e97 100644 (file)
--- a/clang-tools-extra/clang-tidy/utils/ExprMutationAnalyzer.cpp
+++ b/clang-tools-extra/clang-tidy/utils/ExprMutationAnalyzer.cpp
@@ -102,7 +102,7 @@ bool ExprMutationAnalyzer::isUnevaluated(const Expr *Exp) {
                                       hasDescendant(equalsNode(Exp)))),
                                   cxxNoexceptExpr())))))
                          .bind("expr")),
-                 *Stm, *Context)) != nullptr;
+                 Stm, Context)) != nullptr;
 }
 
 const Stmt *
@@ -125,7 +125,7 @@ ExprMutationAnalyzer::findDeclMutation(ArrayRef<BoundNodes> Matches) {
 
 const Stmt *ExprMutationAnalyzer::findDeclMutation(const Decl *Dec) {
   const auto Refs = match(
-      findAll(declRefExpr(to(equalsNode(Dec))).bind("expr")), *Stm, *Context);
+      findAll(declRefExpr(to(equalsNode(Dec))).bind("expr")), Stm, Context);
   for (const auto &RefNodes : Refs) {
     const auto *E = RefNodes.getNodeAs<Expr>("expr");
     if (findMutation(E))
@@ -200,7 +200,7 @@ const Stmt *ExprMutationAnalyzer::findDirectMutation(const Expr *Exp) {
                                AsNonConstRefArg, AsLambdaRefCaptureInit,
                                AsNonConstRefReturn))
                         .bind("stmt")),
-            *Stm, *Context);
+            Stm, Context);
   return selectFirst<Stmt>("stmt", Matches);
 }
 
@@ -211,7 +211,7 @@ const Stmt *ExprMutationAnalyzer::findMemberMutation(const Expr *Exp) {
                                cxxDependentScopeMemberExpr(
                                    hasObjectExpression(equalsNode(Exp)))))
                         .bind("expr")),
-            *Stm, *Context);
+            Stm, Context);
   return findExprMutation(MemberExprs);
 }
 
@@ -220,7 +220,7 @@ const Stmt *ExprMutationAnalyzer::findArrayElementMutation(const Expr *Exp) {
   const auto SubscriptExprs = match(
       findAll(arraySubscriptExpr(hasBase(ignoringImpCasts(equalsNode(Exp))))
                   .bind("expr")),
-      *Stm, *Context);
+      Stm, Context);
   return findExprMutation(SubscriptExprs);
 }
 
@@ -233,7 +233,7 @@ const Stmt *ExprMutationAnalyzer::findCastMutation(const Expr *Exp) {
                                    implicitCastExpr(hasImplicitDestinationType(
                                        nonConstReferenceType()))))
                         .bind("expr")),
-            *Stm, *Context);
+            Stm, Context);
   return findExprMutation(Casts);
 }
 
@@ -245,7 +245,7 @@ const Stmt *ExprMutationAnalyzer::findRangeLoopMutation(const Expr *Exp) {
                 hasLoopVariable(
                     varDecl(hasType(nonConstReferenceType())).bind("decl")),
                 hasRangeInit(equalsNode(Exp)))),
-            *Stm, *Context);
+            Stm, Context);
   return findDeclMutation(LoopVars);
 }
 
@@ -265,7 +265,7 @@ const Stmt *ExprMutationAnalyzer::findReferenceMutation(const Expr *Exp) {
               unless(hasParent(declStmt(hasParent(
                   cxxForRangeStmt(hasRangeStmt(equalsBoundNode("stmt"))))))))
               .bind("decl"))),
-      *Stm, *Context);
+      Stm, Context);
   return findDeclMutation(Refs);
 }
 

diff --git a/clang-tools-extra/clang-tidy/utils/ExprMutationAnalyzer.h b/clang-tools-extra/clang-tidy/utils/ExprMutationAnalyzer.h
index 256bb71..e295de9 100644 (file)
--- a/clang-tools-extra/clang-tidy/utils/ExprMutationAnalyzer.h
+++ b/clang-tools-extra/clang-tidy/utils/ExprMutationAnalyzer.h
@@ -23,7 +23,7 @@ namespace utils {
 /// a given statement.
 class ExprMutationAnalyzer {
 public:
-  ExprMutationAnalyzer(const Stmt *Stm, ASTContext *Context)
+  ExprMutationAnalyzer(const Stmt &Stm, ASTContext &Context)
       : Stm(Stm), Context(Context) {}
 
   bool isMutated(const Decl *Dec) { return findDeclMutation(Dec) != nullptr; }
@@ -44,8 +44,8 @@ private:
   const Stmt *findRangeLoopMutation(const Expr *Exp);
   const Stmt *findReferenceMutation(const Expr *Exp);
 
-  const Stmt *const Stm;
-  ASTContext *const Context;
+  const Stmt &Stm;
+  ASTContext &Context;
   llvm::DenseMap<const Expr *, const Stmt *> Results;
 };
 

diff --git a/clang-tools-extra/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp b/clang-tools-extra/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
index 117bcfe..013493e 100644 (file)
--- a/clang-tools-extra/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
@@ -42,14 +42,14 @@ StmtMatcher withEnclosingCompound(ExprMatcher Matcher) {
 bool isMutated(const SmallVectorImpl<BoundNodes> &Results, ASTUnit *AST) {
   const auto *const S = selectFirst<Stmt>("stmt", Results);
   const auto *const E = selectFirst<Expr>("expr", Results);
-  return utils::ExprMutationAnalyzer(S, &AST->getASTContext()).isMutated(E);
+  return utils::ExprMutationAnalyzer(*S, AST->getASTContext()).isMutated(E);
 }
 
 SmallVector<std::string, 1>
 mutatedBy(const SmallVectorImpl<BoundNodes> &Results, ASTUnit *AST) {
   const auto *const S = selectFirst<Stmt>("stmt", Results);
   SmallVector<std::string, 1> Chain;
-  utils::ExprMutationAnalyzer Analyzer(S, &AST->getASTContext());
+  utils::ExprMutationAnalyzer Analyzer(*S, AST->getASTContext());
   for (const auto *E = selectFirst<Expr>("expr", Results); E != nullptr;) {
     const Stmt *By = Analyzer.findMutation(E);
     std::string buffer;