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

[akhuang]

Summary

This was motivated by the fact that constructor type homing (debug info
optimization that we want to turn on by default) drops some libc++ types,
so an attribute would allow us to override constructor homing and emit
them anyway.

This attribute is called __attribute__((standalone_debug)) and classes with
the attribute will get debug info in the same way as if they were built with
-fstandalone-debug. So, essentially, it overrides all of the debug info optimizations
that are used in -fstandalone-debug.

[rnk]

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
• 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.

clang/include/clang/Basic/Attr.td

1679

I understand that this patch is mainly to open discussion, but we should document the attribute before landing the patch.

clang/test/CodeGenCXX/force-debug-attribute.cpp

10–11 ↗

(On Diff #326188)

This attribute should also work with vtable and extern template instantiation type homing, right? So for example, I would see this template type in the debug info in the standard compilation modes as well:

template <typename T> struct Foo { Foo() {} T x; };
extern template struct DEBUGASNEEDED Foo<int>; // I expect to see a complete Foo<int> DICompositeType

[dblaikie]

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, @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.

clang/lib/CodeGen/CGDebugInfo.cpp

2403–2406

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?

[rnk]

I expected nodebug already applied to types, even though the documentation says it only affects variables and functions. We should update the docs.

I think if we already have nodebug spelling, we don't need to make something general with modes. The "always emit" use case is fairly weak. It's easy to drop in a static_assert(sizeof(Ty) > 0, "emit type info"); to get type info if you want it.

If we're looking at a no-argument attribute, my naming ideas lean towards incorporating "required [to be] complete" in the name. There are many ways that one can "use" or "require" a type that work fine with a forward declaration. Something like emit_debug_typeinfo_if_required_complete? Still too big? emit_debug_if_required_complete?

clang/lib/CodeGen/CGDebugInfo.cpp

2403–2406

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.

[dblaikie]

In D97411#2586055, @rnk wrote:

I expected nodebug already applied to types, even though the documentation says it only affects variables and functions. We should update the docs.

Now that I think about it more, the only thing I made it work for is typedefs, not whole structs - it'd be trickier to do it for a whole struct (what happens if you have a member of that type in another struct? Does it leave a black hole?) - but easy for a typedef: any reference to the typedef is instead a direct reference to the underlying type.

I think if we already have nodebug spelling, we don't need to make something general with modes. The "always emit" use case is fairly weak. It's easy to drop in a static_assert(sizeof(Ty) > 0, "emit type info"); to get type info if you want it.

I don't think that static_assert would cause 'Ty' to be emitted - at least for DWARF/ELF I don't know of a way to force a type to be used that doesn't generate any code, for instance.

If we're looking at a no-argument attribute, my naming ideas lean towards incorporating "required [to be] complete" in the name. There are many ways that one can "use" or "require" a type that work fine with a forward declaration. Something like emit_debug_typeinfo_if_required_complete? Still too big? emit_debug_if_required_complete?

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

[akhuang]

In 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

clang/lib/CodeGen/CGDebugInfo.cpp

2403–2406

Isn't it already above the template specialization part? Or which part is that?

[aaron.ballman]

clang/include/clang/Basic/Attr.td

1678

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?

[dblaikie]

clang/include/clang/Basic/Attr.td

1678

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

clang/lib/CodeGen/CGDebugInfo.cpp

2403–2406

Ah, so it is. Not sure what/how I was reading it.

[akhuang]

clang/include/clang/Basic/Attr.td

1678

Yep, there's a check for LangOpts.CPlusPlus before the debug optimizations.

Maybe should change it to Record anyway, even though it does nothing in C at the moment.

[dblaikie]

clang/include/clang/Basic/Attr.td

1678

I'd probably keep it to C++ if that's the only place it works, so people aren't confused if they put it in C code but it does nothing yet produces no error about misuse.

(only tradeoff there, is if the type is in a C header, meant for use from C and C++ - the attribute would have to be conditional on whether the code is being parsed as C++ - but that seems OK to me)

[aaron.ballman]

clang/include/clang/Basic/Attr.td

1678

I'd probably keep it to C++ if that's the only place it works, so people aren't confused if they put it in C code but it does nothing yet produces no error about misuse.

+1 -- it's easy for us to lift the restriction if/when C support appears.

(only tradeoff there, is if the type is in a C header, meant for use from C and C++ - the attribute would have to be conditional on whether the code is being parsed as C++ - but that seems OK to me)

I think that's defensible behavior.

[akhuang]

@ldionne Do you think it'd be reasonable to add this debug info attribute to some types in libc++? (For types that have constructors but don't call them; some previous discussion in https://reviews.llvm.org/D90719).

[akhuang]

-Add LangOpts[CPlusPlus] to the attribute
-Maybe change the attribute to also override required complete types and change name to standalone_debug

[dblaikie]

clang/test/CodeGenCXX/force-debug-attribute.cpp

4–12 ↗

(On Diff #326438)

Probably rename the macro here to match the new attribute name.

[aaron.ballman]

Can you also add semantic tests that verify the attribute only works in C++, only appertains to a record (as opposed to other kinds of declarations like functions), and doesn't accept arguments. Some other tests that may be interesting:

// Should test redeclaration behavior.
struct [[clang::standalone_debug]] redecl;
struct redecl {};

struct [[clang::standalone_debug]] S; // Does this make sense on forward declare that's never defined?

struct foo {
  struct [[clang::standalone_debug]] { // anonymous types?
    int x;
  };
};

union [[clang::standalone_debug]] U { // Non-struct record types?
  int x;
  float f;
};

union [[clang::standalone_debug]] Oof { // Non-struct record types with a constructor?
  Oof() {}
  int x;
  float f;
};

clang/include/clang/Basic/Attr.td

1679

+1 to this (no new undocumented attributes, please).

clang/test/CodeGenCXX/force-debug-attribute.cpp

1 ↗

(On Diff #326438)

Is there something C++14 specific about the test, or should that be dropped?

Should we specify a triple? (I have no idea if debug info is target-specific.)

[akhuang]

-Add documentation
-Add more test cases and semantics tests

[aaron.ballman]

Thanks! I am still curious about the forward declare/redeclaration behavior and whether that is a situation that makes sense or not. I suspect this case may make sense (and likely already works):

// Should test redeclaration behavior.
struct [[clang::standalone_debug]] redecl;
struct redecl {};

but I'm not certain if this case makes sense:

struct [[clang::standalone_debug]] S; // Does this make sense on forward declare that's never defined?

clang/test/Sema/attr-standalonedebug.cpp

2–5

[ldionne]

In D97411#2588181, @akhuang wrote:

@ldionne Do you think it'd be reasonable to add this debug info attribute to some types in libc++? (For types that have constructors but don't call them; some previous discussion in https://reviews.llvm.org/D90719).

Like I said in D90719, I think it would be best to fix the issue at its root and call those constructors (we must have UB if we use these types but never call any constructors, right?).

[dblaikie]

In D97411#2591030, @aaron.ballman wrote:

Thanks! I am still curious about the forward declare/redeclaration behavior and whether that is a situation that makes sense or not. I suspect this case may make sense (and likely already works):

// Should test redeclaration behavior.
struct [[clang::standalone_debug]] redecl;
struct redecl {};

but I'm not certain if this case makes sense:

struct [[clang::standalone_debug]] S; // Does this make sense on forward declare that's never defined?

It's not meaningful on a pure declaration - and I don't think there's much value in providing support for applying it to a declaration iff that declaration is followed by a definition.

(any prior art for other attributes we should try to be consistent with for an attribute that only has an effect on how a definition is handled?)

In D97411#2591047, @ldionne wrote:

In D97411#2588181, @akhuang wrote:

@ldionne Do you think it'd be reasonable to add this debug info attribute to some types in libc++? (For types that have constructors but don't call them; some previous discussion in https://reviews.llvm.org/D90719).

Like I said in D90719, I think it would be best to fix the issue at its root and call those constructors (we must have UB if we use these types but never call any constructors, right?).

I'm generally in favor of that direction, but it doesn't look like I or @akhuang have the context to fix libc++ - if you or other libc++ maintainers might be able to help it'd be great.

Yes, I believe there is UB - if I recall correctly from some codepaths "create" instances of these objects without calling their ctor & use a custom deleter in a unique_ptr or similar to ensure the dtor doesn't run, while other uses call the ctor and let the dtor run as normal. Given the destruction semantics aren't built into the type, but imposed by the unique_ptr (or similar) outside, it's tricky to change the type. Perhaps changing the type to have a data buffer to construct into, using a factory function for the current "does real construction" case and then the custom deleter inverts - defaulting to doing nothing, but when activated deletes the object in the data buffer.

Though perhaps the issue is that the default codepath (that currently deals with real constructed/destructed objects) doesn't have space for customizing the deleter...

It's weird/complicated, would really like help fixing it, if this attribute sort of path isn't an option.

[akhuang]

In D97411#2591030, @aaron.ballman wrote:

Thanks! I am still curious about the forward declare/redeclaration behavior and whether that is a situation that makes sense or not. I suspect this case may make sense (and likely already works):

// Should test redeclaration behavior.
struct [[clang::standalone_debug]] redecl;
struct redecl {};

but I'm not certain if this case makes sense:

struct [[clang::standalone_debug]] S; // Does this make sense on forward declare that's never defined?

The purpose of the redecl being to test if the attribute still works when there's a redeclaration? I don't know if testing just the forward declare makes sense - I think the attribute wouldn't do anything in that case?

Oh, also, what's the difference between writing __attribute__((attrname)) and [[clang::attrname]]?

In D97411#2591047, @ldionne wrote:

In D97411#2588181, @akhuang wrote:

@ldionne Do you think it'd be reasonable to add this debug info attribute to some types in libc++? (For types that have constructors but don't call them; some previous discussion in https://reviews.llvm.org/D90719).

Like I said in D90719, I think it would be best to fix the issue at its root and call those constructors (we must have UB if we use these types but never call any constructors, right?).

True, I am still planning to look into the libc++ fix, but not sure how large / difficult that change would be. In the meantime we could do this? (And I think it would still be nice to have the attribute, regardless of the libc++ thing).

[akhuang]

address comments

[ldionne]

I don't have an opinion about the attribute itself. I do have an opinion about using that attribute in libc++ instead of fixing the underlying issue (I think we shouldn't do it). Can you confirm what the problematic types are? In another patch I saw __hash_node, __hash_value_type, __tree_node and __value_type. Is that it?

[akhuang]

In D97411#2594345, @ldionne wrote:

I don't have an opinion about the attribute itself. I do have an opinion about using that attribute in libc++ instead of fixing the underlying issue (I think we shouldn't do it). Can you confirm what the problematic types are? In another patch I saw __hash_node, __hash_value_type, __tree_node and __value_type. Is that it?

Not entirely sure - those were pointed out as types with missing debug info, but there might be more. I tried looking for types in libc++ that have a value_type member, since those seem to follow a similar pattern. Possibly __forward_list_node?

[dblaikie]

In D97411#2595049, @akhuang wrote:

In D97411#2594345, @ldionne wrote:

I don't have an opinion about the attribute itself. I do have an opinion about using that attribute in libc++ instead of fixing the underlying issue (I think we shouldn't do it). Can you confirm what the problematic types are? In another patch I saw __hash_node, __hash_value_type, __tree_node and __value_type. Is that it?

