[llvm-branch-commits] [clang] [clang][NFC] Make `CFGStmtMap` `const`-correct (PR #172529)
https://github.com/davidstone created
https://github.com/llvm/llvm-project/pull/172529
None
>From 34fdb92ec425776892a78ec8ca2d49070ea397d2 Mon Sep 17 00:00:00 2001
From: David Stone
Date: Mon, 15 Dec 2025 15:22:47 -0700
Subject: [PATCH] [clang][NFC] Make `CFGStmtMap` `const`-correct
---
clang/include/clang/Analysis/AnalysisDeclContext.h| 2 +-
clang/include/clang/Analysis/CFGStmtMap.h | 10 +-
clang/lib/Analysis/AnalysisDeclContext.cpp| 4 ++--
clang/lib/Analysis/CFGStmtMap.cpp | 11 +--
.../StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp | 3 ++-
clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp | 2 +-
clang/lib/StaticAnalyzer/Core/CallEvent.cpp | 2 +-
7 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/clang/include/clang/Analysis/AnalysisDeclContext.h
b/clang/include/clang/Analysis/AnalysisDeclContext.h
index ced4bb8595bea..76fb96bacc377 100644
--- a/clang/include/clang/Analysis/AnalysisDeclContext.h
+++ b/clang/include/clang/Analysis/AnalysisDeclContext.h
@@ -151,7 +151,7 @@ class AnalysisDeclContext {
CFG *getCFG();
- CFGStmtMap *getCFGStmtMap();
+ const CFGStmtMap *getCFGStmtMap();
CFGReverseBlockReachabilityAnalysis *getCFGReachablityAnalysis();
diff --git a/clang/include/clang/Analysis/CFGStmtMap.h
b/clang/include/clang/Analysis/CFGStmtMap.h
index 2a997df20c5eb..cde24b9f2b282 100644
--- a/clang/include/clang/Analysis/CFGStmtMap.h
+++ b/clang/include/clang/Analysis/CFGStmtMap.h
@@ -23,18 +23,18 @@ class ParentMap;
class Stmt;
class CFGStmtMap {
- using SMap = llvm::DenseMap;
- ParentMap *PM;
+ using SMap = llvm::DenseMap;
+ const ParentMap *PM;
SMap M;
- CFGStmtMap(ParentMap *pm, SMap m) : PM(pm), M(std::move(m)) {}
+ CFGStmtMap(const ParentMap *pm, SMap m) : PM(pm), M(std::move(m)) {}
- static void Accumulate(SMap &SM, CFGBlock *B);
+ static void Accumulate(SMap &SM, const CFGBlock *B);
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(CFG* C, ParentMap *PM);
+ static CFGStmtMap *Build(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 f188fc6921ed1..f683b9efc1d1e 100644
--- a/clang/lib/Analysis/AnalysisDeclContext.cpp
+++ b/clang/lib/Analysis/AnalysisDeclContext.cpp
@@ -250,11 +250,11 @@ CFG *AnalysisDeclContext::getUnoptimizedCFG() {
return completeCFG.get();
}
-CFGStmtMap *AnalysisDeclContext::getCFGStmtMap() {
+const CFGStmtMap *AnalysisDeclContext::getCFGStmtMap() {
if (cfgStmtMap)
return cfgStmtMap.get();
- if (CFG *c = getCFG()) {
+ if (const CFG *c = getCFG()) {
cfgStmtMap.reset(CFGStmtMap::Build(c, &getParentMap()));
return cfgStmtMap.get();
}
diff --git a/clang/lib/Analysis/CFGStmtMap.cpp
b/clang/lib/Analysis/CFGStmtMap.cpp
index a8de07c0ce10a..eb42c8663526f 100644
--- a/clang/lib/Analysis/CFGStmtMap.cpp
+++ b/clang/lib/Analysis/CFGStmtMap.cpp
@@ -34,7 +34,7 @@ const CFGBlock *CFGStmtMap::getBlock(const Stmt *S) const {
return nullptr;
}
-void CFGStmtMap::Accumulate(SMap &SM, CFGBlock *B) {
+void CFGStmtMap::Accumulate(SMap &SM, const CFGBlock *B) {
// First walk the block-level expressions.
for (const CFGElement &CE : *B) {
if (std::optional CS = CE.getAs()) {
@@ -43,17 +43,17 @@ void CFGStmtMap::Accumulate(SMap &SM, CFGBlock *B) {
}
// Look at the label of the block.
- if (Stmt *Label = B->getLabel())
+ if (const Stmt *Label = B->getLabel())
SM[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 (Stmt *Term = B->getTerminatorStmt())
+ if (const Stmt *Term = B->getTerminatorStmt())
SM[Term] = B;
}
-CFGStmtMap *CFGStmtMap::Build(CFG *C, ParentMap *PM) {
+CFGStmtMap *CFGStmtMap::Build(const CFG *C, const ParentMap *PM) {
if (!C || !PM)
return nullptr;
@@ -61,9 +61,8 @@ CFGStmtMap *CFGStmtMap::Build(CFG *C, ParentMap *PM) {
// Walk all blocks, accumulating the block-level expressions, labels,
// and terminators.
- for (CFGBlock *BB : *C)
+ for (const CFGBlock *BB : *C)
Accumulate(SM, BB);
return new CFGStmtMap(PM, std::move(SM));
}
-
diff --git a/clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
index 309e3d250de06..49b20fc877188 100644
--- a/clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
@@ -162,7 +162,8 @@ class AnalysisOrderChecker
return;
llvm::errs() << "CFGElement: ";
- CFG
[llvm-branch-commits] [clang] [clang][NFC] Use constructor instead of factory function in `CFGStmtMap` (PR #172530)
https://github.com/davidstone created
https://github.com/llvm/llvm-project/pull/172530
`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.
>From 65c435c2e939c959c4179d05bd4d51058277b422 Mon Sep 17 00:00:00 2001
From: David Stone
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/include/clang/Analysis/AnalysisDeclContext.h | 4 ++--
clang/include/clang/Analysis/CFGStmtMap.h | 6 +-
clang/lib/Analysis/AnalysisDeclContext.cpp | 6 +++---
clang/lib/Analysis/CFGStmtMap.cpp | 13 +++--
clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp| 1 -
5 files changed, 9 insertions(+), 21 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, completeCFG;
- std::unique_ptr cfgStmtMap;
+ std::optional 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 cde24b9f2b282..c1aa3d2a78bcc 100644
--- a/clang/include/clang/Analysis/CFGStmtMap.h
+++ b/clang/include/clang/Analysis/CFGStmtMap.h
@@ -27,14 +27,10 @@ class CFGStmtMap {
const ParentMap *PM;
SMap M;
- CFGStmtMap(const ParentMap *pm, SMap m) : PM(pm), M(std::move(m)) {}
-
static void Accumulate(SMap &SM, const CFGBlock *B);
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 eb42c8663526f..177aa1a7af686 100644
--- a/clang/lib/Analysis/CFGStmtMap.cpp
+++ b/clang/lib/Analysis/CFGStmtMap.cpp
@@ -5
