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

[Sourabh Singh Tomar via Phabricator via cfe-commits]

SouraVX abandoned this revision.
SouraVX added a comment.

Will work on this later!


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

https://reviews.llvm.org/D68117



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

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

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a subscriber: rsmith.
dblaikie added a comment.

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.


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

https://reviews.llvm.org/D68117



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

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

[Sourabh Singh Tomar via Phabricator via cfe-commits]

SouraVX added a comment.

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.


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

https://reviews.llvm.org/D68117



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

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

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

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.


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

https://reviews.llvm.org/D68117



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

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

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

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.


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

https://reviews.llvm.org/D68117



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

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

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

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


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

https://reviews.llvm.org/D68117



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

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

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

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 

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

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

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.


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

https://reviews.llvm.org/D68117



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

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

[Sourabh Singh Tomar via Phabricator via cfe-commits]

SouraVX added a comment.

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] D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

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?


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

https://reviews.llvm.org/D68117



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

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

[Sourabh Singh Tomar via Phabricator via cfe-commits]

SouraVX added a comment.

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.


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

https://reviews.llvm.org/D68117



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

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

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

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.


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

https://reviews.llvm.org/D68117



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

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

[Sourabh Singh Tomar via Phabricator via cfe-commits]

SouraVX added a comment.

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

...


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

https://reviews.llvm.org/D68117



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

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

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

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.




Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1619
+  else {
+SPFlags |= llvm::DISubprogram::SPFlagNotDefaulted;
+  }

SouraVX wrote:
> 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.
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.



Comment at: clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp:60
 // HAS-ATTR-DAG: DISubprogram(name: "declaration2", {{.*}}, flags: 
DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition
-// HAS-ATTR-DAG: DISubprogram(name: "struct1", {{.*}}, flags: 
DIFlagPrototyped, spFlags: DISPFlagOptimized)
+// HAS-ATTR-DAG: DISubprogram(name: "struct1", {{.*}}, flags: 
DIFlagPrototyped, spFlags: DISPFlagOptimized | DISPFlagNotDefaulted)
 // HAS-ATTR-DAG: DISubprogram(name: "struct1", {{.*}}, flags: DIFlagPrototyped 
| DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition

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



Comment at: clang/test/CodeGenCXX/debug-info-not-defaulted.cpp:9
+
+// ATTR: DISubprogram(name: "not_defaulted", {{.*}}, flags: DIFlagPublic | 
DIFlagPrototyped, spFlags: DISPFlagNotDefaulted
+// ATTR: DISubprogram(name: "not_defaulted", {{.*}}, flags: DIFlagPublic | 
DIFlagPrototyped, spFlags: DISPFlagNotDefaulted

SouraVX wrote:
> SouraVX wrote:
> > This test case is failing, checking DISPFlagNotDefaulted.
> 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.
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.



Comment at: llvm/include/llvm/IR/DebugInfoFlags.def:93
+HANDLE_DISP_FLAG((1u << 10), DefaultedInClass)
+HANDLE_DISP_FLAG((1u << 11), DefaultedOutOfClass)
 

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



Comment at: llvm/include/llvm/IR/DebugInfoFlags.def:98
 // NOTE: Always must be equal to largest flag, check this when adding new 
flags.
-HANDLE_DISP_FLAG((1 << 8), Largest)
+HANDLE_DISP_FLAG((1 << 11), Largest)
 #undef DISP_FLAG_LARGEST_NEEDED

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



Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:1615
 SPFlagVirtuality = SPFlagVirtual | SPFlagPureVirtual,
+SPFlagDefaultedInOrOutOfClass =
+SPFlagDefaultedInClass | SPFlagDefaultedOutOfClass,

I would call this SPFlagDeletedOrDefaulted.



Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:1632
+  static DISPFlags
+  toSPFlags(bool IsLocalToUnit, bool IsDefinition, bool IsOptimized,
+unsigned Virtuality = SPFlagNonvirtual,

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



Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:1777
+  }
+  bool isDeleted() const { return getSPFlags() & SPFlagDeleted; }
 

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.



Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp:1309
+  dwarf::DW_DEFAULTED_no);
+if (SP->isDeleted())
+  addFlag(SPDie, dwarf::DW_AT_deleted);

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

