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
This commit is contained in:
Dodji Seketeli 2012-05-24 19:37:45 +00:00 committed by Dodji Seketeli
parent a5dedb5e59
commit 165ca58dc3
11 changed files with 102 additions and 46 deletions

View File

@ -1,3 +1,10 @@
2012-05-24 Dodji Seketeli <dodji@redhat.com>
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.
2012-05-24 Richard Guenther <rguenther@suse.de>
PR middle-end/53460

View File

@ -1,3 +1,13 @@
2012-05-24 Dodji Seketeli <dodji@redhat.com>
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.
2012-05-24 Paolo Carlini <paolo.carlini@oracle.com>
PR c++/53464

View File

@ -29,5 +29,5 @@ float vfloat;
void h (void)
{
vfloat = INT_MAX; // { dg-message "expanded from here" }
vfloat = INT_MAX; // { dg-message "in expansion of macro 'INT_MAX'" }
}

View File

@ -36,7 +36,7 @@ usual_arithmetic_conversions(void)
local_cf = cf + 1.0; /* { dg-warning "implicit" } */
local_cf = cf - d; /* { dg-warning "implicit" } */
local_cf = cf + 1.0 * ID; /* { dg-message "expanded from here" } */
local_cf = cf + 1.0 * ID; /* { dg-message "in expansion of macro 'ID'" } */
local_cf = cf - cd; /* { dg-warning "implicit" } */
local_f = i ? f : d; /* { dg-warning "implicit" } */

View File

@ -6,16 +6,14 @@
#define OPERATE(OPRD1, OPRT, OPRD2) \
do \
{ \
OPRD1 OPRT OPRD2; /* { dg-message "expansion" }*/ \
OPRD1 OPRT OPRD2; /* { dg-message "definition" }*/ \
} while (0)
#define SHIFTL(A,B) \
OPERATE (A,<<,B) /* { dg-message "expanded|expansion" } */
OPERATE (A,<<,B) /* { dg-error "invalid operands" } */
void
foo ()
{
SHIFTL (0.1,0.2); /* { dg-message "expanded" } */
SHIFTL (0.1,0.2); /* { dg-message "in expansion of macro \[^\n\r\]SHIFTL" } */
}
/* { dg-error "invalid operands" "" { target *-*-* } 13 } */

View File

@ -4,18 +4,17 @@
*/
#define OPERATE(OPRD1, OPRT, OPRD2) \
OPRD1 OPRT OPRD2; /* { dg-message "expansion" } */
OPRD1 OPRT OPRD2; /* { dg-message "in definition of macro 'OPERATE'" } */
#define SHIFTL(A,B) \
OPERATE (A,<<,B) /* { dg-message "expanded|expansion" } */
OPERATE (A,<<,B) /* { dg-message "invalid operands to binary <<" } */
#define MULT(A) \
SHIFTL (A,1) /* { dg-message "expanded|expansion" } */
SHIFTL (A,1) /* { dg-message "in expansion of macro 'SHIFTL'" } */
void
foo ()
{
MULT (1.0); /* { dg-message "expanded" } */
MULT (1.0); /* { dg-message "in expansion of macro 'MULT'" } */
}
/* { dg-error "invalid operands to binary <<" "" { target *-*-* } { 10 } } */

View File

@ -3,12 +3,10 @@
{ dg-do compile }
*/
#define SQUARE(A) A * A /* { dg-message "expansion" } */
#define SQUARE(A) A * A /* { dg-message "in definition of macro 'SQUARE'" } */
void
foo()
{
SQUARE (1 << 0.1); /* { dg-message "expanded" } */
SQUARE (1 << 0.1); /* { dg-error "16:invalid operands to binary <<" } */
}
/* { dg-error "16:invalid operands to binary <<" "" {target *-*-* } { 11 } } */

View File

@ -3,12 +3,11 @@
{ dg-do compile }
*/
#define SQUARE(A) A * A /* { dg-message "expansion" } */
#define SQUARE(A) A * A /* { dg-message "in definition of macro 'SQUARE'" } */
void
foo()
{
SQUARE (1 << 0.1); /* { dg-message "expanded" } */
SQUARE (1 << 0.1); /* { dg-message "13:invalid operands to binary <<" } */
}
/* { dg-error "13:invalid operands to binary <<" "" { target *-*-* } { 11 } } */

View File

@ -3,16 +3,16 @@
{ dg-do compile }
*/
#define PASTED var ## iable /* { dg-error "undeclared" } */
#define PASTED var ## iable /* { dg-error "'variable' undeclared" } */
#define call_foo(p1, p2) \
foo (p1, \
p2); /* { dg-message "in expansion of macro" } */
p2); /* { dg-message "in definition of macro 'call_foo'" } */
void foo(int, char);
void
bar()
{
call_foo(1,PASTED); /* { dg-message "expanded from here" } */
call_foo(1,PASTED); /* { dg-message "in expansion of macro 'PASTED'" } */
}

View File

