* Re: [PATCH 3/6] Emit macro expansion related diagnostics @ 2011-10-18 15:25 David Edelsohn 2011-10-18 15:30 ` Joseph S. Myers ` (2 more replies) 0 siblings, 3 replies; 28+ messages in thread From: David Edelsohn @ 2011-10-18 15:25 UTC (permalink / raw) To: Dodji Seketeli; +Cc: GCC Patches Hey, Dodji, Your patch broke bootstrap on AIX because of the typedef "loc_t" introduced in tree-diagnostics.c. The typedef conflicts with a typedef in an AIX 5.3 header file for locales. AIX should not be using that namespace, but the failure occurs before fix-includes, so it is not possible to fix it there. How can we avoid the namespace conflict? Thanks, David ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/6] Emit macro expansion related diagnostics 2011-10-18 15:25 [PATCH 3/6] Emit macro expansion related diagnostics David Edelsohn @ 2011-10-18 15:30 ` Joseph S. Myers 2011-10-18 15:37 ` Gabriel Dos Reis 2011-10-18 15:34 ` Jakub Jelinek 2011-10-18 16:56 ` Dodji Seketeli 2 siblings, 1 reply; 28+ messages in thread From: Joseph S. Myers @ 2011-10-18 15:30 UTC (permalink / raw) To: David Edelsohn; +Cc: Dodji Seketeli, GCC Patches On Tue, 18 Oct 2011, David Edelsohn wrote: > Hey, Dodji, > > Your patch broke bootstrap on AIX because of the typedef "loc_t" > introduced in tree-diagnostics.c. The typedef conflicts with a > typedef in an AIX 5.3 header file for locales. AIX should not be > using that namespace, but the failure occurs before fix-includes, so > it is not possible to fix it there. The whole *_t namespace is reserved by POSIX, so it's not unreasonable for AIX to have some AIX-specific *_t typedefs although it seems quite rare for this to cause conflicts in practice (and AIX is obviously risking conflicts with future POSIX revisions by using *_t for its own types). -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/6] Emit macro expansion related diagnostics 2011-10-18 15:30 ` Joseph S. Myers @ 2011-10-18 15:37 ` Gabriel Dos Reis 0 siblings, 0 replies; 28+ messages in thread From: Gabriel Dos Reis @ 2011-10-18 15:37 UTC (permalink / raw) To: Joseph S. Myers; +Cc: David Edelsohn, Dodji Seketeli, GCC Patches On Tue, Oct 18, 2011 at 10:19 AM, Joseph S. Myers <joseph@codesourcery.com> wrote: > On Tue, 18 Oct 2011, David Edelsohn wrote: > >> Hey, Dodji, >> >> Your patch broke bootstrap on AIX because of the typedef "loc_t" >> introduced in tree-diagnostics.c. The typedef conflicts with a >> typedef in an AIX 5.3 header file for locales. AIX should not be >> using that namespace, but the failure occurs before fix-includes, so >> it is not possible to fix it there. > > The whole *_t namespace is reserved by POSIX, so it's not unreasonable for > AIX to have some AIX-specific *_t typedefs although it seems quite rare > for this to cause conflicts in practice (and AIX is obviously risking > conflicts with future POSIX revisions by using *_t for its own types). any reason we could not call it location_type? or just location? ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/6] Emit macro expansion related diagnostics 2011-10-18 15:25 [PATCH 3/6] Emit macro expansion related diagnostics David Edelsohn 2011-10-18 15:30 ` Joseph S. Myers @ 2011-10-18 15:34 ` Jakub Jelinek 2011-10-18 16:56 ` Dodji Seketeli 2 siblings, 0 replies; 28+ messages in thread From: Jakub Jelinek @ 2011-10-18 15:34 UTC (permalink / raw) To: David Edelsohn; +Cc: Dodji Seketeli, GCC Patches On Tue, Oct 18, 2011 at 11:09:04AM -0400, David Edelsohn wrote: > Your patch broke bootstrap on AIX because of the typedef "loc_t" > introduced in tree-diagnostics.c. The typedef conflicts with a > typedef in an AIX 5.3 header file for locales. AIX should not be > using that namespace, but the failure occurs before fix-includes, so > it is not possible to fix it there. At least in POSIX *_t is reserved for the implementation (in any header). So gcc should use something else. Jakub ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/6] Emit macro expansion related diagnostics 2011-10-18 15:25 [PATCH 3/6] Emit macro expansion related diagnostics David Edelsohn 2011-10-18 15:30 ` Joseph S. Myers 2011-10-18 15:34 ` Jakub Jelinek @ 2011-10-18 16:56 ` Dodji Seketeli 2011-10-18 18:16 ` Dodji Seketeli 2 siblings, 1 reply; 28+ messages in thread From: Dodji Seketeli @ 2011-10-18 16:56 UTC (permalink / raw) To: David Edelsohn; +Cc: GCC Patches David Edelsohn <dje.gcc@gmail.com> writes: > Your patch broke bootstrap on AIX because of the typedef "loc_t" > introduced in tree-diagnostics.c. The typedef conflicts with a > typedef in an AIX 5.3 header file for locales. AIX should not be > using that namespace, but the failure occurs before fix-includes, so > it is not possible to fix it there. > > How can we avoid the namespace conflict? I'll change the name to location_type as Gaby proposed and commit it as obvious. Sorry for the inconvenience. -- Dodji ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/6] Emit macro expansion related diagnostics 2011-10-18 16:56 ` Dodji Seketeli @ 2011-10-18 18:16 ` Dodji Seketeli 0 siblings, 0 replies; 28+ messages in thread From: Dodji Seketeli @ 2011-10-18 18:16 UTC (permalink / raw) To: David Edelsohn; +Cc: GCC Patches Dodji Seketeli <dodji@redhat.com> writes: > David Edelsohn <dje.gcc@gmail.com> writes: > >> Your patch broke bootstrap on AIX because of the typedef "loc_t" >> introduced in tree-diagnostics.c. I have committed the patch below. Tested on x86_64-unknown-linux-gnu only, unfortunately. I hope it fixes the build on AIX. From: Dodji Seketeli <dodji@seketeli.org> Date: Tue, 18 Oct 2011 18:40:06 +0200 Subject: [PATCH] Do not use loc_t as a type name gcc/ * tree-diagnostic (struct loc_t): Rename into struct loc_map_pair. (maybe_unwind_expanded_macro_loc): Adjust. --- gcc/tree-diagnostic.c | 16 ++++++++-------- 1 files changed, 8 insertions(+), 8 deletions(-) diff --git a/gcc/tree-diagnostic.c b/gcc/tree-diagnostic.c index 53b350b..b4b60dc 100644 --- a/gcc/tree-diagnostic.c +++ b/gcc/tree-diagnostic.c @@ -56,10 +56,10 @@ typedef struct { const struct line_map *map; source_location where; -} loc_t; +} loc_map_pair; -DEF_VEC_O (loc_t); -DEF_VEC_ALLOC_O (loc_t, heap); +DEF_VEC_O (loc_map_pair); +DEF_VEC_ALLOC_O (loc_map_pair, heap); /* Unwind the different macro expansions that lead to the token which location is WHERE and emit diagnostics showing the resulting @@ -111,9 +111,9 @@ maybe_unwind_expanded_macro_loc (diagnostic_context *context, const struct line_map **first_exp_point_map) { const struct line_map *map; - VEC(loc_t,heap) *loc_vec = NULL; + VEC(loc_map_pair,heap) *loc_vec = NULL; unsigned ix; - loc_t loc, *iter; + loc_map_pair loc, *iter; map = linemap_lookup (line_table, where); if (!linemap_macro_expansion_map_p (map)) @@ -132,7 +132,7 @@ maybe_unwind_expanded_macro_loc (diagnostic_context *context, loc.where = where; loc.map = map; - VEC_safe_push (loc_t, heap, loc_vec, &loc); + VEC_safe_push (loc_map_pair, 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 @@ -150,7 +150,7 @@ maybe_unwind_expanded_macro_loc (diagnostic_context *context, 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) + FOR_EACH_VEC_ELT (loc_map_pair, loc_vec, ix, iter) { source_location resolved_def_loc = 0, resolved_exp_loc = 0; diagnostic_t saved_kind; @@ -203,7 +203,7 @@ maybe_unwind_expanded_macro_loc (diagnostic_context *context, context->printer->prefix = saved_prefix; } - VEC_free (loc_t, heap, loc_vec); + VEC_free (loc_map_pair, heap, loc_vec); } /* This is a diagnostic finalizer implementation that is aware of -- 1.7.6.2 -- Dodji ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 0/6] Tracking locations of tokens resulting from macro expansion @ 2011-10-17 9:58 Dodji Seketeli 2011-10-17 9:58 ` [PATCH 3/6] Emit macro expansion related diagnostics Dodji Seketeli 0 siblings, 1 reply; 28+ messages in thread From: Dodji Seketeli @ 2011-10-17 9:58 UTC (permalink / raw) To: gcc-patches; +Cc: jason, tromey, gdr, joseph, burnus, charlet This set of patches to track locations of tokens access macro expansion was reviewed and accepted at http://gcc.gnu.org/ml/gcc-patches/2011-10/msg01346.html. I have bootstrapped and tested it on x86_64-unknown-linux-gnu against trunk and I am checking it in now. Thanks. ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 3/6] Emit macro expansion related diagnostics 2011-10-17 9:58 [PATCH 0/6] Tracking locations of tokens resulting from macro expansion Dodji Seketeli @ 2011-10-17 9:58 ` Dodji Seketeli 2011-10-17 10:56 ` Richard Guenther 0 siblings, 1 reply; 28+ messages in thread From: Dodji Seketeli @ 2011-10-17 9:58 UTC (permalink / raw) To: gcc-patches; +Cc: jason, tromey, gdr, joseph, burnus, charlet 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 gcc/ChangeLog 2011-10-15 Tom Tromey <tromey@redhat.com> Dodji Seketeli <dodji@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 <tromey@redhat.com> Dodji Seketeli <dodji@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 <tromey@redhat.com> Dodji Seketeli <dodji@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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/6] Emit macro expansion related diagnostics 2011-10-17 9:58 ` [PATCH 3/6] Emit macro expansion related diagnostics Dodji Seketeli @ 2011-10-17 10:56 ` Richard Guenther 2011-10-17 12:22 ` Dodji Seketeli 0 siblings, 1 reply; 28+ messages in thread From: Richard Guenther @ 2011-10-17 10:56 UTC (permalink / raw) To: Dodji Seketeli; +Cc: gcc-patches, jason, tromey, gdr, joseph, burnus, charlet On Mon, Oct 17, 2011 at 11:57 AM, Dodji Seketeli <dodji@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 <tromey@redhat.com> > Dodji Seketeli <dodji@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 <tromey@redhat.com> > Dodji Seketeli <dodji@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 <tromey@redhat.com> > Dodji Seketeli <dodji@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 > > ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/6] Emit macro expansion related diagnostics 2011-10-17 10:56 ` Richard Guenther @ 2011-10-17 12:22 ` Dodji Seketeli 2011-10-17 14:11 ` Dodji Seketeli 0 siblings, 1 reply; 28+ messages in thread From: Dodji Seketeli @ 2011-10-17 12:22 UTC (permalink / raw) To: Richard Guenther; +Cc: gcc-patches, jason, tromey, gdr, joseph, burnus, charlet Richard Guenther <richard.guenther@gmail.com> writes: > 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 Sigh. I guess the reason why my testing hasn't caught this is that I am bootstrapping with --enable-checking and so on my system ENABLE_CHECKING is defined and my GCC_VERSION >= 2007. I am bootstrapping and testing the obvious patch below with --disable-bootstrap and I am planning to check it in if it passes, under the obvious rule. Sorry for the inconvenience. commit e957242a9a8ec8f297e05ca0dae1d63bf543fad8 Author: Dodji Seketeli <dodji@redhat.com> Date: Mon Oct 17 13:33:41 2011 +0200 Fix bootstrapping with --disable-checking libcpp/ChangeLog * line-map.c (linemap_macro_map_loc_to_exp_point): Avoid setting a variable without using it if ENABLE_CHECKING is not defined. diff --git a/libcpp/line-map.c b/libcpp/line-map.c index 87b8bfe..a1d0fbb 100644 --- a/libcpp/line-map.c +++ b/libcpp/line-map.c @@ -625,14 +625,12 @@ source_location linemap_macro_map_loc_to_exp_point (const struct line_map *map, source_location location) { - unsigned token_no; - linemap_assert (linemap_macro_expansion_map_p (map) && location >= MAP_START_LOCATION (map)); /* Make sure LOCATION is correct. */ - token_no = location - MAP_START_LOCATION (map); - linemap_assert (token_no < MACRO_MAP_NUM_MACRO_TOKENS (map)); + linemap_assert ((location - MAP_START_LOCATION (map)) + < MACRO_MAP_NUM_MACRO_TOKENS (map)); return MACRO_MAP_EXPANSION_POINT_LOCATION (map); } -- Dodji ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/6] Emit macro expansion related diagnostics 2011-10-17 12:22 ` Dodji Seketeli @ 2011-10-17 14:11 ` Dodji Seketeli 2011-10-17 17:41 ` H.J. Lu 0 siblings, 1 reply; 28+ messages in thread From: Dodji Seketeli @ 2011-10-17 14:11 UTC (permalink / raw) To: Richard Guenther; +Cc: gcc-patches, jason, tromey, gdr, joseph, burnus, charlet Finally here is what I am checking in, which passes bootstrap with --disable-checking --enable-languages=all,ada -- modulo this other bug that breaks bootstrap as well http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50709. It's been OKed off-line by Tom Tromey. commit c1cd2be336ceec75cf40ac5f32cc4f72b8fc5da3 Author: Dodji Seketeli <dodji@redhat.com> Date: Mon Oct 17 13:33:41 2011 +0200 Fix bootstrapping with --disable-checking libcpp/ChangeLog * line-map.c (linemap_macro_map_loc_to_exp_point): Avoid setting a variable without using it if ENABLE_CHECKING is not defined. Mark the LOCATION parameter as being unused. diff --git a/libcpp/line-map.c b/libcpp/line-map.c index 87b8bfe..43e2856 100644 --- a/libcpp/line-map.c +++ b/libcpp/line-map.c @@ -621,18 +621,16 @@ linemap_macro_expansion_map_p (const struct line_map *map) Read the comments of struct line_map and struct line_map_macro in line-map.h to understand what a macro expansion point is. */ -source_location +static source_location linemap_macro_map_loc_to_exp_point (const struct line_map *map, - source_location location) + source_location location ATTRIBUTE_UNUSED) { - unsigned token_no; - linemap_assert (linemap_macro_expansion_map_p (map) && location >= MAP_START_LOCATION (map)); /* Make sure LOCATION is correct. */ - token_no = location - MAP_START_LOCATION (map); - linemap_assert (token_no < MACRO_MAP_NUM_MACRO_TOKENS (map)); + linemap_assert ((location - MAP_START_LOCATION (map)) + < MACRO_MAP_NUM_MACRO_TOKENS (map)); return MACRO_MAP_EXPANSION_POINT_LOCATION (map); } -- Dodji ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/6] Emit macro expansion related diagnostics 2011-10-17 14:11 ` Dodji Seketeli @ 2011-10-17 17:41 ` H.J. Lu 2011-10-18 0:29 ` Dodji Seketeli 0 siblings, 1 reply; 28+ messages in thread From: H.J. Lu @ 2011-10-17 17:41 UTC (permalink / raw) To: Dodji Seketeli Cc: Richard Guenther, gcc-patches, jason, tromey, gdr, joseph, burnus, charlet On Mon, Oct 17, 2011 at 6:41 AM, Dodji Seketeli <dodji@redhat.com> wrote: > Finally here is what I am checking in, which passes bootstrap with > --disable-checking --enable-languages=all,ada -- modulo this other bug > that breaks bootstrap as well > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50709. > > It's been OKed off-line by Tom Tromey. > > commit c1cd2be336ceec75cf40ac5f32cc4f72b8fc5da3 > Author: Dodji Seketeli <dodji@redhat.com> > Date: Mon Oct 17 13:33:41 2011 +0200 > > Fix bootstrapping with --disable-checking > > libcpp/ChangeLog > > * line-map.c (linemap_macro_map_loc_to_exp_point): Avoid setting a > variable without using it if ENABLE_CHECKING is not defined. Mark > the LOCATION parameter as being unused. > There are at least 2 bootstrap problems: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50760 -- H.J. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/6] Emit macro expansion related diagnostics 2011-10-17 17:41 ` H.J. Lu @ 2011-10-18 0:29 ` Dodji Seketeli 2011-10-18 6:07 ` Jason Merrill 0 siblings, 1 reply; 28+ messages in thread From: Dodji Seketeli @ 2011-10-18 0:29 UTC (permalink / raw) To: H.J. Lu Cc: Richard Guenther, gcc-patches, jason, tromey, gdr, joseph, burnus, charlet "H.J. Lu" <hjl.tools@gmail.com> writes: > There are at least 2 bootstrap problems: > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50760 The patch below should address those issues. The first two hunks are to fix bootstrap for targets that have !NO_IMPLICIT_EXTERN_C. To test it, I built the compiler with --target=avr. The next hunk is to fix one part of the issue reported in the PR. It's because size_t on ia32 is unsigned int, while being unsigned long int on x86_64. To fix ia32 in a portable way I am casting size_t to long as it's done in other parts of the compiler when fprinting. The last hunk is because it appears to me that after macro_arg_token_iter_init gets inlined into replace_args, from->token_ptr appears possibly uninitialized. So I am unconditionally initializing it. For the ia32 fixes, as I don't have any such target at hand, I ran configure i686-pc-linux-gnu on my x86_64 host and the bootstrap is underway. It's taking time, sorry. I ran a full bootstrap of the changes which went up to the comparison failure we are currently having on trunk. OK if this appears to fix the raised issues and passes bootstraps on the i686 target? Author: Dodji Seketeli <dodji@redhat.com> Date: Mon Oct 17 22:33:46 2011 +0200 Fix bootstrap on !NO_IMPLICIT_EXTERN_C and ia32 targets libcpp/ * macro.c (macro_arg_token_iter_init): Unconditionally initialize iter->location_ptr. gcc/c-family/ * c-lex.c (fe_file_change): Use LINEMAP_SYSP when !NO_IMPLICIT_EXTERN_C. gcc/ * input.c (dump_line_table_statistics): Cast size_t to long for printing. diff --git a/gcc/c-family/c-lex.c b/gcc/c-family/c-lex.c index be83b61..b151564 100644 --- a/gcc/c-family/c-lex.c +++ b/gcc/c-family/c-lex.c @@ -211,7 +211,7 @@ fe_file_change (const struct line_map *new_map) #ifndef NO_IMPLICIT_EXTERN_C if (c_header_level) ++c_header_level; - else if (new_map->sysp == 2) + else if (LINEMAP_SYSP (new_map) == 2) { c_header_level = 1; ++pending_lang_change; @@ -224,7 +224,7 @@ fe_file_change (const struct line_map *new_map) #ifndef NO_IMPLICIT_EXTERN_C if (c_header_level && --c_header_level == 0) { - if (new_map->sysp == 2) + if (LINEMAP_SYSP (new_map) == 2) warning (0, "badly nested C headers from preprocessor"); --pending_lang_change; } diff --git a/gcc/input.c b/gcc/input.c index 41842b7..8138a65 100644 --- a/gcc/input.c +++ b/gcc/input.c @@ -99,46 +99,46 @@ dump_line_table_statistics (void) + s.macro_maps_used_size + s.macro_maps_locations_size; - fprintf (stderr, "Number of expanded macros: %5lu\n", - s.num_expanded_macros); + fprintf (stderr, "Number of expanded macros: %5ld\n", + (long)s.num_expanded_macros); if (s.num_expanded_macros != 0) - fprintf (stderr, "Average number of tokens per macro expansion: %5lu\n", - s.num_macro_tokens / s.num_expanded_macros); + fprintf (stderr, "Average number of tokens per macro expansion: %5ld\n", + (long)s.num_macro_tokens / s.num_expanded_macros); fprintf (stderr, "\nLine Table allocations during the " "compilation process\n"); - fprintf (stderr, "Number of ordinary maps used: %5lu%c\n", - SCALE (s.num_ordinary_maps_used), + fprintf (stderr, "Number of ordinary maps used: %5ld%c\n", + (long)SCALE (s.num_ordinary_maps_used), STAT_LABEL (s.num_ordinary_maps_used)); - fprintf (stderr, "Ordinary map used size: %5lu%c\n", - SCALE (s.ordinary_maps_used_size), + fprintf (stderr, "Ordinary map used size: %5ld%c\n", + (long)SCALE (s.ordinary_maps_used_size), STAT_LABEL (s.ordinary_maps_used_size)); - fprintf (stderr, "Number of ordinary maps allocated: %5lu%c\n", - SCALE (s.num_ordinary_maps_allocated), + fprintf (stderr, "Number of ordinary maps allocated: %5ld%c\n", + (long)SCALE (s.num_ordinary_maps_allocated), STAT_LABEL (s.num_ordinary_maps_allocated)); - fprintf (stderr, "Ordinary maps allocated size: %5lu%c\n", - SCALE (s.ordinary_maps_allocated_size), + fprintf (stderr, "Ordinary maps allocated size: %5ld%c\n", + (long)SCALE (s.ordinary_maps_allocated_size), STAT_LABEL (s.ordinary_maps_allocated_size)); - fprintf (stderr, "Number of macro maps used: %5lu%c\n", - SCALE (s.num_macro_maps_used), + fprintf (stderr, "Number of macro maps used: %5ld%c\n", + (long)SCALE (s.num_macro_maps_used), STAT_LABEL (s.num_macro_maps_used)); - fprintf (stderr, "Macro maps used size: %5lu%c\n", - SCALE (s.macro_maps_used_size), + fprintf (stderr, "Macro maps used size: %5ld%c\n", + (long)SCALE (s.macro_maps_used_size), STAT_LABEL (s.macro_maps_used_size)); - fprintf (stderr, "Macro maps locations size: %5lu%c\n", - SCALE (s.macro_maps_locations_size), + fprintf (stderr, "Macro maps locations size: %5ld%c\n", + (long)SCALE (s.macro_maps_locations_size), STAT_LABEL (s.macro_maps_locations_size)); - fprintf (stderr, "Macro maps size: %5lu%c\n", - SCALE (macro_maps_size), + fprintf (stderr, "Macro maps size: %5ld%c\n", + (long)SCALE (macro_maps_size), STAT_LABEL (macro_maps_size)); - fprintf (stderr, "Duplicated maps locations size: %5lu%c\n", - SCALE (s.duplicated_macro_maps_locations_size), + fprintf (stderr, "Duplicated maps locations size: %5ld%c\n", + (long)SCALE (s.duplicated_macro_maps_locations_size), STAT_LABEL (s.duplicated_macro_maps_locations_size)); - fprintf (stderr, "Total allocated maps size: %5lu%c\n", - SCALE (total_allocated_map_size), + fprintf (stderr, "Total allocated maps size: %5ld%c\n", + (long)SCALE (total_allocated_map_size), STAT_LABEL (total_allocated_map_size)); - fprintf (stderr, "Total used maps size: %5lu%c\n", - SCALE (total_used_map_size), + fprintf (stderr, "Total used maps size: %5ld%c\n", + (long)SCALE (total_used_map_size), STAT_LABEL (total_used_map_size)); fprintf (stderr, "\n"); } diff --git a/libcpp/macro.c b/libcpp/macro.c index 2d0eeaa..f313959 100644 --- a/libcpp/macro.c +++ b/libcpp/macro.c @@ -1278,6 +1278,10 @@ macro_arg_token_iter_init (macro_arg_token_iter *iter, iter->track_macro_exp_p = track_macro_exp_p; iter->kind = kind; iter->token_ptr = token_ptr; + /* Unconditionally initialize this so that the compiler doesn't warn + about iter->location_ptr being possibly uninitialized later after + this code has been inlined somewhere. */ + iter->location_ptr = NULL; if (track_macro_exp_p) iter->location_ptr = get_arg_token_location (arg, kind); #ifdef ENABLE_CHECKING -- Dodji ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/6] Emit macro expansion related diagnostics 2011-10-18 0:29 ` Dodji Seketeli @ 2011-10-18 6:07 ` Jason Merrill 2011-10-18 9:22 ` Dodji Seketeli 0 siblings, 1 reply; 28+ messages in thread From: Jason Merrill @ 2011-10-18 6:07 UTC (permalink / raw) To: Dodji Seketeli Cc: H.J. Lu, Richard Guenther, gcc-patches, tromey, gdr, joseph, burnus, charlet On 10/17/2011 06:33 PM, Dodji Seketeli wrote: > OK if this appears to fix the raised issues and passes bootstraps on the > i686 target? If you have a patch like this that fixes a major regression, go ahead and check it in without waiting for approval; we can adjust it as necessary after build is working again. > size_t num_expanded_macros; > - fprintf (stderr, "Number of expanded macros: %5lu\n", > - s.num_expanded_macros); > + fprintf (stderr, "Number of expanded macros: %5ld\n", > + (long)s.num_expanded_macros); If we're going to use %l in printf, we should use long rather than size_t in linemap_stats; that way we don't need the cast. Jason ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/6] Emit macro expansion related diagnostics 2011-10-18 6:07 ` Jason Merrill @ 2011-10-18 9:22 ` Dodji Seketeli 0 siblings, 0 replies; 28+ messages in thread From: Dodji Seketeli @ 2011-10-18 9:22 UTC (permalink / raw) To: Jason Merrill Cc: H.J. Lu, Richard Guenther, gcc-patches, tromey, gdr, joseph, burnus, charlet Jason Merrill <jason@redhat.com> writes: > If you have a patch like this that fixes a major regression, go ahead > and check it in without waiting for approval; we can adjust it as > necessary after build is working again. OK. >> size_t num_expanded_macros; > >> - fprintf (stderr, "Number of expanded macros: %5lu\n", >> - s.num_expanded_macros); >> + fprintf (stderr, "Number of expanded macros: %5ld\n", >> + (long)s.num_expanded_macros); > > If we're going to use %l in printf, we should use long rather than > size_t in linemap_stats; that way we don't need the cast. OK, I have checked the below in. libcpp/ * include/line-map.h (struct linemap_stats): Change the type of the members from size_t to long. * macro.c (macro_arg_token_iter_init): Unconditionally initialize iter->location_ptr. gcc/c-family/ * c-lex.c (fe_file_change): Use LINEMAP_SYSP when !NO_IMPLICIT_EXTERN_C. gcc/ * input.c (dump_line_table_statistics): Use long, not size_t. git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@180124 138bc75d-0d04-0410-961f-82ee72b054a4 diff --git a/gcc/c-family/c-lex.c b/gcc/c-family/c-lex.c index be83b61..b151564 100644 --- a/gcc/c-family/c-lex.c +++ b/gcc/c-family/c-lex.c @@ -211,7 +211,7 @@ fe_file_change (const struct line_map *new_map) #ifndef NO_IMPLICIT_EXTERN_C if (c_header_level) ++c_header_level; - else if (new_map->sysp == 2) + else if (LINEMAP_SYSP (new_map) == 2) { c_header_level = 1; ++pending_lang_change; @@ -224,7 +224,7 @@ fe_file_change (const struct line_map *new_map) #ifndef NO_IMPLICIT_EXTERN_C if (c_header_level && --c_header_level == 0) { - if (new_map->sysp == 2) + if (LINEMAP_SYSP (new_map) == 2) warning (0, "badly nested C headers from preprocessor"); --pending_lang_change; } diff --git a/gcc/input.c b/gcc/input.c index 41842b7..a780f5c 100644 --- a/gcc/input.c +++ b/gcc/input.c @@ -80,7 +80,7 @@ void dump_line_table_statistics (void) { struct linemap_stats s; - size_t total_used_map_size, + long total_used_map_size, macro_maps_size, total_allocated_map_size; @@ -99,45 +99,45 @@ dump_line_table_statistics (void) + s.macro_maps_used_size + s.macro_maps_locations_size; - fprintf (stderr, "Number of expanded macros: %5lu\n", + fprintf (stderr, "Number of expanded macros: %5ld\n", s.num_expanded_macros); if (s.num_expanded_macros != 0) - fprintf (stderr, "Average number of tokens per macro expansion: %5lu\n", + fprintf (stderr, "Average number of tokens per macro expansion: %5ld\n", s.num_macro_tokens / s.num_expanded_macros); fprintf (stderr, "\nLine Table allocations during the " "compilation process\n"); - fprintf (stderr, "Number of ordinary maps used: %5lu%c\n", + fprintf (stderr, "Number of ordinary maps used: %5ld%c\n", SCALE (s.num_ordinary_maps_used), STAT_LABEL (s.num_ordinary_maps_used)); - fprintf (stderr, "Ordinary map used size: %5lu%c\n", + fprintf (stderr, "Ordinary map used size: %5ld%c\n", SCALE (s.ordinary_maps_used_size), STAT_LABEL (s.ordinary_maps_used_size)); - fprintf (stderr, "Number of ordinary maps allocated: %5lu%c\n", + fprintf (stderr, "Number of ordinary maps allocated: %5ld%c\n", SCALE (s.num_ordinary_maps_allocated), STAT_LABEL (s.num_ordinary_maps_allocated)); - fprintf (stderr, "Ordinary maps allocated size: %5lu%c\n", + fprintf (stderr, "Ordinary maps allocated size: %5ld%c\n", SCALE (s.ordinary_maps_allocated_size), STAT_LABEL (s.ordinary_maps_allocated_size)); - fprintf (stderr, "Number of macro maps used: %5lu%c\n", + fprintf (stderr, "Number of macro maps used: %5ld%c\n", SCALE (s.num_macro_maps_used), STAT_LABEL (s.num_macro_maps_used)); - fprintf (stderr, "Macro maps used size: %5lu%c\n", + fprintf (stderr, "Macro maps used size: %5ld%c\n", SCALE (s.macro_maps_used_size), STAT_LABEL (s.macro_maps_used_size)); - fprintf (stderr, "Macro maps locations size: %5lu%c\n", + fprintf (stderr, "Macro maps locations size: %5ld%c\n", SCALE (s.macro_maps_locations_size), STAT_LABEL (s.macro_maps_locations_size)); - fprintf (stderr, "Macro maps size: %5lu%c\n", + fprintf (stderr, "Macro maps size: %5ld%c\n", SCALE (macro_maps_size), STAT_LABEL (macro_maps_size)); - fprintf (stderr, "Duplicated maps locations size: %5lu%c\n", + fprintf (stderr, "Duplicated maps locations size: %5ld%c\n", SCALE (s.duplicated_macro_maps_locations_size), STAT_LABEL (s.duplicated_macro_maps_locations_size)); - fprintf (stderr, "Total allocated maps size: %5lu%c\n", + fprintf (stderr, "Total allocated maps size: %5ld%c\n", SCALE (total_allocated_map_size), STAT_LABEL (total_allocated_map_size)); - fprintf (stderr, "Total used maps size: %5lu%c\n", + fprintf (stderr, "Total used maps size: %5ld%c\n", SCALE (total_used_map_size), STAT_LABEL (total_used_map_size)); fprintf (stderr, "\n"); diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h index 1e2a148..ef98f59 100644 --- a/libcpp/include/line-map.h +++ b/libcpp/include/line-map.h @@ -687,17 +687,17 @@ expanded_location linemap_expand_location_full (struct line_maps *, linemap_get_statistics. */ struct linemap_stats { - size_t num_ordinary_maps_allocated; - size_t num_ordinary_maps_used; - size_t ordinary_maps_allocated_size; - size_t ordinary_maps_used_size; - size_t num_expanded_macros; - size_t num_macro_tokens; - size_t num_macro_maps_used; - size_t macro_maps_allocated_size; - size_t macro_maps_used_size; - size_t macro_maps_locations_size; - size_t duplicated_macro_maps_locations_size; + long num_ordinary_maps_allocated; + long num_ordinary_maps_used; + long ordinary_maps_allocated_size; + long ordinary_maps_used_size; + long num_expanded_macros; + long num_macro_tokens; + long num_macro_maps_used; + long macro_maps_allocated_size; + long macro_maps_used_size; + long macro_maps_locations_size; + long duplicated_macro_maps_locations_size; }; /* Compute and return statistics about the memory consumption of some diff --git a/libcpp/line-map.c b/libcpp/line-map.c index 43e2856..fb3be3a 100644 --- a/libcpp/line-map.c +++ b/libcpp/line-map.c @@ -1180,7 +1180,7 @@ void linemap_get_statistics (struct line_maps *set, struct linemap_stats *s) { - size_t ordinary_maps_allocated_size, ordinary_maps_used_size, + long ordinary_maps_allocated_size, ordinary_maps_used_size, macro_maps_allocated_size, macro_maps_used_size, macro_maps_locations_size = 0, duplicated_macro_maps_locations_size = 0; diff --git a/libcpp/macro.c b/libcpp/macro.c index 2d0eeaa..f313959 100644 --- a/libcpp/macro.c +++ b/libcpp/macro.c @@ -1278,6 +1278,10 @@ macro_arg_token_iter_init (macro_arg_token_iter *iter, iter->track_macro_exp_p = track_macro_exp_p; iter->kind = kind; iter->token_ptr = token_ptr; + /* Unconditionally initialize this so that the compiler doesn't warn + about iter->location_ptr being possibly uninitialized later after + this code has been inlined somewhere. */ + iter->location_ptr = NULL; if (track_macro_exp_p) iter->location_ptr = get_arg_token_location (arg, kind); #ifdef ENABLE_CHECKING -- Dodji ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 0/6] Tracking locations of tokens resulting from macro expansion @ 2010-12-10 11:27 Dodji Seketeli 2010-12-10 11:27 ` [PATCH 3/6] Emit macro expansion related diagnostics Dodji Seketeli 0 siblings, 1 reply; 28+ messages in thread From: Dodji Seketeli @ 2010-12-10 11:27 UTC (permalink / raw) To: gcc-patches; +Cc: tromey, joseph, gdr, lopezibanez * Problem statement Let's consider the diagnostic GCC emits when an error occurs on an expression that was defined in a macro that is later expanded: [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 test.c test.c: In function âgâ: test.c:13:3: error: invalid operands to binary << (have âdoubleâ and âintâ) Just looking at line 13 as the diagnostic suggests is not enough to understand why there is an error there. On line 13, there is no "<<" token, but the compiler complains about an opereator "<<" that is taking the wrong types of arguments on /that/ line. Why? Obviously what happened is that the MULT macro defined at line 7 and 8 got expanded. The macro SHIFTL used by MULT and defined at lines 1 and 2 got expanded as well. That macro expansion fest yielded the expression: 1.0 << 1; which is an error as you can't shift a float. The problem is tokens '1.0', '<<' and '1' all have their location set to {line 11, column 3} ({11, 3} for short). That actually is the location of the expansion point of the MULT macro. That's is an interesting observation -- in the current incarnation of GCC the location of each tokens resulting from a macro expansion is set to the location of the expansion point of the macro. * Path for improvement For each token resulting from macro expansion it would be nice if GCC could track two other kinds of locations for tokens resulting from macro expansions: - Spelling location. The location of the point where the token appears in the definition of the macro. The spelling location of "<<" would be {5, 14}. - Macro argument replacement point location. This one only exists for a token that is and arguments of a function-like macro. It's the location of the point in the definition of the macro where the argument replaces the parameter. In other words it's the location of the point where the parameter of a given argument is used in the definition of a function-like macro. For the "<<" token, that would be the location {2,9} of the 'OPRT' token in the definition of the 'OPERATE' macro. By using the three locations exposed above (spelling, argument replacement point and expansion point locations), GCC could emit diagnostics that are hopefully more useful. E.g: [dodji@adjoa gcc]$ ./cc1 -quiet -ftrack-macro-expansion test.c test.c: In function âgâ: test.c:5:14: error: invalid operands to binary << (have âdoubleâ and âintâ) In macro 'OPERATE' at test.c:2:9 Expanded at test.c:5:3 In macro 'SHIFTL' at test.c:5:14 Expanded at test.c:8:3 In macro 'MULT' at test.c:8:3 Expanded at test.c:13:3 Several obversations can be made in that hypothetical example. - The location mentioned in the error message test.c:5:14: error: invalid operands to binary << (have âdoubleâ and âintâ) is the spelling location of the token '<<'. I believe this makes more sense than the original behaviour. - The macro expansion stack following the error message unwinds from the macro which expansion most directly yielded token "<<". Each element of the stack mentions the argument replacement point and the expansion point locations that were exposed earlier. * What it would take Luckily Tom Tromey attached a patch to this bug http://gcc.gnu.org/bugzilla/show_bug.cgi?id=7263 That patch paved the way showing how to build on the locations maps provided by libcpp to come up with what I would call virtual locations. A virtual location is a location that can resolve to many possible physical locuses. For a token that does not result from macro a expansion, a virtual location resolves only to the spelling location of the token. For a token resulting from macro expansion, a virtual location can resolve to either: [I would bet you heard about these different kinds of locations already but here we go] - the expansion point of the macro - the point where the token appears in the definition of the macro - the argument replacement point, in the context of a function-like macro. So I took that patch and massaged it a little bit. The user facing result is actually what I was showing in the [finally not so] hypothetical improved GCC example earlier. With the resulting patch-set the linemap module now provides and API to generate and resolve virtual locations for tokens. When a diagnostic function emit a message about a virtual location, it now can detect [using the new linemap API] if that location belongs to a token resulting from macro expansion. If yes, it can print the macro expansion stack that led to that token. So almost nothing changes for the diagnostics clients. * Limits I guess a pure marketing dude would elide this part :-) The most annoying limit is obviously memory consumption. Tracking the location of every token across every macro expansion does consume memory. I tried compiling some a translation here that heavily uses the C++ standard library and I could see some 13% increase of memory consumption over the entire compilation; note that the compilation was consuming around 250MB without the feature. This is why the feature is triggered by the -ftrack-macro-expansion flag. Without that flag, the memory consumption stays roughly flat. The noticeable downside of that flag is that it makes the code more complex. I think there possibly is room to decrease this, but fundamentaly the consumption is going to increase with the number of macro expanded multiplied by the number of tokens per expansion. The patch-set I have today doesn't track the locations of tokens resulting from pasting and stringification. For those, only the spelling token is tracked for now. I think this can be improved. I just haven't thought about it deeply yet. * Perspectives The patch-set is basically the point A.0) presented in the document http://gcc.gnu.org/wiki/Better_Diagnostics. This is material for GCC 4.7 at best but I felt it would be interesting to post this now. So the patches will follow shortly. For now please find below the summary of the changes. Linemap infrastructure for virtual locations Generate virtual locations for tokens Emit macro expansion related diagnostics Support -fdebug-cpp option Add line map statistics to -fmem-report output Kill pedantic warnings on system headers macros gcc/Makefile.in | 2 +- gcc/ada/gcc-interface/trans.c | 10 +- gcc/c-decl.c | 17 +- gcc/c-family/c-lex.c | 10 +- gcc/c-family/c-opts.c | 17 + gcc/c-family/c-pch.c | 2 +- gcc/c-family/c-ppoutput.c | 92 ++- gcc/c-family/c.opt | 12 + gcc/c-parser.c | 12 +- gcc/c-tree.h | 2 +- gcc/cp/error.c | 2 +- gcc/diagnostic.c | 160 +++- gcc/diagnostic.h | 2 +- gcc/doc/cppopts.texi | 29 + gcc/doc/invoke.texi | 6 +- gcc/fortran/cpp.c | 22 +- gcc/input.c | 107 ++- gcc/input.h | 22 +- gcc/java/jcf-parse.c | 2 +- gcc/testsuite/g++.dg/cpp0x/initlist15.C | 1 + gcc/testsuite/g++.old-deja/g++.robertl/eb43.C | 4 + gcc/testsuite/g++.old-deja/g++.robertl/eb79.C | 4 + gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-1.c | 30 + gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-2.c | 31 + gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-3.c | 18 + gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-4.c | 19 + gcc/testsuite/gcc.dg/cpp/syshdr3.c | 16 + gcc/testsuite/gcc.dg/cpp/syshdr3.h | 7 + gcc/testsuite/gcc.dg/nofixed-point-2.c | 6 +- gcc/testsuite/gcc.target/i386/sse-vect-types.c | 6 + gcc/toplev.c | 1 + gcc/tree-diagnostic.c | 2 +- libcpp/directives-only.c | 7 +- libcpp/directives.c | 19 +- libcpp/errors.c | 21 +- libcpp/expr.c | 176 ++-- libcpp/files.c | 24 +- libcpp/include/cpp-id-data.h | 6 + libcpp/include/cpplib.h | 15 +- libcpp/include/line-map.h | 634 +++++++++++-- libcpp/init.c | 3 +- libcpp/internal.h | 49 +- libcpp/lex.c | 112 ++- libcpp/line-map.c | 997 +++++++++++++++++-- libcpp/macro.c | 1188 ++++++++++++++++++++--- libcpp/traditional.c | 7 +- 46 files changed, 3376 insertions(+), 555 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/syshdr3.c create mode 100644 gcc/testsuite/gcc.dg/cpp/syshdr3.h -- Dodji ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 3/6] Emit macro expansion related diagnostics 2010-12-10 11:27 [PATCH 0/6] Tracking locations of tokens resulting from macro expansion Dodji Seketeli @ 2010-12-10 11:27 ` Dodji Seketeli 2010-12-13 15:25 ` Paolo Bonzini 0 siblings, 1 reply; 28+ messages in thread From: Dodji Seketeli @ 2010-12-10 11:27 UTC (permalink / raw) To: gcc-patches; +Cc: tromey, joseph, gdr, lopezibanez 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: error: invalid operands to binary << (have âdoubleâ and âintâ) In macro 'OPERATE' at test.c:2:9 Expanded at test.c:5:3 In macro 'SHIFTL' at test.c:5:14 Expanded at test.c:8:3 In macro 'MULT' at test.c:8:3 Expanded at test.c:13:3 The combination of this patch and the previous ones boostrapped with --enable-languages=all,ada and passed regression tests on x86_64-unknown-linux-gnu. gcc/ * gcc/diagnostic.h (diagnostic_report_current_module): Add a location parameter. * diagnostic.c (struct loc_t): new struct. (diagnostic_report_current_module): Add a location parameter to the function definition. Use it here instead of input_location. Fully expand the 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. (unwind_expanded_macro_location): New function. (default_diagnostic_finalizer): Use it. * tree-diagnostic.c (diagnostic_report_current_function): Pass the relevant location to diagnostic_report_current_module. gcc/cp/ * error.c (cp_diagnostic_starter): Pass the relevant location to diagnostic_report_current_module. gcc/testsuite/ * 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. * g++.dg/cpp0x/initlist15.C: Discard errors pointing at multiple levels of included files. * g++.old-deja/g++.robertl/eb43.C: Likewise. * g++.old-deja/g++.robertl/eb79.C: Likewise. * gcc.target/i386/sse-vect-types.c: Likewise. --- gcc/Makefile.in | 2 +- gcc/cp/error.c | 2 +- gcc/diagnostic.c | 154 ++++++++++++++++++++++- gcc/diagnostic.h | 2 +- gcc/testsuite/g++.dg/cpp0x/initlist15.C | 1 + gcc/testsuite/g++.old-deja/g++.robertl/eb43.C | 4 + gcc/testsuite/g++.old-deja/g++.robertl/eb79.C | 4 + gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-1.c | 30 +++++ gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-2.c | 31 +++++ gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-3.c | 18 +++ gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-4.c | 19 +++ gcc/testsuite/gcc.target/i386/sse-vect-types.c | 6 + gcc/tree-diagnostic.c | 2 +- 13 files changed, 266 insertions(+), 9 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 diff --git a/gcc/Makefile.in b/gcc/Makefile.in index 717326c..604f626 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -2825,7 +2825,7 @@ fold-const.o : fold-const.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) \ $(GGC_H) $(TM_P_H) langhooks.h $(MD5_H) intl.h $(TARGET_H) \ $(GIMPLE_H) realmpfr.h $(TREE_FLOW_H) diagnostic.o : diagnostic.c $(CONFIG_H) $(SYSTEM_H) coretypes.h \ - version.h $(INPUT_H) intl.h $(DIAGNOSTIC_H) diagnostic.def + version.h $(INPUT_H) intl.h $(DIAGNOSTIC_H) diagnostic.def $(VEC_H) opts.o : opts.c $(OPTS_H) $(OPTIONS_H) $(DIAGNOSTIC_CORE_H) $(CONFIG_H) $(SYSTEM_H) \ coretypes.h $(TM_H) $(RTL_H) \ $(DIAGNOSTIC_H) $(INSN_ATTR_H) intl.h $(TARGET_H) \ diff --git a/gcc/cp/error.c b/gcc/cp/error.c index ed168c4..db35c61 100644 --- a/gcc/cp/error.c +++ b/gcc/cp/error.c @@ -2663,7 +2663,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); diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c index 9df540b..671b6ea 100644 --- a/gcc/diagnostic.c +++ b/gcc/diagnostic.c @@ -30,6 +30,15 @@ along with GCC; see the file COPYING3. If not see #include "input.h" #include "intl.h" #include "diagnostic.h" +#include "vec.h" + +typedef struct +{ + const struct line_map *map; + source_location where; +} loc_t; +DEF_VEC_O(loc_t); +DEF_VEC_ALLOC_O (loc_t,heap); #define pedantic_warning_kind(DC) \ ((DC)->pedantic_errors ? DK_ERROR : DK_WARNING) @@ -45,6 +54,9 @@ static void diagnostic_action_after_output (diagnostic_context *, diagnostic_info *); static void real_abort (void) ATTRIBUTE_NORETURN; +static void unwind_expanded_macro_location (diagnostic_context *, source_location, + const struct line_map **); + /* Name of program invoked, sans directories. */ const char *progname; @@ -254,10 +266,138 @@ diagnostic_action_after_output (diagnostic_context *context, } } +/* Unwind the different macro expansions that lead to the token which + location is WHERE and emit diagnostics showing the resulting + unwound macro expansion stack. If TOPMOST_EXP_POINT_MAP is + non-null, *TOPMOST_EXP_POINT_MAP is set to the map of the + expansion point of the top most macro of the stack. This must be + an ordinary map. */ + +static void +unwind_expanded_macro_location (diagnostic_context *context, + source_location where, + const struct line_map **topmost_exp_point_map) +{ + const struct line_map *map, *resolved_map; + bool unwind = true; + source_location resolved_location; + VEC(loc_t, heap) *loc_vec; + unsigned ix, len; + loc_t loc, *iter; + + map = linemap_lookup (line_table, where); + if (!linemap_macro_expansion_map_p (map)) + return; + + loc_vec = VEC_alloc (loc_t, heap, 4); + + /* Let's unwind the stack of macros that got expanded and that led + to the token which location is WHERE. We are going to store the + stack into MAP_VEC, so that we can later walk MAP_VEC backward to + display a somewhat meaningful trace of the macro expansion + history to the user. Note that the deepest macro expansion is + going to be store at the beginning of MAP_VEC. */ + while (unwind) + { + 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 + *definition* of the macro of MAP, that got expanded at + WHERE. This is basically how we go "up" in the stack of + macro expansions that led to WHERE. */ + resolved_location = + linemap_macro_map_loc_to_def_point (map, where, false); + resolved_map = linemap_lookup (line_table, resolved_location); + + /* If the token at RESOLVED_LOCATION [at macro definition point] + is itself inside an expanded macro then we keep unwinding the + expansion stack by tracing the "parent macro" that got expanded + inside the definition of the macro of MAP... */ + if (linemap_macro_expansion_map_p (resolved_map)) + { + where = resolved_location; + map = resolved_map; + } + else + { + /* Otherwise, let's consider the location of the expansion + point of the macro of MAP. Keep in mind that MAP is a + macro expansion map. To get a "normal map" (i.e a non + macro expansion map) and be done with the unwinding, we + must either consider the location of the location + expansion point of the macro or the location of the token + inside the macro definition that got expanded to + WHERE. */ + where = + linemap_macro_map_loc_to_exp_point (map, where); + map = linemap_lookup (line_table, where); + } + if (!linemap_macro_expansion_map_p (map)) + unwind = false; + } + + if (topmost_exp_point_map) + *topmost_exp_point_map = map; + + /* Walk the map_vec and print the macro expansion stack. */ + len = VEC_length (loc_t, loc_vec); + for (ix = 0; + len && ix < len; + ++ix) + { + expanded_location def_loc, exp_loc; + const struct line_map *def_point_map = NULL, + *exp_point_map = NULL; + + iter = VEC_index (loc_t, loc_vec, ix); + + /* 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. */ + + /* Expand the location iter->where into the locus 1/ of the + comment above. */ + def_loc = + linemap_expand_location_full (line_table, iter->where, + LRK_MACRO_PARM_REPLACEMENT_POINT, + &def_point_map); + + /* Expand the location of the expansion point of the macro + which expansion gave the token at represented by + def_loc. This is the locus 2/ of the earlier comment. */ + exp_loc = + linemap_expand_location_full (line_table, + MACRO_MAP_EXPANSION_POINT_LOCATION + (iter->map), + LRK_MACRO_PARM_REPLACEMENT_POINT, + &exp_point_map); + + if (!LINEMAP_SYSP (resolved_map)) + { + pp_verbatim (context->printer, + "\nIn macro '%s' at %s:%d:%d", + linemap_map_get_macro_name (iter->map), + LINEMAP_FILE (def_point_map), + def_loc.line, def_loc.column); + pp_verbatim (context->printer, + "\n Expanded at %s:%d:%d", + LINEMAP_FILE (exp_point_map), + exp_loc.line, exp_loc.column); + } + } + VEC_free (loc_t, heap, loc_vec); +} + 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 +405,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_expand_location_full (line_table, where, + LRK_MACRO_PARM_REPLACEMENT_POINT, + &map); + if (map && diagnostic_last_module_changed (context, map)) { diagnostic_set_last_module (context, map); @@ -301,7 +444,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)); } @@ -310,6 +453,7 @@ void default_diagnostic_finalizer (diagnostic_context *context, diagnostic_info *diagnostic ATTRIBUTE_UNUSED) { + unwind_expanded_macro_location (context, diagnostic->location, NULL); pp_destroy_prefix (context->printer); } 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/g++.dg/cpp0x/initlist15.C b/gcc/testsuite/g++.dg/cpp0x/initlist15.C index b75cc81..cca56b1 100644 --- a/gcc/testsuite/g++.dg/cpp0x/initlist15.C +++ b/gcc/testsuite/g++.dg/cpp0x/initlist15.C @@ -2,6 +2,7 @@ // Just discard errors pointing at header files // { dg-prune-output "include" } +// { dg-prune-output " from" } #include <vector> #include <typeinfo> diff --git a/gcc/testsuite/g++.old-deja/g++.robertl/eb43.C b/gcc/testsuite/g++.old-deja/g++.robertl/eb43.C index 1dc4328..bd784b1 100644 --- a/gcc/testsuite/g++.old-deja/g++.robertl/eb43.C +++ b/gcc/testsuite/g++.old-deja/g++.robertl/eb43.C @@ -6,6 +6,10 @@ // { dg-prune-output "note" } +// Discard errors pointing at header files +// { dg-prune-output "In file included from" } +// { dg-prune-output " from" } + #include <vector> #include <algorithm> #include <functional> diff --git a/gcc/testsuite/g++.old-deja/g++.robertl/eb79.C b/gcc/testsuite/g++.old-deja/g++.robertl/eb79.C index 1c1ad3e..60cc713 100644 --- a/gcc/testsuite/g++.old-deja/g++.robertl/eb79.C +++ b/gcc/testsuite/g++.old-deja/g++.robertl/eb79.C @@ -1,5 +1,9 @@ // { dg-do assemble } // { dg-prune-output "note" } + +// Discard errors pointing at header files +// { dg-prune-output "In file included from" } +// { dg-prune-output " from" } // Makes bogus x86 assembly code. #include <iostream> 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..a087050 --- /dev/null +++ b/gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-1.c @@ -0,0 +1,30 @@ +/* + { dg-options "-ftrack-macro-expansion=1" } + { dg-do compile } +*/ + +#define OPERATE(OPRD1, OPRT, OPRD2) \ +do \ +{ \ + OPRD1 OPRT OPRD2; \ +} while (0) + +#define SHIFTL(A,B) \ + OPERATE (A,<<,B) /* { dg-error "invalid operands to binary <<" } */ + +void +foo () +{ + SHIFTL (0.1,0.2); +} + +/* + { dg-message "macro 'OPERATE'\[^\n\r\]*:9:8" "In macro OPERATE" { target *-*-* } 0 } +{ dg-message "Expanded at\[^\n\r\]*:13:3" "OPERATE expansion point" { target *-*-* } 0 } + + { dg-message "macro 'SHIFTL'\[^\n\r\]*:13:14" "In macro SHIFTL" { target *-*-* } 0 } + +{ dg-message "Expanded at\[^\n\r\]*:18:3" "SHIFTL expansion point" { target *-*-* } 0 } + + +*/ 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..0f15875 --- /dev/null +++ b/gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-2.c @@ -0,0 +1,31 @@ +/* + { dg-options "-ftrack-macro-expansion=1" } + { dg-do compile } +*/ + +#define OPERATE(OPRD1, OPRT, OPRD2) \ + OPRD1 OPRT OPRD2; + +#define SHIFTL(A,B) \ + OPERATE (A,<<,B) /* { dg-error "invalid operands to binary <<" } */ + +#define MULT(A) \ + SHIFTL (A,1) + +void +foo () +{ + MULT (1.0);/* 1.0 << 1;*/ +} + +/* + { dg-message "macro 'OPERATE'\[^\n\r\]*:7:8*" "In macro OPERATE" { target *-*-* } 0 } + { dg-message "Expanded at\[^\n\r\]*:10:3" "OPERATE expansion point" { target *-*-* } 0 } + + { dg-message "macro 'SHIFTL'\[^\n\r\]*:10:14" "In macro SHIFTL" { target *-*-* } 0 } +{ dg-message "Expanded at\[^\n\r\]*:13:3" "SHIFTL expansion point" { target *-*-* } 0 } + + { dg-message "macro 'MULT'\[^\n\r\]*:13:3" "In macro MULT" { target *-*-* } 0 } +{ dg-message "Expanded at\[^\n\r\]*:18:3" "MULT expansion point" { target *-*-* } 0 } + + */ 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..0b5c662 --- /dev/null +++ b/gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-3.c @@ -0,0 +1,18 @@ +/* + { dg-options "-fshow-column -ftrack-macro-expansion=1" } + { dg-do compile } + */ + +#define SQUARE(A) A * A + +void +foo() +{ + SQUARE (1 << 0.1); /* { dg-error "16:invalid operands to binary <<" } */ +} + +/* +{ dg-message "macro 'SQUARE'\[^\n\r\]*:6:19" "In macro SQUARE" { target *-*-* } 0 } +{ dg-message "Expanded at\[^\n\r\]*:11:3" "SQUARE expansion point" { target *-*-* } 0 } + +*/ 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..f9e4c2d --- /dev/null +++ b/gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-4.c @@ -0,0 +1,19 @@ +/* + { dg-options "-fshow-column -ftrack-macro-expansion=2" } + { dg-do compile } + */ + +#define SQUARE(A) A * A + +void +foo() +{ + SQUARE (1 << 0.1); /* { dg-error "13:invalid operands to binary <<" } */ +} + +/* + +{ dg-message "macro 'SQUARE'\[^\n\r\]*:6:19" "In macro SQUARE" { target *-*-* } 0 } +{ dg-message "Expanded at\[^\n\r\]*:11:3" "SQUARE expansion point" { target *-*-* } 0 } + +*/ diff --git a/gcc/testsuite/gcc.target/i386/sse-vect-types.c b/gcc/testsuite/gcc.target/i386/sse-vect-types.c index 9cb6f3e..ce70125 100644 --- a/gcc/testsuite/gcc.target/i386/sse-vect-types.c +++ b/gcc/testsuite/gcc.target/i386/sse-vect-types.c @@ -1,6 +1,12 @@ /* { dg-do compile } */ /* { dg-options "-O0 -msse2" } */ + +/* + Just discard diagnostic prolog about errors in include files + { dg-prune-output "In file included from" } +*/ + #include <xmmintrin.h> __m128d foo1(__m128d z, __m128d a, int N) { diff --git a/gcc/tree-diagnostic.c b/gcc/tree-diagnostic.c index b456a2a..cbfd81c 100644 --- a/gcc/tree-diagnostic.c +++ b/gcc/tree-diagnostic.c @@ -35,7 +35,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); } -- Dodji ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/6] Emit macro expansion related diagnostics 2010-12-10 11:27 ` [PATCH 3/6] Emit macro expansion related diagnostics Dodji Seketeli @ 2010-12-13 15:25 ` Paolo Bonzini 2010-12-13 15:38 ` Paolo Bonzini ` (3 more replies) 0 siblings, 4 replies; 28+ messages in thread From: Paolo Bonzini @ 2010-12-13 15:25 UTC (permalink / raw) To: Dodji Seketeli; +Cc: gcc-patches, tromey, joseph, gdr, lopezibanez On 12/10/2010 12:11 PM, Dodji Seketeli wrote: > [dodji@adjoa gcc]$ ./cc1 -quiet -ftrack-macro-expansion test.c > test.c: In function âgâ: > test.c:5:14: error: invalid operands to binary<< (have âdoubleâ and âintâ) > In macro 'OPERATE' at test.c:2:9 > Expanded at test.c:5:3 > In macro 'SHIFTL' at test.c:5:14 > Expanded at test.c:8:3 > In macro 'MULT' at test.c:8:3 > Expanded at test.c:13:3 > I'm not sure I share Jeff's doubts about the location to present. Possibly _this_ could be controlled by a flag, though. Also, I know this is just an RFC, but the error message should probably look like either this example: 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 or this shorter example: test.c:5:14: error: invalid operands to binary<< (have âdoubleâ and âintâ) test.c:2:9: note: while expanding macro 'OPERATE' test.c:5:14: note: while expanding macro 'SHIFTL' test.c:8:3: note: while expanding macro 'MULT' test.c:13:3: note: expanded from here or this that does not change the location compared to current GCC: test.c:13:3: error: invalid operands to binary<< (have âdoubleâ and âintâ) test.c:2:9: note: while expanding macro 'OPERATE' test.c:5:14: note: while expanding macro 'SHIFTL' test.c:8:3: note: while expanding macro 'MULT' Paolo ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/6] Emit macro expansion related diagnostics 2010-12-13 15:25 ` Paolo Bonzini @ 2010-12-13 15:38 ` Paolo Bonzini 2010-12-13 16:30 ` Manuel López-Ibáñez ` (2 subsequent siblings) 3 siblings, 0 replies; 28+ messages in thread From: Paolo Bonzini @ 2010-12-13 15:38 UTC (permalink / raw) To: gcc-patches; +Cc: gcc-patches, tromey, joseph, gdr, lopezibanez On 12/10/2010 12:11 PM, Dodji Seketeli wrote: > [dodji@adjoa gcc]$ ./cc1 -quiet -ftrack-macro-expansion test.c > test.c: In function âgâ: > test.c:5:14: error: invalid operands to binary<< (have âdoubleâ and âintâ) > In macro 'OPERATE' at test.c:2:9 > Expanded at test.c:5:3 > In macro 'SHIFTL' at test.c:5:14 > Expanded at test.c:8:3 > In macro 'MULT' at test.c:8:3 > Expanded at test.c:13:3 > I'm not sure I share Jeff's doubts about the location to present. Possibly _this_ could be controlled by a flag, though. Also, I know this is just an RFC, but the error message should probably look like either this example: 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 or this shorter example: test.c:5:14: error: invalid operands to binary<< (have âdoubleâ and âintâ) test.c:2:9: note: while expanding macro 'OPERATE' test.c:5:14: note: while expanding macro 'SHIFTL' test.c:8:3: note: while expanding macro 'MULT' test.c:13:3: note: expanded from here or this that does not change the location compared to current GCC: test.c:13:3: error: invalid operands to binary<< (have âdoubleâ and âintâ) test.c:2:9: note: while expanding macro 'OPERATE' test.c:5:14: note: while expanding macro 'SHIFTL' test.c:8:3: note: while expanding macro 'MULT' Paolo ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/6] Emit macro expansion related diagnostics 2010-12-13 15:25 ` Paolo Bonzini 2010-12-13 15:38 ` Paolo Bonzini @ 2010-12-13 16:30 ` Manuel López-Ibáñez 2010-12-14 7:24 ` Dodji Seketeli 2010-12-14 7:28 ` Dodji Seketeli 3 siblings, 0 replies; 28+ messages in thread From: Manuel López-Ibáñez @ 2010-12-13 16:30 UTC (permalink / raw) To: Paolo Bonzini; +Cc: Dodji Seketeli, gcc-patches, tromey, joseph, gdr On 13 December 2010 15:44, Paolo Bonzini <bonzini@gnu.org> wrote: > On 12/10/2010 12:11 PM, Dodji Seketeli wrote: >> [dodji@adjoa gcc]$ ./cc1 -quiet -ftrack-macro-expansion test.c >> test.c: In function ‘g’: >> test.c:5:14: error: invalid operands to binary<< (have ‘double’ and ‘int’) >> In macro 'OPERATE' at test.c:2:9 >> Expanded at test.c:5:3 >> In macro 'SHIFTL' at test.c:5:14 >> Expanded at test.c:8:3 >> In macro 'MULT' at test.c:8:3 >> Expanded at test.c:13:3 >> > > > or this shorter example: > > test.c:5:14: error: invalid operands to binary<< (have ‘double’ and ‘int’) > test.c:2:9: note: while expanding macro 'OPERATE' > test.c:5:14: note: while expanding macro 'SHIFTL' > test.c:8:3: note: while expanding macro 'MULT' > test.c:13:3: note: expanded from here > This format is closer to what GCC currently prints for templates instantiations, however, in template instantiations, the context goes first and the error/warning goes last. The context is also not marked with "note:". See http://people.redhat.com/bkoz/diagnostics/diagnostics.html#template-instantiate-1 and http://people.redhat.com/bkoz/diagnostics/diagnostics.html#9335 and other examples therein. See gcc/cp/error.c:print_instantiation_partial_context, which also handles eliding excessive number of instantiation contexts (too deep macro expansion). I am not saying that this has to be done in this patch series, but it should eventually be implemented, so why not copy what is already available? > or this that does not change the location compared to current GCC: > > test.c:13:3: error: invalid operands to binary<< (have ‘double’ and ‘int’) > test.c:2:9: note: while expanding macro 'OPERATE' > test.c:5:14: note: while expanding macro 'SHIFTL' > test.c:8:3: note: while expanding macro 'MULT' Interestingly, this is closer to the format adopted by Clang (http://clang.llvm.org/diagnostics.html), which I guess just followed GCC. However, I don't see how changing the locations to the ones proposed by Dodji causes any kind of "havoc" for users. In fact, looking at the caret information of clang, the diagnostic would be clearer if the locations were exchanged, which is what Dodji proposes. I don't think gcc diagnostics can be used to do any kind of automatic rewriting. At most, it is parsed in order to present the location of errors/messages in a nicer way, and the new locations proposed by Dodji do not break that. The question should be what locations are clearer/more informative to the user. I think the new locations proposed by Dodji are more informative and they follow what g++ already does for template errors. In any case, I guess that if there is a flag to disable the tracking of macro expansions for memory concerns, the same flag disables the enhanced locations and reverts to the current status, no? No need for yet another separate flag to control the output. Cheers, Manuel. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/6] Emit macro expansion related diagnostics 2010-12-13 15:25 ` Paolo Bonzini 2010-12-13 15:38 ` Paolo Bonzini 2010-12-13 16:30 ` Manuel López-Ibáñez @ 2010-12-14 7:24 ` Dodji Seketeli 2010-12-14 7:28 ` Gabriel Dos Reis 2010-12-14 7:28 ` Dodji Seketeli 3 siblings, 1 reply; 28+ messages in thread From: Dodji Seketeli @ 2010-12-14 7:24 UTC (permalink / raw) To: Paolo Bonzini; +Cc: gcc-patches, tromey, joseph, gdr, lopezibanez Paolo Bonzini <bonzini@gnu.org> writes: > On 12/10/2010 12:11 PM, Dodji Seketeli wrote: >> [dodji@adjoa gcc]$ ./cc1 -quiet -ftrack-macro-expansion test.c >> test.c: In function ‘g’: >> test.c:5:14: error: invalid operands to binary<< (have ‘double’ and ‘int’) >> In macro 'OPERATE' at test.c:2:9 >> Expanded at test.c:5:3 >> In macro 'SHIFTL' at test.c:5:14 >> Expanded at test.c:8:3 >> In macro 'MULT' at test.c:8:3 >> Expanded at test.c:13:3 >> > > I'm not sure I share Jeff's doubts about the location to present. > Possibly _this_ could be controlled by a flag, though. > > Also, I know this is just an RFC, but the error message should > probably look like either this example: > > 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 Sorry if I am dig into design consideration when we are talking about UI, but I think it is needed to clarify things here. The current custom in the compiler is that the "<locus> note:" prefix is displayed only when client code explicitely calls the 'inform' function. That sets the diagnostic kind to DK_NOTE. This seems a little bit different from what is done for macro expansion contexts. The macro expansion context is unwound implicitly. That is, the client code calls e.g: error_at (some_location, "An error occured") and if some_location appears to be the location of a token resulting from macro expansion the diagnostic machinery unwinds the expansion context and displays it to the user, regardless of what the diagnostic kind was. So the context lines are not prefixed with "<locus> note:" as the context is generated implicitely. This is similar to what G++ does when it displays template instantiation contexts. On the other hand, I find the "<locus> note:" usage more regular and thus easier to parse for tools that interact with the output of the compiler. So there could be some value in us using that prefix. However if we are to use that prefix for implicitely displayed contexts, I think we should factorize diagnostic_build_prefix to allow using the same prefix as the one used for e.g. DK_NOTE. Would that be acceptable? -- Dodji ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/6] Emit macro expansion related diagnostics 2010-12-14 7:24 ` Dodji Seketeli @ 2010-12-14 7:28 ` Gabriel Dos Reis 2010-12-14 8:40 ` Dodji Seketeli 0 siblings, 1 reply; 28+ messages in thread From: Gabriel Dos Reis @ 2010-12-14 7:28 UTC (permalink / raw) To: Dodji Seketeli; +Cc: Paolo Bonzini, gcc-patches, tromey, joseph, lopezibanez On Tue, Dec 14, 2010 at 12:54 AM, Dodji Seketeli <dodji@redhat.com> wrote: > The macro expansion context is unwound implicitly. That is, the client > code calls e.g: > > error_at (some_location, "An error occured") > > and if some_location appears to be the location of a token resulting > from macro expansion the diagnostic machinery unwinds the expansion > context and displays it to the user, regardless of what the diagnostic > kind was. So the context lines are not prefixed with "<locus> note:" as > the context is generated implicitely. This is similar to what G++ does > when it displays template instantiation contexts. Let me add some background about this aspect of the diagnostic machinery. The functions inform(), error(), error_at(), etc. are offered as some `high-level' building blocks for constructing more advanced diagnostic functions. It is just that we have not been very disciplined at factorizing things correctly, and instead we tend to go for the easier road of `inlining' calls to error() or inform(). However, models to look for are print_candidates(), cxx_print_error_function, the newly introduced qualified_name_lookup_errorr, etc. The point here is that I would expect CPP to define its own error print diagnostic function that tracks macro expansion context (at bit like what what we do with template instantiation contexts) and combine calls to error_at() and inform(). Note also that we don't capitalize diagnostic messages (and they don't end with periods either.) -- Gaby ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/6] Emit macro expansion related diagnostics 2010-12-14 7:28 ` Gabriel Dos Reis @ 2010-12-14 8:40 ` Dodji Seketeli 2010-12-14 9:38 ` Gabriel Dos Reis 0 siblings, 1 reply; 28+ messages in thread From: Dodji Seketeli @ 2010-12-14 8:40 UTC (permalink / raw) To: Gabriel Dos Reis; +Cc: Paolo Bonzini, gcc-patches, tromey, joseph, lopezibanez Gabriel Dos Reis <gdr@integrable-solutions.net> writes: > Let me add some background about this aspect of the diagnostic > machinery. The functions inform(), error(), error_at(), etc. are > offered as some `high-level' building blocks for constructing more > advanced diagnostic functions. It is just that we have not been > very disciplined at factorizing things correctly, and instead we > tend to go for the easier road of `inlining' calls to error() or > inform(). Okay. Thank you for this background. > However, models to look for are print_candidates(), > cxx_print_error_function, the newly introduced > qualified_name_lookup_errorr, etc. I see. Though, some of these functions call below the level error and inform by calling pp_base_set_prefix, pp_verbatim and the like. Are those pp_* calls intended as well? > > The point here is that I would expect CPP to define its own error print > diagnostic function that tracks macro expansion context > (at bit like what what we do with template instantiation contexts) > and combine calls to error_at() and inform(). Okay, so I guess I will move the macro expansion unwinder into CPP and make it and use the CPP diagnostic routines that in turn use c_cpp_error (via the cpp_reader callbacks) that is at the same level as inform() error() etc. I will still have to make default_diagnostic_finalizer call that unwinder to make macro expansion context be printed implicitely, though. > Note also that we don't capitalize diagnostic messages (and they don't end > with periods either.) Thanks. I'll fix that. -- Dodji ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/6] Emit macro expansion related diagnostics 2010-12-14 8:40 ` Dodji Seketeli @ 2010-12-14 9:38 ` Gabriel Dos Reis 2010-12-14 9:42 ` Dodji Seketeli 0 siblings, 1 reply; 28+ messages in thread From: Gabriel Dos Reis @ 2010-12-14 9:38 UTC (permalink / raw) To: Dodji Seketeli; +Cc: Paolo Bonzini, gcc-patches, tromey, joseph, lopezibanez On Tue, Dec 14, 2010 at 2:22 AM, Dodji Seketeli <dodji@redhat.com> wrote: >> However, models to look for are print_candidates(), >> cxx_print_error_function, the newly introduced >> qualified_name_lookup_errorr, etc. > > I see. Though, some of these functions call below the level error and > inform by calling pp_base_set_prefix, pp_verbatim and the like. Are > those pp_* calls intended as well? Yes, that is another dimension to the diagnostic machinery. The idea here is that every client (i.e. front-end) may have som front-end specific actions to take (e.g. printing template instantiation contexts) before the core diagnostic message is printed. That is the role of `diagnostic starter' functions (see the comments in diagnostic.h). Similarly we have `diagnostic finalizer' functions which are supposed to do any sort of `cleanup' after a diagnostic is printed. Now, you can consider CPP as a sort of front-end that turns raw input file into a stream of tokens which is then handed over to the C or C++ parsers -- but I'm not sure that is the way it currently works. So, another option is to find a combination of these two dimensions. > >> >> The point here is that I would expect CPP to define its own error print >> diagnostic function that tracks macro expansion context >> (at bit like what what we do with template instantiation contexts) >> and combine calls to error_at() and inform(). > > Okay, so I guess I will move the macro expansion unwinder into CPP and > make it and use the CPP diagnostic routines that in turn use c_cpp_error > (via the cpp_reader callbacks) that is at the same level as inform() > error() etc. It is my belief that the macro-expansion related diagnostics specific improvements belong there. > > I will still have to make default_diagnostic_finalizer call that > unwinder to make macro expansion context be printed implicitely, though. something you can't do with CPP specific diagnostic finalizer? -- Gaby ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/6] Emit macro expansion related diagnostics 2010-12-14 9:38 ` Gabriel Dos Reis @ 2010-12-14 9:42 ` Dodji Seketeli 2010-12-14 9:48 ` Gabriel Dos Reis 0 siblings, 1 reply; 28+ messages in thread From: Dodji Seketeli @ 2010-12-14 9:42 UTC (permalink / raw) To: Gabriel Dos Reis; +Cc: Paolo Bonzini, gcc-patches, tromey, joseph, lopezibanez Gabriel Dos Reis <gdr@integrable-solutions.net> writes: [...] >> I see. Though, some of these functions call below the level error and >> inform by calling pp_base_set_prefix, pp_verbatim and the like. Are >> those pp_* calls intended as well? > > Yes, that is another dimension to the diagnostic machinery. > The idea here is that every client (i.e. front-end) may have som > front-end specific actions to take (e.g. printing template instantiation > contexts) before the core diagnostic message is printed. That > is the role of `diagnostic starter' functions (see the comments > in diagnostic.h). Similarly we have `diagnostic finalizer' functions > which are supposed to do any sort of `cleanup' after a diagnostic > is printed. OK. And front-ends are supposed to be able to provide their own diagnostics starter and finalizer functions. > Now, you can consider CPP as a sort of front-end that turns > raw input file into a stream of tokens which is then handed over > to the C or C++ parsers -- but I'm not sure that is the way it currently > works. A sort of front-end indeed. [..] >> Okay, so I guess I will move the macro expansion unwinder into CPP and >> make it and use the CPP diagnostic routines that in turn use c_cpp_error >> (via the cpp_reader callbacks) that is at the same level as inform() >> error() etc. > > It is my belief that the macro-expansion related diagnostics specific > improvements belong there. > >> >> I will still have to make default_diagnostic_finalizer call that >> unwinder to make macro expansion context be printed implicitely, though. > > something you can't do with CPP specific diagnostic finalizer? Good catch. The thing is CPP is used like a library by the front-end (C, C++, Fortran) that needs to consume the tokens it generates. As such, CPP itself doesn't override the diagnostic finalizer. It's the front-end that does it. And right now the C and Fortran FEs just use the default diagnostic finalizer. If I underrstand correctly, I should now provide a specific diagnostic finalizer for C and Fortran FEs (like what G++ does) and have them call the macro expansion unwinder (that would be) provided by CPP and leave default_diagnostic_finalizer alone. -- Dodji ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/6] Emit macro expansion related diagnostics 2010-12-14 9:42 ` Dodji Seketeli @ 2010-12-14 9:48 ` Gabriel Dos Reis 0 siblings, 0 replies; 28+ messages in thread From: Gabriel Dos Reis @ 2010-12-14 9:48 UTC (permalink / raw) To: Dodji Seketeli; +Cc: Paolo Bonzini, gcc-patches, tromey, joseph, lopezibanez On Tue, Dec 14, 2010 at 3:22 AM, Dodji Seketeli <dodji@redhat.com> wrote: > Gabriel Dos Reis <gdr@integrable-solutions.net> writes: > > [...] > >>> I see. Though, some of these functions call below the level error and >>> inform by calling pp_base_set_prefix, pp_verbatim and the like. Are >>> those pp_* calls intended as well? >> >> Yes, that is another dimension to the diagnostic machinery. >> The idea here is that every client (i.e. front-end) may have som >> front-end specific actions to take (e.g. printing template instantiation >> contexts) before the core diagnostic message is printed. That >> is the role of `diagnostic starter' functions (see the comments >> in diagnostic.h). Similarly we have `diagnostic finalizer' functions >> which are supposed to do any sort of `cleanup' after a diagnostic >> is printed. > > OK. And front-ends are supposed to be able to provide their own > diagnostics starter and finalizer functions. yes. [..] >>> I will still have to make default_diagnostic_finalizer call that >>> unwinder to make macro expansion context be printed implicitely, though. >> >> something you can't do with CPP specific diagnostic finalizer? > > Good catch. The thing is CPP is used like a library by the front-end (C, > C++, Fortran) that needs to consume the tokens it generates. As such, > CPP itself doesn't override the diagnostic finalizer. It's the front-end > that does it. And right now the C and Fortran FEs just use the default > diagnostic finalizer. > > If I underrstand correctly, I should now provide a specific diagnostic > finalizer for C and Fortran FEs (like what G++ does) and have them call > the macro expansion unwinder (that would be) provided by CPP and leave > default_diagnostic_finalizer alone. yes, that sounds good. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/6] Emit macro expansion related diagnostics 2010-12-13 15:25 ` Paolo Bonzini ` (2 preceding siblings ...) 2010-12-14 7:24 ` Dodji Seketeli @ 2010-12-14 7:28 ` Dodji Seketeli 2010-12-14 8:19 ` Gabriel Dos Reis 3 siblings, 1 reply; 28+ messages in thread From: Dodji Seketeli @ 2010-12-14 7:28 UTC (permalink / raw) To: Paolo Bonzini; +Cc: gcc-patches, tromey, joseph, gdr, lopezibanez Paolo Bonzini <bonzini@gnu.org> writes: > > test.c:5:14: error: invalid operands to binary<< (have ‘double’ and ‘int’) > test.c:2:9: note: while expanding macro 'OPERATE' > test.c:5:14: note: while expanding macro 'SHIFTL' > test.c:8:3: note: while expanding macro 'MULT' > test.c:13:3: note: expanded from here > In the absence of caret diagnostic I think it is useful to explicitely make the difference between a location in the definition of the macro, and the location of the macro expansion point, i.e: In this error message: test.c: In function ‘g’: test.c:5:14: error: invalid operands to binary << (have ‘double’ and ‘int’) In macro 'OPERATE' at test.c:2:9 Expanded at test.c:5:3 [...] Here is what we are trying to show: 1 #define OPERATE(OPRD1, OPRT, OPRD2) \ 2 OPRD1 OPRT OPRD2; ^ | ---< location in the /definition/ of OPERATE pointed to by: "In macro OPERATE at test.c:2:9" 3 4 #define SHIFTL(A,B) \ 5 OPERATE (A,<<,B) ^ | ---< location of /expansion point/ of OPERATE pointed to by "Expanded at test.c:5:3" [...] I think that's more explicit (and informative) than just saying: > test.c:5:14: error: invalid operands to binary<< (have ‘double’ and ‘int’) > test.c:2:9: note: while expanding macro 'OPERATE' > test.c:5:14: note: while expanding macro 'SHIFTL' But when we have caret diagnostic where we actually show the user the code we are talking about we can use the "while expanding" just fine. -- Dodji ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/6] Emit macro expansion related diagnostics 2010-12-14 7:28 ` Dodji Seketeli @ 2010-12-14 8:19 ` Gabriel Dos Reis 2010-12-14 8:31 ` Paolo Bonzini 0 siblings, 1 reply; 28+ messages in thread From: Gabriel Dos Reis @ 2010-12-14 8:19 UTC (permalink / raw) To: Dodji Seketeli; +Cc: Paolo Bonzini, gcc-patches, tromey, joseph, lopezibanez On Tue, Dec 14, 2010 at 1:19 AM, Dodji Seketeli <dodji@redhat.com> wrote: > Paolo Bonzini <bonzini@gnu.org> writes: > >> >> test.c:5:14: error: invalid operands to binary<< (have ‘double’ and ‘int’) >> test.c:2:9: note: while expanding macro 'OPERATE' >> test.c:5:14: note: while expanding macro 'SHIFTL' >> test.c:8:3: note: while expanding macro 'MULT' >> test.c:13:3: note: expanded from here >> > > In the absence of caret diagnostic I think it is useful to explicitely > make the difference between a location in the definition of the macro, > and the location of the macro expansion point, i.e: Agreed. But I also agree with Paolo's observation: the prefix "<locus>: note" has to precede the diagnostic message. I know there are other compilers that do it differently, but GCC convention has been to precede diagnostics with loci -- until we move to 2-dimensional diagnostic display (what is sometimes referred to as diagnostics with carets). -- Gaby ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/6] Emit macro expansion related diagnostics 2010-12-14 8:19 ` Gabriel Dos Reis @ 2010-12-14 8:31 ` Paolo Bonzini 2010-12-14 9:23 ` Dodji Seketeli 0 siblings, 1 reply; 28+ messages in thread From: Paolo Bonzini @ 2010-12-14 8:31 UTC (permalink / raw) To: Gabriel Dos Reis; +Cc: Dodji Seketeli, gcc-patches, tromey, joseph, lopezibanez On Tue, Dec 14, 2010 at 08:28, Gabriel Dos Reis <gdr@integrable-solutions.net> wrote: > Agreed. But I also agree with Paolo's observation: the prefix "<locus>: note" > has to precede the diagnostic message. I know there are other compilers > that do it differently, but GCC convention has been to precede diagnostics > with loci That was my main point, yes. I'm not sure about the verboseness of having two lines per level of macro expansion, but I can live with that and it was secondary in my message. Paolo ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/6] Emit macro expansion related diagnostics 2010-12-14 8:31 ` Paolo Bonzini @ 2010-12-14 9:23 ` Dodji Seketeli 0 siblings, 0 replies; 28+ messages in thread From: Dodji Seketeli @ 2010-12-14 9:23 UTC (permalink / raw) To: Paolo Bonzini; +Cc: Gabriel Dos Reis, gcc-patches, tromey, joseph, lopezibanez Paolo Bonzini <bonzini@gnu.org> writes: > On Tue, Dec 14, 2010 at 08:28, Gabriel Dos Reis > <gdr@integrable-solutions.net> wrote: >> Agreed. But I also agree with Paolo's observation: the prefix "<locus>: note" >> has to precede the diagnostic message. I know there are other compilers >> that do it differently, but GCC convention has been to precede diagnostics >> with loci > > That was my main point, yes. I'm not sure about the verboseness of > having two lines per level of macro expansion, but I can live with > that and it was secondary in my message. OK, this is consistent with the discussion we've just had about re-using functions like inform() in another sub-thread. I'll do that then. Thanks. -- Dodji ^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2011-10-18 17:39 UTC | newest] Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-10-18 15:25 [PATCH 3/6] Emit macro expansion related diagnostics David Edelsohn 2011-10-18 15:30 ` Joseph S. Myers 2011-10-18 15:37 ` Gabriel Dos Reis 2011-10-18 15:34 ` Jakub Jelinek 2011-10-18 16:56 ` Dodji Seketeli 2011-10-18 18:16 ` Dodji Seketeli -- strict thread matches above, loose matches on Subject: below -- 2011-10-17 9:58 [PATCH 0/6] Tracking locations of tokens resulting from macro expansion Dodji Seketeli 2011-10-17 9:58 ` [PATCH 3/6] Emit macro expansion related diagnostics Dodji Seketeli 2011-10-17 10:56 ` Richard Guenther 2011-10-17 12:22 ` Dodji Seketeli 2011-10-17 14:11 ` Dodji Seketeli 2011-10-17 17:41 ` H.J. Lu 2011-10-18 0:29 ` Dodji Seketeli 2011-10-18 6:07 ` Jason Merrill 2011-10-18 9:22 ` Dodji Seketeli 2010-12-10 11:27 [PATCH 0/6] Tracking locations of tokens resulting from macro expansion Dodji Seketeli 2010-12-10 11:27 ` [PATCH 3/6] Emit macro expansion related diagnostics Dodji Seketeli 2010-12-13 15:25 ` Paolo Bonzini 2010-12-13 15:38 ` Paolo Bonzini 2010-12-13 16:30 ` Manuel López-Ibáñez 2010-12-14 7:24 ` Dodji Seketeli 2010-12-14 7:28 ` Gabriel Dos Reis 2010-12-14 8:40 ` Dodji Seketeli 2010-12-14 9:38 ` Gabriel Dos Reis 2010-12-14 9:42 ` Dodji Seketeli 2010-12-14 9:48 ` Gabriel Dos Reis 2010-12-14 7:28 ` Dodji Seketeli 2010-12-14 8:19 ` Gabriel Dos Reis 2010-12-14 8:31 ` Paolo Bonzini 2010-12-14 9:23 ` Dodji Seketeli
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).