[PATCH] D150713: [llvm-debuginfo-analyzer] Support both Reference and Type attrs in single DIE

[Carlos Alberto Enciso via Phabricator via llvm-commits]

CarlosAlbertoEnciso added inline comments.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Readers/LVELFReader.cpp:1131
+                                            bool IsType) {
+  auto Iter = ElementTable.try_emplace(Offset).first;
+  // Update the element and all the references pointing to this element.
----------------
scott.linder wrote:
> CarlosAlbertoEnciso wrote:
> > In the case of a new `offset` the original code created:
> > 
> > ```
> > [Offset, LVElementEntry(nullptr, {Element}) ]
> > ```
> > The new code:
> > 
> > ```
> > [Offset, LVElementEntry(nullptr, References{}, Types{}) ]
> > ```
> > The `Element` parameter is not recorded.
> It is handled below now, as the change also removes the `if/else`: we use `try_emplace` to find the entry at `Offset`, creating an empty one if none exists. Then, we always add `Element` to the appropriate set.
> 
> So the previously explicitly special-cased `Iter == ElementTable.end()` case is all contained in this first line of the function.
Sorry; I missed the `Element` assignment.
The change looks great.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150713