[C PATCH] Fix debug info locus of enum with previous forward declaration (PR c/79969)

Jakub Jelinek jakub@redhat.com
Thu Mar 9 15:57:00 GMT 2017


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



More information about the Gcc-patches mailing list