@ -24,5 +24,5 @@ g (void)
void
h (void)
{
CODE_WITH_WARNING; /* { dg-message "expanded" } */
CODE_WITH_WARNING; /* { dg-message "in expansion of macro 'CODE_WITH_WARNING'" } */
}

View File

@ -89,16 +89,13 @@ DEF_VEC_ALLOC_O (loc_map_pair, heap);
Here is the diagnostic that we want the compiler to generate:
test.c: In function 'g':
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 'MULT'
test.c:13:3: note: expanded from here
test.c: In function g:
test.c:5:14: error: invalid operands to binary << (have double and int)
test.c:2:9: note: in definition of macro 'OPERATE'
test.c:8:3: note: in expansion of macro 'SHIFTL'
test.c:13:3: note: in expansion of macro 'MULT'
The part that goes from the third to the eighth line of this
The part that goes from the third to the fifth line of this
diagnostic (the lines containing the 'note:' string) is called the
unwound macro expansion trace. That's the part generated by this
function. */
@ -150,10 +147,38 @@ maybe_unwind_expanded_macro_loc (diagnostic_context *context,
if (!LINEMAP_SYSP (map))
FOR_EACH_VEC_ELT (loc_map_pair, loc_vec, ix, iter)
{
source_location resolved_def_loc = 0, resolved_exp_loc = 0;
source_location resolved_def_loc = 0, resolved_exp_loc = 0,
saved_location = 0;
int resolved_def_loc_line = 0, saved_location_line = 0;
diagnostic_t saved_kind;
const char *saved_prefix;
source_location saved_location;
/* Sometimes, in the unwound macro expansion trace, we want to
print a part of the context that shows where, in the
definition of the relevant macro, is the token (we are
looking at) used. That is the case in the introductory
comment of this function, where we print:
test.c:2:9: note: in definition of macro 'OPERATE'.
We print that "macro definition context" because the
diagnostic line (emitted by the call to
pp_ouput_formatted_text in diagnostic_report_diagnostic):
test.c:5:14: error: invalid operands to binary << (have double and int)
does not point into the definition of the macro where the
token '<<' (that is an argument to the function-like macro
OPERATE) is used. So we must "display" the line of that
macro definition context to the user somehow.
A contrario, when the first interesting diagnostic line
points into the definition of the macro, we don't need to
display any line for that macro definition in the trace
anymore, otherwise it'd be redundant.
This flag is true when we need to display the context of
the macro definition. */
bool print_definition_context_p = false;
/* Okay, now here is what we want. For each token resulting
from macro expansion we want to show: 1/ where in the
@ -176,6 +201,8 @@ maybe_unwind_expanded_macro_loc (diagnostic_context *context,
if (l < RESERVED_LOCATION_COUNT
|| LINEMAP_SYSP (m))
continue;
resolved_def_loc_line = SOURCE_LINE (m, l);
}
/* Resolve the location of the expansion point of the macro
@ -189,22 +216,40 @@ maybe_unwind_expanded_macro_loc (diagnostic_context *context,
saved_kind = diagnostic->kind;
saved_prefix = pp_get_prefix (context->printer);
saved_location = diagnostic->location;
saved_location_line =
expand_location_to_spelling_point (saved_location).line;
diagnostic->kind = DK_NOTE;
diagnostic->location = resolved_def_loc;
pp_set_prefix (context->printer,
diagnostic_build_prefix (context, diagnostic));
pp_newline (context->printer);
pp_printf (context->printer, "in expansion of macro '%s'",
linemap_map_get_macro_name (iter->map));
pp_destroy_prefix (context->printer);
diagnostic_show_locus (context, diagnostic);
diagnostic->location = resolved_exp_loc;
pp_set_prefix (context->printer,
/* We need to print the context of the macro definition only
when the locus of the first displayed diagnostic (displayed
before this trace) was inside the definition of the
macro. */
print_definition_context_p =
(ix == 0 && (saved_location_line != resolved_def_loc_line));
if (print_definition_context_p)
{
diagnostic->location = resolved_def_loc;
pp_set_prefix (context->printer,
diagnostic_build_prefix (context, diagnostic));
pp_newline (context->printer);
pp_printf (context->printer, "in definition of macro '%s'",
linemap_map_get_macro_name (iter->map));
pp_destroy_prefix (context->printer);
diagnostic_show_locus (context, diagnostic);
/* At this step, as we've printed the context of the macro
definition, we don't want to print the context of its
expansion, otherwise, it'd be redundant. */
continue;
}
diagnostic->location = resolved_exp_loc;
pp_set_prefix (context->printer,
diagnostic_build_prefix (context, diagnostic));
pp_newline (context->printer);
pp_string (context->printer, "expanded from here");
pp_newline (context->printer);
pp_printf (context->printer, "in expansion of macro '%s'",
linemap_map_get_macro_name (iter->map));
pp_destroy_prefix (context->printer);
diagnostic_show_locus (context, diagnostic);