[Sourabh Singh Tomar via Phabricator via cfe-commits]

SouraVX marked 2 inline comments as done.
SouraVX added inline comments.



Comment at: clang/test/CodeGenCXX/debug-info-defaulted-out-of-class.cpp:25
+
+  //FIXME -- clang will not mark above member funtions, excluding constructors
+  // as out of class. If we did not mark destructor or other member functions

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



Comment at: clang/test/CodeGenCXX/debug-info-not-defaulted.cpp:9
+
+// ATTR: DISubprogram(name: "not_defaulted", {{.*}}, flags: DIFlagPublic | 
DIFlagPrototyped, spFlags: DISPFlagNotDefaulted
+// ATTR: DISubprogram(name: "not_defaulted", {{.*}}, flags: DIFlagPublic | 
DIFlagPrototyped, spFlags: DISPFlagNotDefaulted

SouraVX wrote:
> This test case is failing, checking DISPFlagNotDefaulted.
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.


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

https://reviews.llvm.org/D68117



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

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

[Sourabh Singh Tomar via Phabricator via cfe-commits]

SouraVX marked 3 inline comments as done.
SouraVX added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1619
+  else {
+SPFlags |= llvm::DISubprogram::SPFlagNotDefaulted;
+  }

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.



Comment at: clang/test/CodeGenCXX/debug-info-not-defaulted.cpp:9
+
+// ATTR: DISubprogram(name: "not_defaulted", {{.*}}, flags: DIFlagPublic | 
DIFlagPrototyped, spFlags: DISPFlagNotDefaulted
+// ATTR: DISubprogram(name: "not_defaulted", {{.*}}, flags: DIFlagPublic | 
DIFlagPrototyped, spFlags: DISPFlagNotDefaulted

This test case is failing, checking DISPFlagNotDefaulted.


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

https://reviews.llvm.org/D68117



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

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

[Sourabh Singh Tomar via Phabricator via cfe-commits]

SouraVX updated this revision to Diff 223954.
SouraVX added a comment.

Addressed comments, regarding flags.


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

https://reviews.llvm.org/D68117

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
  clang/test/CodeGenCXX/debug-info-defaulted-in-class.cpp
  clang/test/CodeGenCXX/debug-info-defaulted-out-of-class.cpp
  clang/test/CodeGenCXX/debug-info-deleted.cpp
  clang/test/CodeGenCXX/debug-info-not-defaulted.cpp
  llvm/include/llvm/BinaryFormat/Dwarf.h
  llvm/include/llvm/IR/DebugInfoFlags.def
  llvm/include/llvm/IR/DebugInfoMetadata.h
  llvm/lib/BinaryFormat/Dwarf.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
  llvm/lib/IR/DebugInfoMetadata.cpp

Index: llvm/lib/IR/DebugInfoMetadata.cpp
===
--- llvm/lib/IR/DebugInfoMetadata.cpp
+++ llvm/lib/IR/DebugInfoMetadata.cpp
@@ -600,6 +600,7 @@
   switch (Flag) {
   // Appease a warning.
   case SPFlagVirtuality:
+  case SPFlagDefaultedInOrOutOfClass:
 return "";
 #define HANDLE_DISP_FLAG(ID, NAME) \
   case SPFlag##NAME:   \
Index: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -1296,6 +1296,19 @@
 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);
+else if (SP->isDefaultedOutOfClass())
+  addUInt(SPDie, dwarf::DW_AT_defaulted, dwarf::DW_FORM_data1,
+  dwarf::DW_DEFAULTED_out_of_class);
+else 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 , const DISubrange *SR,
Index: llvm/lib/BinaryFormat/Dwarf.cpp
===
--- llvm/lib/BinaryFormat/Dwarf.cpp
+++ llvm/lib/BinaryFormat/Dwarf.cpp
@@ -271,6 +271,19 @@
   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:
