On Thu, May 3, 2012 at 5:58 PM, Manuel López-Ibáñez
<lopeziba...@gmail.com> wrote:
> This patch enables caret diagnostics for macro expansions. So now we will get:
>
> /home/manuel/macro-clang.c:2:91: error: invalid operands to binary <
> (have ‘struct mystruct’ and ‘float’)
> #define MYMAX(A,B) __extension__ ({ __typeof__(A) __a = (A);
> __typeof__(B) __b = (B); __a < __b ? __b : __a; })
>
> ^
> /home/manuel/macro-clang.c:2:91: note: in expansion of macro 'MYMAX'
> #define MYMAX(A,B) __extension__ ({ __typeof__(A) __a = (A);
> __typeof__(B) __b = (B); __a < __b ? __b : __a; })
>
> ^
> /home/manuel/macro-clang.c:9:3: note: expanded from here
> MYMAX(p, f);
> ^
>
> In my opinion, the macro unwinder is too verbose. I think the output
> should be just:
>
> 2.91: error: invalid...
> #define MYMAX(A,B) __extension__ ({ __typeof__(A) __a = (A);
> __typeof__(B) __b = (B); __a < __b ? __b : __a; })
>
> ^
> 9.3: note: in expansion of macro 'MYMAX'
> MYMAX(p, f);
> ^
>
> A second option is to use the expansion order, as Clang does:
>
> t.c:80:3: error: invalid operands to binary expression ('typeof(P)'
> (aka 'struct mystruct') and 'typeof(F)' (aka 'float'))
> X = MYMAX(P, F);
> ^~~~~~~~~~~
> t.c:76:94: note: instantiated from:
> #define MYMAX(A,B) __extension__ ({ __typeof__(A) __a = (A);
> __typeof__(B) __b = (B); __a < __b ? __b : __a; })
>
> ~~~ ^ ~~~
>
> (Clang calls this "Automatic Macro Expansion", but it is not actually
> expanding the macro, just showing its definition, like this patch
> does.)
>
> However, this is orthogonal to this patch, which simply prints the
> caret and does not change the unwinder. It would be easier to assess
> changes to the unwinder with the caret enabled than without.
>
> Bootstrapped and regression tested.
>
> OK?
The patch changes indentation in several places (violating, I think
coding conventions) which are unrelated to the functionality
that the patch is supposed to implement. Could you resend
a version unencumbered by those changes for review?
The changes to use pp_get_prefix and pp_set_prefix are OK.
-- Gaby
