[PATCH] D37996: [LoopInfo] Make LoopBase and Loop destructors non-public
Chandler Carruth via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 19 14:49:37 PDT 2017
chandlerc accepted this revision.
chandlerc added a comment.
This revision is now accepted and ready to land.
LGTM, nits below.
================
Comment at: include/llvm/Analysis/LoopInfo.h:343-349
+ // Since loop passes like SCEV are allowed to key analysis results off of
+ // llvm::Loop pointers, we cannot re-use pointers within a loop pass manager.
+ // This means loop passes should not be `delete` ing llvm::Loop objects
+ // directly (and risk a later llvm::Loop allocation re-using the address of a
+ // previous one) but should be using LoopInfo::markAsRemoved, which keeps
+ // around the llvm::Loop pointer till the end of the lifetime of the
+ // llvm::LoopInfo object.
----------------
Rather than using `llvm::` prefixes, I'd just use markdown around the name, for example: ##`Loop`## and ##`LoopInfo`##.
================
Comment at: lib/Analysis/LoopInfo.cpp:625
-void LoopInfo::markAsRemoved(Loop *Unloop) {
- assert(!Unloop->isInvalid() && "Loop has already been removed");
+void LoopInfo::markAsErased(Loop *Unloop) {
+ assert(!Unloop->isInvalid() && "Loop has already been erased!");
----------------
This should really be `erase` rather than `markAsErased`... But happy for that to be a later change.
https://reviews.llvm.org/D37996
More information about the llvm-commits
mailing list