[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 a comment.

In D97411#2591030 <https://reviews.llvm.org/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 <https://reviews.llvm.org/D97411#2591047>, @ldionne wrote:

> In D97411#2588181 <https://reviews.llvm.org/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 <https://reviews.llvm.org/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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97411