[PATCH] D50213: DebugInfo: Add metadata support for disabling DWARF pub sections

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In https://reviews.llvm.org/D50213#1200917, @aprantl wrote:

> > Currently has no effect, but see above ^ - I reckon it probably should. Maybe we can generalize the name of the property? "nameTable: " {None, Default (v4 or v5 based on DWARF version), GNU (only makes sense for v4 though - ignored if you end up building v5? seems reasonable to me)}
>
> Is there a test covering the DWARF5 + GNU CU flag combination?


Nah - for now they are orthogonal (pubnames (GNU or otherwise) are emitted wherever they're emitted, regardless of whether DWARF5 is used) so a test doesn't seem appropriate.

But, yeah, I think in a follow-up patch making DWARF5 disable any form of pubnames would be appropriate (given the amount of work needed for a consumer to support DWARF5, it doesn't seem unreasonable to say "hey, also you've got to support the new name table if you want a name table" (if the hash table is too complex for the consumer - they can ignore that part and use the rest of teh name table still as something like pubnames - a flat list  - if I recall the format))



================
Comment at: lib/Bitcode/Reader/MetadataLoader.cpp:1393
         Record.size() <= 17 ? false : Record[17],
-        Record.size() <= 18 ? false : Record[18]);
+        Record.size() <= 18 ? 0 : Record[18]);
 
----------------
aprantl wrote:
> Can you add a bitcode upgrade test?
I could - but I'm not sure it would test anything. The format has in some sense remained the same - this field is still zero/one before/after the change, but now new values are also valid.

Reckon it's still worth testing? What do you reckon that testing would cover?


Repository:
  rL LLVM

https://reviews.llvm.org/D50213