ymandel marked 2 inline comments as done. ymandel added inline comments.
================ Comment at: clang/lib/Analysis/IntervalPartition.cpp:121 + Index.emplace(N, ID); + Graph.Intervals.emplace_back(ID, Header, std::move(Data.Nodes)); } ---------------- xazax.hun wrote: > It would probably take for me some time to understand what is going on here, > but I think this part might worth a comment. In particular, I have the > impression that `Graph.Intervals` is the owner of all the intervals. But we > use pointers to refer to these intervals. What would guarantee that those > pointers are not being invalidated by this `emplace_back` operations? Excellent question, not least because I *wasn't* careful the first around and indeed ran up against this as a memory bug. :) That's the reason I added IDs, so that I could _avoid_ taking the addresses of elements until after we finish growing the vector. See lines 148-165: after we finish building the intervals, we go through and take addresses. I can add comments to this effect, but perhaps the safe option is to use a std::dequeue. What do you think? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153058/new/ https://reviews.llvm.org/D153058 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
