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

David Blaikie via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Oct 16 12:40:46 PDT 2019


dblaikie added a comment.

In D68117#1711154 <https://reviews.llvm.org/D68117#1711154>, @probinson wrote:

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


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

https://reviews.llvm.org/D68117





More information about the cfe-commits mailing list