This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [C PATCH] Fix debug info locus of enum with previous forward declaration (PR c/79969)
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Marek Polacek <polacek at redhat dot com>
- Cc: "Joseph S. Myers" <joseph at codesourcery dot com>, gcc-patches at gcc dot gnu dot org
- Date: Thu, 9 Mar 2017 16:57:42 +0100
- Subject: Re: [C PATCH] Fix debug info locus of enum with previous forward declaration (PR c/79969)
- Authentication-results: sourceware.org; auth=none
- References: <20170309092443.GF22703@tucnak> <20170309154931.GD3172@redhat.com>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
On Thu, Mar 09, 2017 at 04:49:31PM +0100, Marek Polacek wrote:
> On Thu, Mar 09, 2017 at 10:24:43AM +0100, Jakub Jelinek wrote:
> > Hi!
> >
> > Similarly to https://gcc.gnu.org/ml/gcc-patches/2009-09/msg01161.html
> > the C FE gets wrong the location of DW_TAG_enumeral_type if there is a
> > forward declaration. If we e.g. have a variable that is first declared
> > extern and then defined, we emit DW_TAG_variable with the location of the
> > first declaration and then another DW_TAG_variable with DW_AT_specification
> > pointing to the previous one for the definition, with locus of the
> > definition. That is not what we do for enums, there is just one DIE, so we
> > should use the more descriptive from the locations, which is the definition.
> >
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> >
> > 2017-03-09 Jakub Jelinek <jakub@redhat.com>
> >
> > PR c/79969
> > * c-decl.c (start_enum): Adjust DECL_SOURCE_LOCATION of
> > TYPE_STUB_DECL.
>
> How about doing this in finish_enum, similarly to finish_struct? You'd need to
> pass a location from the parser, but that's easy:
That would be just for consistency with finish_struct, right?
Not sure if we need such consistency, but I don't care that much. The point
to put it into start_enum was that we don't really care about the location
of the forward declaration after that.
That said, there is one thing I'm wondering about:
if (name != NULL_TREE)
enumtype = lookup_tag (ENUMERAL_TYPE, name, true, &enumloc);
if (enumtype == NULL_TREE || TREE_CODE (enumtype) != ENUMERAL_TYPE)
{
enumtype = make_node (ENUMERAL_TYPE);
pushtag (loc, name, enumtype);
}
with the optional patched spot after this. Now, if somebody does:
enum E;
enum E { A, B, C };
enum E { D, F };
then I think we'll complain about line 3 overriding previous definition
at line 1 (which is not right). Maybe if there is TYPE_STUB_DECL (do we
have it always?), we can override enumloc = DECL_SOURCE_LOCATION
(TYPE_STUB_DECL (enumtype));? I bet trying to change the binding ->locus
would be too much work.
Jakub