Not entirely sure - those were pointed out as types with missing debug info, but there might be more. I tried looking for types in libc++ that have a value_type member, since those seem to follow a similar pattern. Possibly __forward_list_node?

Since we'll need to identify this list of types either way (to attribute or to fix) might be worth making the list - I'd guess building all the libc++ tests with/without ctor homing and seeing which types go missing as a result might be informative?

[akhuang]

In D97411#2595155, @dblaikie wrote:

In D97411#2595049, @akhuang wrote:

In D97411#2594345, @ldionne wrote:

I don't have an opinion about the attribute itself. I do have an opinion about using that attribute in libc++ instead of fixing the underlying issue (I think we shouldn't do it). Can you confirm what the problematic types are? In another patch I saw __hash_node, __hash_value_type, __tree_node and __value_type. Is that it?

Not entirely sure - those were pointed out as types with missing debug info, but there might be more. I tried looking for types in libc++ that have a value_type member, since those seem to follow a similar pattern. Possibly __forward_list_node?

Since we'll need to identify this list of types either way (to attribute or to fix) might be worth making the list - I'd guess building all the libc++ tests with/without ctor homing and seeing which types go missing as a result might be informative?

I started looking into some diffs of debug info in libc++ tests, but it's pretty hard to tell what's different - as far as I can see, there are just a bunch of __hash_value_types and __value_types.

