Re: [PATCH] Emit DW_AT_inline for C++17 inline variables (take 2)

[Mark Wielaard]

On Wed, 2016-11-02 at 00:08 +0100, Mark Wielaard wrote:
> That seemed to have been the last usage of origin_die in gen_variable_die.
> So removing that caused:
> 
> /home/mark/src/gcc/gcc/dwarf2out.c: In function ‘void gen_variable_die(tree, 
> tree, dw_die_ref)’:
> /home/mark/src/gcc/gcc/dwarf2out.c:22454:14: error: variable ‘origin_die’ set 
> but not used [-Werror=unused-but-set-variable]
>dw_die_ref origin_die = NULL;
>   ^~
> cc1plus: all warnings being treated as errors
> make[3]: *** [dwarf2out.o] Error 1
> 
> The following seems to work around that and makes things build again for me:

Which is precisely what Jakub checked in 5 minutes before I sent that
email.

Sorry for the noise. Everything builds again.

Thanks,

Mark

Re: [PATCH] Emit DW_AT_inline for C++17 inline variables (take 2)

[Mark Wielaard]

On Tue, Nov 01, 2016 at 01:20:23PM -0400, Jason Merrill wrote:
> On Tue, Nov 1, 2016@12:10 PM, Jakub Jelinek  wrote:
> > + && !get_AT (var_die, DW_AT_inline)
> > + && (origin_die == NULL || get_AT (origin_die, DW_AT_inline) == 
> > NULL)
> 
> Can we drop this repetition (here and for DW_AT_const_expr)?  get_AT
> should look through DW_AT_abstract_origin, and we should have added
> that earlier in gen_variable_die.
> 
> OK with that change.

That seemed to have been the last usage of origin_die in gen_variable_die.
So removing that caused:

/home/mark/src/gcc/gcc/dwarf2out.c: In function ‘void gen_variable_die(tree, 
tree, dw_die_ref)’:
/home/mark/src/gcc/gcc/dwarf2out.c:22454:14: error: variable ‘origin_die’ set 
but not used [-Werror=unused-but-set-variable]
   dw_die_ref origin_die = NULL;
  ^~
cc1plus: all warnings being treated as errors
make[3]: *** [dwarf2out.o] Error 1

The following seems to work around that and makes things build again for me:

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 5ff6f97..24b85c1 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -22451,7 +22451,6 @@ gen_variable_die (tree decl, tree origin, dw_die_ref 
context_die)
   tree ultimate_origin;
   dw_die_ref var_die;
   dw_die_ref old_die = decl ? lookup_decl_die (decl) : NULL;
-  dw_die_ref origin_die = NULL;
   bool declaration = (DECL_EXTERNAL (decl_or_origin)
  || class_or_namespace_scope_p (context_die));
   bool specialization_p = false;
@@ -22627,7 +22626,7 @@ gen_variable_die (tree decl, tree origin, dw_die_ref 
context_die)
 var_die = new_die (DW_TAG_variable, context_die, decl);
 
   if (origin != NULL)
