[debug-early] Handle specification of class scoped static functions

On 03/20/2015 02:21 PM, Jason Merrill wrote:
> I think we want to drop the debug_early check there entirely; the added
> conditions seem to be gutting it.  If is_cu_die (old_die->die_parent) is
> false, then class_or_namespace_scope_p (old_die->die_parent) ought to be
> true.
>
> Jason
>

Good catch.  I am so glad you are keeping track of all this spaghetti, 
but in my defense, it was pasta already.

With the attached I also got rid of one superfluous check for `old_die', 
as well as your suggestion.  We get rid of one more gdb regression.  Yay.

I'll wait for your high-five (or OK) before committing to branch.

Thanks.
Aldy
commit b3d910713d27dc29801f3ddbe8671a4a6e0de4c1
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Fri Mar 20 09:55:31 2015 -0700

    Handle specification of class scoped static functions.
    
    Remove superfluous check for old_die.

On 03/20/2015 08:11 PM, Aldy Hernandez wrote:
> +	   /* For class scoped static functions, the dumped early
> +	      version was the declaration, whereas the next time
> +	      around with a different context should be the
> +	      specification.  In this case, avoid reusing the DIE, but
> +	      generate a specification below. E.g.:
> +
> +	      class C {
> +	      public:
> +	        static void moo () {}
> +	      };  */
> +	   || !is_cu_die (context_die))

Why do we still need this added (relative to trunk)?  Are we getting 
here multiple times with class context_die?

Also, the comment seems redundant with the comment immediately above.

Jason

On 04/03/2015 07:48 AM, Jason Merrill wrote:
> On 03/20/2015 08:11 PM, Aldy Hernandez wrote:
>> +       /* For class scoped static functions, the dumped early
>> +          version was the declaration, whereas the next time
>> +          around with a different context should be the
>> +          specification.  In this case, avoid reusing the DIE, but
>> +          generate a specification below. E.g.:
>> +
>> +          class C {
>> +          public:
>> +            static void moo () {}
>> +          };  */
>> +       || !is_cu_die (context_die))
>
> Why do we still need this added (relative to trunk)?  Are we getting
> here multiple times with class context_die?

Apparently we no longer need it for the C++ case above, so the comment 
certainly needs updating, but we need it for fortran:

module some_m
contains
    logical function funky (FLAG)
      funky = .true.
   end function
end module

The first time through gen_subprogram_die() we generate the DIE with a 
context of DW_TAG_module (early dwarf).  The second time, in late dwarf, 
we get here with a DW_TAG_module context again, so the above code will 
allow us to reuse the DIE, instead of creating a DW_AT_specification.

...or perhaps we could change the condition to:

       if ((is_cu_die (old_die->die_parent)
+	   || old_die->die_parent->die_tag == DW_TAG_module
	   || context_die == NULL

??
Aldy

On 04/13/2015 02:01 PM, Aldy Hernandez wrote:
> ...or perhaps we could change the condition to:
>
>        if ((is_cu_die (old_die->die_parent)
> +       || old_die->die_parent->die_tag == DW_TAG_module
>         || context_die == NULL

Does checking context_die == old_die->die_parent work?

Jason

On 04/13/2015 05:04 PM, Jason Merrill wrote:
> On 04/13/2015 02:01 PM, Aldy Hernandez wrote:
>> ...or perhaps we could change the condition to:
>>
>>        if ((is_cu_die (old_die->die_parent)
>> +       || old_die->die_parent->die_tag == DW_TAG_module
>>         || context_die == NULL
>
> Does checking context_die == old_die->die_parent work?

Actually, no.  The comment here is saying that we don't want to reuse 
the DIE in this case, because we want the DIE with the source location 
info to be at CU scope.  We only want to reuse if the function is at CU 
scope anyway or it's a nested function.

Jason