dblaikie added a comment.

Generally OK with this, as it seemed the libc++ issue might be a bit thorny to 
solve. Though I'd like to hear from libc++ maintainers about how they feel to 
ensure they're comfortable adding this attribute in the interim (or if this 
might be a motivation to fix libc++ instead of adding the attribute)

As for the name - I take it the "if required" is meant to connote the fact that 
this attribute only has an effect if the type is used/reachable from the DWARF, 
and doesn't force the type to be emitted even when unused/unreferenced?

Maybe "full_debug_if_used"? or "debug_full_if_used" (or maybe even reuse the 
equivalent compiler flag: standalone_debug?)

In D97411#2585910 <https://reviews.llvm.org/D97411#2585910>, @rnk wrote:

> To help bikeshed the name, I can imagine a few other use cases:
>
> - an attribute to suppress type info emission, never emit full type info for 
> this type (similar to nodebug / artificial attributes for functions) -- this 
> could help optimize debug info size

nodebug is already supported on types for this purpose - we used it in libc++ 
to remove some type trait debug info to trim it down.

> - an attribute to always emit type information, whether it is required to be 
> complete or not (similar to -fno-eliminate-unused-types behavior)
> - non-use-case: It's not clear if it's useful to have an attribute to 
> explicitly indicate that a type should use the vptr, constructor, or extern 
> template heuristics.
>
> I thought maybe we could have a single attribute that takes a mode, something 
> like 
> `emit_debug_type_info("never/always/when_required_complete/with_ctor/with_vtable")`.
>  Or maybe shorter is better: `emit_typeinfo("when_required_complete")`. But 
> that sounds like we're talking about RTTI, not debug info.

Yeah, might be useful to have something that supports the different type homing 
strategies separately, though it is a more complicated user facing feature. The 
"always" mode could be handy, though I hesitate to build more features/surface 
area without users in mind, especially since these sort of things may paper 
over debug info issues we might be better off fixing generally.

I'd like to keep "debug" or "debuginfo" in the name, as you say, to avoid some 
confusion with RTTI for instance.



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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97411

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to