[PATCH] D50094: Introduce DebugCounter into ConstProp pass

George Burgess IV via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 1 13:32:01 PDT 2018


george.burgess.iv added inline comments.


================
Comment at: lib/Transforms/Scalar/ConstantProp.cpp:88
+  // Iterate element from the worklist in stable order
+  for (Instruction *I: WorkListVec) {
+    WorkList.erase(I);                  // Remove element from the worklist...
----------------
george.burgess.iv wrote:
> We can't do a range for loop here: they expand to (roughly; I'm too lazy to bring up the standard, and the differences don't matter here :) )
> 
> ```
> for (auto Begin = WorkListVec.begin(), End = WorkListVec.end(); Begin != End; ++Begin) {
>   Instruction *I = *Begin;
>   {
>     // Loop body
>   }
> }
> ```
> 
> This is problematic, since `WorkListVec.push_back()` invalidates iterators, but we're still using the now-invalid iterators.
> 
> Please either make this use indices, or a `while (!vec.empty()) { auto *i = vec.pop_back_val(); }`-like thing
I think there was a misunderstanding here. Please use an index instead of an iterator. The loop should look like

```
for (unsigned I = 0; I < WorkListVec.size(); ++I) {
  ...
}
```


Repository:
  rL LLVM

https://reviews.llvm.org/D50094





More information about the llvm-commits mailing list