[PATCH] D59552: [Linker] Fix crash handling appending linkage

[Rafael Auler via Phabricator via llvm-commits]

rafauler marked 3 inline comments as done.
rafauler added a comment.

Thanks for your feedback, Eli



================
Comment at: lib/Linker/IRMover.cpp:1059
+void IRLinker::flushRAUWWorklist() {
+  std::reverse(RAUWWorklist.begin(), RAUWWorklist.end());
+  while (!RAUWWorklist.empty()) {
----------------
efriedma wrote:
> Why is the call to std::reverse() necessary?  There shouldn't be any other code touching the worklist, so you can just iterate over it normally.
Indeed, I'll fix this


================
Comment at: lib/Linker/IRMover.cpp:1067
+    Old->replaceAllUsesWith(New);
+    Old->eraseFromParent();
+  }
----------------
efriedma wrote:
> ValueMap and AliasValueMap use raw pointers for keys.  Do we need to update them if one of those pointers is deleted?
This is handled in Value's destructor


================
Comment at: lib/Linker/IRMover.cpp:1396
       return std::move(*FoundError);
+    flushRAUWWorklist();
   }
----------------
efriedma wrote:
> I guess you can just flush the worklist here because the mapValue() invariant only applies inside a single call?
That's correct


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59552/new/

https://reviews.llvm.org/D59552