[DWARF-5] Support for C++11 defaulted, deleted member functions.

[SouraVX]

Summary

This patch provides DWARF5 support for C++11 defaulted, deleted member.
Added support in clang C++ frontend, llvm, and llvm-dwarfdump.

[SouraVX]

Minor refactor.

[aprantl]

Thanks, this looks like a good addition!

clang/lib/CodeGen/CGDebugInfo.cpp

1608

Please try to always upload patches with more context (git diff -U9999 works fine). I can't even tell which function this is in otherwise.

[aprantl]

This needs a lot more test coverage: The frontend cases aren't all covered by frontend checks and neither is the dwarf output.

[SouraVX]

Minor refactor for context.

[SouraVX]

In D68117#1686235, @aprantl wrote:

This needs a lot more test coverage: The frontend cases aren't all covered by frontend checks and neither is the dwarf output.

Do you mean to add more test cases ? Could you please elaborate on this?

clang/lib/CodeGen/CGDebugInfo.cpp

1608

Thanks, for correcting this. Seems to miss context part every time.

[aprantl]

In D68117#1686929, @SouraVX wrote:

In D68117#1686235, @aprantl wrote:

This needs a lot more test coverage: The frontend cases aren't all covered by frontend checks and neither is the dwarf output.

Do you mean to add more test cases ? Could you please elaborate on this

I think it would be good to add a separate CFE test for this instead of piggybacking on clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp whose indiates that it is really testing something else. The new test should fail if I comment out any of the changes in CreateCXXMemberFunction. Second, there needs to be a test in llvm/test/DebugInfo that takes LLVM IR as input and checks that the new attributes are showing up in the dwarfdump output.

clang/lib/CodeGen/CGDebugInfo.cpp

1608

