[clang] [clang-tools-extra] [analyzer][clang-tidy][NFC] Clean up eagerly-assume handling (PR #112209)
Donát Nagy via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 14 07:18:34 PDT 2024
https://github.com/NagyDonat created https://github.com/llvm/llvm-project/pull/112209
This commit is a collection of several very minor code quality improvements. The main goal is removing the misleading "Bin" substring from the names of several methods and variables (like `evalEagerlyAssumedBinOpBifurcation`) that are also applied for the unary logical not operator.
In addition to this, I clarified the doc-comment of the method `evalEagerlyAssumedBinOpBifurcation` and refactored the body of this method to fix the capitalization of variable names and replace an obsolete use of `std::tie` with a structured binding.
Finally, the data member `eagerlyAssumeBinOpBifurcation` of the class `AnalyzerOptions` was completely removed (including a line in clang-tidy that sets it to true), because it was never read by any code.
Note that the eagerly-assume mode of the analyzer is controlled by a different boolean member of `AnalyzerOptions` which is called `ShouldEagerlyAssume` and is defined via the macro magic from `AnalyzerOptions.def`.
>From ea6ab3fe84e5ac89f82def877c37c8409889d01d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Mon, 14 Oct 2024 15:34:55 +0200
Subject: [PATCH] [analyzer][clang-tidy][NFC] Clean up eagerly-assume handling
This commit is a collection of several very minor code quality
improvements. The main goal is removing the misleading "Bin" substring
from the names of several methods and variables (like
`evalEagerlyAssumedBinOpBifurcation`) that are also applied for the
unary logical not operator.
In addition to this, I clarified the doc-comment of the method
`evalEagerlyAssumedBinOpBifurcation` and refactored the body of this
method to fix the capitalization of variable names and replace an
obsolete use of `std::tie` with a structured binding.
Finally, the data member `eagerlyAssumeBinOpBifurcation` of the class
`AnalyzerOptions` was completely removed (including a line in clang-tidy
that sets it to true), because it was never read by any code.
Note that the eagerly-assume mode of the analyzer is controlled by a
different boolean member of `AnalyzerOptions` which is called
`ShouldEagerlyAssume` and is defined via the macro magic from
`AnalyzerOptions.def`.
---
clang-tools-extra/clang-tidy/ClangTidy.cpp | 1 -
.../StaticAnalyzer/Core/AnalyzerOptions.h | 8 ++--
.../Core/PathSensitive/ExprEngine.h | 12 +++---
.../Core/BugReporterVisitors.cpp | 2 +-
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 41 +++++++++----------
5 files changed, 29 insertions(+), 35 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp
index 62f9d19b2a362f..c4cac7d27b77c2 100644
--- a/clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -458,7 +458,6 @@ ClangTidyASTConsumerFactory::createASTConsumer(
if (!AnalyzerOptions.CheckersAndPackages.empty()) {
setStaticAnalyzerCheckerOpts(Context.getOptions(), AnalyzerOptions);
AnalyzerOptions.AnalysisDiagOpt = PD_NONE;
- AnalyzerOptions.eagerlyAssumeBinOpBifurcation = true;
std::unique_ptr<ento::AnalysisASTConsumer> AnalysisConsumer =
ento::CreateAnalysisConsumer(Compiler);
AnalysisConsumer->AddDiagnosticConsumer(
diff --git a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
index 3a3c1a13d67dd5..2f4cd277cccdc6 100644
--- a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -229,8 +229,6 @@ class AnalyzerOptions : public RefCountedBase<AnalyzerOptions> {
unsigned AnalyzerDisplayProgress : 1;
unsigned AnalyzerNoteAnalysisEntryPoints : 1;
- unsigned eagerlyAssumeBinOpBifurcation : 1;
-
unsigned TrimGraph : 1;
unsigned visualizeExplodedGraphWithGraphViz : 1;
unsigned UnoptimizedCFG : 1;
@@ -293,9 +291,9 @@ class AnalyzerOptions : public RefCountedBase<AnalyzerOptions> {
ShowConfigOptionsList(false),
ShouldEmitErrorsOnInvalidConfigValue(false), AnalyzeAll(false),
AnalyzerDisplayProgress(false), AnalyzerNoteAnalysisEntryPoints(false),
- eagerlyAssumeBinOpBifurcation(false), TrimGraph(false),
- visualizeExplodedGraphWithGraphViz(false), UnoptimizedCFG(false),
- PrintStats(false), NoRetryExhausted(false), AnalyzerWerror(false) {}
+ TrimGraph(false), visualizeExplodedGraphWithGraphViz(false),
+ UnoptimizedCFG(false), PrintStats(false), NoRetryExhausted(false),
+ AnalyzerWerror(false) {}
/// Interprets an option's string value as a boolean. The "true" string is
/// interpreted as true and the "false" string is interpreted as false.
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index 04eacd1df7ffe2..ad21943f56fc8e 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -583,14 +583,14 @@ class ExprEngine {
ExplodedNode *Pred,
ExplodedNodeSet &Dst);
- /// evalEagerlyAssumeBinOpBifurcation - Given the nodes in 'Src', eagerly assume symbolic
- /// expressions of the form 'x != 0' and generate new nodes (stored in Dst)
- /// with those assumptions.
- void evalEagerlyAssumeBinOpBifurcation(ExplodedNodeSet &Dst, ExplodedNodeSet &Src,
- const Expr *Ex);
+ /// evalEagerlyAssumeOpBifurcation - Given the nodes in 'Src', eagerly assume
+ /// comparison operator expressions like 'x != 0' or logical negation like
+ /// '!foo' and generate new nodes (stored in Dst) with those assumptions.
+ void evalEagerlyAssumeOpBifurcation(ExplodedNodeSet &Dst,
+ ExplodedNodeSet &Src, const Expr *Ex);
static std::pair<const ProgramPointTag *, const ProgramPointTag *>
- geteagerlyAssumeBinOpBifurcationTags();
+ getEagerlyAssumeOpBifurcationTags();
ProgramStateRef handleLValueBitCast(ProgramStateRef state, const Expr *Ex,
const LocationContext *LCtx, QualType T,
diff --git a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index 68c8a8dc682507..de2d89bf09c3b9 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -2695,7 +2695,7 @@ ConditionBRVisitor::VisitNodeImpl(const ExplodedNode *N,
PathSensitiveBugReport &BR) {
ProgramPoint ProgPoint = N->getLocation();
const std::pair<const ProgramPointTag *, const ProgramPointTag *> &Tags =
- ExprEngine::geteagerlyAssumeBinOpBifurcationTags();
+ ExprEngine::getEagerlyAssumeOpBifurcationTags();
// If an assumption was made on a branch, it should be caught
// here by looking at the state transition.
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 43ab646d398b31..9bfe93b6fc7ba6 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -2129,7 +2129,7 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
(B->isRelationalOp() || B->isEqualityOp())) {
ExplodedNodeSet Tmp;
VisitBinaryOperator(cast<BinaryOperator>(S), Pred, Tmp);
- evalEagerlyAssumeBinOpBifurcation(Dst, Tmp, cast<Expr>(S));
+ evalEagerlyAssumeOpBifurcation(Dst, Tmp, cast<Expr>(S));
}
else
VisitBinaryOperator(cast<BinaryOperator>(S), Pred, Dst);
@@ -2402,7 +2402,7 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
if (AMgr.options.ShouldEagerlyAssume && (U->getOpcode() == UO_LNot)) {
ExplodedNodeSet Tmp;
VisitUnaryOperator(U, Pred, Tmp);
- evalEagerlyAssumeBinOpBifurcation(Dst, Tmp, U);
+ evalEagerlyAssumeOpBifurcation(Dst, Tmp, U);
}
else
VisitUnaryOperator(U, Pred, Dst);
@@ -3742,20 +3742,18 @@ void ExprEngine::evalLocation(ExplodedNodeSet &Dst,
BldrTop.addNodes(Tmp);
}
-std::pair<const ProgramPointTag *, const ProgramPointTag*>
-ExprEngine::geteagerlyAssumeBinOpBifurcationTags() {
- static SimpleProgramPointTag
- eagerlyAssumeBinOpBifurcationTrue(TagProviderName,
- "Eagerly Assume True"),
- eagerlyAssumeBinOpBifurcationFalse(TagProviderName,
- "Eagerly Assume False");
- return std::make_pair(&eagerlyAssumeBinOpBifurcationTrue,
- &eagerlyAssumeBinOpBifurcationFalse);
+std::pair<const ProgramPointTag *, const ProgramPointTag *>
+ExprEngine::getEagerlyAssumeOpBifurcationTags() {
+ static SimpleProgramPointTag eagerlyAssumeOpBifurcationTrue(
+ TagProviderName, "Eagerly Assume True"),
+ eagerlyAssumeOpBifurcationFalse(TagProviderName, "Eagerly Assume False");
+ return std::make_pair(&eagerlyAssumeOpBifurcationTrue,
+ &eagerlyAssumeOpBifurcationFalse);
}
-void ExprEngine::evalEagerlyAssumeBinOpBifurcation(ExplodedNodeSet &Dst,
- ExplodedNodeSet &Src,
- const Expr *Ex) {
+void ExprEngine::evalEagerlyAssumeOpBifurcation(ExplodedNodeSet &Dst,
+ ExplodedNodeSet &Src,
+ const Expr *Ex) {
StmtNodeBuilder Bldr(Src, Dst, *currBldrCtx);
for (const auto Pred : Src) {
@@ -3767,28 +3765,27 @@ void ExprEngine::evalEagerlyAssumeBinOpBifurcation(ExplodedNodeSet &Dst,
continue;
}
- ProgramStateRef state = Pred->getState();
- SVal V = state->getSVal(Ex, Pred->getLocationContext());
+ ProgramStateRef State = Pred->getState();
+ SVal V = State->getSVal(Ex, Pred->getLocationContext());
std::optional<nonloc::SymbolVal> SEV = V.getAs<nonloc::SymbolVal>();
if (SEV && SEV->isExpression()) {
- const std::pair<const ProgramPointTag *, const ProgramPointTag*> &tags =
- geteagerlyAssumeBinOpBifurcationTags();
+ const std::pair<const ProgramPointTag *, const ProgramPointTag *> &Tags =
+ getEagerlyAssumeOpBifurcationTags();
- ProgramStateRef StateTrue, StateFalse;
- std::tie(StateTrue, StateFalse) = state->assume(*SEV);
+ auto [StateTrue, StateFalse] = State->assume(*SEV);
// First assume that the condition is true.
if (StateTrue) {
SVal Val = svalBuilder.makeIntVal(1U, Ex->getType());
StateTrue = StateTrue->BindExpr(Ex, Pred->getLocationContext(), Val);
- Bldr.generateNode(Ex, Pred, StateTrue, tags.first);
+ Bldr.generateNode(Ex, Pred, StateTrue, Tags.first);
}
// Next, assume that the condition is false.
if (StateFalse) {
SVal Val = svalBuilder.makeIntVal(0U, Ex->getType());
StateFalse = StateFalse->BindExpr(Ex, Pred->getLocationContext(), Val);
- Bldr.generateNode(Ex, Pred, StateFalse, tags.second);
+ Bldr.generateNode(Ex, Pred, StateFalse, Tags.second);
}
}
}
More information about the cfe-commits
mailing list