[PATCH] D37996: [LoopInfo] Make LoopBase and Loop destructors non-public

[Chandler Carruth via Phabricator via llvm-commits]

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