Author: lebedevri Date: Mon Sep 10 12:59:18 2018 New Revision: 341854 URL: http://llvm.org/viewvc/llvm-project?rev=341854&view=rev Log: [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 Modified: clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.cpp clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.h clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp Modified: clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp?rev=341854&r1=341853&r2=341854&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp Mon Sep 10 12:59:18 2018 @@ -88,8 +88,8 @@ bool ForRangeCopyCheck::handleCopyIsOnly // 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()) { Modified: clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp?rev=341854&r1=341853&r2=341854&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp Mon Sep 10 12:59:18 2018 @@ -95,14 +95,14 @@ void UnnecessaryValueParamCheck::check(c // 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; } Modified: clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.cpp?rev=341854&r1=341853&r2=341854&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.cpp Mon Sep 10 12:59:18 2018 @@ -102,7 +102,7 @@ bool ExprMutationAnalyzer::isUnevaluated hasDescendant(equalsNode(Exp)))), cxxNoexceptExpr()))))) .bind("expr")), - *Stm, *Context)) != nullptr; + Stm, Context)) != nullptr; } const Stmt * @@ -125,7 +125,7 @@ ExprMutationAnalyzer::findDeclMutation(A 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::findDi AsNonConstRefArg, AsLambdaRefCaptureInit, AsNonConstRefReturn)) .bind("stmt")), - *Stm, *Context); + Stm, Context); return selectFirst<Stmt>("stmt", Matches); } @@ -211,7 +211,7 @@ const Stmt *ExprMutationAnalyzer::findMe cxxDependentScopeMemberExpr( hasObjectExpression(equalsNode(Exp))))) .bind("expr")), - *Stm, *Context); + Stm, Context); return findExprMutation(MemberExprs); } @@ -220,7 +220,7 @@ const Stmt *ExprMutationAnalyzer::findAr 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::findCa implicitCastExpr(hasImplicitDestinationType( nonConstReferenceType())))) .bind("expr")), - *Stm, *Context); + Stm, Context); return findExprMutation(Casts); } @@ -245,7 +245,7 @@ const Stmt *ExprMutationAnalyzer::findRa hasLoopVariable( varDecl(hasType(nonConstReferenceType())).bind("decl")), hasRangeInit(equalsNode(Exp)))), - *Stm, *Context); + Stm, Context); return findDeclMutation(LoopVars); } @@ -265,7 +265,7 @@ const Stmt *ExprMutationAnalyzer::findRe unless(hasParent(declStmt(hasParent( cxxForRangeStmt(hasRangeStmt(equalsBoundNode("stmt")))))))) .bind("decl"))), - *Stm, *Context); + Stm, Context); return findDeclMutation(Refs); } Modified: clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.h?rev=341854&r1=341853&r2=341854&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.h (original) +++ clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.h Mon Sep 10 12:59:18 2018 @@ -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; }; Modified: clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp?rev=341854&r1=341853&r2=341854&view=diff ============================================================================== --- clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp (original) +++ clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp Mon Sep 10 12:59:18 2018 @@ -42,14 +42,14 @@ StmtMatcher withEnclosingCompound(ExprMa 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; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
