https://github.com/davidstone updated https://github.com/llvm/llvm-project/pull/172530
>From 3a8a9dd1d6e9da537a47c94ffbc9fe9b94e7bbb1 Mon Sep 17 00:00:00 2001 From: David Stone <[email protected]> Date: Tue, 16 Dec 2025 11:15:00 -0700 Subject: [PATCH] [clang][NFC] Use constructor instead of factory function in `CFGStmtMap` `CFGStmtMap::Build` accepts pointers and returns a pointer to dynamically allocated memory. In the one location where the type is actually constructed, the pointers are guaranteed to be non-null. By accepting references to statically enforce this, we can remove the only way for the construction to fail. By making this change, we also allow our user to decide how they want to own the memory (either directly or indirectly). The user does not actually need dynamic allocation here, so we replace the `std::unique_ptr` with `std::optional`. This simplifies the code by requiring fewer checks, makes comments on what happens redundant because the code can obviously do only one thing, avoids potential bugs, and improves performance by allocating less. --- .../clang/Analysis/AnalysisDeclContext.h | 4 ++-- clang/include/clang/Analysis/CFGStmtMap.h | 9 ++------- clang/lib/Analysis/AnalysisDeclContext.cpp | 6 +++--- clang/lib/Analysis/CFGStmtMap.cpp | 17 +++++------------ clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp | 1 - 5 files changed, 12 insertions(+), 25 deletions(-) diff --git a/clang/include/clang/Analysis/AnalysisDeclContext.h b/clang/include/clang/Analysis/AnalysisDeclContext.h index 76fb96bacc377..2368b8ed911e7 100644 --- a/clang/include/clang/Analysis/AnalysisDeclContext.h +++ b/clang/include/clang/Analysis/AnalysisDeclContext.h @@ -20,6 +20,7 @@ #include "clang/AST/DeclBase.h" #include "clang/Analysis/BodyFarm.h" #include "clang/Analysis/CFG.h" +#include "clang/Analysis/CFGStmtMap.h" #include "clang/Analysis/CodeInjector.h" #include "clang/Basic/LLVM.h" #include "llvm/ADT/DenseMap.h" @@ -37,7 +38,6 @@ class ASTContext; class BlockDecl; class BlockInvocationContext; class CFGReverseBlockReachabilityAnalysis; -class CFGStmtMap; class ImplicitParamDecl; class LocationContext; class LocationContextManager; @@ -77,7 +77,7 @@ class AnalysisDeclContext { const Decl *const D; std::unique_ptr<CFG> cfg, completeCFG; - std::unique_ptr<CFGStmtMap> cfgStmtMap; + std::optional<CFGStmtMap> cfgStmtMap; CFG::BuildOptions cfgBuildOptions; CFG::BuildOptions::ForcedBlkExprs *forcedBlkExprs = nullptr; diff --git a/clang/include/clang/Analysis/CFGStmtMap.h b/clang/include/clang/Analysis/CFGStmtMap.h index e83a00a2508e9..ad58f0b806c52 100644 --- a/clang/include/clang/Analysis/CFGStmtMap.h +++ b/clang/include/clang/Analysis/CFGStmtMap.h @@ -23,16 +23,11 @@ class ParentMap; class Stmt; class CFGStmtMap { - using SMap = llvm::DenseMap<const Stmt *, const CFGBlock *>; const ParentMap *PM; - SMap M; - - CFGStmtMap(const ParentMap *pm, SMap m) : PM(pm), M(std::move(m)) {} + llvm::DenseMap<const Stmt *, const CFGBlock *> M; public: - /// Returns a new CFGMap for the given CFG. It is the caller's - /// responsibility to 'delete' this object when done using it. - static CFGStmtMap *Build(const CFG *C, const ParentMap *PM); + CFGStmtMap(const CFG &C, const ParentMap &PM); /// Returns the CFGBlock the specified Stmt* appears in. For Stmt* that /// are terminators, the CFGBlock is the block they appear as a terminator, diff --git a/clang/lib/Analysis/AnalysisDeclContext.cpp b/clang/lib/Analysis/AnalysisDeclContext.cpp index f683b9efc1d1e..6f153e7e65255 100644 --- a/clang/lib/Analysis/AnalysisDeclContext.cpp +++ b/clang/lib/Analysis/AnalysisDeclContext.cpp @@ -252,11 +252,11 @@ CFG *AnalysisDeclContext::getUnoptimizedCFG() { const CFGStmtMap *AnalysisDeclContext::getCFGStmtMap() { if (cfgStmtMap) - return cfgStmtMap.get(); + return &*cfgStmtMap; if (const CFG *c = getCFG()) { - cfgStmtMap.reset(CFGStmtMap::Build(c, &getParentMap())); - return cfgStmtMap.get(); + cfgStmtMap.emplace(*c, getParentMap()); + return &*cfgStmtMap; } return nullptr; diff --git a/clang/lib/Analysis/CFGStmtMap.cpp b/clang/lib/Analysis/CFGStmtMap.cpp index adbec58210954..ae8955922fef5 100644 --- a/clang/lib/Analysis/CFGStmtMap.cpp +++ b/clang/lib/Analysis/CFGStmtMap.cpp @@ -33,31 +33,24 @@ const CFGBlock *CFGStmtMap::getBlock(const Stmt *S) const { return nullptr; } -CFGStmtMap *CFGStmtMap::Build(const CFG *C, const ParentMap *PM) { - if (!C || !PM) - return nullptr; - - SMap SM; - +CFGStmtMap::CFGStmtMap(const CFG &C, const ParentMap &PM) : PM(&PM) { // Walk all blocks, accumulating the block-level expressions, labels, // and terminators. - for (const CFGBlock *B : *C) { + for (const CFGBlock *B : C) { // First walk the block-level expressions. for (const CFGElement &CE : *B) { if (std::optional<CFGStmt> CS = CE.getAs<CFGStmt>()) - SM.try_emplace(CS->getStmt(), B); + M.try_emplace(CS->getStmt(), B); } // Look at the label of the block. if (const Stmt *Label = B->getLabel()) - SM[Label] = B; + M[Label] = B; // Finally, look at the terminator. If the terminator was already added // because it is a block-level expression in another block, overwrite // that mapping. if (const Stmt *Term = B->getTerminatorStmt()) - SM[Term] = B; + M[Term] = B; } - - return new CFGStmtMap(PM, std::move(SM)); } diff --git a/clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp b/clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp index 53fec3467b835..159555aad4a7e 100644 --- a/clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp @@ -16,7 +16,6 @@ #include "clang/AST/ExprObjC.h" #include "clang/AST/ParentMap.h" #include "clang/AST/Stmt.h" -#include "clang/Analysis/CFGStmtMap.h" #include "clang/Analysis/ProgramPoint.h" #include "clang/Analysis/Support/BumpVector.h" #include "clang/Basic/LLVM.h" _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
