Jason Merrill <ja...@redhat.com> writes: > On 05/22/2012 05:04 AM, Dodji Seketeli wrote: >> The problem is that cpp_get_token_1 can be called when we are at the >> beginning of a macro expansion (inside enter_macro_expansion, called >> from cpp_get_token_1), *before* context->c.macro is set. This happens >> e.g, when we call funlike_invocation_p to know if the current macro is >> function-like or not. > > OK, sounds like we need some additional code to handle that. I guess > we could do something in funlike_invocation_p to prevent > cpp_get_token_1 from setting invocation_location, or change the check > we use to decide whether or not we already have an invocation > location, perhaps by looking at invocation_location itself (and > clearing it when we finish a macro).
So. After thinking about this a bit more, here is how I phrase the issue. There is a small interval of time between when we decide to start the expansion of a macro (when we get into enter_macro_context), and when we really start that expansion (when push the context of macro the macro) where we can recursively call cpp_get_token_1. In that time interval, cpp_get_token_1 might wrongly think that no macro expansion is in progress. And I think that's the issue root issue over which the cpp_reader::set_invocation_location flag was papering. It seems to me that a small and maintainable option to tackle this is to introduce a flag cpp_reader::about_to_expand_macro_p, that is 'on' during that time interval. A new predicate function in_macro_expansion_p then can now accurately tell cpp_get_token_1 if we are in the process of expanding a macro or not, doing away with the need for casual users of cpp_get_token_1 to set a flag to deal with macro expansion point handling. Tested and bootstrapped and tested on x86_64-unknown-linux-gnu against trunk. libcpp/ PR preprocessor/53229 * internal.h (cpp_reader::set_invocation_location): Remove. (cpp_reader::about_to_expand_macro_p): New member flag. * directives.c (do_pragma): Remove Kludge as pfile->set_invocation_location is no more. * macro.c (cpp_get_token_1): Do away with the use of cpp_reader::set_invocation_location. Just collect the macro expansion point when we are about to expand the top-most macro. Do not override cpp_reader::about_to_expand_macro_p. This fixes gcc.dg/cpp/paste12.c by making get_token_no_padding properly handle locations of expansion points. (cpp_get_token_with_location): Adjust, as cpp_reader::set_invocation_location is no more. (paste_tokens): Take a virtual location parameter for the LHS of the pasting operator. Use it in diagnostics. Update comments. (paste_all_tokens): Tighten the assert. Propagate the location of the expansion point when no virtual locations are available. Pass the virtual location to paste_tokens. (in_macro_expansion_p): New static function. (enter_macro_context): Set the cpp_reader::about_to_expand_macro_p flag until we really start expanding the macro. (_cpp_push_token_context, push_ptoken_context) (push_extended_tokens_context): Unset the cpp_reader::about_to_expand_macro_p flag when we push the macro context. gcc/testsuite/ PR preprocessor/53229 * gcc.dg/cpp/paste6.c: Force to run without -ftrack-macro-expansion. * gcc.dg/cpp/paste8.c: Likewise. * gcc.dg/cpp/paste8-2.c: New test, like paste8.c but run with -ftrack-macro-expansion. * gcc.dg/cpp/paste12.c: Force to run without -ftrack-macro-expansion. * gcc.dg/cpp/paste12-2.c: New test, like paste12.c but run with -ftrack-macro-expansion. * gcc.dg/cpp/paste13.c: Likewise. * gcc.dg/cpp/paste14.c: Likewise. * gcc.dg/cpp/paste14-2.c: New test, like paste14.c but run with -ftrack-macro-expansion. * gcc.dg/cpp/paste18.c: New test. --- gcc/testsuite/gcc.dg/cpp/paste12-2.c | 11 +++++ gcc/testsuite/gcc.dg/cpp/paste12.c | 5 ++- gcc/testsuite/gcc.dg/cpp/paste13.c | 5 ++- gcc/testsuite/gcc.dg/cpp/paste14-2.c | 11 +++++ gcc/testsuite/gcc.dg/cpp/paste14.c | 5 ++- gcc/testsuite/gcc.dg/cpp/paste18.c | 16 +++++++ gcc/testsuite/gcc.dg/cpp/paste6.c | 5 ++- gcc/testsuite/gcc.dg/cpp/paste8-2.c | 15 ++++++ gcc/testsuite/gcc.dg/cpp/paste8.c | 2 +- libcpp/directives.c | 30 +------------ libcpp/internal.h | 10 +++- libcpp/macro.c | 82 ++++++++++++++++++++++++++------- 12 files changed, 142 insertions(+), 55 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/cpp/paste12-2.c create mode 100644 gcc/testsuite/gcc.dg/cpp/paste14-2.c create mode 100644 gcc/testsuite/gcc.dg/cpp/paste18.c create mode 100644 gcc/testsuite/gcc.dg/cpp/paste8-2.c diff --git a/gcc/testsuite/gcc.dg/cpp/paste12-2.c b/gcc/testsuite/gcc.dg/cpp/paste12-2.c new file mode 100644 index 0000000..6e2e4f1 --- /dev/null +++ b/gcc/testsuite/gcc.dg/cpp/paste12-2.c @@ -0,0 +1,11 @@ +/* + { dg-options "-ftrack-macro-expansion=2" } + { dg-do preprocess } + */ + +/* Test correct diagnostics when pasting in #include. + Source: PR preprocessor/6780. */ + +#define inc2(a,b) <##a.b> /* { dg-error "pasting \"<\" and \"stdio\" does not" } */ +#define INC(X) inc2(X,h) +#include INC(stdio) diff --git a/gcc/testsuite/gcc.dg/cpp/paste12.c b/gcc/testsuite/gcc.dg/cpp/paste12.c index e61ec51..3e0f7b9 100644 --- a/gcc/testsuite/gcc.dg/cpp/paste12.c +++ b/gcc/testsuite/gcc.dg/cpp/paste12.c @@ -1,4 +1,7 @@ -/* { dg-do preprocess } */ +/* + { dg-options "-ftrack-macro-expansion=0" } + { dg-do preprocess } +*/ /* Test correct diagnostics when pasting in #include. Source: PR preprocessor/6780. */ diff --git a/gcc/testsuite/gcc.dg/cpp/paste13.c b/gcc/testsuite/gcc.dg/cpp/paste13.c index 62c72d4..f0f4fd8 100644 --- a/gcc/testsuite/gcc.dg/cpp/paste13.c +++ b/gcc/testsuite/gcc.dg/cpp/paste13.c @@ -1,6 +1,9 @@ /* Copyright (C) 2000 Free Software Foundation, Inc. */ -/* { dg-do preprocess } */ +/* + { dg-options "-ftrack-macro-expansion=0" } + { dg-do preprocess } +*/ /* This used to be recognized as a comment when lexing after pasting spellings. Neil Booth, 9 Oct 2002. */ diff --git a/gcc/testsuite/gcc.dg/cpp/paste14-2.c b/gcc/testsuite/gcc.dg/cpp/paste14-2.c new file mode 100644 index 0000000..3b23ada --- /dev/null +++ b/gcc/testsuite/gcc.dg/cpp/paste14-2.c @@ -0,0 +1,11 @@ +/* PR preprocessor/28709 */ +/* + { dg-options "-ftrack-macro-expansion=2" } + { dg-do preprocess } +*/ + +#define foo - ## >> /* { dg-error "pasting \"-\" and \">>\"" } */ +foo +#define bar = ## == /* { dg-error "pasting \"=\" and \"==\"" } */ +bar + diff --git a/gcc/testsuite/gcc.dg/cpp/paste14.c b/gcc/testsuite/gcc.dg/cpp/paste14.c index ec243c2..043d5e5 100644 --- a/gcc/testsuite/gcc.dg/cpp/paste14.c +++ b/gcc/testsuite/gcc.dg/cpp/paste14.c @@ -1,5 +1,8 @@ /* PR preprocessor/28709 */ -/* { dg-do preprocess } */ +/* + { dg-options "-ftrack-macro-expansion=0" } + { dg-do preprocess } +*/ #define foo - ## >> foo /* { dg-error "pasting \"-\" and \">>\"" } */ diff --git a/gcc/testsuite/gcc.dg/cpp/paste18.c b/gcc/testsuite/gcc.dg/cpp/paste18.c new file mode 100644 index 0000000..2888144 --- /dev/null +++ b/gcc/testsuite/gcc.dg/cpp/paste18.c @@ -0,0 +1,16 @@ +/* + { dg-options "-ftrack-macro-expansion=2" } + { dg-do compile } + */ + +struct x { + int i; +}; +struct x x; + +#define TEST(X) x.##X /* { dg-error "pasting\[^\n\r\]*does not give\[^\n\r\]*token" } */ + +void foo (void) +{ + TEST(i) = 0; +} diff --git a/gcc/testsuite/gcc.dg/cpp/paste6.c b/gcc/testsuite/gcc.dg/cpp/paste6.c index ac9ae39..a4e70e4 100644 --- a/gcc/testsuite/gcc.dg/cpp/paste6.c +++ b/gcc/testsuite/gcc.dg/cpp/paste6.c @@ -2,7 +2,10 @@ actual arguments. Original bug exposed by Linux kernel. Problem reported by Jakub Jelinek <ja...@redhat.com>. */ -/* { dg-do compile } */ +/* + { dg-options "-ftrack-macro-expansion=0" } + { dg-do compile } +*/ extern int foo(int x); diff --git a/gcc/testsuite/gcc.dg/cpp/paste8-2.c b/gcc/testsuite/gcc.dg/cpp/paste8-2.c new file mode 100644 index 0000000..c037e99 --- /dev/null +++ b/gcc/testsuite/gcc.dg/cpp/paste8-2.c @@ -0,0 +1,15 @@ +/* { dg-do preprocess } */ +/* { dg-options "-ftrack-macro-expansion=2" } */ + +int foo(int, ...); + +#define a(x, y...) foo(x, ##y) +a(1) +a(1, 2, 3) +#define b(x, y, z...) foo(x, ##y) /* { dg-error "valid preprocessing token" } */ +b(1, 2, 3) +#define c(x, y, z...) foo(x, ##z) +c(1, 2) +c(1, 2, 3) +#define d(x) fo(##x) /* { dg-error "valid preprocessing token" } */ +d(1) diff --git a/gcc/testsuite/gcc.dg/cpp/paste8.c b/gcc/testsuite/gcc.dg/cpp/paste8.c index ab01779..db1416c 100644 --- a/gcc/testsuite/gcc.dg/cpp/paste8.c +++ b/gcc/testsuite/gcc.dg/cpp/paste8.c @@ -1,5 +1,5 @@ /* { dg-do preprocess } */ -/* { dg-options "" } */ +/* { dg-options "-ftrack-macro-expansion=0" } */ int foo(int, ...); diff --git a/libcpp/directives.c b/libcpp/directives.c index e46280e..66fa66d 100644 --- a/libcpp/directives.c +++ b/libcpp/directives.c @@ -1362,35 +1362,7 @@ do_pragma (cpp_reader *pfile) { bool allow_name_expansion = p->allow_expansion; if (allow_name_expansion) - { - pfile->state.prevent_expansion--; - /* - Kludge ahead. - - Consider this code snippet: - - #define P parallel - #pragma omp P for - ... a for loop ... - - Once we parsed the 'omp' namespace of the #pragma - directive, we then parse the 'P' token that represents the - pragma name. P being a macro, it is expanded into the - resulting 'parallel' token. - - At this point the 'p' variable contains the 'parallel' - pragma name. And pfile->context->macro is non-null - because we are still right at the end of the macro - context of 'P'. The problem is, if we are being - (indirectly) called by cpp_get_token_with_location, - that function might test pfile->context->macro to see - if we are in the context of a macro expansion, (and we - are) and then use pfile->invocation_location as the - location of the macro invocation. So we must instruct - cpp_get_token below to set - pfile->invocation_location. */ - pfile->set_invocation_location = true; - } + pfile->state.prevent_expansion--; token = cpp_get_token (pfile); if (token->type == CPP_NAME) diff --git a/libcpp/internal.h b/libcpp/internal.h index 5b3731b..37aac82 100644 --- a/libcpp/internal.h +++ b/libcpp/internal.h @@ -413,9 +413,13 @@ struct cpp_reader macro invocation. */ source_location invocation_location; - /* True if this call to cpp_get_token should consider setting - invocation_location. */ - bool set_invocation_location; + /* Nonzero if we are about to expand a macro. Note that if we are + really expanding a macro, the function macro_of_context returns + the macro being expanded and this flag is set to false. Client + code should use the function in_macro_expansion_p to know if we + are either about to expand a macro, or are actually expanding + one. */ + bool about_to_expand_macro_p; /* Search paths for include files. */ struct cpp_dir *quote_include; /* "" */ diff --git a/libcpp/macro.c b/libcpp/macro.c index c4e2a23..48f2473 100644 --- a/libcpp/macro.c +++ b/libcpp/macro.c @@ -100,7 +100,8 @@ static void expand_arg (cpp_reader *, macro_arg *); static const cpp_token *new_string_token (cpp_reader *, uchar *, unsigned int); static const cpp_token *stringify_arg (cpp_reader *, macro_arg *); static void paste_all_tokens (cpp_reader *, const cpp_token *); -static bool paste_tokens (cpp_reader *, const cpp_token **, const cpp_token *); +static bool paste_tokens (cpp_reader *, source_location, + const cpp_token **, const cpp_token *); static void alloc_expanded_arg_mem (cpp_reader *, macro_arg *, size_t); static void ensure_expanded_arg_room (cpp_reader *, macro_arg *, size_t, size_t *); static void delete_macro_args (_cpp_buff*, unsigned num_args); @@ -167,6 +168,8 @@ static const cpp_token* cpp_get_token_1 (cpp_reader *, source_location *); static cpp_hashnode* macro_of_context (cpp_context *context); +static bool in_macro_expansion_p (cpp_reader *pfile); + /* Statistical counter tracking the number of macros that got expanded. */ unsigned num_expanded_macros_counter = 0; @@ -544,9 +547,11 @@ stringify_arg (cpp_reader *pfile, macro_arg *arg) /* Try to paste two tokens. On success, return nonzero. In any case, PLHS is updated to point to the pasted token, which is - guaranteed to not have the PASTE_LEFT flag set. */ + guaranteed to not have the PASTE_LEFT flag set. LOCATION is + the virtual location used for error reporting. */ static bool -paste_tokens (cpp_reader *pfile, const cpp_token **plhs, const cpp_token *rhs) +paste_tokens (cpp_reader *pfile, source_location location, + const cpp_token **plhs, const cpp_token *rhs) { unsigned char *buf, *end, *lhsend; cpp_token *lhs; @@ -590,7 +595,7 @@ paste_tokens (cpp_reader *pfile, const cpp_token **plhs, const cpp_token *rhs) /* Mandatory error for all apart from assembler. */ if (CPP_OPTION (pfile, lang) != CLK_ASM) - cpp_error (pfile, CPP_DL_ERROR, + cpp_error_with_line (pfile, CPP_DL_ERROR, location, 0, "pasting \"%s\" and \"%s\" does not give a valid preprocessing token", buf, cpp_token_as_text (pfile, rhs)); return false; @@ -615,9 +620,10 @@ paste_all_tokens (cpp_reader *pfile, const cpp_token *lhs) cpp_context *context = pfile->context; source_location virt_loc = 0; - /* We must have been called on a token that appears at the left - hand side of a ## operator. */ - if (!(lhs->flags & PASTE_LEFT)) + /* We are expanding a macro and we must have been called on a token + that appears at the left hand side of a ## operator. */ + if (macro_of_context (pfile->context) == NULL + || (!(lhs->flags & PASTE_LEFT))) abort (); if (context->tokens_kind == TOKENS_KIND_EXTENDED) @@ -628,6 +634,11 @@ paste_all_tokens (cpp_reader *pfile, const cpp_token *lhs) resulting pasted token to have the location of the current *LHS, though. */ virt_loc = context->c.mc->cur_virt_loc[-1]; + else + /* We are not tracking macro expansion. So the best virtual + location we can get here is the expansion point of the macro we + are currently expanding. */ + virt_loc = pfile->invocation_location; do { @@ -661,7 +672,7 @@ paste_all_tokens (cpp_reader *pfile, const cpp_token *lhs) if (rhs->flags & PASTE_LEFT) abort (); } - if (!paste_tokens (pfile, &lhs, rhs)) + if (!paste_tokens (pfile, virt_loc, &lhs, rhs)) break; } while (rhs->flags & PASTE_LEFT); @@ -1018,6 +1029,17 @@ enter_macro_context (cpp_reader *pfile, cpp_hashnode *node, pfile->state.angled_headers = false; + /* From here to when we push the context for the macro later down + this function, we need to flag the fact that we are about to + expand a macro. This is useful when -ftrack-macro-expansion is + turned off. In that case, we need to record the location of the + expansion point of the top-most macro we are about to to expand, + into pfile->invocation_location. But we must not record any such + location once the process of expanding the macro starts; that is, + we must not do that recording between now and later down this + function where set this flag to FALSE. */ + pfile->about_to_expand_macro_p = true; + if ((node->flags & NODE_BUILTIN) && !(node->flags & NODE_USED)) { node->flags |= NODE_USED; @@ -1057,6 +1079,7 @@ enter_macro_context (cpp_reader *pfile, cpp_hashnode *node, if (pragma_buff) _cpp_release_buff (pfile, pragma_buff); + pfile->about_to_expand_macro_p = false; return 0; } @@ -1804,6 +1827,10 @@ push_ptoken_context (cpp_reader *pfile, cpp_hashnode *macro, _cpp_buff *buff, cpp_context *context = next_context (pfile); context->tokens_kind = TOKENS_KIND_INDIRECT; + + if (macro != NULL) + pfile->about_to_expand_macro_p = false; + context->c.macro = macro; context->buff = buff; FIRST (context).ptoken = first; @@ -1825,6 +1852,9 @@ _cpp_push_token_context (cpp_reader *pfile, cpp_hashnode *macro, if (macro == NULL) macro = macro_of_context (pfile->context); + if (macro != NULL) + pfile->about_to_expand_macro_p = false; + context = next_context (pfile); context->tokens_kind = TOKENS_KIND_DIRECT; context->c.macro = macro; @@ -1858,6 +1888,9 @@ push_extended_tokens_context (cpp_reader *pfile, if (macro == NULL) macro = macro_of_context (pfile->context); + if (macro != NULL) + pfile->about_to_expand_macro_p = false; + context = next_context (pfile); context->tokens_kind = TOKENS_KIND_EXTENDED; context->buff = token_buff; @@ -2143,6 +2176,20 @@ macro_of_context (cpp_context *context) : context->c.macro; } +/* Return TRUE iff we are expanding a macro or are about to start + expanding one. If we are effectively expanding a macro, the + function macro_of_context returns a pointer to the macro being + expanded. */ +static bool +in_macro_expansion_p (cpp_reader *pfile) +{ + if (pfile == NULL) + return false; + + return (pfile->about_to_expand_macro_p + || macro_of_context (pfile->context)); +} + /* Pop the current context off the stack, re-enabling the macro if the context represented a macro's replacement list. Initially the context structure was not freed so that we can re-use it later, but @@ -2298,11 +2345,13 @@ static const cpp_token* cpp_get_token_1 (cpp_reader *pfile, source_location *location) { const cpp_token *result; - bool can_set = pfile->set_invocation_location; /* This token is a virtual token that either encodes a location related to macro expansion or a spelling location. */ source_location virt_loc = 0; - pfile->set_invocation_location = false; + /* pfile->about_to_expand_macro_p can be overriden by indirect calls + to functions that push macro contexts. So let's save it so that + we can restore it when we are about to leave this routine. */ + bool saved_about_to_expand_macro = pfile->about_to_expand_macro_p; for (;;) { @@ -2355,7 +2404,7 @@ cpp_get_token_1 (cpp_reader *pfile, source_location *location) int ret = 0; /* If not in a macro context, and we're going to start an expansion, record the location. */ - if (can_set && !context->c.macro) + if (!in_macro_expansion_p (pfile)) pfile->invocation_location = result->src_loc; if (pfile->state.prevent_expansion) break; @@ -2423,8 +2472,7 @@ cpp_get_token_1 (cpp_reader *pfile, source_location *location) *location = virt_loc; if (!CPP_OPTION (pfile, track_macro_expansion) - && can_set - && pfile->context->c.macro != NULL) + && macro_of_context (pfile->context) != NULL) /* We are in a macro expansion context, are not tracking virtual location, but were asked to report the location of the expansion point of the macro being expanded. */ @@ -2432,6 +2480,8 @@ cpp_get_token_1 (cpp_reader *pfile, source_location *location) *location = maybe_adjust_loc_for_trad_cpp (pfile, *location); } + + pfile->about_to_expand_macro_p = saved_about_to_expand_macro; return result; } @@ -2493,11 +2543,7 @@ cpp_get_token (cpp_reader *pfile) const cpp_token * cpp_get_token_with_location (cpp_reader *pfile, source_location *loc) { - const cpp_token *result; - - pfile->set_invocation_location = true; - result = cpp_get_token_1 (pfile, loc); - return result; + return cpp_get_token_1 (pfile, loc); } /* Returns true if we're expanding an object-like macro that was -- Dodji