[PATCH] D38055: Tighten the invariants around LoopBase::invalidate

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added a comment.

Generally like the API changes here. The enum seems a big improvement.



================
Comment at: include/llvm/Analysis/LoopInfo.h:68-82
+template <class BlockT, class LoopT> class LoopBase {
   LoopT *ParentLoop;
   // Loops contained entirely within this one.
   std::vector<LoopT *> SubLoops;
 
   // The list of blocks in this loop. First entry is the header node.
+  std::vector<BlockT *> Blocks;
----------------
Given the number of formatting changes here, can you do an initial NFC commit w/ clang-format and rebase?


================
Comment at: include/llvm/Analysis/LoopInfo.h:85
+
+  void invalidate() {
+    IsInvalid = true;
----------------
I find `clear` a better name for these methods. And eventually you may just run the destructor.

I don't want it to be confused with invalidating analyses *on* the loop with a loop analysis manager.


================
Comment at: include/llvm/Analysis/LoopInfo.h:86
+  void invalidate() {
+    IsInvalid = true;
+    SubLoops.clear();
----------------
This seems redundant if you set `ParentLoop` to null? I guess not for top-level loops.

But it is also redundant with an empty `Blocks` vector. Anyways, I'd try to remove this at some point.


https://reviews.llvm.org/D38055