[PATCH] D53545: [DWARF][NFC] NFC patch for reverted r342218 (refactoring rangelist handling)

[Paul Robinson via Phabricator via llvm-commits]

probinson added inline comments.


================
Comment at: llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFListTable.h:165
+  /// True if this list is located in a split-DWARF (dwo or dwp) file.
+  bool isDWO;
   /// This string is displayed as a heading before the list is dumped
----------------
wolfgangp wrote:
> dblaikie wrote:
> > wolfgangp wrote:
> > > dblaikie wrote:
> > > > echristo wrote:
> > > > > dblaikie wrote:
> > > > > > echristo wrote:
> > > > > > > Trying to figure out a way around isDWO as a boolean argument and data member would be great.
> > > > > > Avoiding it being a boolean, or avoiding having the same name for an argument and data member at the same time?
> > > > > The former. I'd like to avoid the boolean existing if we can avoid it, and definitely as an argument to the constructor as well. 
> > > > Ah, OK. In the "boolean arguments are hard to read" (eg: replace them with an enum) or the "this is extra state/information that this part of the code might, hopefully, not need to know"?
> > > Yeah, it looks like it's always available from the context. If all goes well with this patch I'll try to do this as a follow-on if that's OK with you.
> > Worth a shot - though one wrinkle: Support for non-split Split DWARF. I know the llvm-dwarfdump support for this is incomplete, but it's something to keep in mind - "isDWO" might not be a constant/globally true thing even for a single object file - it may contain both split and non-split content in the same file.
> I had interpreted "isDWO" as something that derives from the section you're extracting from, i.e. when extracting a table from either .debug_rnglists or .debug_rnglists.dwo, so we should be able to treat the presence of the non-split content orthogonally, hopefully.
My impression is that "isDWO" generally guides how to interpret cross-section references, so references from debug_foo.dwo would go to debug_bar.dwo instead of debug_bar.  The dumper and verifier don't do cross-file stuff?
The debugger would though... so in the interest of converging the LLVM and LLDB readers, we might want to upgrade the bool to a 3-value enum at some point.


Repository:
  rL LLVM

https://reviews.llvm.org/D53545