This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] C++/DWARF : Add 'using' support - take 2
- From: Jason Merrill <jason at redhat dot com>
- To: Devang Patel <dpatel at apple dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Tue, 13 Jan 2004 17:32:25 -0500
- Subject: Re: [PATCH] C++/DWARF : Add 'using' support - take 2
- References: <0B6FDAF0-2C47-11D8-AD81-000393A91CAA@apple.com><xypr7zapmwy.fsf@miranda.boston.redhat.com><204973B6-2CD3-11D8-8D51-000393A91CAA@apple.com><xyp65glnpwb.fsf@miranda.boston.redhat.com><BB6DFCC8-3274-11D8-B338-000393A91CAA@apple.com><xyp7jzvmuc2.fsf@miranda.boston.redhat.com><1A0648CD-4613-11D8-8C16-000393A91CAA@apple.com>
On Tue, 13 Jan 2004 13:54:46 -0800, Devang Patel <dpatel@apple.com> wrote:
> On Jan 13, 2004, at 1:06 PM, Jason Merrill wrote:
>> These hunks seem useless; you aren't using tmp_die anywhere. You really
>> ought to review your own patches for this sort of thing before you ask
>> someone else to review them.
>
> I intentionally added it to note that now gen_subprogram_die returns new
> die. If you want I will remove it.
Please.
>> I'd prefer to do without all the changes to make gen_subprogram_die return
>> a value. I don't see a good reason to break the convention of "generate,
>> then look up" for just this one variety of DIE.
>
> We do not equate abstract dies with decl number
Huh?
else if (DECL_ABSTRACT (decl))
{
...
equate_decl_number_to_die (decl, subr_die);
}
>> You should handle this in the "else if (old_die)" block below, which also
>> deals with reusing a die.
>
> It handles reusing decl die as definition die. We need to complete this
> die which is the else section after "else if (old_die)".
What do you mean by "complete"? The die created by force_decl_die should
be a declaration die, so reusing it as a definition die should be what we
want.
>> I don't see how checking die_definition makes any sense here. What
>> situation is this code trying to handle?
>
> If variable for forced out earlier than later on add_AT_specification()
> aborts because die_definition is non-zero.
Why is it non-zero?
>>> ! force_decl_die (tree decl)
>>> ! {
>>> ...
>>> ! else if (DECL_INLINE (decl))
>>> ! /* Force die to represent this function as abstract
>>> ! function for our reference. */
>>> ! dwarf2out_abstract_function (decl);
>>
>> I realize that I suggested this in my earlier review, but I think we
>> shouldn't handle inline functions differently here. If all we care about
>> is having a DIE to refer to, we don't need to emit the whole abstract
>> function.
>
> so instead what do you suggest here?
I think we want to force a declaration die in all cases, regardless of
whether or not the function has been defined. I'm reluctant to change the
order in which definition dies are emitted.
>> (btw, checking DECL_INLINE is the wrong way to test this)
>
> why?
Because DECL_INLINE doesn't tell you whether or not we want an abstract
instance of a particular function. cgraph_function_possibly_inlined_p is
better, but still misses cases where we use abstract instances in the
absence of inlining. Cloning, for instance.
>>> ! case FIELD_DECL:
>>> ! case VAR_DECL:
>>> ! gen_decl_die (decl, context_die);
>>> ! break;
>>> ! default:
>>> ! dwarf2out_decl (decl);
>>> ! break;
>>
>> What other decls are likely to come up? Why is dwarf2out_decl the right
>> thing for them?
>
> I do not remember. May be it is here because force_decl_die() is a
> replacement for original dwarf2out_decl() call I had. I'll test again
> by adding abort() for other decls.
Thanks.
>>> *************** do_using_directive (tree namespace)
>> Also, this is wrong for function local using-directives; they need to be
>> in current_scope(), not current_namespace.
>
> I use current_scope() in do_local_using_decl().
And you should use it here, too; that deals with using-declaration, this
deals with using-directives.
Jason