[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#1187506, @JDevlieghere wrote:

> Regarding the naming: I like `PubnameKind` because of the aforementioned consistency (we also have `AccelTableKind`).


Fair enough - happy to go with that. (though AccelTableKind isn't as interesting an example - since it doesn't appear in the IR, unlike pubnames and emissionKind - which is really where I'm more interested in the consistency of the name (I don't so much mind if the type itself is called PubnamesKind, but about whether the IR field should be called "pubnames: " or "pubnamesKind: ", etc (currently it's a boolean, "gnuPubnames: true" for example)))

> Can't we use an `enum class` to prevent collisions?

Yep, though that'd be a bit inconsistent with the other enums that don't use that. But I'm OK with that.

Actually - potentially we could merge the accelerator table options into this too, to promote them to an IR feature as well. But it looks like they aren't really supported on a per-CU basis, but are across the whole compilation only. (read the spec of debug_names - I see, best to emit the widest entry possible (ie: bad idea to emit one table per CU if we're emitting multiple CUs in a single object file (LTO) - and no need not to emit it if it's been requested, I suppose - though I guess a user might reasonably request no accelerator table for /some/ CUs they expect not to debug much/are OK with the debugger indexing slowly later on if they do)). But Apple's names don't support multiple CUs at all, so they can't be unified there anyway.

OK, so maybe renaming this to something generic like "nameTable" {on, off, default} on a per-CU basis. If accelerator tables are in-use, then filter the names that go into it on the basis of this property (well, maybe Apple doesn't want to support that option with Apple tables? There doesn't seem to be any notion that the Apple accelerator table could be incomplete - CUs don't have a flag saying "I'm covered by an accelerator table" and the tables don't have an entry saying "these CUs are covered here").

@aprantl - reckon it'd be too much of a foot-gun to have a -gno-pubnames (or perhaps we could use a more general name in addition to this) that'd probably produce a rather bad (names not found) experience when combined with Apple accelerator tables.

In https://reviews.llvm.org/D50213#1187570, @probinson wrote:

> How does this interact with v5 .debug_names?


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)}


Repository:
  rL LLVM

https://reviews.llvm.org/D50213