[PATCH] D96045: [llvm-dwarfdump][locstats] Unify handling of inlined vars with no loc

[Djordje Todorovic via Phabricator via llvm-commits]

djtodoro added inline comments.


================
Comment at: llvm/tools/llvm-dwarfdump/Statistics.cpp:653-655
+          InlinedVars.erase(std::remove(InlinedVars.begin(), InlinedVars.end(),
+                                        (*OffsetVar).getRawUValue()),
+                            InlinedVars.end());
----------------
dblaikie wrote:
> djtodoro wrote:
> > dblaikie wrote:
> > > jmorse wrote:
> > > > Isn't this "removing" twice, once with std::remove, the other with the erase method?
> > > Ah the joys of C++. This code is correct and an application of the classic "erase remove" idiom ( https://en.wikipedia.org/wiki/Erase%E2%80%93remove_idiom ).
> > > 
> > > Though we do have an llmv wrapper that tidies this up a bit, llvm::erase_if and llvm::erase_value
> > Yeah, it is an idiom for eliminating elements from stl containers.
> > 
> > @dblaikie I think that using of the `std::` ones is recommended approach, right? Or do you proposing using of the `llvm::` ones?
> generally we use the llvm ones, that's why we have them - they simplify the API (make it range based instead of iterator pair based, and in this case wrap up the common erase+remove idiom into a single more legible function call (avoiding the confusion @jmorse ran into when reading this code, for instance))
OK, it sounds reasonable to me. Thanks.


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

https://reviews.llvm.org/D96045