Message ID | m3y5xk8whp.fsf@redhat.com |
---|---|
State | New |
Headers | show |
On 09/19/2011 09:58 AM, Dodji Seketeli wrote: > + The part that goes from the third to the heighth line of this An extra 'h' snuck in there. :) > Inside the definition of a macro M, you can have another macro M'. > And M' is going to be eventually expanded into a token FOO. > > So, to paraphrase what I said earlier, FOO [which is at a point in the > definition of the macro M] is itself inside the expanded macro M'. So what we're dealing with here is the token FOO. We start with its fully-expanded location, and then step out to see where it was expanded from. If that location is still within a macro expansion, we step out again until we are at the point in the source that originally triggered a macro expansion. Right? I don't understand how that unwinding operation maps onto a function called linemap_macro_map_loc_to_def_point, and why we need to use both that function and linemap_macro_map_loc_to_exp_point here. I'm afraid this is leading me to question the basic model again. > + 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 "1.0" has the locations 2(OPRD1), 5(A), 8(A), 13(1.0). "<<" has the locations 2(OPRT), 5(<<), 8(SHIFTL), 13(MULT). Each token has 4 locations, one for each of the macros involved plus one for the original expansion location. So it seems like we ought to be able to get away with only storing one location per token in a macro expansion map. Am I missing something? Jason
On 09/19/2011 02:29 PM, Jason Merrill wrote: > expansion location. So it seems like we ought to be able to get away > with only storing one location per token in a macro expansion map. Am I > missing something? I notice that here: > + /* 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_PARM_REPLACEMENT_POINT, 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_PARM_REPLACEMENT_POINT, NULL); You're using LRK_MACRO_PARM_REPLACEMENT_POINT in both places for printing (and thus the second of the two token locations), whereas you used the first location in the unwinding process. It is sounding to me like the first location (xI) gets you the next virtual location in the unwinding process, whereas the second location (yI) gets you the spelling location of the token in the definition of a macro. Certainly all the calls to tokens_buff_add_token pass src->src_loc for the second. So why don't we look up the second location in the macro definition when we need it rather than store a copy in the map? Jason
Jason Merrill <jason@redhat.com> writes: > On 09/19/2011 09:58 AM, Dodji Seketeli wrote: > > + The part that goes from the third to the heighth line of this > > An extra 'h' snuck in there. :) Oops, fixed in my local copy, sorry. > > > Inside the definition of a macro M, you can have another macro M'. > > And M' is going to be eventually expanded into a token FOO. > > > > So, to paraphrase what I said earlier, FOO [which is at a point in the > > definition of the macro M] is itself inside the expanded macro M'. > > So what we're dealing with here is the token FOO. We start with its > fully-expanded location, and then step out to see where it was > expanded from. If that location is still within a macro expansion, we > step out again until we are at the point in the source that originally > triggered a macro expansion. Right? Right. > I don't understand how that unwinding operation maps onto a function > called linemap_macro_map_loc_to_def_point, In the example I used in the comment of that function, suppose that during 'stepping out', as you put it, we don't deal with the place in the source where '<<' appears in the definition of a macro. We'd then only deal with the expansion points of the macros involved. That would mean that we would only record in the trace the following locations (and be able to only display this trace): 1/ test.c:5:14: error: invalid operands to binary << (have 'double' and 'int') test.c:5:3: note: expanded from here test.c:8:3: note: expanded from here test.c:13:3: note: expanded from here But I also want to display where "<<" appears in the definition of the relevant macro ... > and why we need to use both that function and > linemap_macro_map_loc_to_exp_point here. ... so that I can have a trace that shows the locations also in the context of the definitions of the macros: 2/ 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 > > I'm afraid this is leading me to question the basic model again. > > > + 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 "1.0" has the locations 2(OPRD1), 5(A), 8(A), 13(1.0). "<<" > has the locations 2(OPRT), 5(<<), 8(SHIFTL), 13(MULT). Each token > has 4 locations, one for each of the macros involved plus one for > the original expansion location. So it seems like we ought to be > able to get away with only storing one location per token in a macro > expansion map. This is essentially what we do, for tokens of macro replacement list that are not macro parameters. In a function-like macro, tokens of macro parameters are replaced by the tokens of the arguments; if we just store the spelling location of a given parameter (and forget about the location of the matching argument as you are suggesting), getting the location of that argument at virtual location resolution time can be difficult. In your example, suppose we didn't store the location of "<<" inside the map for OPERATE, and that we only stored the location (2:9) of OPRT, as you suggest. How, to generate the trace, would we step out to get the location (5:14) (of "<<") from that (2:9)? For that, I guess we'd need to store the locations of the arguments of OPERATE, somehow, figure out that OPRT is the parameter for the argument "<<" and act from there. This is the argument replacement replay I talked about in another thread when you first raised this point. We'd need to do some parts of what replace_args does and handle cases like pasting etc. Tom and I figured this would be a bit more difficult so we preferred staying with this simpler scheme for now. The simpler scheme being to store the location of the argument "<<" and the location of the parameter OPRT in the map.
Jason Merrill <jason@redhat.com> writes: > It is sounding to me like the first location (xI) gets you the next > virtual location in the unwinding process, whereas the second location > (yI) gets you the spelling location of the token in the definition of > a macro. Right. > Certainly all the calls to tokens_buff_add_token pass src->src_loc for > the second. So why don't we look up the second location in the macro > definition when we need it rather than store a copy in the map? Because when you have the first location, looking up the second is not easy. Note that the information about the arguments of a function-like macro is freed right in enter_macro_context by delete_macro_args once we have recorded the locations for the macro expansion. Getting the src that matches the loc of a token coming from an argument of the macro expansion, once that argument has been freed is not easier than just storing a copy of the src->src_loc we need. So Tom and I decided to let that optimization for later once we are sure the whole thing works and performs well enough.
On 09/20/2011 03:23 AM, Dodji Seketeli wrote: > Jason Merrill<jason@redhat.com> writes: >> Certainly all the calls to tokens_buff_add_token pass src->src_loc for >> the second. So why don't we look up the second location in the macro >> definition when we need it rather than store a copy in the map? > > Because when you have the first location, looking up the second is not > easy. In linemap_macro_map_loc_to_def_point you get the token number and then use that to index into MACRO_MAP_LOCATIONS. Can't you use the same token number to index into macro->exp.tokens instead? Jason
Jason Merrill <jason@redhat.com> writes: > On 09/20/2011 03:23 AM, Dodji Seketeli wrote: >> Jason Merrill<jason@redhat.com> writes: > >>> Certainly all the calls to tokens_buff_add_token pass src->src_loc for >>> the second. So why don't we look up the second location in the macro >>> definition when we need it rather than store a copy in the map? >> >> Because when you have the first location, looking up the second is not >> easy. > > In linemap_macro_map_loc_to_def_point you get the token number and > then use that to index into MACRO_MAP_LOCATIONS. Can't you use the > same token number to index into macro->exp.tokens instead? No, because a macro argument can be made of several tokens. So after the first macro parameter of the replacement-list has been replaced by the tokens of the argument, there is a shift between the indexes of the tokens resulting from the replacement, and the original tokens of the macro replacement list.
On 09/20/2011 10:05 AM, Dodji Seketeli wrote: > Jason Merrill<jason@redhat.com> writes: >> In linemap_macro_map_loc_to_def_point you get the token number and >> then use that to index into MACRO_MAP_LOCATIONS. Can't you use the >> same token number to index into macro->exp.tokens instead? > No, because a macro argument can be made of several tokens. Ah, I see. >> I don't understand how that unwinding operation maps onto a function >> called linemap_macro_map_loc_to_def_point, > In the example I used in the comment of that function, suppose that > during 'stepping out', as you put it, we don't deal with the place in > the source where '<<' appears in the definition of a macro. We'd then > only deal with the expansion points of the macros involved. I think I wasn't expressing clearly enough the point I was trying to make. My point was more that the name of the function is confusing; if what you get back is another virtual location, that's not what I would consider a "def point". The only time you get a source location in the actual definition of the macro is when you ask for the "macro parm usage point". And so when you go to print actual diagnostics you use LRK_MACRO_PARM_REPLACEMENT_POINT. When we start out, we have a virtual location that represents << in the expansion of OPERATE. Then we call linemap_macro_map_loc_to_def_point to get a virtual location that represents << in the expansion of SHIFTL. This is not part of the definition of OPERATE, and shouldn't be described as such. It seems that this function steps out until we hit the spelling location, and then we have a real source location and stop, which is why the loop in maybe_unwind_expanded_macro_loc needs to use linemap_macro_map_loc_to_exp_point as well. Right? In the example, once we hit << in SHIFTL unwinding needs to switch to the expansion point. It seems to me that we should encapsulate all of this in a function that expresses this unwinding operation, say "linemap_macro_map_loc_unwind_once". So the loop would look like + 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 in 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_once (where, &map); + } while (linemap_macro_expansion_map_p (map)); I think that linemap_macro_loc_to_def_point when called with false returns the unwound spelling location of the token (and thus is used for LRK_SPELLING_LOCATION), or when called with true returns the (not-unwound) location in the expanded macro (and so I think LRK_MACRO_PARM_REPLACEMENT_POINT should be renamed to LRK_MACRO_EXPANDED_LOCATION or something similar). It seems to me that either we should split those functions apart in to two functions with clearer names, or bundle them and linemap_macro_loc_to_exp_point into linemap_resolve_location; if linemap_location_in_system_header_p used linemap_resolve_location it would be more readable anyway. I'm having trouble thinking of a less misleading name for linemap_macro_map_loc_to_def_point, since the two locations serve completely different purposes. Maybe something vague like linemap_macro_map_loc_get_stored_loc. Or split it into two functions like linemap_virtual_loc_unwind_once_toward_spelling and linemap_virtual_loc_get_expanded_location or something like that. What do you think? Jason
Jason Merrill <jason@redhat.com> writes: > My point was more that the name of the function is confusing; Sorry about that. > if what you get back is another virtual location, that's not what I > would consider a "def point". For tokens that are not function-like macro arguments, I think the linemap_macro_loc_to_def_point makes sense because what we get then is the source location in the actual definition of the macro. Actually I think that's where the confusion comes from. I first devised the name while thinking of the case of macros that are not function-like. And now it falls short for the general case. I should have caught that but for some reason I was just seeing through the name is if it was transparent. Sigh. I take your point; that name doesn't make sense in the general case. > The only time you get a source location in the actual definition of > the macro is when you ask for the "macro parm usage point". Yes that, and in the case above; in which case xI and yI are equal, by the way. > When we start out, we have a virtual location that represents << in > the expansion of OPERATE. Then we call > linemap_macro_map_loc_to_def_point to get a virtual location that > represents << in the expansion of SHIFTL. This is not part of the > definition of OPERATE, and shouldn't be described as such. Agreed. > It seems that this function steps out until we hit the spelling > location, and then we have a real source location and stop, which is > why the loop in maybe_unwind_expanded_macro_loc needs to use > linemap_macro_map_loc_to_exp_point as well. Right? Right. > It seems to me that we should encapsulate all of this in a function > that expresses this unwinding operation, say > "linemap_macro_map_loc_unwind_once". So the loop would look like > > + 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 in 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_once (where, &map); > + } while (linemap_macro_expansion_map_p (map)); > OK. > I think that linemap_macro_loc_to_def_point when called with false > returns the unwound spelling location of the token (and thus is used > for LRK_SPELLING_LOCATION), Right. > or when called with true returns the (not-unwound) location in the > expanded macro (and so I think LRK_MACRO_PARM_REPLACEMENT_POINT > should be renamed to LRK_MACRO_EXPANDED_LOCATION or something > similar). FWIW, I'd like the LRK_MACRO_PARM_REPLACEMENT name (or its replacement. ha ha) to hint at the fact that it really has to do with a token that is an /argument/ for a function-like macro. So maybe LRK_MACRO_PARM_FOR_ARG_LOCATION? LRK_MACRO_EXPANDED_LOCATION really seems too vague to me. After all, pretty much everything is about *EXPAND*ing macros here. :-) > It seems to me that either we should split those functions apart in > to two functions with clearer names, or bundle them and > linemap_macro_loc_to_exp_point into linemap_resolve_location; if > linemap_location_in_system_header_p used linemap_resolve_location it > would be more readable anyway. OK. > I'm having trouble thinking of a less misleading name for > linemap_macro_map_loc_to_def_point, since the two locations serve > completely different purposes. Maybe something vague like > linemap_macro_map_loc_get_stored_loc. Or split it into two > functions like linemap_virtual_loc_unwind_once_toward_spelling and > linemap_virtual_loc_get_expanded_location or something like that. So would a function named linemap_macro_map_loc_to_def_point that returns the second location (yI) only, and a second function linemap_macro_map_loc_unwind_once be less confusing to you? If yes, then I'll send an updated patch for that in a short while. Thanks.
On 09/21/2011 02:32 PM, Dodji Seketeli wrote: > FWIW, I'd like the LRK_MACRO_PARM_REPLACEMENT name (or its > replacement. ha ha) to hint at the fact that it really has to do with > a token that is an /argument/ for a function-like macro. I disagree; arguments are the situation when the two locations differ, but this one (yI) is always the source location in the macro definition, while the first one (xI) is either that or, for macro arguments, a virtual location. So the association with arguments seems backwards to me. > So would a function named linemap_macro_map_loc_to_def_point that > returns the second location (yI) only, and a second function > linemap_macro_map_loc_unwind_once be less confusing to you? Yes, that sounds good. Jason
Jason Merrill <jason@redhat.com> writes: > On 09/21/2011 02:32 PM, Dodji Seketeli wrote: >> FWIW, I'd like the LRK_MACRO_PARM_REPLACEMENT name (or its >> replacement. ha ha) to hint at the fact that it really has to do with >> a token that is an /argument/ for a function-like macro. > > I disagree; arguments are the situation when the two locations differ, > but this one (yI) is always the source location in the macro > definition I think an interesting question to ask here is "the source location of what exactly?". The value of the source location is always the same, yes, but its meaning is different depending on if the token in question is an argument or not. When the token is not an argument, then yI is a source location for *that* token in the macro definition. It's then the spelling location for the token in question. When the token is an argument, then yI is a source location for *another* token. Namely, the parameter for that argument. Not necessarily the spelling location for the token we are observing. > while the first one (xI) is either that or, for macro arguments, a > virtual location. Yes, but it's always a locus of the observed token. Would LRK_MACRO_DEFINITION_LOCATION be better then? :-)
On 09/26/2011 04:21 PM, Dodji Seketeli wrote: > Jason Merrill<jason@redhat.com> writes: > >> On 09/21/2011 02:32 PM, Dodji Seketeli wrote: >>> FWIW, I'd like the LRK_MACRO_PARM_REPLACEMENT name (or its >>> replacement. ha ha) to hint at the fact that it really has to do with >>> a token that is an /argument/ for a function-like macro. >> >> I disagree; arguments are the situation when the two locations differ, >> but this one (yI) is always the source location in the macro >> definition > > I think an interesting question to ask here is "the source location of > what exactly?". It's the source location of what we're currently looking at, which is a virtual token in the macro expansion. My point is that yI is always a source location, whereas xI may or may not be. > Would LRK_MACRO_DEFINITION_LOCATION be better then? :-) Yes. :) Jason
diff --git a/gcc/Makefile.in b/gcc/Makefile.in index 92016f2..d26c682 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -2796,7 +2796,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 598ddf1..8fa163f 100644 --- a/gcc/cp/error.c +++ b/gcc/cp/error.c @@ -2767,7 +2767,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); @@ -2777,8 +2777,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..0344937 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_PARM_REPLACEMENT_POINT, + &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 de0a58a..5f63b69 100644 --- a/gcc/toplev.c +++ b/gcc/toplev.c @@ -1132,6 +1132,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..5a7e6c2 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,212 @@ 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 heighth 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, *resolved_map; + source_location resolved_location; + 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 + *definition* of the macro of MAP, that got expanded at WHERE. + This is basically how we go "down" in the trace 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); + + /* In the definition of a macro M, we can have another macro M'. + M' is eventually going to be expanded into a token FOO, which + location is RESOLVED_LOCATION. MAP would be the map of M. + + So if we are in this case, i.e If FOO is itself inside an + expanded macro M' then we keep unwinding the trace to reach + the "parent macro" M'. RESOLVED_MAP would then be the map of + M'. */ + 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 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); + } + } 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_PARM_REPLACEMENT_POINT, 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_PARM_REPLACEMENT_POINT, 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 */