[PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens

[PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens

[Dodji Seketeli]

Hello,

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 get_token_no_padding doesn't
use cpp_get_token_with_location.  The patch fixes that as well.

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
	* directives.c (get_token_no_padding): Allow use of virtual
	locations in diagnostics emitted by the processor when handling
	directives.  Update comments.  This fixes gcc.dg/cpp/paste12.c.
	* macro.c (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.

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                  |    7 +++++--
 libcpp/macro.c                       |   26 ++++++++++++++++++--------
 11 files changed, 93 insertions(+), 15 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 <jakub@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..e161810 100644
--- a/libcpp/directives.c
+++ b/libcpp/directives.c
@@ -1660,13 +1660,16 @@ do_pragma_dependency (cpp_reader *pfile)
   free ((void *) fname);
 }
 
-/* Get a token but skip padding.  */
+/* Get a token but skip padding.
+
+   Handles macro expansion and allows the use of virtual locations in
+   diagnostics emitted by the preprocessor.  */
 static const cpp_token *
 get_token_no_padding (cpp_reader *pfile)
 {
   for (;;)
     {
-      const cpp_token *result = cpp_get_token (pfile);
+      const cpp_token *result = cpp_get_token_with_location (pfile, NULL);
       if (result->type != CPP_PADDING)
 	return result;
     }
diff --git a/libcpp/macro.c b/libcpp/macro.c
index c4e2a23..7d71ae1 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);
@@ -544,9 +545,12 @@ 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.  LHS_LOCATION is
+   the virtual location for the LHS token pointed to by PLHS.  This
+   is 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 lhs_location,
+	      const cpp_token **plhs, const cpp_token *rhs)
 {
   unsigned char *buf, *end, *lhsend;
   cpp_token *lhs;
@@ -590,7 +594,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, lhs_location, 0,
 	 "pasting \"%s\" and \"%s\" does not give a valid preprocessing token",
 		   buf, cpp_token_as_text (pfile, rhs));
       return false;
@@ -615,9 +619,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 +633,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 +671,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);
-- 
		Dodji

Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens

[Manuel López-Ibáñez]

On 15 May 2012 13:18, Dodji Seketeli <dodji@redhat.com> wrote:
>
> 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;
>       ^
>    $

I like the screenshot. ;-)

Thanks for fixing this!

Cheers,

Manuel.

Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens

[Jason Merrill]

