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

[Wolfgang Pieb via Phabricator via llvm-commits]

wolfgangp 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
----------------
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.


Repository:
  rL LLVM

https://reviews.llvm.org/D53545