If I look in Visual Studio's debug info (on a file where I just construct all the libc++ types) I get errors for __list_node, __tree_node, and __hash_node. That's probably about it, and if there end up being more, it should be pretty straightforward to fix those later?

[dblaikie]

In D97411#2598625, @akhuang wrote:

In D97411#2595155, @dblaikie wrote:

In D97411#2595049, @akhuang wrote:

In D97411#2594345, @ldionne wrote:

I don't have an opinion about the attribute itself. I do have an opinion about using that attribute in libc++ instead of fixing the underlying issue (I think we shouldn't do it). Can you confirm what the problematic types are? In another patch I saw __hash_node, __hash_value_type, __tree_node and __value_type. Is that it?

Not entirely sure - those were pointed out as types with missing debug info, but there might be more. I tried looking for types in libc++ that have a value_type member, since those seem to follow a similar pattern. Possibly __forward_list_node?

Since we'll need to identify this list of types either way (to attribute or to fix) might be worth making the list - I'd guess building all the libc++ tests with/without ctor homing and seeing which types go missing as a result might be informative?

I started looking into some diffs of debug info in libc++ tests, but it's pretty hard to tell what's different - as far as I can see, there are just a bunch of __hash_value_types and __value_types.

Happy to help if there's anything I can do there (did a lot of comparisons like this when adding other type homing strategies and diffing GCC and Clang output in the past) dumping the fully qualified names (or mangled names) of all the types (llvm-dwarfdump has a mode where it specifically dumps the names of types in type units that I've used for this purpose in the past, for instance) then diffing, stripping off uninteresting template parameters to deduplicate, etc, possibly.

