[PATCH] D101025: [llvm-dwarfdump] Fix abstract origin vars location stats calculation

[David Blaikie via Phabricator via llvm-commits]

dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Looks good, thanks!



================
Comment at: llvm/tools/llvm-dwarfdump/Statistics.cpp:461-462
   // Handle any kind of lexical scope.
+  const bool IsPotentialAbstractOrigin =
+      Die.find(dwarf::DW_AT_abstract_origin) != None;
   const bool IsFunction = Tag == dwarf::DW_TAG_subprogram;
----------------
maybe more accurately named `HasAbstractOrigin`? (`IsPotentialAbstractOrigin` sounds like this DIE /is/ the one referenced by an abstract_origin, rather than being the DIE with the abstract_origin)


================
Comment at: llvm/tools/llvm-dwarfdump/Statistics.cpp:225-226
+static void
+collectStatsForDie(DWARFDie Die, std::string FnPrefix, std::string VarPrefix,
+                   uint64_t BytesInScope, uint32_t InlineDepth,
+                   StringMap<PerFunctionStats> &FnStatMap,
----------------
djtodoro wrote:
> dblaikie wrote:
> > Side note: why are we passing around std::strings by value? Should these be being passed by const ref? 
> I think we should be using const ref instead.
> But it is existing code, and the change for that should be a separate commit, right?
oh, sure - easier to see this is untouched now that the renaming is separated, thanks!


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

https://reviews.llvm.org/D101025