[PATCH] D73086: [DWARF5] Added support for debug_macro section parsing and dumping in llvm-dwarfdump.

Sourabh Singh Tomar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 30 02:07:49 PDT 2020


SouraVX marked 3 inline comments as done.
SouraVX added inline comments.


================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h:111-113
+    MACINFO = 1,
+    MACINFODWO,
+    MACRO
----------------
SouraVX wrote:
> jhenderson wrote:
> > Why did you change the naming style of this? LLVM coding standards say that enums should use the same naming scheme as types: https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly.
> > 
> > Also, why the `= 1`?
> Ah Sorry, I renamed them this way, to avoid confusion of naming while calling
> `parseMacroOrMacinfo(Macro, getRecoverableErrorHandler(), MACRO);`
> 
> I'll change this as per Coding Standard.
Just want to drop a forward note here since, naming enum also as `Macro` `Macinfo`and so forth was colliding with previous definition `std::unique_ptr<DWARFDebugMacro> Macro;`
`error: Macro conflicts with a previous declaration Macro`
So I've extended enum names as
`MacroSection` `MacinfoSection` `MacinfoDwoSection`.



================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:300
+  auto Macro = std::make_unique<DWARFDebugMacro>();
+  auto parseAndDump = [&](DWARFDataExtractor &Data, bool IsMacro) {
+    if (Error Err = Macro->parse(getStringExtractor(), Data, IsMacro))
----------------
jhenderson wrote:
> SouraVX wrote:
> > SouraVX wrote:
> > > jhenderson wrote:
> > > > `ParseAndDump`. This is a (callable) object, not a function.
> > > Sorry, didn't get what you meant here ?
> > Is naming not conforming to standard are you referring here `parseAndDump`  ?
> Right. Lambdas are objects, not functions, and so should follow the variable naming scheme, not the function naming scheme.
Thanks! done locally, we'll wait for @Ikudrin before revising this.


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

https://reviews.llvm.org/D73086





More information about the llvm-commits mailing list