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

[Amy Huang via Phabricator via cfe-commits]

akhuang marked an inline comment as done.
akhuang added a comment.

In D97411#2586201 <https://reviews.llvm.org/D97411#2586201>, @dblaikie wrote:

> Oh, that might be a bit different again - if the type isn't required to be complete in this translation unit, should this attribute override the "required to be complete" homing strategy? (that's the oldest standing strategy - if the type isn't required to be complete, the definition is omitted) I'd have thought this should override that behavior too. Perhaps the type is never dereferenced, but is somehow still useful (eg: you might have one translation unit that only uses handles, and another translation unit that never has a variable of that type (maybe deals with void*) and casts to the defined type and dereferences it - that would break the "required to be complete" homing strategy (though it's unlikely/weird - if you're passing around void* it's unlikely your caller would somehow see and use the declared type anyway))

Yeah, seems like it makes sense to have the attribute do the same thing as -fstandalone-debug if just for consistency



================
Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:2402-2405
+  // Don't omit definition if marked with attribute.
+  if (RD->hasAttr<ForceDebugIfRequiredTypeAttr>())
+    return false;
+
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97411