@@ -612,6 +625,8 @@
 return ArrayOrderString(Val);
   case DW_AT_APPLE_runtime_class:
 return LanguageString(Val);
+  case DW_AT_defaulted:
+return DefaultedMemberString(Val);
   }
 
   return StringRef();
Index: llvm/include/llvm/IR/DebugInfoMetadata.h
===
--- llvm/include/llvm/IR/DebugInfoMetadata.h
+++ llvm/include/llvm/IR/DebugInfoMetadata.h
@@ -1610,7 +1610,10 @@
 #define DISP_FLAG_LARGEST_NEEDED
 #include "llvm/IR/DebugInfoFlags.def"
 SPFlagNonvirtual = SPFlagZero,
+SPFlagNotDefaulted = SPFlagZero,
 SPFlagVirtuality = SPFlagVirtual | SPFlagPureVirtual,
+SPFlagDefaultedInOrOutOfClass =
+SPFlagDefaultedInClass | SPFlagDefaultedOutOfClass,
 LLVM_MARK_AS_BITMASK_ENUM(SPFlagLargest)
   };
 
@@ -1625,10 +1628,11 @@
   SmallVectorImpl );
 
   // Helper for converting old bitfields to new flags word.
-  static DISPFlags toSPFlags(bool IsLocalToUnit, bool IsDefinition,
- bool IsOptimized,
- unsigned Virtuality = SPFlagNonvirtual,
- bool IsMainSubprogram = false) {
+  static DISPFlags
+  toSPFlags(bool IsLocalToUnit, bool IsDefinition, bool IsOptimized,
+unsigned Virtuality = SPFlagNonvirtual,
+unsigned DefaultedInOrOutOfClass = SPFlagNotDefaulted,
+bool IsMainSubprogram = false) {
 // We're assuming virtuality is the low-order field.
 static_assert(
 int(SPFlagVirtual) == int(dwarf::DW_VIRTUALITY_virtual) &&
@@ -1636,6 +1640,7 @@
 "Virtuality constant mismatch");
 return static_cast(
 (Virtuality & SPFlagVirtuality) |
+(DefaultedInOrOutOfClass & SPFlagDefaultedInOrOutOfClass) |
 (IsLocalToUnit ? SPFlagLocalToUnit : SPFlagZero) |
 (IsDefinition ? SPFlagDefinition : SPFlagZero) |
 (IsOptimized ? SPFlagOptimized : SPFlagZero) |
@@ -1758,6 +1763,18 @@
   bool isPure() const { return 

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

[Paul Robinson via Phabricator via cfe-commits]

probinson added inline comments.



Comment at: llvm/include/llvm/IR/DebugInfoFlags.def:91
 HANDLE_DISP_FLAG((1u << 8), MainSubprogram)
+HANDLE_DISP_FLAG((1u << 9), NotDefaulted)
+HANDLE_DISP_FLAG((1u << 10), DefaultedInClass)

probinson wrote:
> SouraVX wrote:
> > aprantl wrote:
> > > 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.
> > 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?
> 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.
> 
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.


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

https://reviews.llvm.org/D68117



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

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

[Paul Robinson via Phabricator via cfe-commits]

probinson added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1618
+  return;
+} else if (const auto Def = Method->getDefinition()) {
+  if (Def->isDefaulted()) {

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



Comment at: llvm/include/llvm/IR/DebugInfoFlags.def:91
 HANDLE_DISP_FLAG((1u << 8), MainSubprogram)
+HANDLE_DISP_FLAG((1u << 9), NotDefaulted)
+HANDLE_DISP_FLAG((1u << 10), DefaultedInClass)

SouraVX wrote:
> aprantl wrote:
> > 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.
> 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?
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.



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

https://reviews.llvm.org/D68117



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

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

[Sourabh Singh Tomar via Phabricator via cfe-commits]

SouraVX marked 7 inline comments as done.
SouraVX added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1610
+  if (const auto *CXXC = dyn_cast(Method)) {
+if (CXXC->getCanonicalDecl()->isDeleted())
+  SPFlags |= llvm::DISubprogram::SPFlagDeleted;

aprantl wrote:
> can you factor this block out into a static function or lambda, since it is 
> repeated three times?
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.



Comment at: clang/test/CodeGenCXX/debug-info-defaulted-in-class.cpp:6
+// RUN:   -O0 -disable-llvm-passes \
+// RUN:   -debug-info-kind=standalone -dwarf-version=5 \
+// RUN: | FileCheck %s -check-prefix=ATTR

aprantl wrote:
> the -dwarf-version flag should be unnecessary now, right?
Thanks for correcting again! Removed unnecessary flags from test cases.



Comment at: llvm/include/llvm/IR/DebugInfoFlags.def:91
 HANDLE_DISP_FLAG((1u << 8), MainSubprogram)
+HANDLE_DISP_FLAG((1u << 9), NotDefaulted)
+HANDLE_DISP_FLAG((1u << 10), DefaultedInClass)

aprantl wrote:
> 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.
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?


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

https://reviews.llvm.org/D68117



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

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

[Sourabh Singh Tomar via Phabricator via cfe-commits]

SouraVX updated this revision to Diff 223240.

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

https://reviews.llvm.org/D68117

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
  clang/test/CodeGenCXX/debug-info-defaulted-in-class.cpp
  clang/test/CodeGenCXX/debug-info-defaulted-out-of-class.cpp
  clang/test/CodeGenCXX/debug-info-deleted.cpp
  clang/test/CodeGenCXX/debug-info-not-defaulted.cpp
  llvm/include/llvm/BinaryFormat/Dwarf.h
  llvm/include/llvm/IR/DebugInfoFlags.def
  llvm/include/llvm/IR/DebugInfoMetadata.h
  llvm/lib/BinaryFormat/Dwarf.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp

Index: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -1296,6 +1296,19 @@
 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 , const DISubrange *SR,
Index: llvm/lib/BinaryFormat/Dwarf.cpp
===
--- llvm/lib/BinaryFormat/Dwarf.cpp
+++ llvm/lib/BinaryFormat/Dwarf.cpp
@@ -271,6 +271,19 @@
   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:
@@ -601,6 +614,8 @@
 return ArrayOrderString(Val);
   case DW_AT_APPLE_runtime_class:
 return LanguageString(Val);
+  case DW_AT_defaulted:
+return DefaultedMemberString(Val);
   }
 
   return StringRef();
Index: llvm/include/llvm/IR/DebugInfoMetadata.h
===
--- llvm/include/llvm/IR/DebugInfoMetadata.h
+++ llvm/include/llvm/IR/DebugInfoMetadata.h
@@ -1758,6 +1758,14 @@
   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.
   ///
Index: llvm/include/llvm/IR/DebugInfoFlags.def
===
--- llvm/include/llvm/IR/DebugInfoFlags.def
+++ llvm/include/llvm/IR/DebugInfoFlags.def
@@ -88,11 +88,15 @@
 HANDLE_DISP_FLAG((1u << 6), Elemental)
 HANDLE_DISP_FLAG((1u << 7), Recursive)
 HANDLE_DISP_FLAG((1u << 8), MainSubprogram)
+HANDLE_DISP_FLAG((1u << 9), Deleted)
+HANDLE_DISP_FLAG((1u << 10), DefaultedInClass)
+HANDLE_DISP_FLAG((1u << 11), DefaultedOutOfClass)
+HANDLE_DISP_FLAG((1u << 12), NotDefaulted)
 
 #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
 
Index: llvm/include/llvm/BinaryFormat/Dwarf.h
===
--- llvm/include/llvm/BinaryFormat/Dwarf.h
+++ llvm/include/llvm/BinaryFormat/Dwarf.h
@@ -411,6 +411,7 @@
 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);
Index: clang/test/CodeGenCXX/debug-info-not-defaulted.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-not-defaulted.cpp
@@ -0,0 +1,30 

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

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1610
+  if (const auto *CXXC = dyn_cast(Method)) {
+if (CXXC->getCanonicalDecl()->isDeleted())
+  SPFlags |= llvm::DISubprogram::SPFlagDeleted;

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



Comment at: clang/test/CodeGenCXX/debug-info-defaulted-in-class.cpp:6
+// RUN:   -O0 -disable-llvm-passes \
+// RUN:   -debug-info-kind=standalone -dwarf-version=5 \
+// RUN: | FileCheck %s -check-prefix=ATTR

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



Comment at: clang/test/CodeGenCXX/debug-info-defaulted-out-of-class.cpp:6
+// RUN:   -O0 -disable-llvm-passes \
+// RUN:   -debug-info-kind=standalone -dwarf-version=5 \
+// RUN: | FileCheck %s -check-prefix=ATTR

same here



Comment at: clang/test/CodeGenCXX/debug-info-not-defaulted.cpp:3
+
+//Supported: DWARF5, -O0, standalone DI
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-linux-gnu %s -o - \

and here



Comment at: llvm/include/llvm/IR/DebugInfoFlags.def:91
 HANDLE_DISP_FLAG((1u << 8), MainSubprogram)
+HANDLE_DISP_FLAG((1u << 9), NotDefaulted)
+HANDLE_DISP_FLAG((1u << 10), DefaultedInClass)

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.


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

https://reviews.llvm.org/D68117



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

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

[Sourabh Singh Tomar via Phabricator via cfe-commits]

SouraVX marked 2 inline comments as done.
SouraVX added a comment.

Will be adding llvm-dwarfdump tests soon.




Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1608
 
+  if (CGM.getCodeGenOpts().DwarfVersion >= 5) {
+// DWARF-5 support for, defaulted, deleted member functions

aprantl wrote:
> 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).
Make sense, removed check from frontend.


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

https://reviews.llvm.org/D68117



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

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

[Sourabh Singh Tomar via Phabricator via cfe-commits]

SouraVX updated this revision to Diff 222701.
SouraVX added a comment.

Added test cases for debug info Clang frontend.


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

https://reviews.llvm.org/D68117

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
  clang/test/CodeGenCXX/debug-info-defaulted-in-class.cpp
  clang/test/CodeGenCXX/debug-info-defaulted-out-of-class.cpp
  clang/test/CodeGenCXX/debug-info-deleted.cpp
  clang/test/CodeGenCXX/debug-info-not-defaulted.cpp
  llvm/include/llvm/BinaryFormat/Dwarf.h
  llvm/include/llvm/IR/DebugInfoFlags.def
  llvm/include/llvm/IR/DebugInfoMetadata.h
  llvm/lib/BinaryFormat/Dwarf.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp

Index: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -1296,6 +1296,19 @@
 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 , const DISubrange *SR,
Index: llvm/lib/BinaryFormat/Dwarf.cpp
===
--- llvm/lib/BinaryFormat/Dwarf.cpp
+++ llvm/lib/BinaryFormat/Dwarf.cpp
@@ -271,6 +271,19 @@
   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:
@@ -601,6 +614,8 @@
 return ArrayOrderString(Val);
   case DW_AT_APPLE_runtime_class:
 return LanguageString(Val);
+  case DW_AT_defaulted:
+return DefaultedMemberString(Val);
   }
 
   return StringRef();
Index: llvm/include/llvm/IR/DebugInfoMetadata.h
===
--- llvm/include/llvm/IR/DebugInfoMetadata.h
+++ llvm/include/llvm/IR/DebugInfoMetadata.h
@@ -1758,6 +1758,14 @@
   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.
   ///
Index: llvm/include/llvm/IR/DebugInfoFlags.def
===
--- llvm/include/llvm/IR/DebugInfoFlags.def
+++ llvm/include/llvm/IR/DebugInfoFlags.def
@@ -88,11 +88,15 @@
 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
 
Index: llvm/include/llvm/BinaryFormat/Dwarf.h
===
--- llvm/include/llvm/BinaryFormat/Dwarf.h
+++ llvm/include/llvm/BinaryFormat/Dwarf.h
@@ -411,6 +411,7 @@
 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);
Index: clang/test/CodeGenCXX/debug-info-not-defaulted.cpp
===
--- 

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

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

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.




Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1608
 
+  if (CGM.getCodeGenOpts().DwarfVersion >= 5) {
+// DWARF-5 support for, defaulted, deleted member functions

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


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

https://reviews.llvm.org/D68117



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

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

[Sourabh Singh Tomar via Phabricator via cfe-commits]

SouraVX updated this revision to Diff 86.
SouraVX added a comment.

Minor refactor for context.


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

https://reviews.llvm.org/D68117

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
  llvm/include/llvm/BinaryFormat/Dwarf.h
  llvm/include/llvm/IR/DebugInfoFlags.def
  llvm/include/llvm/IR/DebugInfoMetadata.h
  llvm/lib/BinaryFormat/Dwarf.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp

Index: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -1296,6 +1296,19 @@
 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 , const DISubrange *SR,
Index: llvm/lib/BinaryFormat/Dwarf.cpp
===
--- llvm/lib/BinaryFormat/Dwarf.cpp
+++ llvm/lib/BinaryFormat/Dwarf.cpp
@@ -271,6 +271,19 @@
   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:
@@ -601,6 +614,8 @@
 return ArrayOrderString(Val);
   case DW_AT_APPLE_runtime_class:
 return LanguageString(Val);
+  case DW_AT_defaulted:
+return DefaultedMemberString(Val);
   }
 
   return StringRef();
Index: llvm/include/llvm/IR/DebugInfoMetadata.h
===
--- llvm/include/llvm/IR/DebugInfoMetadata.h
+++ llvm/include/llvm/IR/DebugInfoMetadata.h
@@ -1758,6 +1758,14 @@
   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.
   ///
Index: llvm/include/llvm/IR/DebugInfoFlags.def
===
--- llvm/include/llvm/IR/DebugInfoFlags.def
+++ llvm/include/llvm/IR/DebugInfoFlags.def
@@ -88,11 +88,15 @@
 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
 
Index: llvm/include/llvm/BinaryFormat/Dwarf.h
===
--- llvm/include/llvm/BinaryFormat/Dwarf.h
+++ llvm/include/llvm/BinaryFormat/Dwarf.h
@@ -411,6 +411,7 @@
 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);
Index: clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
===
--- clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
+++ clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
@@ -4,8 +4,7 @@
 // RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple %s -o - \
 // RUN:   -O1 -disable-llvm-passes \
 // 

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

[Sourabh Singh Tomar via Phabricator via cfe-commits]

SouraVX marked 2 inline comments as done.
SouraVX added a comment.

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?




Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1608
 
+  if (CGM.getCodeGenOpts().DwarfVersion >= 5) {
+// DWARF-5 support for, defaulted, deleted member functions

aprantl wrote:
> Please try to always upload patches with more context (git diff -U works 
> fine). I can't even tell which function this is in otherwise.
Thanks, for correcting  this. Seems to miss context part every time.


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

https://reviews.llvm.org/D68117



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

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

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

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


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

https://reviews.llvm.org/D68117



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

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

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

Thanks, this looks like a good addition!




Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1608
 
+  if (CGM.getCodeGenOpts().DwarfVersion >= 5) {
+// DWARF-5 support for, defaulted, deleted member functions

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


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

https://reviews.llvm.org/D68117



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

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

[Sourabh Singh Tomar via Phabricator via cfe-commits]

SouraVX updated this revision to Diff 222079.
SouraVX added a comment.

Minor refactor.


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

https://reviews.llvm.org/D68117

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
  llvm/include/llvm/BinaryFormat/Dwarf.h
  llvm/include/llvm/IR/DebugInfoFlags.def
  llvm/include/llvm/IR/DebugInfoMetadata.h
  llvm/lib/BinaryFormat/Dwarf.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp

Index: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -1296,6 +1296,19 @@
 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 , const DISubrange *SR,
Index: llvm/lib/BinaryFormat/Dwarf.cpp
===
--- llvm/lib/BinaryFormat/Dwarf.cpp
+++ llvm/lib/BinaryFormat/Dwarf.cpp
@@ -271,6 +271,19 @@
   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:
@@ -601,6 +614,8 @@
 return ArrayOrderString(Val);
   case DW_AT_APPLE_runtime_class:
 return LanguageString(Val);
+  case DW_AT_defaulted:
+return DefaultedMemberString(Val);
   }
 
   return StringRef();
Index: llvm/include/llvm/IR/DebugInfoMetadata.h
===
--- llvm/include/llvm/IR/DebugInfoMetadata.h
+++ llvm/include/llvm/IR/DebugInfoMetadata.h
@@ -1758,6 +1758,14 @@
   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.
   ///
Index: llvm/include/llvm/IR/DebugInfoFlags.def
===
--- llvm/include/llvm/IR/DebugInfoFlags.def
+++ llvm/include/llvm/IR/DebugInfoFlags.def
@@ -88,11 +88,15 @@
 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
 
Index: llvm/include/llvm/BinaryFormat/Dwarf.h
===
--- llvm/include/llvm/BinaryFormat/Dwarf.h
+++ llvm/include/llvm/BinaryFormat/Dwarf.h
@@ -411,6 +411,7 @@
 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);
Index: clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
===
--- clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
+++ clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
@@ -4,8 +4,7 @@
 // RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple %s -o - \
 // RUN:   -O1 -disable-llvm-passes \
 // RUN:   

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

[Sourabh Singh Tomar via Phabricator via cfe-commits]

SouraVX created this revision.
SouraVX added reviewers: aprantl, dblaikie, probinson.
SouraVX added projects: LLVM, clang, debug-info.
Herald added a subscriber: hiraditya.

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


https://reviews.llvm.org/D68117

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
  clang/test/CodeGenCXX/debug-info-decl-nested.cpp
  llvm/include/llvm/BinaryFormat/Dwarf.h
  llvm/include/llvm/IR/DebugInfoFlags.def
  llvm/include/llvm/IR/DebugInfoMetadata.h
  llvm/lib/BinaryFormat/Dwarf.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp

Index: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -1296,6 +1296,19 @@
 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 , const DISubrange *SR,
Index: llvm/lib/BinaryFormat/Dwarf.cpp
===
--- llvm/lib/BinaryFormat/Dwarf.cpp
+++ llvm/lib/BinaryFormat/Dwarf.cpp
@@ -271,6 +271,19 @@
   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:
@@ -601,6 +614,8 @@
 return ArrayOrderString(Val);
   case DW_AT_APPLE_runtime_class:
 return LanguageString(Val);
+  case DW_AT_defaulted:
+return DefaultedMemberString(Val);
   }
 
   return StringRef();
Index: llvm/include/llvm/IR/DebugInfoMetadata.h
===
--- llvm/include/llvm/IR/DebugInfoMetadata.h
+++ llvm/include/llvm/IR/DebugInfoMetadata.h
@@ -1758,6 +1758,14 @@
   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.
   ///
Index: llvm/include/llvm/IR/DebugInfoFlags.def
===
--- llvm/include/llvm/IR/DebugInfoFlags.def
+++ llvm/include/llvm/IR/DebugInfoFlags.def
@@ -88,11 +88,15 @@
 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
 
Index: llvm/include/llvm/BinaryFormat/Dwarf.h
===
--- llvm/include/llvm/BinaryFormat/Dwarf.h
+++ llvm/include/llvm/BinaryFormat/Dwarf.h
@@ -411,6 +411,7 @@
 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);
Index: clang/test/CodeGenCXX/debug-info-decl-nested.cpp
===
--- clang/test/CodeGenCXX/debug-info-decl-nested.cpp
+++