Skip to content

Commit 0c787ef

Browse files
usx95usx95
committed
bring back comments
1 parent 927f92a commit 0c787ef

File tree

2 files changed

+91
-57
lines changed

2 files changed

+91
-57
lines changed

‎clang/lib/Analysis/LifetimeSafety.cpp

Lines changed: 76 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -503,13 +503,18 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
503503
///
504504
/// The derived class is expected to provide:
505505
/// - A `Lattice` type.
506-
/// - `Lattice getInitialState()`
507-
/// - `Lattice join(Lattice, Lattice)`
508-
/// - `Lattice transfer(Lattice, const FactType&)` for relevant fact types.
506+
/// - `const char *getAnalysisName() const`
507+
/// - `Lattice getInitialState();` The initial state at the function entry.
508+
/// - `Lattice join(Lattice, Lattice);` Merges states from multiple CFG paths.
509+
/// - `Lattice transfer(Lattice, const FactType&);` Defines how a single
510+
/// lifetime-relevant `Fact` transforms the lattice state. Only overloads
511+
/// for facts relevant to the analysis need to be implemented.
509512
///
510513
/// \tparam Derived The CRTP derived class that implements the specific
511514
/// analysis.
512515
/// \tparam LatticeType The lattice type used by the analysis.
516+
/// TODO: Maybe use the dataflow framework! The framework might need changes
517+
/// to support the current comparison done at block-entry.
513518
template <typename Derived, typename LatticeType> class DataflowAnalysis {
514519
public:
515520
using Lattice = LatticeType;
@@ -530,10 +535,12 @@ template <typename Derived, typename LatticeType> class DataflowAnalysis {
530535

531536
public:
532537
void run() {
533-
Derived &d = static_cast<Derived &>(*this);
538+
Derived &D = static_cast<Derived &>(*this);
539+
llvm::TimeTraceScope Time(D.getAnalysisName());
540+
534541
ForwardDataflowWorklist Worklist(Cfg, AC);
535542
const CFGBlock *Entry = &Cfg.getEntry();
536-
BlockEntryStates[Entry] = d.getInitialState();
543+
BlockEntryStates[Entry] = D.getInitialState();
537544
Worklist.enqueueBlock(Entry);
538545

539546
while (const CFGBlock *B = Worklist.dequeue()) {
@@ -545,9 +552,11 @@ template <typename Derived, typename LatticeType> class DataflowAnalysis {
545552
auto SuccIt = BlockEntryStates.find(Successor);
546553
Lattice OldSuccEntryState = (SuccIt != BlockEntryStates.end())
547554
? SuccIt->second
548-
: d.getInitialState();
549-
Lattice NewSuccEntryState = d.join(OldSuccEntryState, ExitState);
550-
555+
: D.getInitialState();
556+
Lattice NewSuccEntryState = D.join(OldSuccEntryState, ExitState);
557+
// Enqueue the successor if its entry state has changed.
558+
// TODO(opt): Consider changing 'join' to report a change if !=
559+
// comparison is found expensive.
551560
if (SuccIt == BlockEntryStates.end() ||
552561
NewSuccEntryState != OldSuccEntryState) {
553562
BlockEntryStates[Successor] = NewSuccEntryState;
@@ -565,7 +574,20 @@ template <typename Derived, typename LatticeType> class DataflowAnalysis {
565574
return BlockExitStates.lookup(B);
566575
}
567576

577+
void dump() const {
578+
const Derived *D = static_cast<const Derived *>(this);
579+
llvm::dbgs() << "==========================================\n";
580+
llvm::dbgs() << " " << D->getAnalysisName() << " results:\n";
581+
llvm::dbgs() << "==========================================\n";
582+
const CFGBlock &B = Cfg.getExit();
583+
getExitState(&B).dump(llvm::dbgs());
584+
}
585+
568586
private:
587+
/// Computes the exit state of a block by applying all its facts sequentially
588+
/// to a given entry state.
589+
/// TODO: We might need to store intermediate states per-fact in the block for
590+
/// later analysis.
569591
Lattice transferBlock(const CFGBlock *Block, Lattice EntryState) {
570592
Lattice BlockState = EntryState;
571593
for (const Fact *F : AllFacts.getFacts(Block)) {
@@ -618,24 +640,24 @@ struct LifetimeFactory {
618640
}
619641
};
620642

621-
/// LifetimeLattice represents the state of our analysis at a given program
622-
/// point. It is an immutable object, and all operations produce a new
643+
/// LoanPropagationLattice represents the state of our analysis at a given
644+
/// program point. It is an immutable object, and all operations produce a new
623645
/// instance rather than modifying the existing one.
624-
struct LifetimeLattice {
646+
struct LoanPropagationLattice {
625647
/// The map from an origin to the set of loans it contains.
626648
/// The lattice has a finite height: An origin's loan set is bounded by the
627649
/// total number of loans in the function.
628650
/// TODO(opt): To reduce the lattice size, propagate origins of declarations,
629651
/// not expressions, because expressions are not visible across blocks.
630652
OriginLoanMap Origins = OriginLoanMap(nullptr);
631653

632-
explicit LifetimeLattice(const OriginLoanMap &S) : Origins(S) {}
633-
LifetimeLattice() = default;
654+
explicit LoanPropagationLattice(const OriginLoanMap &S) : Origins(S) {}
655+
LoanPropagationLattice() = default;
634656

635-
bool operator==(const LifetimeLattice &Other) const {
657+
bool operator==(const LoanPropagationLattice &Other) const {
636658
return Origins == Other.Origins;
637659
}
638-
bool operator!=(const LifetimeLattice &Other) const {
660+
bool operator!=(const LoanPropagationLattice &Other) const {
639661
return !(*this == Other);
640662
}
641663

@@ -653,7 +675,7 @@ struct LifetimeLattice {
653675
};
654676

655677
class LoanPropagationAnalysis
656-
: public DataflowAnalysis<LoanPropagationAnalysis, LifetimeLattice> {
678+
: public DataflowAnalysis<LoanPropagationAnalysis, LoanPropagationLattice> {
657679

658680
LifetimeFactory &Factory;
659681

@@ -662,49 +684,58 @@ class LoanPropagationAnalysis
662684
LifetimeFactory &Factory)
663685
: DataflowAnalysis(C, AC, F), Factory(Factory) {}
664686

665-
// Make the base class's transfer overloads visible.
666687
using DataflowAnalysis<LoanPropagationAnalysis, Lattice>::transfer;
667688

689+
const char *getAnalysisName() const { return "LoanPropagation"; }
690+
668691
Lattice getInitialState() { return Lattice{}; }
669692

670-
Lattice join(Lattice L1, Lattice L2) {
693+
/// Computes the union of two lattices by performing a key-wise join of
694+
/// their OriginLoanMaps.
695+
// TODO(opt): This key-wise join is a performance bottleneck. A more
696+
// efficient merge could be implemented using a Patricia Trie or HAMT
697+
// instead of the current AVL-tree-based ImmutableMap.
698+
// TODO(opt): Keep the state small by removing origins which become dead.
699+
Lattice join(Lattice A, Lattice B) {
671700
/// Merge the smaller map into the larger one ensuring we iterate over the
672701
/// smaller map.
673-
if (L1.Origins.getHeight() < L2.Origins.getHeight())
674-
std::swap(L1, L2);
702+
if (A.Origins.getHeight() < B.Origins.getHeight())
703+
std::swap(A, B);
675704

676-
OriginLoanMap JoinedState = L1.Origins;
705+
OriginLoanMap JoinedState = A.Origins;
677706
// For each origin in the other map, union its loan set with ours.
678-
for (const auto &Entry : L2.Origins) {
707+
for (const auto &Entry : B.Origins) {
679708
OriginID OID = Entry.first;
680709
LoanSet OtherLoanSet = Entry.second;
681710
JoinedState = Factory.OriginMapFactory.add(
682-
JoinedState, OID, join(getLoans(L1, OID), OtherLoanSet));
711+
JoinedState, OID, join(getLoans(A, OID), OtherLoanSet));
683712
}
684713
return Lattice(JoinedState);
685714
}
686715

687-
LoanSet join(LoanSet S1, LoanSet S2) {
688-
if (S1.getHeight() < S2.getHeight())
689-
std::swap(S1, S2);
690-
for (LoanID L : S2)
691-
S1 = Factory.LoanSetFact.add(S1, L);
692-
return S1;
716+
LoanSet join(LoanSet A, LoanSet B) {
717+
if (A.getHeight() < B.getHeight())
718+
std::swap(A, B);
719+
for (LoanID L : B)
720+
A = Factory.LoanSetFact.add(A, L);
721+
return A;
693722
}
694723

695-
// Overloads for specific fact types this transferer cares about.
724+
/// A new loan is issued to the origin. Old loans are erased.
696725
Lattice transfer(Lattice In, const IssueFact &F) {
697726
OriginID OID = F.getOriginID();
698727
LoanID LID = F.getLoanID();
699-
return LifetimeLattice(Factory.OriginMapFactory.add(
728+
return LoanPropagationLattice(Factory.OriginMapFactory.add(
700729
In.Origins, OID, Factory.createLoanSet(LID)));
701730
}
702731

732+
/// The destination origin's loan set is replaced by the source's.
733+
/// This implicitly "resets" the old loans of the destination.
703734
Lattice transfer(Lattice In, const AssignOriginFact &F) {
704735
OriginID DestOID = F.getDestOriginID();
705736
OriginID SrcOID = F.getSrcOriginID();
706737
LoanSet SrcLoans = getLoans(In, SrcOID);
707-
return LifetimeLattice(
738+
return LoanPropagationLattice(
708739
Factory.OriginMapFactory.add(In.Origins, DestOID, SrcLoans));
709740
}
710741

@@ -743,7 +774,6 @@ struct ExpiredLattice {
743774
}
744775
};
745776

746-
/// Transfer function for the expired loans analysis.
747777
class ExpiredLoansAnalysis
748778
: public DataflowAnalysis<ExpiredLoansAnalysis, ExpiredLattice> {
749779

@@ -756,6 +786,8 @@ class ExpiredLoansAnalysis
756786

757787
using DataflowAnalysis<ExpiredLoansAnalysis, Lattice>::transfer;
758788

789+
const char *getAnalysisName() const { return "ExpiredLoans"; }
790+
759791
Lattice getInitialState() { return Lattice(SetFactory.getEmptySet()); }
760792

761793
Lattice join(Lattice L1, Lattice L2) const {
@@ -765,7 +797,6 @@ class ExpiredLoansAnalysis
765797
return Lattice(JoinedSet);
766798
}
767799

768-
// Overloads for specific fact types this transferer cares about.
769800
Lattice transfer(Lattice In, const ExpireFact &F) {
770801
return Lattice(SetFactory.add(In.Expired, F.getLoanID()));
771802
}
@@ -786,20 +817,23 @@ void runLifetimeSafetyAnalysis(const DeclContext &DC, const CFG &Cfg,
786817
FactGen.run();
787818
DEBUG_WITH_TYPE("LifetimeFacts", FactMgr.dump(Cfg, AC));
788819

789-
// Run Loan Propagation Analysis
820+
/// TODO(opt): Consider optimizing individual blocks before running the
821+
/// dataflow analysis.
822+
/// 1. Expression Origins: These are assigned once and read at most once,
823+
/// forming simple chains. These chains can be compressed into a single
824+
/// assignment.
825+
/// 2. Block-Local Loans: Origins of expressions are never read by other
826+
/// blocks; only Decls are visible. Therefore, loans in a block that
827+
/// never reach an Origin associated with a Decl can be safely dropped by
828+
/// the analysis.
790829
LifetimeFactory LifetimeFact;
791830
LoanPropagationAnalysis LoanPropagation(Cfg, AC, FactMgr, LifetimeFact);
792831
LoanPropagation.run();
793-
DEBUG_WITH_TYPE(
794-
"LifetimeDataflow",
795-
LoanPropagation.getExitState(&Cfg.getExit()).dump(llvm::dbgs()));
832+
DEBUG_WITH_TYPE("LifetimeLoanPropagation", LoanPropagation.dump());
796833

797-
// Run Expired Loans Analysis
798834
ExpiredLoansAnalysis ExpiredAnalysis(Cfg, AC, FactMgr,
799835
LifetimeFact.LoanSetFact);
800836
ExpiredAnalysis.run();
801-
DEBUG_WITH_TYPE(
802-
"ExpiredLoans",
803-
ExpiredAnalysis.getExitState(&Cfg.getExit()).dump(llvm::dbgs()));
837+
DEBUG_WITH_TYPE("LifetimeExpiredLoans", ExpiredAnalysis.dump());
804838
}
805839
} // namespace clang

‎clang/test/Sema/warn-lifetime-safety-dataflow.cpp

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %clang_cc1 -mllvm -debug-only=LifetimeFacts,LifetimeDataflow -Wexperimental-lifetime-safety %s 2>&1 | FileCheck %s
1+
// RUN: %clang_cc1 -mllvm -debug-only=LifetimeFacts,LifetimeLoanPropagation -Wexperimental-lifetime-safety %s 2>&1 | FileCheck %s
22
// REQUIRES: asserts
33

44
struct MyObj {
@@ -19,7 +19,7 @@ MyObj* return_local_addr() {
1919
// CHECK: ReturnOfOrigin (OriginID: [[O_RET_VAL]])
2020
// CHECK: Expire (LoanID: [[L_X]])
2121
}
22-
// CHECK: Dataflow results:
22+
// CHECK: LoanPropagation results:
2323
// CHECK-DAG: Origin [[O_ADDR_X]] contains Loan [[L_X]]
2424
// CHECK-DAG: Origin [[O_P]] contains Loan [[L_X]]
2525
// CHECK-DAG: Origin [[O_RET_VAL]] contains Loan [[L_X]]
@@ -47,7 +47,7 @@ MyObj* assign_and_return_local_addr() {
4747
// CHECK: ReturnOfOrigin (OriginID: [[O_PTR2_RVAL_2]])
4848
// CHECK: Expire (LoanID: [[L_Y]])
4949
}
50-
// CHECK: Dataflow results:
50+
// CHECK: LoanPropagation results:
5151
// CHECK-DAG: Origin [[O_ADDR_Y]] contains Loan [[L_Y]]
5252
// CHECK-DAG: Origin [[O_PTR1]] contains Loan [[L_Y]]
5353
// CHECK-DAG: Origin [[O_PTR2]] contains Loan [[L_Y]]
@@ -65,7 +65,7 @@ int return_int_val() {
6565
return x;
6666
}
6767
// CHECK-NEXT: End of Block
68-
// CHECK: Dataflow results:
68+
// CHECK: LoanPropagation results:
6969
// CHECK: <empty>
7070

7171

@@ -79,7 +79,7 @@ void loan_expires_cpp() {
7979
// CHECK: AssignOrigin (DestID: [[O_POBJ:[0-9]+]], SrcID: [[O_ADDR_OBJ]])
8080
// CHECK: Expire (LoanID: [[L_OBJ]])
8181
}
82-
// CHECK: Dataflow results:
82+
// CHECK: LoanPropagation results:
8383
// CHECK-DAG: Origin [[O_ADDR_OBJ]] contains Loan [[L_OBJ]]
8484
// CHECK-DAG: Origin [[O_POBJ]] contains Loan [[L_OBJ]]
8585

@@ -96,7 +96,7 @@ void loan_expires_trivial() {
9696
// CHECK-NEXT: End of Block
9797
// FIXME: Add check for Expire once trivial destructors are handled for expiration.
9898
}
99-
// CHECK: Dataflow results:
99+
// CHECK: LoanPropagation results:
100100
// CHECK-DAG: Origin [[O_ADDR_TRIVIAL_OBJ]] contains Loan [[L_TRIVIAL_OBJ]]
101101
// CHECK-DAG: Origin [[O_PTOBJ]] contains Loan [[L_TRIVIAL_OBJ]]
102102

@@ -119,7 +119,7 @@ void conditional(bool condition) {
119119
// CHECK: AssignOrigin (DestID: [[O_P_RVAL:[0-9]+]], SrcID: [[O_P]])
120120
// CHECK: AssignOrigin (DestID: [[O_Q:[0-9]+]], SrcID: [[O_P_RVAL]])
121121
}
122-
// CHECK: Dataflow results:
122+
// CHECK: LoanPropagation results:
123123
// CHECK-DAG: Origin [[O_ADDR_A]] contains Loan [[L_A]]
124124
// CHECK-DAG: Origin [[O_ADDR_B]] contains Loan [[L_B]]
125125
// CHECK-DAG: Origin [[O_P]] contains Loan [[L_A]]
@@ -163,7 +163,7 @@ void pointers_in_a_cycle(bool condition) {
163163
}
164164
// At the end of the analysis, the origins for the pointers involved in the cycle
165165
// (p1, p2, p3, temp) should all contain the loans from v1, v2, and v3 at the fixed point.
166-
// CHECK: Dataflow results:
166+
// CHECK: LoanPropagation results:
167167
// CHECK-DAG: Origin [[O_P1]] contains Loan [[L_V1]]
168168
// CHECK-DAG: Origin [[O_P1]] contains Loan [[L_V2]]
169169
// CHECK-DAG: Origin [[O_P1]] contains Loan [[L_V3]]
@@ -195,7 +195,7 @@ void overwrite_origin() {
195195
// CHECK: Expire (LoanID: [[L_S2]])
196196
// CHECK: Expire (LoanID: [[L_S1]])
197197
}
198-
// CHECK: Dataflow results:
198+
// CHECK: LoanPropagation results:
199199
// CHECK: Origin [[O_P]] contains Loan [[L_S2]]
200200
// CHECK-NOT: Origin [[O_P]] contains Loan [[L_S1]]
201201

@@ -213,7 +213,7 @@ void reassign_to_null() {
213213
}
214214
// FIXME: Have a better representation for nullptr than just an empty origin.
215215
// It should be a separate loan and origin kind.
216-
// CHECK: Dataflow results:
216+
// CHECK: LoanPropagation results:
217217
// CHECK: Origin [[O_P]] contains no loans
218218

219219

@@ -235,7 +235,7 @@ void reassign_in_if(bool condition) {
235235
// CHECK: Expire (LoanID: [[L_S2]])
236236
// CHECK: Expire (LoanID: [[L_S1]])
237237
}
238-
// CHECK: Dataflow results:
238+
// CHECK: LoanPropagation results:
239239
// CHECK-DAG: Origin [[O_P]] contains Loan [[L_S1]]
240240
// CHECK-DAG: Origin [[O_P]] contains Loan [[L_S2]]
241241
// CHECK-DAG: Origin [[O_ADDR_S1]] contains Loan [[L_S1]]
@@ -276,7 +276,7 @@ void assign_in_switch(int mode) {
276276
// CHECK-DAG: Expire (LoanID: [[L_S2]])
277277
// CHECK-DAG: Expire (LoanID: [[L_S1]])
278278
}
279-
// CHECK: Dataflow results:
279+
// CHECK: LoanPropagation results:
280280
// CHECK-DAG: Origin [[O_P]] contains Loan [[L_S1]]
281281
// CHECK-DAG: Origin [[O_P]] contains Loan [[L_S2]]
282282
// CHECK-DAG: Origin [[O_P]] contains Loan [[L_S3]]
@@ -299,7 +299,7 @@ void loan_in_loop(bool condition) {
299299
// CHECK: Expire (LoanID: [[L_INNER]])
300300
}
301301
}
302-
// CHECK: Dataflow results:
302+
// CHECK: LoanPropagation results:
303303
// CHECK-DAG: Origin [[O_P]] contains Loan [[L_INNER]]
304304
// CHECK-DAG: Origin [[O_ADDR_INNER]] contains Loan [[L_INNER]]
305305

@@ -326,7 +326,7 @@ void loop_with_break(int count) {
326326
// CHECK: Expire (LoanID: [[L_S1]])
327327
}
328328

329-
// CHECK-LABEL: Dataflow results:
329+
// CHECK-LABEL: LoanPropagation results:
330330
// CHECK-DAG: Origin [[O_P]] contains Loan [[L_S1]]
331331
// CHECK-DAG: Origin [[O_P]] contains Loan [[L_S2]]
332332
// CHECK-DAG: Origin [[O_ADDR_S1]] contains Loan [[L_S1]]
@@ -355,7 +355,7 @@ void nested_scopes() {
355355
// CHECK: Expire (LoanID: [[L_OUTER]])
356356
}
357357

358-
// CHECK-LABEL: Dataflow results:
358+
// CHECK-LABEL: LoanPropagation results:
359359
// CHECK-DAG: Origin [[O_P]] contains Loan [[L_INNER]]
360360
// CHECK-DAG: Origin [[O_ADDR_INNER]] contains Loan [[L_INNER]]
361361
// CHECK-DAG: Origin [[O_ADDR_OUTER]] contains Loan [[L_OUTER]]

0 commit comments

Comments
 (0)