This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PR c++/24985 caret diagnostics for macro expansions


On Thu, May 3, 2012 at 5:58 PM, Manuel López-Ibáñez
<lopezibanez@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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]