If I look in Visual Studio's debug info (on a file where I just construct all the libc++ types) I get errors for __list_node, __tree_node, and __hash_node. That's probably about it, and if there end up being more, it should be pretty straightforward to fix those later?

Yeah - though having at least a rough list to get a sense of the scale of the work's probably helpful. Maybe that list is these 3-4 names. (are they each singular types - or are there several distinct uses of "__tree_node" as a name in different contexts that would need to be fixed separately, for instance?)

[akhuang]

I think they're all singular types, like __hash_node. Actually, __tree_node might not be problematic.

I could look more at the types if it's helpful; I think the other issue is that there are a lot of libc++ tests and most are not really useful, and I'm kind of building them all and putting all the diffs together. It also seems like the __hash_node definitions are all present in the ctor homing dwarfdumps; it's just __hash_value_type and __value_type that are missing. Not sure why; I would have expected stuff like __hash_node to also be missing.

(I saw some other missing types while diffing (like _Storage and __shared_mutex_base) but looking at the code, it seems like both of those types get constructed, so not sure what's going on there..)

[probinson]

In D97411#2598625, @akhuang wrote:

I started looking into some diffs of debug info in libc++ tests, but it's pretty hard to tell what's different - as far as I can see, there are just a bunch of __hash_value_types and __value_types.

This is a job for.... llvm-dva! See the preliminary patch at D88661, although it's getting a bit old and might not apply/build cleanly.

(llvm-dva is undergoing an internal review at the moment, we hope to have a proper reviewable patch series up soon-ish.)

[CarlosAlbertoEnciso]

In D97411#2611142, @probinson wrote:

In D97411#2598625, @akhuang wrote:

I started looking into some diffs of debug info in libc++ tests, but it's pretty hard to tell what's different - as far as I can see, there are just a bunch of __hash_value_types and __value_types.

This is a job for.... llvm-dva! See the preliminary patch at D88661, although it's getting a bit old and might not apply/build cleanly.

(llvm-dva is undergoing an internal review at the moment, we hope to have a proper reviewable patch series up soon-ish.)

Uploaded an updated llvm-dva single patch the builds with the TOT.

[akhuang]

@EricWF, comments on using this attribute in libc++ vs fixing the code otherwise?

[EricWF]

Due to some complexities libc++ has dealing with the allocator, as well some optimizations it would be a serious undertaking to restructure libc++ to properly construct the types in the hash containers,
This change would be greatly simpler and much appreciated.

[dblaikie]

Looks good to me!

clang/lib/CodeGen/CGDebugInfo.cpp

2388–2390

Perfect - I love the symmetry/simplicity of this approach, easy to explain (as you have in the documentation) to users, etc.

[aaron.ballman]

LGTM with a small, technical nit to be fixed.

clang/include/clang/Basic/Attr.td

1677

I think this should be Clang<"standalone_debug", /*allowInC*/0> so that __has_c_attribute reports false for it.

[aaron.ballman]

I should note, you need to also fix the CI build failures: https://reviews.llvm.org/harbormaster/unit/view/421174/

[akhuang]

update test case

[akhuang]

add allowInC to attribute

[akhuang]

In D97411#2611142, @probinson wrote:

In D97411#2598625, @akhuang wrote:

I started looking into some diffs of debug info in libc++ tests, but it's pretty hard to tell what's different - as far as I can see, there are just a bunch of __hash_value_types and __value_types.

This is a job for.... llvm-dva! See the preliminary patch at D88661, although it's getting a bit old and might not apply/build cleanly.

(llvm-dva is undergoing an internal review at the moment, we hope to have a proper reviewable patch series up soon-ish.)

cool, thanks! I tried using it to compare a file that just constructs all of the libc++ types, and it works pretty well (definitely nicer than processing/diffing the dwarf dumps).

One thing I ran into is that I think it'll classify a type as missing/added if it appears in a different order. Not sure if there's a way to make it ignore order, but it wasn't too hard to look through them all.

Anyway, comparing the types did find one class (allocator) that I didn't include the list of types to add the attribute to. Seems like it has some empty inline constructors that are marked constexpr after c++17. Maybe we don't need to have it because it doesn't affect printing the types in the debugger.

[dblaikie]

In D97411#2638609, @akhuang wrote:

In D97411#2611142, @probinson wrote:

In D97411#2598625, @akhuang wrote:

I started looking into some diffs of debug info in libc++ tests, but it's pretty hard to tell what's different - as far as I can see, there are just a bunch of __hash_value_types and __value_types.

This is a job for.... llvm-dva! See the preliminary patch at D88661, although it's getting a bit old and might not apply/build cleanly.

(llvm-dva is undergoing an internal review at the moment, we hope to have a proper reviewable patch series up soon-ish.)

cool, thanks! I tried using it to compare a file that just constructs all of the libc++ types, and it works pretty well (definitely nicer than processing/diffing the dwarf dumps).

One thing I ran into is that I think it'll classify a type as missing/added if it appears in a different order. Not sure if there's a way to make it ignore order, but it wasn't too hard to look through them all.

Anyway, comparing the types did find one class (allocator) that I didn't include the list of types to add the attribute to. Seems like it has some empty inline constructors that are marked constexpr after c++17. Maybe we don't need to have it because it doesn't affect printing the types in the debugger.

Hmm - is that type used in a way that invokes Undefined Behavior? Or is this a gap/bug in the ctor homing? I thought there was already a special case for constexpr ctors that opted them out of ctor homing.

[akhuang]

Hmm - is that type used in a way that invokes Undefined Behavior? Or is this a gap/bug in the ctor homing? I thought there was already a special case for constexpr ctors that opted them out of ctor homing.

Right, I think the constexpr is only used in >c++17 for some reason, so doesn't apply here. I looked around a bit but still not sure how/where allocators are constructed.

[dblaikie]

In D97411#2642194, @akhuang wrote:

Hmm - is that type used in a way that invokes Undefined Behavior? Or is this a gap/bug in the ctor homing? I thought there was already a special case for constexpr ctors that opted them out of ctor homing.

Right, I think the constexpr is only used in >c++17 for some reason, so doesn't apply here. I looked around a bit but still not sure how/where allocators are constructed.

Fair enough - guess that's a discussion to have on a separate thread/when we talk about adding the attribute to libc++/or when bugfixing the ctor homing, if it turns out/looks like a gap in ctor homing to address there. (happy to discuss it whenever - email/chat/etc)

[akhuang]

In D97411#2642782, @dblaikie wrote:

In D97411#2642194, @akhuang wrote:

Hmm - is that type used in a way that invokes Undefined Behavior? Or is this a gap/bug in the ctor homing? I thought there was already a special case for constexpr ctors that opted them out of ctor homing.

Right, I think the constexpr is only used in >c++17 for some reason, so doesn't apply here. I looked around a bit but still not sure how/where allocators are constructed.

Fair enough - guess that's a discussion to have on a separate thread/when we talk about adding the attribute to libc++/or when bugfixing the ctor homing, if it turns out/looks like a gap in ctor homing to address there. (happy to discuss it whenever - email/chat/etc)

Oh, I have already created a patch for adding to libc++ (https://reviews.llvm.org/D98750), although no comments on it yet.

Yeah, I'll try to figure out what's going on with allocator; I wasn't planning on adding the attribute there though because it didn't seem to affect displaying the debug info

clang/include/clang/Basic/Attr.td

 }
 
 def NoDebug : InheritableAttr {
   let Spellings = [GCC<"nodebug">];
   let Subjects = SubjectList<[TypedefName, FunctionLike, ObjCMethod, NonParmVar]>;
   let Documentation = [NoDebugDocs];
 }
 
+def StandaloneDebug : InheritableAttr {
+  let Spellings = [Clang<"standalone_debug", /*allowInC =*/0>];
+  let Subjects = SubjectList<[CXXRecord]>;
+  let Documentation = [StandaloneDebugDocs];
+  let SimpleHandler = 1;
+  let LangOpts = [CPlusPlus];
+}
+
 def NoDuplicate : InheritableAttr {
   let Spellings = [Clang<"noduplicate">];
   let Subjects = SubjectList<[Function]>;
   let Documentation = [NoDuplicateDocs];
   let SimpleHandler = 1;
 }
 
 def Convergent : InheritableAttr {

clang/include/clang/Basic/AttrDocs.td

def NoDebugDocs : Documentation {
   let Category = DocCatVariable;
   let Content = [{
 The ``nodebug`` attribute allows you to suppress debugging information for a
 function or method, for a variable that is not a parameter or a non-static
 data member, or for a typedef or using declaration.
   }];
 }
 
+def StandaloneDebugDocs : Documentation {
+  let Category = DocCatVariable;
+  let Content = [{
+The ``standalone_debug`` attribute causes debug info to be emitted for a record
+type regardless of the debug info optimizations that are enabled with
+-fno-standalone-debug. This attribute only has an effect when debug info
+optimizations are enabled (e.g. with -fno-standalone-debug), and is C++-only.
+  }];
+}
+
 def NoDuplicateDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{
 The ``noduplicate`` attribute can be placed on function declarations to control
 whether function calls to this function can be duplicated or not as a result of
 optimizations. This is required for the implementation of functions with
 certain special requirements, like the OpenCL "barrier" function, that might
 need to be run concurrently by all the threads that are executing in lockstep

clang/lib/CodeGen/CGDebugInfo.cpp

if (ES->hasExternalDefinitions(RD) == ExternalASTSource::EK_Always)
       return true;
 
   // Only emit forward declarations in line tables only to keep debug info size
   // small. This only applies to CodeView, since we don't emit types in DWARF
   // line tables only.
   if (DebugKind == codegenoptions::DebugLineTablesOnly)
     return true;
 
-  if (DebugKind > codegenoptions::LimitedDebugInfo)
+  if (DebugKind > codegenoptions::LimitedDebugInfo ||
+      RD->hasAttr<StandaloneDebugAttr>())
     return false;
 
   if (!LangOpts.CPlusPlus)
     return false;
 
   if (!RD->isCompleteDefinitionRequired())
     return true;
 
   const auto *CXXDecl = dyn_cast<CXXRecordDecl>(RD);
 
   if (!CXXDecl)
     return false;
 
   // Only emit complete debug info for a dynamic class when its vtable is
   // emitted.  However, Microsoft debuggers don't resolve type information
   // across DLL boundaries, so skip this optimization if the class or any of its
   // methods are marked dllimport. This isn't a complete solution, since objects
   // without any dllimport methods can be used in one DLL and constructed in
   // another, but it is the current behavior of LimitedDebugInfo.
   if (CXXDecl->hasDefinition() && CXXDecl->isDynamicClass() &&
       !isClassOrMethodDLLImport(CXXDecl))
     return true;
 
   TemplateSpecializationKind Spec = TSK_Undeclared;
   if (const auto *SD = dyn_cast<ClassTemplateSpecializationDecl>(RD))

clang/test/CodeGenCXX/standalone-debug-attribute.cpp

+// RUN: %clang_cc1 -DSETATTR=0 -triple x86_64-unknown-linux-gnu -emit-llvm -debug-info-kind=constructor %s -o - | FileCheck %s --check-prefix=DEBUG
+// RUN: %clang_cc1 -DSETATTR=1 -triple x86_64-unknown-linux-gnu -emit-llvm -debug-info-kind=constructor %s -o - | FileCheck %s --check-prefix=WITHATTR
+// Use -debug-info-kind=constructor because it includes all the optimizations.
+
+#if SETATTR
+#define STANDALONEDEBUGATTR __attribute__((standalone_debug))
+#else
+#define STANDALONEDEBUGATTR
+#endif
+
+struct STANDALONEDEBUGATTR StructWithConstructor {
+  StructWithConstructor() {}
+};
+void f(StructWithConstructor s) {}
+// DEBUG:  !DICompositeType({{.*}}name: "StructWithConstructor"
+// DEBUG-SAME:              flags: {{.*}}DIFlagFwdDecl
+// WITHATTR: !DICompositeType({{.*}}name: "StructWithConstructor"
+// WITHATTR-NOT: DIFlagFwdDecl
+
+union STANDALONEDEBUGATTR UnionWithConstructor {
+  UnionWithConstructor() {}
+};
+void f(UnionWithConstructor u) {}
+// DEBUG:  !DICompositeType({{.*}}name: "UnionWithConstructor"
+// DEBUG-SAME:              flags: {{.*}}DIFlagFwdDecl
+// WITHATTR: !DICompositeType({{.*}}name: "UnionWithConstructor"
+// WITHATTR-NOT: DIFlagFwdDecl
+
+template <typename T> struct ExternTemplate {
+  ExternTemplate() {}
+  T x;
+};
+extern template struct STANDALONEDEBUGATTR ExternTemplate<int>;
+void f(ExternTemplate<int> s) {}
+// DEBUG: !DICompositeType({{.*}}name: "ExternTemplate<int>"
+// DEBUG-SAME:             flags: {{.*}}DIFlagFwdDecl
+// WITHATTR: !DICompositeType({{.*}}name: "ExternTemplate<int>"
+// WITHATTR-NOT: DIFlagFwdDecl
+
+struct STANDALONEDEBUGATTR CompleteTypeRequired {};
+void f(CompleteTypeRequired &s) {}
+// DEBUG: !DICompositeType({{.*}}name: "CompleteTypeRequired"
+// DEBUG-SAME:             flags: {{.*}}DIFlagFwdDecl
+// WITHATTR: !DICompositeType({{.*}}name: "CompleteTypeRequired"
+// WITHATTR-NOT: DIFlagFwdDecl
+
+struct STANDALONEDEBUGATTR Redecl;
+struct Redecl {};
+void f(Redecl &s) {}
+// DEBUG: !DICompositeType({{.*}}name: "Redecl"
+// DEBUG-SAME:             flags: {{.*}}DIFlagFwdDecl
+// WITHATTR: !DICompositeType({{.*}}name: "Redecl"
+// WITHATTR-NOT: DIFlagFwdDecl
+

clang/test/Misc/pragma-attribute-supported-attributes-list.test

 // CHECK-NEXT: Restrict (SubjectMatchRule_function)
 // CHECK-NEXT: ReturnTypestate (SubjectMatchRule_function, SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: ReturnsNonNull (SubjectMatchRule_objc_method, SubjectMatchRule_function)
 // CHECK-NEXT: ReturnsTwice (SubjectMatchRule_function)
 // CHECK-NEXT: ScopedLockable (SubjectMatchRule_record)
 // CHECK-NEXT: Section (SubjectMatchRule_function, SubjectMatchRule_variable_is_global, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property)
 // CHECK-NEXT: SetTypestate (SubjectMatchRule_function_is_member)
 // CHECK-NEXT: SpeculativeLoadHardening (SubjectMatchRule_function, SubjectMatchRule_objc_method)
+// CHECK-NEXT: StandaloneDebug (SubjectMatchRule_record)
 // CHECK-NEXT: SwiftAsync (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: SwiftAsyncError (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: SwiftAsyncName (SubjectMatchRule_objc_method, SubjectMatchRule_function)
 // CHECK-NEXT: SwiftBridgedTypedef (SubjectMatchRule_type_alias)
 // CHECK-NEXT: SwiftContext (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: SwiftError (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: SwiftErrorResult (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: SwiftIndirectResult (SubjectMatchRule_variable_is_parameter)

clang/test/Sema/attr-standalonedebug.cpp

+// RUN: %clang_cc1 %s -verify -fsyntax-only
+// RUN: %clang_cc1 %s -verify -fsyntax-only -x c
+
+#ifdef __cplusplus
+int a __attribute__((standalone_debug)); // expected-warning {{'standalone_debug' attribute only applies to classes}}
+
+void __attribute__((standalone_debug)) b(); // expected-warning {{'standalone_debug' attribute only applies to classes}}
+
+struct __attribute__((standalone_debug(1))) c {}; // expected-error {{'standalone_debug' attribute takes no arguments}}
+
+#else
+// Check that attribute only works in C++.
+struct __attribute__((standalone_debug)) a {}; // expected-warning {{'standalone_debug' attribute ignored}}
+#endif