[clang-tools-extra] r341854 - [clang-tidy] ExprMutationAnalyzer: construct from references. Fixes PR38888
Roman Lebedev via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 10 12:59:19 PDT 2018
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;
More information about the cfe-commits
mailing list