[llvm] r191407 - Unify pubsection/gnu pubsection printing.

Eric Christopher echristo at gmail.com
Wed Sep 25 16:53:54 PDT 2013


>
> The two conditions here are basically the same for the Offset and Name
> fields, except for two extra spaces of indent (and whether or not the name
> is quoted?). Should we just make them actually the same & conditionalize the
> extra field like this:
>
>
> OS << format("0x%8.8x ", dieRef)
> if (GnuStyle) {
>   PubIndexEntryDescriptor desc(pubNames.getU8(&offset));
>   OS << format("%-8s ", dwarf::GDBIndexEntryLinkageString(desc.Linkage))
>         << format("%-8s ", dwarf::GDBIndexEntryKindString(desc.Kind));
> }
> OS << '\"' << pubNames.getCStr(&offset) << "\"\n";
>
> (alternatively, rather than hiding the spaces at the end of the format
> strings, they could be printed at the beginning of the field that requires
> them - I don't feel strongly either way)
>

No preference really here. I think the spaces should definitely be at
the end of the strings, but it might be nice to have them still listed
explicitly. Hard to see otherwise.

>>    if (DumpType == DIDT_All || DumpType == DIDT_GnuPubnames)
>>      dumpPubSection(OS, "debug_gnu_pubnames", getGnuPubNamesSection(),
>> -                   isLittleEndian());
>> +                   isLittleEndian(), true /* GnuStyle */);
>
>
> If you feel the need to comment these parameters (which I don't disagree
> with, as we've discussed) - perhaps just introduce a trivial two-state enum?
>

Works too. Sure.

>
> Your change looked like it didn't include quoting here - but perhaps that
> was added in one of your follow-on changes.
>

It was... as the bots noticed. :)

-eric



More information about the llvm-commits mailing list