PR preprocessor/53229 - Fix diagnostics location when pasting tokens

As stated in the audit trail of this problem report, consider this
test case:

    $ cat test.c
	 1	struct x {
	 2	  int i;
	 3	};
	 4	struct x x;
	 5
	 6	#define TEST(X) x.##X
	 7
	 8	void foo (void)
	 9	{
	10	  TEST(i) = 0;
	11	}
    $

    $ cc1 -quiet test.c
    test.c: In function 'foo':
    test.c:10:1: error: pasting "." and "i" does not give a valid preprocessing token
       TEST(i) = 0;
     ^
    $

So, when pasting tokens, the error diagnostic uses the global and
imprecise input_location variable, leading to an imprecise output.

To properly fix this, I think libcpp should keep the token of the
pasting operator '##', instead of representing it with flag on the LHS
operand's token.  That way, it could use its location.  Doing that
would be quite intrusive though.  So this patch just uses the location
of the LHS of the pasting operator, for now.  It's IMHO better than
the current situation.

The patch makes paste_tokens take a location parameter that is used in
the diagnostics.  This change can still be useful later when we can
use the location of the pasting operator, because paste_tokens will
just be passed the new, more precise location.

Incidentally, it appeared that when getting tokens from within
preprocessor directives (like what is done in gcc.dg/cpp/paste12.c),
with -ftrack-macro-expansion disabled, the location of the expansion
point of macros was being lost because
cpp_reader::set_invocation_location wasn't being properly set.  It's
because when cpp_get_token_1 calls enter_macro_context, there is a
little period of time between the beginning of that later function and
when the macro is really pushed (and thus when the macro is really
expanded) where we wrongly consider that we are not expanding the
macro because macro_of_context is still NULL.  In that period of time,
in the occurrences of indirect recursive calls to cpp_get_token_1,
this later function wrongly sets cpp_reader::invocation_location
because cpp_reader::set_invocation_location is not being properly set.

To avoid that confusion the patch does away with
cpp_reader::set_invocation_location and introduces a new flag
cpp_reader::about_to_expand_macro_p that is set in the small time
interval exposed earlier.  A new in_macro_expansion_p is introduced as
well, so that cpp_get_token_1 can now accurately detect when we are in
the process of expanding a macro, and thus correctly collect the
location of the expansion point.

People seem to like screenshots.

Thus, after the patch, we now have:

    $ cc1 -quiet test.c
    test.c: In function 'foo':
    test.c:6:18: error: pasting "." and "i" does not give a valid preprocessing token
     #define TEST(X) x.##X
		      ^
    test.c:10:3: note: in expansion of macro 'TEST'
       TEST(i) = 0;
       ^
    $

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.

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.

From-SVN: r187945
This commit is contained in:
Dodji Seketeli 2012-05-29 09:36:29 +00:00 committed by Dodji Seketeli
parent 0de9dab58d
commit 828a7f76c7
14 changed files with 178 additions and 55 deletions

View File

@ -1,3 +1,21 @@
2012-05-29 Dodji Seketeli <dodji@redhat.com>
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.
2012-05-29 Hans-Peter Nilsson <hp@axis.com>
* gcc.target/cris/torture/trap-1.c,

View File

@ -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)

View File

@ -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. */

View File

@ -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. */

View File

@ -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

View File

@ -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 \">>\"" } */

View File

@ -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;
}

View File

@ -2,7 +2,10 @@
actual arguments. Original bug exposed by Linux kernel. Problem
reported by Jakub Jelinek <jakub@redhat.com>. */
/* { dg-do compile } */
/*
{ dg-options "-ftrack-macro-expansion=0" }
{ dg-do compile }
*/
extern int foo(int x);

View File

@ -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)

View File

@ -1,5 +1,5 @@
/* { dg-do preprocess } */
/* { dg-options "" } */
/* { dg-options "-ftrack-macro-expansion=0" } */
int foo(int, ...);

View File

@ -1,3 +1,28 @@
2012-05-29 Dodji Seketeli <dodji@redhat.com>
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.
2012-05-24 Dodji Seketeli <dodji@redhat.com>
PR bootstrap/53459

View File

@ -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)

View File

@ -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; /* "" */

View File

@ -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;
}
@ -1146,12 +1169,15 @@ enter_macro_context (cpp_reader *pfile, cpp_hashnode *node,
}
while (pragma_buff != NULL);
pfile->about_to_expand_macro_p = false;
return 2;
}
pfile->about_to_expand_macro_p = false;
return 1;
}
pfile->about_to_expand_macro_p = false;
/* Handle built-in macros and the _Pragma operator. */
return builtin_macro (pfile, node);
}
@ -2143,6 +2169,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 +2338,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 +2397,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 +2465,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 +2473,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 +2536,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