The clang frontend should not be the one making the decision here, if possible it would be better to unconditionally emit this info in LLVM IR and then filter it out in the backend. I could imagine that the PDB backend might find this info useful, too (but I don't know).

[SouraVX]

Added test cases for debug info Clang frontend.

[SouraVX]

Will be adding llvm-dwarfdump tests soon.

clang/lib/CodeGen/CGDebugInfo.cpp

1608

Make sense, removed check from frontend.

[aprantl]

clang/lib/CodeGen/CGDebugInfo.cpp

1610

can you factor this block out into a static function or lambda, since it is repeated three times?

clang/test/CodeGenCXX/debug-info-defaulted-in-class.cpp

6 ↗

(On Diff #222701)

the -dwarf-version flag should be unnecessary now, right?

clang/test/CodeGenCXX/debug-info-defaulted-out-of-class.cpp

6 ↗

(On Diff #222701)

same here

clang/test/CodeGenCXX/debug-info-not-defaulted.cpp

3 ↗

(On Diff #222701)

and here

llvm/include/llvm/IR/DebugInfoFlags.def

91

Since these are all mutually exclusive, it might make sense to do the same thing as we did for virtuality above and thus save a bit or two.

[SouraVX]

clang/lib/CodeGen/CGDebugInfo.cpp

1610

Refactored this into lambda, with additional return statement after every check, since every attribute is mutually exclusive. No need to check others once an attribute is set.

clang/test/CodeGenCXX/debug-info-defaulted-in-class.cpp

6 ↗

(On Diff #222701)

Thanks for correcting again! Removed unnecessary flags from test cases.

llvm/include/llvm/IR/DebugInfoFlags.def

91

Had this in mind when I added 4 flags but couldn't able to solve this back then so I put this.

I tried refactoring this couple times based on your comments with results in failure. Virtuality and Defaulted member have same encodings "00-01-02".
My approach was conflicting with Virtuality attributes, Since Virtuality here is already using 2 bits of LSB.
Could you please suggest or hints to tackle this?

[probinson]

clang/lib/CodeGen/CGDebugInfo.cpp

1618

LLVM style is not to use 'else' after 'return'.

llvm/include/llvm/IR/DebugInfoFlags.def

91

Wouldn't the values for those constants be (1u << 9), (2u << 9), (3u << 9), (4u << 9) ? Look at how the Single/Multiple/VirtualInheritance flags work, earlier in this file.

And if we said a zero value meant NotDefaulted (which is what zero has meant up to now, so might as well keep that), then you need only 3 new flag values which will fit in 2 bits.

[probinson]

llvm/include/llvm/IR/DebugInfoFlags.def

91

And if you need to create an additional zero enum or mask enum, you can do that in DebugInfoMetadata.h; both DIFlags and DISPFlags do this kind of thing.

[SouraVX]

Addressed comments, regarding flags.

[SouraVX]

clang/lib/CodeGen/CGDebugInfo.cpp

1619

Previously SPFlagNotDefaulted is setted to SPFlagZero as it's normal value is, to save a bit. Hence in generated IR this flag is not getting set. instead 0 is getting emitted.
As a result, test cases checking DISPFlagNotDefaulted in IR are failing.

clang/test/CodeGenCXX/debug-info-not-defaulted.cpp

9 ↗

(On Diff #223954)

This test case is failing, checking DISPFlagNotDefaulted.

[SouraVX]

clang/test/CodeGenCXX/debug-info-defaulted-out-of-class.cpp

25 ↗

(On Diff #223954)

This is the case, checking for Out of class definition. I've been mentioning in llvm-dev mails.

clang/test/CodeGenCXX/debug-info-not-defaulted.cpp

9 ↗

(On Diff #223954)

Please note here that, backend and llvm-dwarfdump is fine without this.
Since it's value is '0' , we are able to query this using isNotDefaulted() -- hence attribute
DW_AT_defaulted having value DW_DEFAULTED_no is getting set and emitted and dumped fine by llvm-dwarfdump.

[probinson]

We really do want to pack the four mutually exclusive cases into two bits. I have tried to give more explicit comments inline to explain how you would do this. It really should work fine, recognizing that the "not defaulted" case is not explicitly represented in the textual IR because it uses a zero value in the defaulted/deleted subfield of SPFlags.

clang/lib/CodeGen/CGDebugInfo.cpp

1619

Given that DISPFlagNotDefaulted is represented by the absence of the other related flags, that makes sense. Those tests would verify the DISPFlagNotDefaulted case by showing none of those flags are present.

clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp

60

Because DISPFlagNotDefaulted has a zero value, the unmodified test correctly verifies that no other defaulted/deleted flags are present.

clang/test/CodeGenCXX/debug-info-not-defaulted.cpp

9 ↗

(On Diff #223954)

DISPFlagNotDefaulted is not explicitly represented in the textual IR; it is implied by the absence of any of the other deleted/defaulted values. The test needs to verify that spFlags is omitted from these DISubprogram entries; or if there are other spFlags present, it must verify that the other deleted/defaulted values are not present.

llvm/include/llvm/IR/DebugInfoFlags.def

93

There are 4 mutually exclusive cases, which can be handled using 4 values in a 2-bit field. We will give NotDefaulted the zero value, so it is not explicitly defined here. So we would have:

HANDLE_DISP_FLAG((1u << 9), Deleted)
HANDLE_DISP_FLAG((2u << 9), DefaultedInClass)
HANDLE_DISP_FLAG((3u << 9), DefaultedOutOfClass)

98

This can be 10, because we used only 2 bits for deleted/defaulted.

llvm/include/llvm/IR/DebugInfoMetadata.h

1615

I would call this SPFlagDeletedOrDefaulted.

1632

No, you don't want to modify this function. It is for converting from older bitcode formats that did not have a DISPFlags field.

1777

With all 4 values encoded in one field, isDeleted would become

return (getSPFlags() & SPFlagDefaultedOrDeleted) == SPFlagDeleted;

and of course the others would use the new mask name as well.

llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp

1309

else if here. It cannot be both defaulted and deleted.

llvm/lib/IR/DebugInfoMetadata.cpp

603 ↗

(On Diff #223954)

This would go away, if we pack 4 values into 2 bits.

[SouraVX]

In D68117#1702595, @probinson wrote:

We really do want to pack the four mutually exclusive cases into two bits. I have tried to give more explicit comments inline to explain how you would do this. It really should work fine, recognizing that the "not defaulted" case is not explicitly represented in the textual IR because it uses a zero value in the defaulted/deleted subfield of SPFlags.

Thanks Paul, for suggesting this. Your approach works fine. But as I was working on some lvm-dwarfdump test cases. We seems to miss one corner case --
Consider this test case;
class foo{

foo() = default;
~foo() = default;
 void not_special() {}

};
void not_a_member_of_foo(){}

Now I'm getting DW_AT_defaulted getting emitted with value DW_DEFAULTED_no, for functions "not_special" and "not_a_member_of_foo". This behavior is undesirable since, DW_AT_defaulted attributes is only valid for C++ special member functions{Constructors/Destructors, ...}.

Please correct me if I'm wrong -- Now This attributes to- implicitly defined "0" NotDefaulted bit. which is getting checked{that's fine as long as we have a dedicated bits for distinguishing} and true for every subprogram or function in a CU.
void DwarfUnit::applySubprogramAttributes( ...
...
else if (SP->isNotDefaulted())

addUInt(SPDie, dwarf::DW_AT_defaulted, dwarf::DW_FORM_data1,
        dwarf::DW_DEFAULTED_no);

...

[dblaikie]

In D68117#1707578, @SouraVX wrote:

In D68117#1702595, @probinson wrote:

We really do want to pack the four mutually exclusive cases into two bits. I have tried to give more explicit comments inline to explain how you would do this. It really should work fine, recognizing that the "not defaulted" case is not explicitly represented in the textual IR because it uses a zero value in the defaulted/deleted subfield of SPFlags.

Thanks Paul, for suggesting this. Your approach works fine. But as I was working on some lvm-dwarfdump test cases. We seems to miss one corner case --
Consider this test case;
class foo{

foo() = default;
~foo() = default;
 void not_special() {}

};
void not_a_member_of_foo(){}

Now I'm getting DW_AT_defaulted getting emitted with value DW_DEFAULTED_no, for functions "not_special" and "not_a_member_of_foo". This behavior is undesirable since, DW_AT_defaulted attributes is only valid for C++ special member functions{Constructors/Destructors, ...}.

Please correct me if I'm wrong -- Now This attributes to- implicitly defined "0" NotDefaulted bit. which is getting checked{that's fine as long as we have a dedicated bits for distinguishing} and true for every subprogram or function in a CU.
void DwarfUnit::applySubprogramAttributes( ...
...
else if (SP->isNotDefaulted())

addUInt(SPDie, dwarf::DW_AT_defaulted, dwarf::DW_FORM_data1,
        dwarf::DW_DEFAULTED_no);

...

Perhaps we should only emit DEFAULTED_yes, and assume anything that's not DEFAULTED_yes, is... not defaulted?

Also: What features is anyone planning to build with this information? I'm sort of inclined not to implement features without some use-case in mind/planned.

[SouraVX]

In D68117#1707680, @dblaikie wrote:

In D68117#1707578, @SouraVX wrote:

In D68117#1702595, @probinson wrote:

We really do want to pack the four mutually exclusive cases into two bits. I have tried to give more explicit comments inline to explain how you would do this. It really should work fine, recognizing that the "not defaulted" case is not explicitly represented in the textual IR because it uses a zero value in the defaulted/deleted subfield of SPFlags.

Thanks Paul, for suggesting this. Your approach works fine. But as I was working on some lvm-dwarfdump test cases. We seems to miss one corner case --
Consider this test case;
class foo{

foo() = default;
~foo() = default;
 void not_special() {}

};
void not_a_member_of_foo(){}

Now I'm getting DW_AT_defaulted getting emitted with value DW_DEFAULTED_no, for functions "not_special" and "not_a_member_of_foo". This behavior is undesirable since, DW_AT_defaulted attributes is only valid for C++ special member functions{Constructors/Destructors, ...}.

Please correct me if I'm wrong -- Now This attributes to- implicitly defined "0" NotDefaulted bit. which is getting checked{that's fine as long as we have a dedicated bits for distinguishing} and true for every subprogram or function in a CU.
void DwarfUnit::applySubprogramAttributes( ...
...
else if (SP->isNotDefaulted())

addUInt(SPDie, dwarf::DW_AT_defaulted, dwarf::DW_FORM_data1,
        dwarf::DW_DEFAULTED_no);

...

Perhaps we should only emit DEFAULTED_yes, and assume anything that's not DEFAULTED_yes, is... not defaulted?

Also: What features is anyone planning to build with this information? I'm sort of inclined not to implement features without some use-case in mind/planned.

Hi David, thanks for your suggestion. But, if we do that way, we may not be able to capture it's location and, whether that function was defaulted in or out of class.

Regarding the intent behind doing this, we have an initial internal requirement for 100% compliance towards DWARF-5 from producer{Clang} side.

[dblaikie]

In D68117#1708114, @SouraVX wrote:

In D68117#1707680, @dblaikie wrote:

In D68117#1707578, @SouraVX wrote:

In D68117#1702595, @probinson wrote:

We really do want to pack the four mutually exclusive cases into two bits. I have tried to give more explicit comments inline to explain how you would do this. It really should work fine, recognizing that the "not defaulted" case is not explicitly represented in the textual IR because it uses a zero value in the defaulted/deleted subfield of SPFlags.

Thanks Paul, for suggesting this. Your approach works fine. But as I was working on some lvm-dwarfdump test cases. We seems to miss one corner case --
Consider this test case;
class foo{

foo() = default;
~foo() = default;
 void not_special() {}

};
void not_a_member_of_foo(){}

Now I'm getting DW_AT_defaulted getting emitted with value DW_DEFAULTED_no, for functions "not_special" and "not_a_member_of_foo". This behavior is undesirable since, DW_AT_defaulted attributes is only valid for C++ special member functions{Constructors/Destructors, ...}.

Please correct me if I'm wrong -- Now This attributes to- implicitly defined "0" NotDefaulted bit. which is getting checked{that's fine as long as we have a dedicated bits for distinguishing} and true for every subprogram or function in a CU.
void DwarfUnit::applySubprogramAttributes( ...
...
else if (SP->isNotDefaulted())

addUInt(SPDie, dwarf::DW_AT_defaulted, dwarf::DW_FORM_data1,
        dwarf::DW_DEFAULTED_no);

...

Perhaps we should only emit DEFAULTED_yes, and assume anything that's not DEFAULTED_yes, is... not defaulted?

Also: What features is anyone planning to build with this information? I'm sort of inclined not to implement features without some use-case in mind/planned.

Hi David, thanks for your suggestion. But, if we do that way, we may not be able to capture it's location and, whether that function was defaulted in or out of class.

Not sure I follow - for an out-of-class defaulting, I'd expect the non-defining (declaration) DW_TAG_subprogram inside the class to not have the DW_AT_defaulted attribute - and then the out of line definition would have DW_AT_defaulted = DEFAULTED_yes. For an inline defaulted definition, the non-defining DW_TAG_subprogram would have DW_AT_defaulted= DEFAULTED_yes, and the defining DW_TAG_subprogram would have no DW_AT_defaulted, it would inherit it from the declaration via DW_AT_specification.

Regarding the intent behind doing this, we have an initial internal requirement for 100% compliance towards DWARF-5 from producer{Clang} side.

I'd like to discuss that requirement a bit further - obviously I'm not your management/customers/etc, so I may not be able to sway you, but I don't believe absence of DW_AT_defaulted would classify as DWARFv5 non-conformance to me.

Producing, say, debug_ranges instead of debug_rnglists (both in terms of teh section used, and the format of the bytes in that section) would be non-conformant. But DWARF only suggests what some forms/attributes/etc might be useful for, it doesn't require them by any means.

Any idea what the particular motivation for compliance is? So you/we could evaluate whether this feature is meeting a need or not?

[SouraVX]

In D68117#1708115, @dblaikie wrote:

In D68117#1708114, @SouraVX wrote:

In D68117#1707680, @dblaikie wrote:

In D68117#1707578, @SouraVX wrote:

In D68117#1702595, @probinson wrote:

We really do want to pack the four mutually exclusive cases into two bits. I have tried to give more explicit comments inline to explain how you would do this. It really should work fine, recognizing that the "not defaulted" case is not explicitly represented in the textual IR because it uses a zero value in the defaulted/deleted subfield of SPFlags.

Thanks Paul, for suggesting this. Your approach works fine. But as I was working on some lvm-dwarfdump test cases. We seems to miss one corner case --
Consider this test case;
class foo{

foo() = default;
~foo() = default;
 void not_special() {}

};
void not_a_member_of_foo(){}

Now I'm getting DW_AT_defaulted getting emitted with value DW_DEFAULTED_no, for functions "not_special" and "not_a_member_of_foo". This behavior is undesirable since, DW_AT_defaulted attributes is only valid for C++ special member functions{Constructors/Destructors, ...}.

Please correct me if I'm wrong -- Now This attributes to- implicitly defined "0" NotDefaulted bit. which is getting checked{that's fine as long as we have a dedicated bits for distinguishing} and true for every subprogram or function in a CU.
void DwarfUnit::applySubprogramAttributes( ...
...
else if (SP->isNotDefaulted())

addUInt(SPDie, dwarf::DW_AT_defaulted, dwarf::DW_FORM_data1,
        dwarf::DW_DEFAULTED_no);

...

Perhaps we should only emit DEFAULTED_yes, and assume anything that's not DEFAULTED_yes, is... not defaulted?

Also: What features is anyone planning to build with this information? I'm sort of inclined not to implement features without some use-case in mind/planned.

Hi David, thanks for your suggestion. But, if we do that way, we may not be able to capture it's location and, whether that function was defaulted in or out of class.

Not sure I follow - for an out-of-class defaulting, I'd expect the non-defining (declaration) DW_TAG_subprogram inside the class to not have the DW_AT_defaulted attribute - and then the out of line definition would have DW_AT_defaulted = DEFAULTED_yes. For an inline defaulted definition, the non-defining DW_TAG_subprogram would have DW_AT_defaulted= DEFAULTED_yes, and the defining DW_TAG_subprogram would have no DW_AT_defaulted, it would inherit it from the declaration via DW_AT_specification.

Regarding the intent behind doing this, we have an initial internal requirement for 100% compliance towards DWARF-5 from producer{Clang} side.

I'd like to discuss that requirement a bit further - obviously I'm not your management/customers/etc, so I may not be able to sway you, but I don't believe absence of DW_AT_defaulted would classify as DWARFv5 non-conformance to me.

Producing, say, debug_ranges instead of debug_rnglists (both in terms of teh section used, and the format of the bytes in that section) would be non-conformant. But DWARF only suggests what some forms/attributes/etc might be useful for, it doesn't require them by any means.

Any idea what the particular motivation for compliance is? So you/we could evaluate whether this feature is meeting a need or not?

Hi David,
I did some digging about DW_AT_defaulted and stuff, not much of a success but. Here's what I found -- http://dwarfstd.org/Issues.php?type=closed4 -- here it;s still marked as open, not sure what that means. Abbreviations on this page doesn't describe what "open" meant. But since, it's already in DWARF5 Spec -- it must be accepted.

Their's not much information available behind the suggestion or intention for adding this feature to Spec. http://dwarfstd.org/ShowIssue.php?issue=141215.3 -- I think Paul can better provide remarks on this one.

GCC and GDB side of things-- I've checked GCC-9.2.0 implements this feature, but didn't noticed any use of this feature from GDB side{GDB.8.3}. It's merely declaration of the forms available. GCC's implementation doesn't emit DW_DEFAULTED_no -- skipping DW_AT_defaulted attribute for that function. Current GCC implementation addresses in_class, out_of_class attributes and of_course DW_AT_deleted.

Regarding my patch and whether we should add this in clang/llvm--
Please correct me, in case I'm mistaken. David are you suggesting that, may be just "DW_DEFAULTED_yes" can suffice our needs instead of using the Spec {in_class, out_of_class, DEFAULTED_no}. We could do that, that would incur mostly adding a custom "DW_DEFAULTED_yes" {non-conflicting to Spec} opcode in LLVM, Not sure about this to addition to LLVM ??.
Or we can choose same approach as GCC.
Please share your thoughts on this. which direction should we choose ?? Or you guys have altogether something different in mind.

[probinson]

In D68117#1709557, @SouraVX wrote:

Hi David,
I did some digging about DW_AT_defaulted and stuff, not much of a success but. Here's what I found -- http://dwarfstd.org/Issues.php?type=closed4 -- here it;s still marked as open, not sure what that means. Abbreviations on this page doesn't describe what "open" meant. But since, it's already in DWARF5 Spec -- it must be accepted.

"open" here means the web page wasn't updated correctly. :-) Yes, it was accepted and incorporated into DWARF 5, see http://dwarfstd.org/ShowIssue.php?issue=141215.3 (notes at the bottom).

Their's not much information available behind the suggestion or intention for adding this feature to Spec. http://dwarfstd.org/ShowIssue.php?issue=141215.3 -- I think Paul can better provide remarks on this one.

"It affects overload resolution" according to my record of the DWARF meeting where we discussed this. Although "overload" resolution might not be the technically correct term. Deleted is different from omitted, when trying to determine what to do with a particular source-language expression.

GCC and GDB side of things-- I've checked GCC-9.2.0 implements this feature, but didn't noticed any use of this feature from GDB side{GDB.8.3}. It's merely declaration of the forms available. GCC's implementation doesn't emit DW_DEFAULTED_no -- skipping DW_AT_defaulted attribute for that function. Current GCC implementation addresses in_class, out_of_class attributes and of_course DW_AT_deleted.

Regarding my patch and whether we should add this in clang/llvm--
Please correct me, in case I'm mistaken. David are you suggesting that, may be just "DW_DEFAULTED_yes" can suffice our needs instead of using the Spec {in_class, out_of_class, DEFAULTED_no}. We could do that, that would incur mostly adding a custom "DW_DEFAULTED_yes" {non-conflicting to Spec} opcode in LLVM, Not sure about this to addition to LLVM ??.
Or we can choose same approach as GCC.
Please share your thoughts on this. which direction should we choose ?? Or you guys have altogether something different in mind.

I don't see any problem with omitting the attribute instead of explicitly saying DEFAULTED_no. There is no DW_DEFAULTED_yes, if we provide the attribute at all it would have to distinguish in-class vs out-of-class in order to conform. I know the compiler does treat them a little differently depending on in-class vs out-of-class; if nothing else, in-class is inlined more aggressively and might not have an out-of-line instance at all. This might matter to a debugger trying to support source-language expression evaluation.

[dblaikie]

In D68117#1709712, @probinson wrote:

In D68117#1709557, @SouraVX wrote:

Their's not much information available behind the suggestion or intention for adding this feature to Spec. http://dwarfstd.org/ShowIssue.php?issue=141215.3 -- I think Paul can better provide remarks on this one.

"It affects overload resolution" according to my record of the DWARF meeting where we discussed this. Although "overload" resolution might not be the technically correct term. Deleted is different from omitted, when trying to determine what to do with a particular source-language expression.

Carey's notes on the original give some idea: http://dwarfstd.org/ShowIssue.php?issue=141003.1 - with regards to the calling ABI. But Richard Smith explained that any solution like this would not be complete - and indeed, even in that issue the incompleteness is discussed and a second attribute DW_AT_layout to help address this and Carey filed another issue with a different proposed solution: http://dwarfstd.org/ShowIssue.php?issue=141215.1 that has some of the same phrasing and a different approach to address this, proposing a DW_AT_calling_convention attribute - this attribute was accepted.

So, by my reckoning, I don't know why the DW_AT_defaulted was kept/made it into the standard - it was an incomplete solution to the problem it was proposed to fix, and a complete solution was added as well.

(deleted is different - and while recording all deleted functions is probably impractical, though could benefit consumers (as could having all function overloads, and having raw template descriptions) - as without all the original code, overloads, and templates, expression evaluation will always be inaccurate (a consumer may choose overload candidates that the same expression in the original source would not have chosen - not just rejecting expressions that call functions that were not code generated), deleted special members are a bit special and may be useful to include to ensure the consumer doesn't attempt to synthesize trivial or default implementations)

GCC and GDB side of things-- I've checked GCC-9.2.0 implements this feature, but didn't noticed any use of this feature from GDB side{GDB.8.3}. It's merely declaration of the forms available. GCC's implementation doesn't emit DW_DEFAULTED_no -- skipping DW_AT_defaulted attribute for that function. Current GCC implementation addresses in_class, out_of_class attributes and of_course DW_AT_deleted.

Regarding my patch and whether we should add this in clang/llvm--
Please correct me, in case I'm mistaken. David are you suggesting that, may be just "DW_DEFAULTED_yes" can suffice our needs instead of using the Spec {in_class, out_of_class, DEFAULTED_no}. We could do that, that would incur mostly adding a custom "DW_DEFAULTED_yes" {non-conflicting to Spec} opcode in LLVM, Not sure about this to addition to LLVM ??.
Or we can choose same approach as GCC.
Please share your thoughts on this. which direction should we choose ?? Or you guys have altogether something different in mind.

I don't see any problem with omitting the attribute instead of explicitly saying DEFAULTED_no. There is no DW_DEFAULTED_yes, if we provide the attribute at all it would have to distinguish in-class vs out-of-class in order to conform. I know the compiler does treat them a little differently depending on in-class vs out-of-class; if nothing else, in-class is inlined more aggressively and might not have an out-of-line instance at all. This might matter to a debugger trying to support source-language expression evaluation.

Yes, sorry, my "DEFAULTED_yes" wasn't an attempt to propose a new DWARF feature, just ignorance having not recently read what the existing feature contains.

I think the existing feature's perhaps a bit misspecified - because where you put the DEFAULTED_{in_class,out_of_class} would communicate that without needing the two values - if you put it on an out of line function definition, it's defaulted out of line, if you put it on the in-class declaration then it's defaulted inline.

But spec'd the way it is, if we want to implement this at all/if there's some actual user need (admittedly the noreturn attribute has gone in recently without a discussion of usage... so I'm not the only gatekeeper here & other people have other opinions, and that's OK - if someone (@probinson @aprantl etc) want to tell me I'm being unreasonable here & we should just put it in - it's only a bit here or there & not likely to make a huge difference to DWARF size & possibly enable some scenarios we haven't imagined yet and avoid the chicken-and-egg problem for the future implementer of such features, that's OK) - I'd essentially implement it that way. Put DEFAULTED_out_of_class on the member function definition DIE if it's defaulted there, and put DEFAULTED_in_class on the declaration DIE if it's defaulted there.

And I'd probably only spend one bit of flags on this, if possible - "not defaulted (0/assumed for all subprograms) or defaulted (1)" and translate it into one of the two values (in_class or out_of_class) in the backend based on which subprogram DIE it's attached to.

[probinson]

In D68117#1710295, @dblaikie wrote:

I think the existing feature's perhaps a bit misspecified - because where you put the DEFAULTED_{in_class,out_of_class} would communicate that without needing the two values - if you put it on an out of line function definition, it's defaulted out of line, if you put it on the in-class declaration then it's defaulted inline.

But spec'd the way it is, if we want to implement this at all/if there's some actual user need (admittedly the noreturn attribute has gone in recently without a discussion of usage... so I'm not the only gatekeeper here & other people have other opinions, and that's OK - if someone (@probinson @aprantl etc) want to tell me I'm being unreasonable here & we should just put it in - it's only a bit here or there & not likely to make a huge difference to DWARF size & possibly enable some scenarios we haven't imagined yet and avoid the chicken-and-egg problem for the future implementer of such features, that's OK) - I'd essentially implement it that way. Put DEFAULTED_out_of_class on the member function definition DIE if it's defaulted there, and put DEFAULTED_in_class on the declaration DIE if it's defaulted there.

And I'd probably only spend one bit of flags on this, if possible - "not defaulted (0/assumed for all subprograms) or defaulted (1)" and translate it into one of the two values (in_class or out_of_class) in the backend based on which subprogram DIE it's attached to.

That seems reasonable too. Of course if we're already spending a bit on Deleted, and the same subprogram can't be both Deleted and Defaulted, it costs no extra DISP bits to represent the 4 cases (defaulted-in-class, defaulted-out-of-class, deleted, none-of-the-above).

@SouraVX I would say we should never emit DEFAULTED_no. If the compiler organized its internal data differently, and had a canned abbreviation for ctors/dtors that included DW_AT_defaulted, then we'd need to fill that in; but that's not how LLVM handles DWARF, it creates abbreviations on demand. So, doing nothing in the none-of-the-above case would be best here.

(@dblaikie Aside regarding noreturn, the original DWARF proposal was for a debugger wanting to know a given function will not return. As a flag, in the DWARF it costs an extra abbrev entry but no space in .debug_info. Cheap enough for me.)

[dblaikie]

In D68117#1711154, @probinson wrote:

In D68117#1710295, @dblaikie wrote:

I think the existing feature's perhaps a bit misspecified - because where you put the DEFAULTED_{in_class,out_of_class} would communicate that without needing the two values - if you put it on an out of line function definition, it's defaulted out of line, if you put it on the in-class declaration then it's defaulted inline.

But spec'd the way it is, if we want to implement this at all/if there's some actual user need (admittedly the noreturn attribute has gone in recently without a discussion of usage... so I'm not the only gatekeeper here & other people have other opinions, and that's OK - if someone (@probinson @aprantl etc) want to tell me I'm being unreasonable here & we should just put it in - it's only a bit here or there & not likely to make a huge difference to DWARF size & possibly enable some scenarios we haven't imagined yet and avoid the chicken-and-egg problem for the future implementer of such features, that's OK) - I'd essentially implement it that way. Put DEFAULTED_out_of_class on the member function definition DIE if it's defaulted there, and put DEFAULTED_in_class on the declaration DIE if it's defaulted there.

And I'd probably only spend one bit of flags on this, if possible - "not defaulted (0/assumed for all subprograms) or defaulted (1)" and translate it into one of the two values (in_class or out_of_class) in the backend based on which subprogram DIE it's attached to.

That seems reasonable too. Of course if we're already spending a bit on Deleted, and the same subprogram can't be both Deleted and Defaulted, it costs no extra DISP bits to represent the 4 cases (defaulted-in-class, defaulted-out-of-class, deleted, none-of-the-above).

Understood - though I think it's probably technically better to only store defaulted on/off to remove possible incorrect representations at the IR level at least:

Since out of line defaulted can't be reliably provided on the in-class declaration of the special member, it'd be unreliable to put it on that declaration even if one translation unit did see the out of line definition. And it'd be questionable to put it on the out of line definition if it's defaulted inline? (though I guess one could argue that would be consistent - always attaching it to the definition, never the declaration?)

But I still come back to: Why do we/anyone want this? Is anyone planning on using this information for any purpose? DWARF doesn't require us to emit everything - it just says we can if it's useful.

@SouraVX I would say we should never emit DEFAULTED_no. If the compiler organized its internal data differently, and had a canned abbreviation for ctors/dtors that included DW_AT_defaulted, then we'd need to fill that in; but that's not how LLVM handles DWARF, it creates abbreviations on demand. So, doing nothing in the none-of-the-above case would be best here.

(@dblaikie Aside regarding noreturn, the original DWARF proposal was for a debugger wanting to know a given function will not return.

I'd still be curious to know which consumer, if they're still interested, what feature they're trying to build with it, if they're using Clang/LLVM's output, etc.

As a flag, in the DWARF it costs an extra abbrev entry but no space in .debug_info. Cheap enough for me.)

Oh, sure - I don't think the output cost is high here or there - but it's more feature surface area, the time spent designing, code reviewing, supporting, etc, another place for bugs, etc. So I'd generally like to have some justification beyond "the spec says we can" - ideally, I'd like a DWARF consumer that's actively trying to build a feature that they can't without this new information.

[probinson]

In D68117#1711714, @dblaikie wrote:

In D68117#1711154, @probinson wrote:

(@dblaikie Aside regarding noreturn, the original DWARF proposal was for a debugger wanting to know a given function will not return.

I'd still be curious to know which consumer, if they're still interested, what feature they're trying to build with it, if they're using Clang/LLVM's output, etc.

DW_AT_noreturn (http://dwarfstd.org/ShowIssue.php?issue=140331.1) was requested by Mark Wielaard, who AFAICT works for Red Hat. Now you know as much as I do.

As a flag, in the DWARF it costs an extra abbrev entry but no space in .debug_info. Cheap enough for me.)

Oh, sure - I don't think the output cost is high here or there - but it's more feature surface area, the time spent designing, code reviewing, supporting, etc, another place for bugs, etc. So I'd generally like to have some justification beyond "the spec says we can" - ideally, I'd like a DWARF consumer that's actively trying to build a feature that they can't without this new information.

Fair.

[SouraVX]

Hi @probinson @dblaikie @aprantl , I've was investigating and working on your inputs regarding the problem with DW_at_defaulted once. I think clang has also some issues. Though I'm not able to precisely point out. I ranned into some problems in CFE while marking out_of_class functions. i.e consider this for instance "debug-info-defaulted-out_of_class.cpp" FIXME:. Causing too much trouble and possibly can introduce some bugs in clang/llvm.
May be we'll reconsider this in future. Thanks for putting time in reviewing and discussing this.

Anyway here's the next course of action I'm considering. I'll be marking this review as abandoned. I'll raise a fresh new review for DW_AT_deleted feature, that;s pretty straight forward, considering C++ spec. If it's deleted, it has to be declared/defined{whatever} deleted in it's first declaration. This feature also augments the consumer information regarding which spl member functions are deleted, hence restrict their calling for user or do something useful with this at it's disposal.

@probinson @dblaikie @aprantl Please share your thoughts/comments inclination regarding adding this DW_AT_deleted to clang/llvm.

[dblaikie]

In D68117#1714091, @SouraVX wrote:

Hi @probinson @dblaikie @aprantl , I've was investigating and working on your inputs regarding the problem with DW_at_defaulted once. I think clang has also some issues.

You think clang has bugs, independent of the debug info?

Though I'm not able to precisely point out. I ranned into some problems in CFE while marking out_of_class functions. i.e consider this for instance "debug-info-defaulted-out_of_class.cpp" FIXME:. Causing too much trouble and possibly can introduce some bugs in clang/llvm.
May be we'll reconsider this in future. Thanks for putting time in reviewing and discussing this.

Yeah, I looked at it a bit & asked @rsmith for some thoughts. Eventually wrapped my head around it.

Clang's codegen occurs during parsing - rather than parsing/AST building entirely, then code generating. So to take a smaller example:

struct foo {
  foo();
  // virtual
  void f1();
  void f2();
};
void foo::f2() { }
foo::foo() = default;

Without the virtual keyword, the first time the debug info needs the type (when building the description for f1's definition's "this" parameter) the foo::foo() = default definition hasn't been parsed yet, so there is no definition of foo::foo() available. (so the out of class defaulting isn't visible)

The reason virtual matters is that the actual class definition of 'foo' is never built - check the DWARF and metadata, only a declaration of "foo" is built. And the declarations of the member functions are injected into the declaration as the definitions are parsed, not before - so they observe the correct defaulting state.

Another way to show the bug is with -fstandalone-debug, even with a virtual function it shows the problem.

And moving 'f2' around (before/after 'foo()') also shows the problem - because having f2 first means the type definition (without virtual, or while using -fstandalone-debug) is built before any defaulting is seen - so 'foo()' isn't shown as defaulted.

All of this somewhat goes to my point - out of line defaulting should be tested and recorded on the out of line definition, not on the declaration, of a special member.

Anyway here's the next course of action I'm considering. I'll be marking this review as abandoned. I'll raise a fresh new review for DW_AT_deleted feature, that;s pretty straight forward, considering C++ spec. If it's deleted, it has to be declared/defined{whatever} deleted in it's first declaration. This feature also augments the consumer information regarding which spl member functions are deleted, hence restrict their calling for user or do something useful with this at it's disposal.

@probinson @dblaikie @aprantl Please share your thoughts/comments inclination regarding adding this DW_AT_deleted to clang/llvm.

Sure, deleted support seems fine/reasonable/good to me.

[SouraVX]

Will work on this later!

clang/lib/CodeGen/CGDebugInfo.cpp

     ContainingType = RecordTy;
   }
 
+  if (CGM.getCodeGenOpts().DwarfVersion >= 5) {
+    // DWARF-5 support for, defaulted, deleted member functions
+    if (const auto *CXXC = dyn_cast<CXXConstructorDecl>(Method)) {
+      if (CXXC->getCanonicalDecl()->isDeleted())
+        SPFlags |= llvm::DISubprogram::SPFlagDeleted;
+
+      if (CXXC->getCanonicalDecl()->isDefaulted())
+        SPFlags |= llvm::DISubprogram::SPFlagDefaultedInClass;
+      else if (const auto Def = CXXC->getDefinition()) {
+        if (Def->isDefaulted())
+          SPFlags |= llvm::DISubprogram::SPFlagDefaultedOutOfClass;
+        else
+          SPFlags |= llvm::DISubprogram::SPFlagNotDefaulted;
+      }
+    }
+    if (const auto *DXXC = dyn_cast<CXXDestructorDecl>(Method)) {
+      if (DXXC->getCanonicalDecl()->isDeleted())
+        SPFlags |= llvm::DISubprogram::SPFlagDeleted;
+
+      if (DXXC->getCanonicalDecl()->isDefaulted())
+        SPFlags |= llvm::DISubprogram::SPFlagDefaultedInClass;
+      else if (const auto Def = DXXC->getDefinition()) {
+        if (Def->isDefaulted()) {
+          SPFlags |= llvm::DISubprogram::SPFlagDefaultedOutOfClass;
+        } else {
+          SPFlags |= llvm::DISubprogram::SPFlagNotDefaulted;
+        }
+      }
+    }
+    if (Method->isCopyAssignmentOperator() ||
+        Method->isMoveAssignmentOperator()) {
+      if (Method->getCanonicalDecl()->isDeleted())
+        SPFlags |= llvm::DISubprogram::SPFlagDeleted;
+
+      if (Method->getCanonicalDecl()->isDefaulted())
+        SPFlags |= llvm::DISubprogram::SPFlagDefaultedInClass;
+      else if (const auto Def = Method->getDefinition()) {
+        if (Def->isDefaulted())
+          SPFlags |= llvm::DISubprogram::SPFlagDefaultedOutOfClass;
+        else
+          SPFlags |= llvm::DISubprogram::SPFlagNotDefaulted;
+      }
+    }
+  }
   if (Method->isStatic())
     Flags |= llvm::DINode::FlagStaticMember;
   if (Method->isImplicit())

clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp

 // RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple %s -o - \
 // RUN:   -O1 -disable-llvm-passes \
 // RUN:   -debug-info-kind=standalone -dwarf-version=5 \
-// RUN: | FileCheck %s -check-prefix=HAS-ATTR \
-// RUN:     -implicit-check-not=DISubprogram -implicit-check-not=DIFlagAllCallsDescribed
+// RUN: | FileCheck %s -check-prefix=HAS-DWARF5
 
 // Supported: DWARF4 + LLDB tuning, -O1, limited DI
 // RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple %s -o - \

 // HAS-ATTR-DAG: DISubprogram(name: "method1", {{.*}}, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition
 // HAS-ATTR-DAG: DISubprogram(name: "force_irgen", {{.*}}, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition
 
+// HAS-DWARF5: DISubprogram(name: "struct1", {{.*}}, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized | DISPFlagNotDefaulted)
+
 // LINE-TABLES-ONLY: DISubprogram(name: "force_irgen", {{.*}}, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition
 
 void declaration1();

llvm/include/llvm/BinaryFormat/Dwarf.h

 StringRef DecimalSignString(unsigned Sign);
 StringRef EndianityString(unsigned Endian);
 StringRef AccessibilityString(unsigned Access);
+StringRef DefaultedMemberString(unsigned DefaultedEncodings);
 StringRef VisibilityString(unsigned Visibility);
 StringRef VirtualityString(unsigned Virtuality);
 StringRef LanguageString(unsigned Language);

llvm/include/llvm/IR/DebugInfoFlags.def

 HANDLE_DISP_FLAG((1u << 6), Elemental)
 HANDLE_DISP_FLAG((1u << 7), Recursive)
 HANDLE_DISP_FLAG((1u << 8), MainSubprogram)
+HANDLE_DISP_FLAG((1u << 9), NotDefaulted)
+HANDLE_DISP_FLAG((1u << 10), DefaultedInClass)
+HANDLE_DISP_FLAG((1u << 11), DefaultedOutOfClass)
+HANDLE_DISP_FLAG((1u << 12), Deleted)
 
 #ifdef DISP_FLAG_LARGEST_NEEDED
 // Intended to be used with ADT/BitmaskEnum.h.
 // NOTE: Always must be equal to largest flag, check this when adding new flags.
-HANDLE_DISP_FLAG((1 << 8), Largest)
+HANDLE_DISP_FLAG((1 << 12), Largest)
 #undef DISP_FLAG_LARGEST_NEEDED
 #endif
 

llvm/include/llvm/IR/DebugInfoMetadata.h

   bool isPure() const { return getSPFlags() & SPFlagPure; }
   bool isElemental() const { return getSPFlags() & SPFlagElemental; }
   bool isRecursive() const { return getSPFlags() & SPFlagRecursive; }
+  bool isDefaultedInClass() const {
+    return getSPFlags() & SPFlagDefaultedInClass;
+  }
+  bool isDefaultedOutOfClass() const {
+    return getSPFlags() & SPFlagDefaultedOutOfClass;
+  }
+  bool isNotDefaulted() const { return getSPFlags() & SPFlagNotDefaulted; }
+  bool isDeleted() const { return getSPFlags() & SPFlagDeleted; }
 
   /// Check if this is reference-qualified.
   ///

llvm/lib/BinaryFormat/Dwarf.cpp

   return StringRef();
 }
 
+StringRef llvm::dwarf::DefaultedMemberString(unsigned DefaultedEncodings) {
+  switch (DefaultedEncodings) {
+  // Defaulted Member Encodings codes
+  case DW_DEFAULTED_no:
+    return "DW_DEFAULTED_no";
+  case DW_DEFAULTED_in_class:
+    return "DW_DEFAULTED_in_class";
+  case DW_DEFAULTED_out_of_class:
+    return "DW_DEFAULTED_out_of_class";
+  }
+  return StringRef();
+}
+
 StringRef llvm::dwarf::VisibilityString(unsigned Visibility) {
   switch (Visibility) {
   case DW_VIS_local:

     return ArrayOrderString(Val);
   case DW_AT_APPLE_runtime_class:
     return LanguageString(Val);
+  case DW_AT_defaulted:
+    return DefaultedMemberString(Val);
   }
 
   return StringRef();

llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp

     addFlag(SPDie, dwarf::DW_AT_elemental);
   if (SP->isRecursive())
     addFlag(SPDie, dwarf::DW_AT_recursive);
+  if (DD->getDwarfVersion() >= 5) {
+    if (SP->isDefaultedInClass())
+      addUInt(SPDie, dwarf::DW_AT_defaulted, dwarf::DW_FORM_data1,
+              dwarf::DW_DEFAULTED_in_class);
+    if (SP->isDefaultedOutOfClass())
+      addUInt(SPDie, dwarf::DW_AT_defaulted, dwarf::DW_FORM_data1,
+              dwarf::DW_DEFAULTED_out_of_class);
+    if (SP->isNotDefaulted())
+      addUInt(SPDie, dwarf::DW_AT_defaulted, dwarf::DW_FORM_data1,
+              dwarf::DW_DEFAULTED_no);
+    if (SP->isDeleted())
+      addFlag(SPDie, dwarf::DW_AT_deleted);
+  }
 }
 
 void DwarfUnit::constructSubrangeDIE(DIE &Buffer, const DISubrange *SR,