]> git.ipfire.org Git - thirdparty/gcc.git/commit
Make unwound macro expansion trace less redundant
authorDodji Seketeli <dodji@redhat.com>
Thu, 24 May 2012 19:37:45 +0000 (19:37 +0000)
committerDodji Seketeli <dodji@gcc.gnu.org>
Thu, 24 May 2012 19:37:45 +0000 (21:37 +0200)
commit165ca58dc37d9c10443e5cf1daa836edb355a0f0
tree9447b3399ccf2b1a74d55172202e0c240c861cdf
parenta5dedb5e59b72167bcc501f28a8a8c08b9465070
Make unwound macro expansion trace less redundant

As discussed previously, the unwinder for macro expansion is quite
verbose [1].  This patch proposes to address that shortcoming.

Consider this test case:

    $ cat -n test.c
 1 #define MYMAX(A,B) __extension__ ({ __typeof__(A) __a = (A); \
 2  __typeof__(B) __b = (B); __a < __b ? __b : __a; })
 3
 4 struct mystruct {};
 5 void
 6 foo()
 7 {
 8   struct mystruct p;
 9   float f = 0.0;
10   MYMAX (p, f);
11 }
    $

The output of the compiler from trunk yields:

    $ cc1 -quiet ./test.c
    ./test.c: In function ‘foo’:
    ./test.c:2:31: error: invalid operands to binary < (have ‘struct mystruct’ and ‘float’)
      __typeof__(B) __b = (B); __a < __b ? __b : __a; })
   ^
    ./test.c:2:31: note: in expansion of macro 'MYMAX'
      __typeof__(B) __b = (B); __a < __b ? __b : __a; })
   ^
    ./test.c:10:3: note: expanded from here
       MYMAX (p, f);
       ^
    $

After this patch, the compiler yields:

    $ ./cc1 -quiet ./test.c
    ./test.c: In function ‘foo’:
    ./test.c:2:31: error: invalid operands to binary < (have ‘struct mystruct’ and ‘float’)
      __typeof__(B) __b = (B); __a < __b ? __b : __a; })
   ^
    ./test.c:10:3: note: in expansion of macro 'MYMAX'
       MYMAX (p, f);
       ^
    $

The gotcha is, in the general case, we cannot simply eliminate the
context of the macro definition.  That is, the line from the first
output that is redundant with the first diagnostic line that has
line/column number:

    ./test.c:2:31: note: in expansion of macro 'MYMAX'
      __typeof__(B) __b = (B); __a < __b ? __b : __a; })
                                   ^

We cannot simply eliminate that context of macro definition because
there are cases where the first diagnostic that has a line/column
number doesn't point to a location inside the definition of the macro
where the relevant token is used.  For instance:

    $ cat -n test2.c
 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 }
    $

Which yields without the patch:

    $ cc1 -quiet ./test2.c
    ./test2.c: In function ‘g’:
    ./test2.c:5:14: error: invalid operands to binary << (have ‘double’ and ‘int’)
       OPERATE (A,<<,B)
  ^
    ./test2.c:2:9: note: in expansion of macro 'OPERATE'
       OPRD1 OPRT OPRD2;
     ^
    ./test2.c:5:3: note: expanded from here
       OPERATE (A,<<,B)
       ^
    ./test2.c:5:14: note: in expansion of macro 'SHIFTL'
       OPERATE (A,<<,B)
  ^
    ./test2.c:8:3: note: expanded from here
       SHIFTL (A,1)
       ^
    ./test2.c:8:3: note: in expansion of macro 'MULT'
       SHIFTL (A,1)
       ^
    ./test2.c:13:3: note: expanded from here
       MULT (1.0);// 1.0 << 1; <-- so this is an error.
       ^
    $

Here, the line that has the context of macro definition:

    ./test2.c:2:9: note: in expansion of macro 'OPERATE'
       OPRD1 OPRT OPRD2;
     ^
is useful, because the first diagnostic that has line/column number
wasn't pointing into the definition of the macro OPERATE, where the
token '<<' is used.

    ./test2.c:5:14: error: invalid operands to binary << (have ‘double’ and ‘int’)
       OPERATE (A,<<,B)
  ^
So in this this case, displaying the macro definition context is not
redundant.  I think it is even desirable.

The patch changes the output in that case to be:

    ./test2.c: In function ‘g’:
    ./test2.c:5:14: erreur: invalid operands to binary << (have ‘double’ and ‘int’)
       OPERATE (A,<<,B)
  ^
    ./test2.c:2:9: note: in definition of macro 'OPERATE'
       OPRD1 OPRT OPRD2;
     ^
    ./test2.c:8:3: note: in expansion of macro 'SHIFTL'
       SHIFTL (A,1)
       ^
    ./test2.c:13:3: note: in expansion of macro 'MULT'
       MULT (1.0);// 1.0 << 1; <-- so this is an error.
       ^
    $

It's shorter, but I believe it has all the information that was
present before the patch.

[1]: http://gcc.gnu.org/ml/gcc-patches/2012-05/msg00321.html

Bootstrapped and tested on x86_64-unknown-linux-gnu against trunk.

gcc/

Make unwound macro expansion trace less redundant
* tree-diagnostic.c (maybe_unwind_expanded_macro_loc): Don't print
context of macro definition in the trace, when it's redundant.
Update comments.

gcc/testsuite/

Make unwound macro expansion trace less redundant
* gcc.dg/cpp/macro-exp-tracking-1.c: Adjust.
* gcc.dg/cpp/macro-exp-tracking-2.c: Likewise.
* gcc.dg/cpp/macro-exp-tracking-3.c: Likewise.
* gcc.dg/cpp/macro-exp-tracking-4.c: Likewise.
* gcc.dg/cpp/macro-exp-tracking-5.c: Likewise.
* gcc.dg/cpp/pragma-diagnostic-2.c: Likewise.

From-SVN: r187845
gcc/ChangeLog
gcc/testsuite/ChangeLog
gcc/testsuite/g++.dg/warn/Wconversion-real-integer2.C
gcc/testsuite/g++.dg/warn/Wdouble-promotion.C
gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-1.c
gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-2.c
gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-3.c
gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-4.c
gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-5.c
gcc/testsuite/gcc.dg/cpp/pragma-diagnostic-2.c
gcc/tree-diagnostic.c