llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tidy @llvm/pr-subscribers-clang Author: Donát Nagy (NagyDonat) <details> <summary>Changes</summary> 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`. --- Full diff: https://github.com/llvm/llvm-project/pull/112209.diff 5 Files Affected: - (modified) clang-tools-extra/clang-tidy/ClangTidy.cpp (-1) - (modified) clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h (+3-5) - (modified) clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h (+6-6) - (modified) clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (+1-1) - (modified) clang/lib/StaticAnalyzer/Core/ExprEngine.cpp (+19-22) ``````````diff 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); } } } `````````` </details> https://github.com/llvm/llvm-project/pull/112209 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
