Re: [PATCH 3/7] Emit macro expansion related diagnostics

[Dodji Seketeli <dodji@redhat.com>]

Jason Merrill <jason@redhat.com> writes:

> On 09/19/2011 09:58 AM, Dodji Seketeli wrote:
> > +   The part that goes from the third to the heighth line of this
> 
> An extra 'h' snuck in there. :)

Oops, fixed in my local copy, sorry.

> 
> > Inside the definition of a macro M, you can have another macro M'.
> > And M' is going to be eventually expanded into a token FOO.
> >
> > So, to paraphrase what I said earlier, FOO [which is at a point in the
> > definition of the macro M] is itself inside the expanded macro M'.
> 
> So what we're dealing with here is the token FOO.  We start with its
> fully-expanded location, and then step out to see where it was
> expanded from.  If that location is still within a macro expansion, we
> step out again until we are at the point in the source that originally
> triggered a macro expansion.  Right?

Right.

> I don't understand how that unwinding operation maps onto a function
> called linemap_macro_map_loc_to_def_point,

In the example I used in the comment of that function, suppose that
during 'stepping out', as you put it, we don't deal with the place in
the source where '<<' appears in the definition of a macro.  We'd then
only deal with the expansion points of the macros involved.  That
would mean that we would only record in the trace the following
locations (and be able to only display this trace):

1/

    test.c:5:14: error: invalid operands to binary << (have 'double' and 'int')
    test.c:5:3: note: expanded from here
    test.c:8:3: note: expanded from here
    test.c:13:3: note: expanded from here

But I also want to display where "<<" appears in the definition of the
relevant macro ...

> and why we need to use both that function and
> linemap_macro_map_loc_to_exp_point here.

... so that I can have a trace that shows the locations also in the
context of the definitions of the macros:

2/

    test.c:5:14: error: invalid operands to binary << (have 'double' and 'int')
    test.c:2:9: note: in expansion of macro 'OPERATE'
    test.c:5:3: note: expanded from here
    test.c:5:14: note: in expansion of macro 'SHIFTL'
    test.c:8:3: note: expanded from here
    test.c:8:3: note: in expansion of macro 'MULT2'
    test.c:13:3: note: expanded from here

> 
> I'm afraid this is leading me to question the basic model again.
> 
> > +      1    #define OPERATE(OPRD1, OPRT, OPRD2) \
> > +      2      OPRD1 OPRT OPRD2;
> > +      3
> > +      4    #define SHIFTL(A,B) \
> > +      5      OPERATE (A,<<,B)
> > +      6
> > +      7    #define MULT(A) \
> > +      8      SHIFTL (A,1)
> > +      9
> > +     10    void
> > +     11    g ()
> > +     12    {
> > +     13      MULT (1.0);// 1.0 << 1; <-- so this is an error.
> > +     14    }
> 
> Here "1.0" has the locations 2(OPRD1), 5(A), 8(A), 13(1.0).  "<<"
> has the locations 2(OPRT), 5(<<), 8(SHIFTL), 13(MULT).  Each token
> has 4 locations, one for each of the macros involved plus one for
> the original expansion location.  So it seems like we ought to be
> able to get away with only storing one location per token in a macro
> expansion map.

This is essentially what we do, for tokens of macro replacement list
that are not macro parameters.

In a function-like macro, tokens of macro parameters are replaced by
the tokens of the arguments; if we just store the spelling location of
a given parameter (and forget about the location of the matching
argument as you are suggesting), getting the location of that argument
at virtual location resolution time can be difficult.

In your example, suppose we didn't store the location of "<<" inside
the map for OPERATE, and that we only stored the location (2:9) of
OPRT, as you suggest.  How, to generate the trace, would we step out
to get the location (5:14) (of "<<") from that (2:9)?  For that, I
guess we'd need to store the locations of the arguments of OPERATE,
somehow, figure out that OPRT is the parameter for the argument "<<"
and act from there.  This is the argument replacement replay I talked
about in another thread when you first raised this point.  We'd need
to do some parts of what replace_args does and handle cases like
pasting etc.  Tom and I figured this would be a bit more difficult so
we preferred staying with this simpler scheme for now.  The simpler
scheme being to store the location of the argument "<<" and the
location of the parameter OPRT in the map.

-- 
		Dodji