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

4 files changed, 90 insertions, 50 deletions

diff --git a/libcpp/ChangeLog b/libcpp/ChangeLog
index 45ff6ca2d89..19936fe431d 100644
--- a/libcpp/ChangeLog
+++ b/libcpp/ChangeLog
@@ -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
diff --git a/libcpp/directives.c b/libcpp/directives.c
index e46280e46b0..66fa66daa64 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 5b3731bba1d..37aac821d62 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 c4e2a23c56b..89d70f1da9e 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