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

[Jeremy Morse via Phabricator via llvm-commits]

jmorse added a comment.

Looking good with a couple of comments and the harder case of the DW_AT_inline copy being out-of-order wanting to be solved.



================
Comment at: llvm/test/tools/llvm-dwarfdump/X86/locstats-for-inlined-vars.yaml:36-37
+##       DW_AT_call_column     (0x01)
+##       DW_TAG_formal_parameter
+##         DW_AT_abstract_origin       (0x00000018)
+##       DW_TAG_variable
----------------
Could I request a third inlining site with no DIE at all for this variable -- that'll ensure we test the behaviour that D94976 is trying to create.


================
Comment at: llvm/tools/llvm-dwarfdump/Statistics.cpp:439
   const bool IsInlinedFunction = Tag == dwarf::DW_TAG_inlined_subroutine;
+  std::string FnID;
   if (IsFunction || IsInlinedFunction || IsBlock) {
----------------
This doesn't need to be in the outer scope, does it? IMO better to keep the declaration where it was, as it's only used in that block.


================
Comment at: llvm/tools/llvm-dwarfdump/Statistics.cpp:556-558
+    InlineSiteVars.erase(
+        std::remove(InlineSiteVars.begin(), InlineSiteVars.end(), Offset),
+        InlineSiteVars.end());
----------------
I get the feeling modifying this vector while iterating over it is vulnerable to re-ordering / re-allocating.


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

https://reviews.llvm.org/D96045