On 05/15/2012 07:18 AM, Dodji Seketeli wrote:
> +paste_tokens (cpp_reader *pfile, source_location lhs_location,

If in the long run we want the location passed in to be the ## location, 
let's drop the "lhs" from the parameter name.

> -      const cpp_token *result = cpp_get_token (pfile);
> +      const cpp_token *result = cpp_get_token_with_location (pfile, NULL);

I find the difference between these two functions confusing, since you 
aren't passing in a pointer for the location to go into.  I see that 
cpp_get_token_with_location sets pfile->set_invocation_location, which 
is documented to mean

>   /* When expanding a macro at top-level, this is the location of the
>      macro invocation.  */
>   source_location invocation_location;
>
>   /* True if this call to cpp_get_token should consider setting
>      invocation_location.  */
>   bool set_invocation_location;

But presumably get_token_no_padding isn't only called when we're 
starting to expand a top-level macro.  And as far as I can tell the 
value of invocation_location is only used when we aren't tracking 
virtual locations anyway.  So why does the change above fix the testcase?

Jason

Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens

[Dodji Seketeli]

Jason Merrill <jason@redhat.com> writes:

> On 05/15/2012 07:18 AM, Dodji Seketeli wrote:
>> +paste_tokens (cpp_reader *pfile, source_location lhs_location,
>
> If in the long run we want the location passed in to be the ##
> location, let's drop the "lhs" from the parameter name.

Dropped.

>
>> -      const cpp_token *result = cpp_get_token (pfile);
>> +      const cpp_token *result = cpp_get_token_with_location (pfile, NULL);
>
> I find the difference between these two functions confusing, since you
> aren't passing in a pointer for the location to go into.  I see that
> cpp_get_token_with_location sets pfile->set_invocation_location, which
> is documented to mean
>
>>   /* When expanding a macro at top-level, this is the location of the
>>      macro invocation.  */
>>   source_location invocation_location;
>>
>>   /* True if this call to cpp_get_token should consider setting
>>      invocation_location.  */
>>   bool set_invocation_location;
>
> But presumably get_token_no_padding isn't only called when we're
> starting to expand a top-level macro.  And as far as I can tell the
> value of invocation_location is only used when we aren't tracking
> virtual locations anyway.  So why does the change above fix the
> testcase?

It fixes the test case gcc.dg/cpp/paste12.c because that test case runs
with -ftrack-macro-expansion turned off.  Otherwise, you are right that
the issue exists only when we aren't tracking virtual locations.

FWIW, here is the updated patch I have.


libcpp/

	PR preprocessor/53229
	* directives.c (get_token_no_padding): Allow use of virtual
	locations in diagnostics emitted by the processor when handling
	directives.  Update comments.  This fixes gcc.dg/cpp/paste12.c.
	* macro.c (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.

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                  |    7 +++++--
 libcpp/macro.c                       |   25 +++++++++++++++++--------
 11 files changed, 92 insertions(+), 15 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 <jakub@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..e161810 100644
--- a/libcpp/directives.c
+++ b/libcpp/directives.c
@@ -1660,13 +1660,16 @@ do_pragma_dependency (cpp_reader *pfile)
   free ((void *) fname);
 }
 
-/* Get a token but skip padding.  */
+/* Get a token but skip padding.
+
+   Handles macro expansion and allows the use of virtual locations in
+   diagnostics emitted by the preprocessor.  */
 static const cpp_token *
 get_token_no_padding (cpp_reader *pfile)
 {
   for (;;)
     {
-      const cpp_token *result = cpp_get_token (pfile);
+      const cpp_token *result = cpp_get_token_with_location (pfile, NULL);
       if (result->type != CPP_PADDING)
 	return result;
     }
diff --git a/libcpp/macro.c b/libcpp/macro.c
index c4e2a23..04810b3 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);
@@ -544,9 +545,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 +593,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 +618,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 +632,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 +670,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);
-- 
1.7.6.5


-- 
		Dodji

Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens

[Tom Tromey]

>>>>> "Dodji" == Dodji Seketeli <dodji@redhat.com> writes:

Dodji> To properly fix this, I think libcpp should keep the token of the
Dodji> pasting operator '##', instead of representing it with flag on the LHS
Dodji> operand's token.  That way, it could use its location.

Originally I had thought that a pasted token should have a special
virtual location, pointing to the locations of its source tokens.
This would let later code "undo" the paste, if need be.

I don't know any more if this idea makes sense or not.

Tom

Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens

[Jason Merrill]

On 05/15/2012 10:17 AM, Dodji Seketeli wrote:
> It fixes the test case gcc.dg/cpp/paste12.c because that test case runs
> with -ftrack-macro-expansion turned off.  Otherwise, you are right that
> the issue exists only when we aren't tracking virtual locations.

I still don't understand why this change should be needed; it seems like 
a kludge disguised as a trivial change from one function to another.  If 
we're going to kludge, I'd rather explicitly set the flag with a comment.

But why do we need the set_invocation_location flag at all?  When do we 
not want to set invocation_location if we're beginning to expand a macro?

Jason

Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens

[Dodji Seketeli]

Jason Merrill <jason@redhat.com> writes:

> On 05/15/2012 10:17 AM, Dodji Seketeli wrote:
>> It fixes the test case gcc.dg/cpp/paste12.c because that test case runs
>> with -ftrack-macro-expansion turned off.  Otherwise, you are right that
>> the issue exists only when we aren't tracking virtual locations.
>
> I still don't understand why this change should be needed; it seems
> like a kludge disguised as a trivial change from one function to
> another.  If we're going to kludge, I'd rather explicitly set the flag
> with a comment.
>
> But why do we need the set_invocation_location flag at all?

I understand this set_invocation_location business as the poor man's
approach of tracking the expansion point location prior to the
ftrack-macro-expansion machinery.

When we are about to expand a macro M that is the top-most macro of the
expansion stack, we set it to record the expansion point of M.  This is
what cpp_get_token_with_location does.

> When do we not want to set invocation_location if we're beginning to
> expand a macro?

If M itself involves expanding another macro M', we are supposed to call
the (internal function) cpp_get_token_1 that does not set
set_invocation_location.  So that the only expansion point recorded is
the one for M, not the one for M'.

Said otherwise, cpp_get_token_with_location is the preferred external top-most
entry point to use to get a token (otherwise, we don't handle expansion
point locations), and cpp_get_token_1 should only used internally.

Now, looking at how surprising this is turning out to be, I am thinking
that cpp_get_token should, like cpp_get_token_with_location just set
pfile->set_invocation_location, just like cpp_get_token_with_location.

Would you agree?

-- 
		Dodji

Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens

[Dodji Seketeli]

Tom Tromey <tromey@redhat.com> writes:

>>>>>> "Dodji" == Dodji Seketeli <dodji@redhat.com> writes:
>
> Dodji> To properly fix this, I think libcpp should keep the token of the
> Dodji> pasting operator '##', instead of representing it with flag on the LHS
> Dodji> operand's token.  That way, it could use its location.
>
> Originally I had thought that a pasted token should have a special
> virtual location, pointing to the locations of its source tokens.
> This would let later code "undo" the paste, if need be.
>
> I don't know any more if this idea makes sense or not.

I think this makes sense, at least in the grand scheme of things.

The same underlying mechanism that would associate one or several
locations (for instance, the locations of the two operands of the
pasting operator) to a given location (for instance the location for the
token resulting from the pasting operation) could be used to e.g, make a
token carry two locations: one for the beginning of the token, and
another one for the end of the token.

cpp_get_token_with_location would just return the location for the
beginning of the token, like it does today, and we'd be able to retrieve
the second token by mean of the earlier association.

This whole shebang would be then used for the mighty range based
diagnostic[1].

Does that make sense?

[1]: http://gcc.gnu.org/wiki/Better_Diagnostics

-- 
		Dodji

Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens

[Jason Merrill]

On 05/21/2012 10:07 AM, Dodji Seketeli wrote:
> Jason Merrill<jason@redhat.com>  writes:
>> When do we not want to set invocation_location if we're beginning to
>> expand a macro?
>
> If M itself involves expanding another macro M', we are supposed to call
> the (internal function) cpp_get_token_1 that does not set
> set_invocation_location.  So that the only expansion point recorded is
> the one for M, not the one for M'.

But if we're already in a macro expansion (if context->c.macro is set) 
cpp_get_token_1 doesn't set invocation_location even if 
set_invocation_location is true.  So we should only get the expansion 
point for M even if set_invocation_location is always true.

> Now, looking at how surprising this is turning out to be, I am thinking
> that cpp_get_token should, like cpp_get_token_with_location just set
> pfile->set_invocation_location, just like cpp_get_token_with_location.
>
> Would you agree?

Yes, except that I would go farther and do away with the flag entirely.

Jason

Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens

[Dodji Seketeli]

Jason Merrill <jason@redhat.com> writes:

> On 05/21/2012 10:07 AM, Dodji Seketeli wrote:
>> Jason Merrill<jason@redhat.com>  writes:
>>> When do we not want to set invocation_location if we're beginning to
>>> expand a macro?
>>
>> If M itself involves expanding another macro M', we are supposed to call
>> the (internal function) cpp_get_token_1 that does not set
>> set_invocation_location.  So that the only expansion point recorded is
>> the one for M, not the one for M'.
>
> But if we're already in a macro expansion (if context->c.macro is set)
> cpp_get_token_1 doesn't set invocation_location even if
> set_invocation_location is true.  So we should only get the expansion
> point for M even if set_invocation_location is always true.

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.

In that case, if we don't have the pfile->set_invocation_location flag,
we'll record the expansion point of M', if M' is an argument of the
function-like macro M.

-- 
		Dodji

Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens

[Jason Merrill]

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

Jason

Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens

[Dodji Seketeli]

Jason Merrill <jason@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 <jakub@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

Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens

[Jason Merrill]

On 05/24/2012 12:03 PM, Dodji Seketeli wrote:
> +  if (macro != NULL)
> +    pfile->about_to_expand_macro_p = false;

The approach sounds good, but why do this in the push_token_context 
functions rather than in enter_macro_context?

Jason

Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens

[Dodji Seketeli]

Jason Merrill <jason@redhat.com> writes:

> On 05/24/2012 12:03 PM, Dodji Seketeli wrote:
>> +  if (macro != NULL)
>> +    pfile->about_to_expand_macro_p = false;
>
> The approach sounds good, but why do this in the push_token_context
> functions rather than in enter_macro_context?

Because, it's not only in enter_macro_context that I'd have to put it,
but also in replace_args, called from enter_macro_context, for instance.

Another way of seeing it is to say that, from the beginning of
enter_macro_context, we are in a state of "about to expand a macro"
until we actually push the macro context.  So it seems to make sense to
flip the flag where the the macro context push happens, IMHO.

-- 
		Dodji

Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens

[Jason Merrill]

On 05/24/2012 01:41 PM, Dodji Seketeli wrote:
> Jason Merrill<jason@redhat.com>  writes:
>>
>> The approach sounds good, but why do this in the push_token_context
>> functions rather than in enter_macro_context?
>
> Because, it's not only in enter_macro_context that I'd have to put it,
> but also in replace_args, called from enter_macro_context, for instance.

Why?

> Another way of seeing it is to say that, from the beginning of
> enter_macro_context, we are in a state of "about to expand a macro"
> until we actually push the macro context.  So it seems to make sense to
> flip the flag where the the macro context push happens, IMHO.

Sure, that makes sense.  It just seems better to me to have the flag set 
a little longer than necessary in order to keep the setting and 
unsetting closer together for maintainability.

Jason

Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens

[Dodji Seketeli]

Jason Merrill <jason@redhat.com> writes:

> On 05/24/2012 01:41 PM, Dodji Seketeli wrote:

[...]

>> Another way of seeing it is to say that, from the beginning of
>> enter_macro_context, we are in a state of "about to expand a macro"
>> until we actually push the macro context.  So it seems to make sense to
>> flip the flag where the the macro context push happens, IMHO.
>
> Sure, that makes sense.  It just seems better to me to have the flag
> set a little longer than necessary in order to keep the setting and
> unsetting closer together for maintainability.

Like the below?  Bootstrap is under way.
    
    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.

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 <jakub@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..89d70f1 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;
 	    }
 
@@ -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

-- 
		Dodji

Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens

[Dodji Seketeli]

Dodji Seketeli <dodji@redhat.com> writes:

> Jason Merrill <jason@redhat.com> writes:
>
>> On 05/24/2012 01:41 PM, Dodji Seketeli wrote:
>
> [...]
>
>>> Another way of seeing it is to say that, from the beginning of
>>> enter_macro_context, we are in a state of "about to expand a macro"
>>> until we actually push the macro context.  So it seems to make sense to
>>> flip the flag where the the macro context push happens, IMHO.
>>
>> Sure, that makes sense.  It just seems better to me to have the flag
>> set a little longer than necessary in order to keep the setting and
>> unsetting closer together for maintainability.
>
> Like the below?  Bootstrap is under way.

FWIW, the patch passed bootstrap and testing 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.

Thanks.

-- 
		Dodji

Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens

[Jason Merrill]

On 05/24/2012 03:18 PM, Dodji Seketeli wrote:
> Like the below?

Yep, thanks.  The patch is OK.

Jason