On Mon, Oct 17, 2011 at 11:57 AM, Dodji Seketeli <do...@redhat.com> wrote: > In this third instalment the diagnostic machinery -- when faced with > the virtual location of a token resulting from macro expansion -- uses > the new linemap APIs to unwind the stack of macro expansions that led > to that token and emits a [hopefully] more useful message than what we > have today. > > diagnostic_report_current_module has been slightly changed to use the > location given by client code instead of the global input_location > variable. This results in more precise diagnostic locations in > general but then the patch adjusts some C++ tests which output changed > as a result of this. > > Three new regression tests have been added. > > The mandatory screenshot goes like this: > > [dodji@adjoa gcc]$ cat -n test.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 } > > [dodji@adjoa gcc]$ ./cc1 -quiet -ftrack-macro-expansion test.c > test.c: In function 'g': > test.c:5:14: erreur: 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 'MULT2' > test.c:13:3: note: expanded from here
This broke bootstrap on x86_64-linux. /space/rguenther/src/svn/trunk/libcpp/line-map.c: In function 'source_location linemap_macro_map_loc_to_exp_point(const line_map*, source_location)': /space/rguenther/src/svn/trunk/libcpp/line-map.c:628:12: error: variable 'token_no' set but not used [-Werror=unused-but-set-variable] cc1plus: all warnings being treated as errors make[3]: *** [line-map.o] Error 1 > gcc/ChangeLog > 2011-10-15 Tom Tromey <tro...@redhat.com> > Dodji Seketeli <do...@redhat.com> > > * gcc/diagnostic.h (diagnostic_report_current_module): Add a > location parameter. > * diagnostic.c (diagnostic_report_current_module): Add a location > parameter to the function definition. Use it instead of > input_location. Resolve the virtual location rather than just > looking up its map and risking to touch a resulting macro map. > (default_diagnostic_starter): Pass the relevant diagnostic > location to diagnostic_report_current_module. > * tree-diagnostic.c (maybe_unwind_expanded_macro_loc): New. > (virt_loc_aware_diagnostic_finalizer): Likewise. > (diagnostic_report_current_function): Pass the > relevant location to diagnostic_report_current_module. > * tree-diagnostic.h (virt_loc_aware_diagnostic_finalizer): Declare > new function. > * toplev.c (general_init): By default, use the new > virt_loc_aware_diagnostic_finalizer as diagnostic finalizer. > * Makefile.in: Add vec.h dependency to tree-diagnostic.c. > > gcc/cp/ChangeLog > 2011-10-15 Tom Tromey <tro...@redhat.com> > Dodji Seketeli <do...@redhat.com> > > * error.c (cp_diagnostic_starter): Pass the relevant location to > diagnostic_report_current_module. > (cp_diagnostic_finalizer): Call virt_loc_aware_diagnostic_finalizer. > > gcc/testsuite/ChangeLog > 2011-10-15 Tom Tromey <tro...@redhat.com> > Dodji Seketeli <do...@redhat.com> > > * lib/prune.exp (prune_gcc_output): Prune output referring to > included files. > * gcc.dg/cpp/macro-exp-tracking-1.c: New test. > * gcc.dg/cpp/macro-exp-tracking-2.c: Likewise. > * gcc.dg/cpp/macro-exp-tracking-3.c: Likewise. > * gcc.dg/cpp/pragma-diagnostic-2.c: Likewise. > > --- > gcc/ChangeLog | 21 +++ > gcc/Makefile.in | 3 +- > gcc/cp/ChangeLog | 7 + > gcc/cp/error.c | 5 +- > gcc/diagnostic.c | 13 +- > gcc/diagnostic.h | 2 +- > gcc/testsuite/ChangeLog | 10 ++ > gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-1.c | 21 +++ > gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-2.c | 21 +++ > gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-3.c | 14 ++ > gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-4.c | 14 ++ > gcc/testsuite/gcc.dg/cpp/pragma-diagnostic-2.c | 34 +++++ > gcc/testsuite/lib/prune.exp | 1 + > gcc/toplev.c | 3 + > gcc/tree-diagnostic.c | 182 > ++++++++++++++++++++++- > gcc/tree-diagnostic.h | 3 +- > 16 files changed, 343 insertions(+), 11 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-1.c > create mode 100644 gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-2.c > create mode 100644 gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-3.c > create mode 100644 gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-4.c > create mode 100644 gcc/testsuite/gcc.dg/cpp/pragma-diagnostic-2.c > > diff --git a/gcc/Makefile.in b/gcc/Makefile.in > index baec5fe..70dc0b8 100644 > --- a/gcc/Makefile.in > +++ b/gcc/Makefile.in > @@ -2805,7 +2805,8 @@ tree-pretty-print.o : tree-pretty-print.c $(CONFIG_H) > $(SYSTEM_H) \ > $(TM_H) coretypes.h tree-iterator.h $(SCEV_H) langhooks.h \ > $(TREE_PASS_H) value-prof.h output.h tree-pretty-print.h > tree-diagnostic.o : tree-diagnostic.c $(CONFIG_H) $(SYSTEM_H) coretypes.h \ > - $(TREE_H) $(DIAGNOSTIC_H) tree-diagnostic.h langhooks.h $(LANGHOOKS_DEF_H) > + $(TREE_H) $(DIAGNOSTIC_H) tree-diagnostic.h langhooks.h > $(LANGHOOKS_DEF_H) \ > + $(VEC_H) > fold-const.o : fold-const.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) \ > $(TREE_H) $(FLAGS_H) $(DIAGNOSTIC_CORE_H) $(HASHTAB_H) $(EXPR_H) $(RTL_H) \ > $(GGC_H) $(TM_P_H) langhooks.h $(MD5_H) intl.h $(TARGET_H) \ > diff --git a/gcc/cp/error.c b/gcc/cp/error.c > index 4d12a0d..c7c4525 100644 > --- a/gcc/cp/error.c > +++ b/gcc/cp/error.c > @@ -2768,7 +2768,7 @@ static void > cp_diagnostic_starter (diagnostic_context *context, > diagnostic_info *diagnostic) > { > - diagnostic_report_current_module (context); > + diagnostic_report_current_module (context, diagnostic->location); > cp_print_error_function (context, diagnostic); > maybe_print_instantiation_context (context); > maybe_print_constexpr_context (context); > @@ -2778,8 +2778,9 @@ cp_diagnostic_starter (diagnostic_context *context, > > static void > cp_diagnostic_finalizer (diagnostic_context *context, > - diagnostic_info *diagnostic ATTRIBUTE_UNUSED) > + diagnostic_info *diagnostic) > { > + virt_loc_aware_diagnostic_finalizer (context, diagnostic); > pp_base_destroy_prefix (context->printer); > } > > diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c > index b46eb35..a8c0e66 100644 > --- a/gcc/diagnostic.c > +++ b/gcc/diagnostic.c > @@ -255,9 +255,9 @@ diagnostic_action_after_output (diagnostic_context > *context, > } > > void > -diagnostic_report_current_module (diagnostic_context *context) > +diagnostic_report_current_module (diagnostic_context *context, location_t > where) > { > - const struct line_map *map; > + const struct line_map *map = NULL; > > if (pp_needs_newline (context->printer)) > { > @@ -265,10 +265,13 @@ diagnostic_report_current_module (diagnostic_context > *context) > pp_needs_newline (context->printer) = false; > } > > - if (input_location <= BUILTINS_LOCATION) > + if (where <= BUILTINS_LOCATION) > return; > > - map = linemap_lookup (line_table, input_location); > + linemap_resolve_location (line_table, where, > + LRK_MACRO_DEFINITION_LOCATION, > + &map); > + > if (map && diagnostic_last_module_changed (context, map)) > { > diagnostic_set_last_module (context, map); > @@ -301,7 +304,7 @@ void > default_diagnostic_starter (diagnostic_context *context, > diagnostic_info *diagnostic) > { > - diagnostic_report_current_module (context); > + diagnostic_report_current_module (context, diagnostic->location); > pp_set_prefix (context->printer, diagnostic_build_prefix (context, > diagnostic)); > } > diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h > index 8074354..4b1265b 100644 > --- a/gcc/diagnostic.h > +++ b/gcc/diagnostic.h > @@ -253,7 +253,7 @@ extern diagnostic_context *global_dc; > /* Diagnostic related functions. */ > extern void diagnostic_initialize (diagnostic_context *, int); > extern void diagnostic_finish (diagnostic_context *); > -extern void diagnostic_report_current_module (diagnostic_context *); > +extern void diagnostic_report_current_module (diagnostic_context *, > location_t); > > /* Force diagnostics controlled by OPTIDX to be kind KIND. */ > extern diagnostic_t diagnostic_classify_diagnostic (diagnostic_context *, > diff --git a/gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-1.c > b/gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-1.c > new file mode 100644 > index 0000000..d975c8c > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-1.c > @@ -0,0 +1,21 @@ > +/* > + { dg-options "-ftrack-macro-expansion=1" } > + { dg-do compile } > +*/ > + > +#define OPERATE(OPRD1, OPRT, OPRD2) \ > +do \ > +{ \ > + OPRD1 OPRT OPRD2; /* { dg-message "expansion" }*/ \ > +} while (0) > + > +#define SHIFTL(A,B) \ > + OPERATE (A,<<,B) /* { dg-message "expanded|expansion" } */ > + > +void > +foo () > +{ > + SHIFTL (0.1,0.2); /* { dg-message "expanded" } */ > +} > + > +/* { dg-error "invalid operands" "" { target *-*-* } 13 } */ > diff --git a/gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-2.c > b/gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-2.c > new file mode 100644 > index 0000000..684af4c > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-2.c > @@ -0,0 +1,21 @@ > +/* > + { dg-options "-ftrack-macro-expansion=1" } > + { dg-do compile } > +*/ > + > +#define OPERATE(OPRD1, OPRT, OPRD2) \ > + OPRD1 OPRT OPRD2; /* { dg-message "expansion" } */ > + > +#define SHIFTL(A,B) \ > + OPERATE (A,<<,B) /* { dg-message "expanded|expansion" } */ > + > +#define MULT(A) \ > + SHIFTL (A,1) /* { dg-message "expanded|expansion" } */ > + > +void > +foo () > +{ > + MULT (1.0); /* { dg-message "expanded" } */ > +} > + > +/* { dg-error "invalid operands to binary <<" "" { target *-*-* } { 10 } } */ > diff --git a/gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-3.c > b/gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-3.c > new file mode 100644 > index 0000000..119053e > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-3.c > @@ -0,0 +1,14 @@ > +/* > + { dg-options "-fshow-column -ftrack-macro-expansion=1" } > + { dg-do compile } > + */ > + > +#define SQUARE(A) A * A /* { dg-message "expansion" } */ > + > +void > +foo() > +{ > + SQUARE (1 << 0.1); /* { dg-message "expanded" } */ > +} > + > +/* { dg-error "16:invalid operands to binary <<" "" {target *-*-* } { 11 } } > */ > diff --git a/gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-4.c > b/gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-4.c > new file mode 100644 > index 0000000..1f9fe6a > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-4.c > @@ -0,0 +1,14 @@ > +/* > + { dg-options "-fshow-column -ftrack-macro-expansion=2" } > + { dg-do compile } > + */ > + > +#define SQUARE(A) A * A /* { dg-message "expansion" } */ > + > +void > +foo() > +{ > + SQUARE (1 << 0.1); /* { dg-message "expanded" } */ > +} > + > +/* { dg-error "13:invalid operands to binary <<" "" { target *-*-* } { 11 } > } */ > diff --git a/gcc/testsuite/gcc.dg/cpp/pragma-diagnostic-2.c > b/gcc/testsuite/gcc.dg/cpp/pragma-diagnostic-2.c > new file mode 100644 > index 0000000..7ab95b0 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/cpp/pragma-diagnostic-2.c > @@ -0,0 +1,34 @@ > +/* > + { dg-options "-Wuninitialized -ftrack-macro-expansion=2" } > + { dg-do compile } > +*/ > + > +void f (unsigned); > + > +#define CODE_WITH_WARNING \ > + int a; /* { dg-message "expansion|declared here" } */ \ > + f (a) /* { dg-message "expansion" } */ > + > +#pragma GCC diagnostic ignored "-Wuninitialized" > + > +void > +g (void) > +{ > + CODE_WITH_WARNING; > +} > + > +#pragma GCC diagnostic push > + > +#pragma GCC diagnostic error "-Wuninitialized" > + > +void > +h (void) > +{ > + CODE_WITH_WARNING; /* { dg-message "expanded" } */ > +} > + > +/* > + { dg-message "some warnings being treated as errors" "" {target *-*-*} 0 } > +*/ > + > +/* { dg-error "uninitialized" "" { target *-*-* } { 10 } } */ > diff --git a/gcc/testsuite/lib/prune.exp b/gcc/testsuite/lib/prune.exp > index 4683f93..09d2581 100644 > --- a/gcc/testsuite/lib/prune.exp > +++ b/gcc/testsuite/lib/prune.exp > @@ -29,6 +29,7 @@ proc prune_gcc_output { text } { > regsub -all "(^|\n)collect: re(compiling|linking)\[^\n\]*" $text "" text > regsub -all "(^|\n)Please submit.*instructions\[^\n\]*" $text "" text > regsub -all "(^|\n)\[0-9\]\[0-9\]* errors\." $text "" text > + regsub -all "(^|\n)(In file included|\[ \]+from)\[^\n\]*" $text "" text > > # Ignore informational notes. > regsub -all "(^|\n)\[^\n\]*: note: \[^\n\]*" $text "" text > diff --git a/gcc/toplev.c b/gcc/toplev.c > index ab6b5a4..0188755 100644 > --- a/gcc/toplev.c > +++ b/gcc/toplev.c > @@ -1171,6 +1171,9 @@ general_init (const char *argv0) > can give warnings and errors. */ > diagnostic_initialize (global_dc, N_OPTS); > diagnostic_starter (global_dc) = default_tree_diagnostic_starter; > + /* By default print macro expansion contexts in the diagnostic > + finalizer -- for tokens resulting from macro macro expansion. */ > + diagnostic_finalizer (global_dc) = virt_loc_aware_diagnostic_finalizer; > /* Set a default printer. Language specific initializations will > override it later. */ > pp_format_decoder (global_dc->printer) = &default_tree_printer; > diff --git a/gcc/tree-diagnostic.c b/gcc/tree-diagnostic.c > index b456a2a..53b350b 100644 > --- a/gcc/tree-diagnostic.c > +++ b/gcc/tree-diagnostic.c > @@ -28,6 +28,7 @@ along with GCC; see the file COPYING3. If not see > #include "tree-diagnostic.h" > #include "langhooks.h" > #include "langhooks-def.h" > +#include "vec.h" > > /* Prints out, if necessary, the name of the current function > that caused an error. Called from all error and warning functions. */ > @@ -35,7 +36,7 @@ void > diagnostic_report_current_function (diagnostic_context *context, > diagnostic_info *diagnostic) > { > - diagnostic_report_current_module (context); > + diagnostic_report_current_module (context, diagnostic->location); > lang_hooks.print_error_function (context, input_filename, diagnostic); > } > > @@ -47,3 +48,182 @@ default_tree_diagnostic_starter (diagnostic_context > *context, > pp_set_prefix (context->printer, diagnostic_build_prefix (context, > diagnostic)); > } > + > +/* This is a pair made of a location and the line map it originated > + from. It's used in the maybe_unwind_expanded_macro_loc function > + below. */ > +typedef struct > +{ > + const struct line_map *map; > + source_location where; > +} loc_t; > + > +DEF_VEC_O (loc_t); > +DEF_VEC_ALLOC_O (loc_t, heap); > + > +/* Unwind the different macro expansions that lead to the token which > + location is WHERE and emit diagnostics showing the resulting > + unwound macro expansion trace. Let's look at an example to see how > + the trace looks like. Suppose we have this piece of code, > + artificially annotated with the line numbers to increase > + legibility: > + > + $ cat -n test.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 } > + > + 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 'MULT2' > + test.c:13:3: note: expanded from here > + > + The part that goes from the third to the eighth 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. > + > + If FIRST_EXP_POINT_MAP is non-null, *FIRST_EXP_POINT_MAP is set to > + the map of the location in the source that first triggered the > + macro expansion. This must be an ordinary map. */ > + > +static void > +maybe_unwind_expanded_macro_loc (diagnostic_context *context, > + diagnostic_info *diagnostic, > + source_location where, > + const struct line_map **first_exp_point_map) > +{ > + const struct line_map *map; > + VEC(loc_t,heap) *loc_vec = NULL; > + unsigned ix; > + loc_t loc, *iter; > + > + map = linemap_lookup (line_table, where); > + if (!linemap_macro_expansion_map_p (map)) > + return; > + > + /* Let's unwind the macros that got expanded and led to the token > + which location is WHERE. We are going to store these macros into > + LOC_VEC, so that we can later walk it at our convenience to > + display a somewhat meaningful trace of the macro expansion > + history to the user. Note that the first macro of the trace > + (which is OPERATE in the example above) is going to be stored at > + the beginning of LOC_VEC. */ > + > + do > + { > + loc.where = where; > + loc.map = map; > + > + VEC_safe_push (loc_t, heap, loc_vec, &loc); > + > + /* WHERE is the location of a token inside the expansion of a > + macro. MAP is the map holding the locations of that macro > + expansion. Let's get the location of the token inside the > + context that triggered the expansion of this macro. > + This is basically how we go "down" in the trace of macro > + expansions that led to WHERE. */ > + where = linemap_unwind_toward_expansion (line_table, where, &map); > + } while (linemap_macro_expansion_map_p (map)); > + > + if (first_exp_point_map) > + *first_exp_point_map = map; > + > + /* Walk LOC_VEC and print the macro expansion trace, unless the > + first macro which expansion triggered this trace was expanded > + inside a system header. */ > + if (!LINEMAP_SYSP (map)) > + FOR_EACH_VEC_ELT (loc_t, loc_vec, ix, iter) > + { > + source_location resolved_def_loc = 0, resolved_exp_loc = 0; > + diagnostic_t saved_kind; > + const char *saved_prefix; > + source_location saved_location; > + > + /* Okay, now here is what we want. For each token resulting > + from macro expansion we want to show: 1/ where in the > + definition of the macro the token comes from; 2/ where the > + macro got expanded. */ > + > + /* Resolve the location iter->where into the locus 1/ of the > + comment above. */ > + resolved_def_loc = > + linemap_resolve_location (line_table, iter->where, > + LRK_MACRO_DEFINITION_LOCATION, NULL); > + > + /* Resolve the location of the expansion point of the macro > + which expansion gave the token represented by def_loc. > + This is the locus 2/ of the earlier comment. */ > + resolved_exp_loc = > + linemap_resolve_location (line_table, > + MACRO_MAP_EXPANSION_POINT_LOCATION > (iter->map), > + LRK_MACRO_DEFINITION_LOCATION, NULL); > + > + saved_kind = diagnostic->kind; > + saved_prefix = context->printer->prefix; > + saved_location = diagnostic->location; > + > + diagnostic->kind = DK_NOTE; > + diagnostic->location = resolved_def_loc; > + pp_base_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->location = resolved_exp_loc; > + pp_base_set_prefix (context->printer, > + diagnostic_build_prefix (context, > + diagnostic)); > + pp_newline (context->printer); > + pp_printf (context->printer, "expanded from here"); > + pp_destroy_prefix (context->printer); > + > + diagnostic->kind = saved_kind; > + diagnostic->location = saved_location; > + context->printer->prefix = saved_prefix; > + } > + > + VEC_free (loc_t, heap, loc_vec); > +} > + > +/* This is a diagnostic finalizer implementation that is aware of > + virtual locations produced by libcpp. > + > + It has to be called by the diagnostic finalizer of front ends that > + uses libcpp and wish to get diagnostics involving tokens resulting > + from macro expansion. > + > + For a given location, if said location belongs to a token > + resulting from a macro expansion, this starter prints the context > + of the token. E.g, for multiply nested macro expansion, it > + unwinds the nested macro expansions and prints them in a manner > + that is similar to what is done for function call stacks, or > + template instantiation contexts. */ > +void > +virt_loc_aware_diagnostic_finalizer (diagnostic_context *context, > + diagnostic_info *diagnostic) > +{ > + maybe_unwind_expanded_macro_loc (context, diagnostic, > + diagnostic->location, > + NULL); > +} > diff --git a/gcc/tree-diagnostic.h b/gcc/tree-diagnostic.h > index 7d88089..6b8e8e6 100644 > --- a/gcc/tree-diagnostic.h > +++ b/gcc/tree-diagnostic.h > @@ -52,5 +52,6 @@ along with GCC; see the file COPYING3. If not see > void default_tree_diagnostic_starter (diagnostic_context *, diagnostic_info > *); > extern void diagnostic_report_current_function (diagnostic_context *, > diagnostic_info *); > - > +void virt_loc_aware_diagnostic_finalizer (diagnostic_context *, > + diagnostic_info *); > #endif /* ! GCC_TREE_DIAGNOSTIC_H */ > -- > 1.7.6.4 > >