[PATCH] D82974: [DebugInfo] Allow GNU macro extension to be read
David Stenberg via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 6 12:54:26 PDT 2020
dstenb added inline comments.
================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:3055-3059
+ auto FormToString = [](unsigned Form) {
+ return dwarf::MacroString(Form, /*GNU=*/false);
+ };
emitMacroFileImpl(F, U, dwarf::DW_MACRO_start_file,
+ dwarf::DW_MACRO_end_file, FormToString);
----------------
dblaikie wrote:
> I'd probably inline the lambda into the call expression - it's short/simple enough.
This change is now removed. I'll inline the expression in the follow-up patch.
================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h:535
+ unsigned EndFile,
+ std::function<StringRef(unsigned Form)> MacroFormToString);
void handleMacroNodes(DIMacroNodeArray Nodes, DwarfCompileUnit &U);
----------------
dblaikie wrote:
> Probably llvm::function_ref here, since the functor doesn't escape the callee
Yes, thanks! Since I moved out the GNU defines to a separate GnuMacroString() function this change is done in the patch which introduces emission of the extension.
================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugMacro.cpp:57
+ WithColor(OS, HighlightColor::Macro).get()
+ << MacroString(E.Type, /*GNU=*/Macros.Header.Version < 5);
else
----------------
ikudrin wrote:
> If this is written like
> ```
> << (Macros.Header.Version < 5 ? GnuMacroString(E.type) : MacroString(E.type)) ;
> ```
>
> then `llvm::dwarf::MacroString()` can be left as is and most of the changes are not needed.
Yes, thanks! Since there are so few uses where we conditionally want to select either the DWARF or GNU defines that seems cleaner.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82974/new/
https://reviews.llvm.org/D82974
More information about the llvm-commits
mailing list