-origin_die = add_abstract_origin_attribute (var_die, origin);
+add_abstract_origin_attribute (var_die, origin);
 
   /* Loop unrolling can create multiple blocks that refer to the same
  static variable, so we must test for the DW_AT_declaration flag.

Cheers,

Mark

Re: [PATCH] Emit DW_AT_inline for C++17 inline variables (take 2)

[Jason Merrill]

On Tue, Nov 1, 2016 at 12:10 PM, Jakub Jelinek  wrote:
> + && !get_AT (var_die, DW_AT_inline)
> + && (origin_die == NULL || get_AT (origin_die, DW_AT_inline) == NULL)

Can we drop this repetition (here and for DW_AT_const_expr)?  get_AT
should look through DW_AT_abstract_origin, and we should have added
that earlier in gen_variable_die.

OK with that change.

Jason

Re: [PATCH] Emit DW_AT_inline for C++17 inline variables

[Jason Merrill]

On Mon, Oct 31, 2016 at 10:25 AM, Jakub Jelinek  wrote:
> On Mon, Oct 31, 2016 at 09:52:28AM -0400, Jason Merrill wrote:
>> On 10/14/2016 01:33 PM, Jakub Jelinek wrote:
>> >This also uses the infrastructure of the langhook patch I've sent earlier.
>> >It emits (if not strict dwarf) DW_AT_inline on explicit or implicit inline
>> >variables, and also tweaks dwarf2out so that for inline static data members
>> >we consider in-class declarations as definitions (emit DW_AT_linkage_name
>> >and no DW_AT_declaration) for them.
>>
>> Hmm, so there's no DW_TAG_variable for the inline static data member? That
>> seems problematic to me.  The DWARF 3 convention that static data members
>> use DW_TAG_member seems rather misguided, since in fact they are variables.
>> Why did that change?
>
> I had to bisect it, apparently it has been changed by myself in
> r145770, rationale in thread starting at
> https://gcc.gnu.org/ml/gcc-patches/2009-04/msg00039.html
>
> The current DWARF 5 wording is:
> "If the variable entry represents the defining declaration for a C++ static 
> data
> member of a structure, class or union, the entry has a DW_AT_specification
> attribute, whose value is a reference to the debugging information entry
> representing the declaration of this data member. The referenced entry has
> the tag DW_TAG_member and will be a child of some class, structure or
> union type entry." on page 98 in DWARF5_Public_Review.pdf.

Yes, this changed in DWARF 3; DWARF 2 didn't specify the tag.  I think
this was a mistake.

> Incidentally, I've filed today a DWARF issue that Appendix A doesn't list
> for DW_TAG_member lots of attributes that are allowed for DW_TAG_variable
> and are useful for static data members.

Using DW_TAG_variable would address that, too.

Jason

Re: [PATCH] Emit DW_AT_inline for C++17 inline variables

[Jakub Jelinek]

On Mon, Oct 31, 2016 at 09:52:28AM -0400, Jason Merrill wrote:
> On 10/14/2016 01:33 PM, Jakub Jelinek wrote:
> >This also uses the infrastructure of the langhook patch I've sent earlier.
> >It emits (if not strict dwarf) DW_AT_inline on explicit or implicit inline
> >variables, and also tweaks dwarf2out so that for inline static data members
> >we consider in-class declarations as definitions (emit DW_AT_linkage_name
> >and no DW_AT_declaration) for them.
> 
> Hmm, so there's no DW_TAG_variable for the inline static data member? That
> seems problematic to me.  The DWARF 3 convention that static data members
> use DW_TAG_member seems rather misguided, since in fact they are variables.
> Why did that change?

I had to bisect it, apparently it has been changed by myself in
r145770, rationale in thread starting at
https://gcc.gnu.org/ml/gcc-patches/2009-04/msg00039.html

The current DWARF 5 wording is:
"If the variable entry represents the defining declaration for a C++ static data
member of a structure, class or union, the entry has a DW_AT_specification
attribute, whose value is a reference to the debugging information entry
representing the declaration of this data member. The referenced entry has
the tag DW_TAG_member and will be a child of some class, structure or
union type entry." on page 98 in DWARF5_Public_Review.pdf.
And page 117 has in 5.7.6:
"A data member (as opposed to a member function) is represented by a
debugging information entry with the tag DW_TAG_member."

So, using DW_TAG_variable as DW_TAG_{structure,class}_type children
for the non-inline static data members would violate these two.
C++17 is outside of the scope of DWARF5 I guess, so the question is to find
out what do we want for those and agree on that with consumers and other
producers.

Incidentally, I've filed today a DWARF issue that Appendix A doesn't list
for DW_TAG_member lots of attributes that are allowed for DW_TAG_variable
and are useful for static data members.  Some of them we want anyway, at
least unless we agree on the non-inline static data members no longer be
DW_TAG_member in the structure/class type and change the standard.
In particular DW_AT_const_value and DW_AT_const_expr are desirable even
for the static data members in C++98..14 (or non-inline ones in C++17).
The rest, like DW_AT_linkage_name, DW_AT_location, etc. are generally more
useful mainly for the inline ones.

Jakub

Re: [PATCH] Emit DW_AT_inline for C++17 inline variables

[Jason Merrill]

On 10/14/2016 01:33 PM, Jakub Jelinek wrote:

This also uses the infrastructure of the langhook patch I've sent earlier.
It emits (if not strict dwarf) DW_AT_inline on explicit or implicit inline
variables, and also tweaks dwarf2out so that for inline static data members
we consider in-class declarations as definitions (emit DW_AT_linkage_name
and no DW_AT_declaration) for them.


Hmm, so there's no DW_TAG_variable for the inline static data member? 
That seems problematic to me.  The DWARF 3 convention that static data 
members use DW_TAG_member seems rather misguided, since in fact they are 
variables.  Why did that change?


Jason