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

["Manuel López-Ibáñez" <lopezibanez@gmail.com>]

On 13 December 2010 15:44, Paolo Bonzini <bonzini@gnu.org> wrote:
> On 12/10/2010 12:11 PM, Dodji Seketeli wrote:
>> [dodji@adjoa gcc]$ ./cc1 -quiet -ftrack-macro-expansion test.c
>> test.c: In function ‘g’:
>> test.c:5:14: error: invalid operands to binary<<  (have ‘double’ and ‘int’)
>> In macro 'OPERATE' at test.c:2:9
>>      Expanded at test.c:5:3
>> In macro 'SHIFTL' at test.c:5:14
>>      Expanded at test.c:8:3
>> In macro 'MULT' at test.c:8:3
>>      Expanded at test.c:13:3
>>
>
>
> or this shorter example:
>
> test.c:5:14: error: invalid operands to binary<<  (have ‘double’ and ‘int’)
> test.c:2:9: note: while expanding macro 'OPERATE'
> test.c:5:14: note: while expanding macro 'SHIFTL'
> test.c:8:3: note: while expanding macro 'MULT'
> test.c:13:3: note: expanded from here
>

This format is closer to what GCC currently prints for templates
instantiations, however, in template instantiations, the context goes
first and the error/warning goes last. The context is also not marked
with "note:". See
http://people.redhat.com/bkoz/diagnostics/diagnostics.html#template-instantiate-1
and http://people.redhat.com/bkoz/diagnostics/diagnostics.html#9335
and other examples therein.

See gcc/cp/error.c:print_instantiation_partial_context, which also
handles eliding excessive number of instantiation contexts (too deep
macro expansion). I am not saying that this has to be done in this
patch series, but it should eventually be implemented, so why not copy
what is already available?

> or this that does not change the location compared to current GCC:
>
> test.c:13:3: error: invalid operands to binary<<  (have ‘double’ and ‘int’)
> test.c:2:9: note: while expanding macro 'OPERATE'
> test.c:5:14: note: while expanding macro 'SHIFTL'
> test.c:8:3: note: while expanding macro 'MULT'

Interestingly, this is closer to the format adopted by Clang
(http://clang.llvm.org/diagnostics.html), which I guess just followed
GCC. However, I don't see how changing the locations to the ones
proposed by Dodji causes any kind of "havoc" for users. In fact,
looking at the caret information of clang, the diagnostic would be
clearer if the locations were exchanged, which is what Dodji proposes.

I don't think gcc diagnostics can be used to do any kind of automatic
rewriting. At most, it is parsed in order to present the location of
errors/messages in a nicer way, and the new locations proposed by
Dodji do not break that.

The question should be what locations are clearer/more informative to
the user. I think the new locations proposed by Dodji are more
informative and they follow what g++ already does for template errors.

In any case, I guess that if there is a flag to disable the tracking
of macro expansions for memory concerns, the same flag disables the
enhanced locations and reverts to the current status, no? No need for
yet another separate flag to control the output.

Cheers,

Manuel.