Skip to content

Commit bdec502

Browse files
committed
remove AccessPath::Kind
1 parent 634ba4b commit bdec502

File tree

3 files changed

+21
-29
lines changed

3 files changed

+21
-29
lines changed

clang/include/clang/Analysis/Analyses/LifetimeSafety.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@
2222
#include "clang/Analysis/CFG.h"
2323
namespace clang {
2424

25-
void runLifetimeAnalysis(const DeclContext &DC, const CFG &Cfg,
26-
AnalysisDeclContext &AC);
25+
void runLifetimeSafetyAnalysis(const DeclContext &DC, const CFG &Cfg,
26+
AnalysisDeclContext &AC);
2727

2828
} // namespace clang
2929

clang/lib/Analysis/LifetimeSafety.cpp

Lines changed: 15 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -25,21 +25,11 @@ namespace {
2525

2626
/// Represents the storage location being borrowed, e.g., a specific stack
2727
/// variable.
28+
/// TODO: Model access paths of other types, e.g., s.field, heap and globals.
2829
struct AccessPath {
2930
const clang::ValueDecl *D;
3031

31-
enum class Kind : uint8_t {
32-
StackVariable,
33-
Temporary, // TODO: Handle.
34-
Field, // TODO: Handle like `s.y`.
35-
Heap, // TODO: Handle.
36-
ArrayElement, // TODO: Handle.
37-
Static, // TODO: Handle.
38-
};
39-
40-
Kind PathKind;
41-
42-
AccessPath(const clang::ValueDecl *D, Kind K) : D(D), PathKind(K) {}
32+
AccessPath(const clang::ValueDecl *D) : D(D) {}
4333
};
4434

4535
/// A generic, type-safe wrapper for an ID, distinguished by its `Tag` type.
@@ -399,11 +389,11 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
399389
if (!hasOrigin(ICE->getType()))
400390
return;
401391
Visit(ICE->getSubExpr());
402-
/// TODO: Consider if this is actually useful in practice. Alternatively, we
403-
/// could directly use the sub-expression's OriginID instead of creating a
404-
/// new one.
405392
// An ImplicitCastExpr node itself gets an origin, which flows from the
406393
// origin of its sub-expression (after stripping its own parens/casts).
394+
// TODO: Consider if this is actually useful in practice. Alternatively, we
395+
// could directly use the sub-expression's OriginID instead of creating a
396+
// new one.
407397
addAssignOriginFact(*ICE, *ICE->getSubExpr());
408398
}
409399

@@ -415,9 +405,9 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
415405
// Check if it's a local variable.
416406
if (VD->hasLocalStorage()) {
417407
OriginID OID = FactMgr.getOriginMgr().getOrCreate(*UO);
418-
AccessPath AddrOfLocalVarPath(VD, AccessPath::Kind::StackVariable);
419-
Loan &L = FactMgr.getLoanMgr().addLoan(AddrOfLocalVarPath,
420-
UO->getOperatorLoc());
408+
AccessPath AddrOfLocalVarPath(VD);
409+
const Loan &L = FactMgr.getLoanMgr().addLoan(AddrOfLocalVarPath,
410+
UO->getOperatorLoc());
421411
CurrentBlockFacts.push_back(
422412
FactMgr.createFact<IssueFact>(L.ID, OID));
423413
}
@@ -469,6 +459,8 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
469459
/// TODO: Also handle trivial destructors (e.g., for `int`
470460
/// variables) which will never have a CFGAutomaticObjDtor node.
471461
/// TODO: Handle loans to temporaries.
462+
/// TODO: Consider using clang::CFG::BuildOptions::AddLifetime to reuse the
463+
/// lifetime ends.
472464
const VarDecl *DestructedVD = DtorOpt.getVarDecl();
473465
if (!DestructedVD)
474466
return;
@@ -479,9 +471,8 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
479471
const AccessPath &LoanPath = L.Path;
480472
// Check if the loan is for a stack variable and if that variable
481473
// is the one being destructed.
482-
if (LoanPath.PathKind == AccessPath::Kind::StackVariable)
483-
if (LoanPath.D == DestructedVD)
484-
CurrentBlockFacts.push_back(FactMgr.createFact<ExpireFact>(L.ID));
474+
if (LoanPath.D == DestructedVD)
475+
CurrentBlockFacts.push_back(FactMgr.createFact<ExpireFact>(L.ID));
485476
}
486477
}
487478

@@ -495,9 +486,9 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
495486
// ========================================================================= //
496487
} // anonymous namespace
497488

498-
void runLifetimeAnalysis(const DeclContext &DC, const CFG &Cfg,
499-
AnalysisDeclContext &AC) {
500-
llvm::TimeTraceScope TimeProfile("Lifetime Analysis");
489+
void runLifetimeSafetyAnalysis(const DeclContext &DC, const CFG &Cfg,
490+
AnalysisDeclContext &AC) {
491+
llvm::TimeTraceScope TimeProfile("LifetimeSafetyAnalysis");
501492
DEBUG_WITH_TYPE("PrintCFG", Cfg.dump(AC.getASTContext().getLangOpts(),
502493
/*ShowColors=*/true));
503494
FactManager FactMgr;

clang/lib/Sema/AnalysisBasedWarnings.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2746,6 +2746,8 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings(
27462746
.setAlwaysAdd(Stmt::UnaryOperatorClass);
27472747
}
27482748

2749+
bool EnableLifetimeSafetyAnalysis = !Diags.isIgnored(
2750+
diag::warn_experimental_lifetime_safety_dummy_warning, D->getBeginLoc());
27492751
// Install the logical handler.
27502752
std::optional<LogicalErrorHandler> LEH;
27512753
if (LogicalErrorHandler::hasActiveDiagnostics(Diags, D->getBeginLoc())) {
@@ -2870,11 +2872,10 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings(
28702872

28712873
// TODO: Enable lifetime safety analysis for other languages once it is
28722874
// stable.
2873-
if (!Diags.isIgnored(diag::warn_experimental_lifetime_safety_dummy_warning,
2874-
D->getBeginLoc()) &&
2875+
if (EnableLifetimeSafetyAnalysis &&
28752876
S.getLangOpts().CPlusPlus) {
28762877
if (CFG *cfg = AC.getCFG())
2877-
runLifetimeAnalysis(*cast<DeclContext>(D), *cfg, AC);
2878+
runLifetimeSafetyAnalysis(*cast<DeclContext>(D), *cfg, AC);
28782879
}
28792880
// Check for violations of "called once" parameter properties.
28802881
if (S.getLangOpts().ObjC && !S.getLangOpts().CPlusPlus &&

0 commit comments

Comments
 (0)