[PATCH] D97411: [DebugInfo] Add an attribute to force type info to be emitted for types that are required to be complete.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.


================
Comment at: clang/include/clang/Basic/Attr.td:1665
+  let Spellings = [Clang<"force_debug_if_required_type">];
+  let Subjects = SubjectList<[CXXRecord]>;
+  let Documentation = [Undocumented];
----------------
aaron.ballman wrote:
> Does this attribute have effect in C? If so, this should be `Record` instead of `CXXRecord`. If not, should this get a `LangOpts` field so the attribute is explicitly unused in C?
Seems (at least based on some limited testing I just did) we don't do type homing in C at all, even the basic "is the type required to be complete" sort of thing, like this:
```
struct s { int i; };
struct s *g;
```
compiled as C, that produces a definition of `s`, compiled as C++ it produces a declaration of `s`

(& because I was curious - we don't home enums under these rules (we handle enums differently anyway - because sometimes they're used as bags of constants, so we have to preserve their definition even when they're not referenced through the usual function/variable type links, etc))


================
Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:2402-2405
+  // Don't omit definition if marked with attribute.
+  if (RD->hasAttr<ForceDebugIfRequiredTypeAttr>())
+    return false;
+
----------------
akhuang wrote:
> rnk wrote:
> > dblaikie wrote:
> > > Perahps this should be a bit further up in this function? For instance the template specialization homing seems like it'd override this behavior. Perhaps someone has a template specialization that shouldn't be homed for some reason? & similarly with modular debug info (both the "isDefinediInClangModule" and "hasExternalDefinitions" cases are different kinds of modular debug info homing)
> > > 
> > > Hmm, I guess the modular debug info is less likely to have these sort of problems - it's more explicit about what is homed, both explicitly making a home, and explicitly communicating the presence of a home to other compilations that rely on that data. So it might be unfortunate to pessimize that scenario unnecessarily.
> > > 
> > > @rnk - any thoughts on the tradeoff of uniformity of the attribute (ie: applies to all type homing strategies) V applying the attribute to address shortcomings in the source that only affect some homing strategies and not others?
> > I think it makes sense to move this up so that the user can work around extern template type homing, but we probably don't need to override module type homing behavior.
> > 
> > I think if we're using modular debug info, we can be pretty confident that the module will provide debug info, but it's easy to construct scenarios where the extern template is provided by a TU that doesn't enable debug info. This attribute would allow the user to work around that without going all the way to enabling fstandalone-debug, which typically generates too much data to be usable.
> Isn't it already above the template specialization part? Or which part is that?
Ah, so it is. Not sure what/how I was reading it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97411