[PATCH] D43473: [mem2reg] Use range loops (NFCI)

[Brian Gesiak via Phabricator via llvm-commits]

modocache added inline comments.


================
Comment at: llvm/trunk/lib/Transforms/Utils/PromoteMemoryToRegister.cpp:329
+  for (User *U : AI->users()) {
+    Instruction *I = cast<Instruction>(U);
     if (isa<LoadInst>(I) || isa<StoreInst>(I))
----------------
bjope wrote:
> I think the old code was more safe since it does not depend as much on how the iterators are implemented. The instruction "I" is actually deleted inside  the loop. So stepping the iterator after deleting "I" could be dangerous if stepping of the iterator involves dereferencing of the object that the iterator refers. Or am I missing something?
> I would at least assume that the old code was written the way it was just because the datastructure that we iterate over can change during iteration. So it should be more safe to step the iterator to the "next" element first, and then remove the "current" element.
> 
Aha, that's a very good point, thanks! I was looking at the references to `U` but didn't see that `I` was being erased. I guess the same can be said for the loop below as well. So of the three loops in this diff, only the first loop appears to be convertible to a range loop.


Repository:
  rL LLVM

https://reviews.llvm.org/D43473