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

[Dodji Seketeli <dodji at redhat dot com>]

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