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

[David Blaikie via Phabricator via llvm-commits]

dblaikie 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());
----------------
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))


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

https://reviews.llvm.org/D96045