Patch for bug c++/17964 (cpp error locations for C++)

["Joseph S. Myers" <joseph at codesourcery dot com>]

This patch fixes bug 17964, a regression in preprocessor diagnostics
for C++ caused by lexing up front but deferring string interpretation
(which depends on the parse context) until later, so the
preprocessor's notion of source location does not relate to the actual
source location at that point.

The method is to arrange for some preprocessor diagnostics (those
after lexing) to go through the common diagnostic.c machinery, whose
notion of input_location is the right one for diagnostics at this
stage (although it doesn't include column numbers).  In principle it
is desirable for all preprocessor diagnostics to use the common
infrastructure, but doing so depends on having
--enable-mapped-location on unconditionally so preprocessor
source_locations are the same as location_t used by diagnostic.c; this
would also restore column numbers to these string diagnostics.  (Bug
24009 is another regression associated with lexing up front that could
be fixed using --enable-mapped-location.)

Thus, there is a callback to pass diagnostics through the cpplib
client, and a flag to say whether to use this callback.  Even with all
diagnostics going through diagnostic.c, such a flag will still be
needed, but its meaning will change to indicate only whether the
preprocessor or front-end notion of location is to be used; during
lexing the preprocessor location will be used but after then the
front-end location.

There is exactly one cpplib diagnostic outside the printf subset
implemented by pretty-print.c, "unknown escape sequence: '\\%03o'".
This diagnostic is one of those from string interpretation.  In
principle support for the '0' flag for for widths should be added to
pretty-print.c, but in practice for this regression fix (and bearing
in mind that this fix should be appropriate for 4.0 branch as well as
mainline) it seems to make more sense to format the number locally
with sprintf and then this can be changed back later once suitable
support is in pretty-print.c.

The core diagnostic infrastructure handles translating messages using
the "gcc" domain.  cpplib messages are in a different domain
"cpplib".  The simplest way of ensuring messages always get translated
in the right domain seemed to be to have cpplib handle translation
before using the callback, and to add diagnostic_set_info_translated
which takes an already-translated message.

The callback function pointer needs to be declared with a format
attribute to avoid warnings when assigning cp_cpp_error to it.  This
attribute can't be the ATTRIBUTE_GCC_CXXDIAG used for cp_cpp_error
because of ordering problems (that attribute would require cp-tree.h
to be included before cpplib.h, but cp-tree.h includes c-common.h
which includes cpplib.h and the attribute definitions in cp-tree.h
override those in c-common.h), so it's a printf attribute.

Bootstrapped with no regressions on i686-pc-linux-gnu.  OK to commit
to mainline, and to 4.0 branch after testing a backport there?

gcc:
2005-11-02  Joseph S. Myers  <joseph@codesourcery.com>

	PR c++/17964
	* diagnostic.c (diagnostic_set_info_translated): New function.
	(diagnostic_set_info): Use it.  Add comment.
	* diagnostic.h (diagnostic_set_info_translated): Declare.

gcc/cp:
2005-11-02  Joseph S. Myers  <joseph@codesourcery.com>

	PR c++/17964
	* error.c (cp_cpp_error): New function.
	* cp-tree.h (cp_cpp_error): Declare.
	* parser.c (cp_lexer_new_main): Set CPP option client_diagnostic
	and error callback after lexing.

gcc/testsuite:
2005-11-02  Joseph S. Myers  <joseph@codesourcery.com>

	PR c++/17964
	* g++.dg/cpp/string-1.C: New test.

libcpp:
2005-11-02  Joseph S. Myers  <joseph@codesourcery.com>

	PR c++/17964
	* include/cpplib.h (struct cpp_options): Add client_diagnostic.
	(struct cpp_callbacks): Add error.
	* errors.c (cpp_error): If client_diagnostic, use error callback.
	* charset.c (convert_escape): Don't use %03o in diagnostic.

diff -rupN GCC.orig/gcc/cp/cp-tree.h GCC/gcc/cp/cp-tree.h
--- GCC.orig/gcc/cp/cp-tree.h	2005-10-28 23:30:53.000000000 +0000
+++ GCC/gcc/cp/cp-tree.h	2005-11-01 22:49:44.000000000 +0000
@@ -4437,5 +4437,8 @@ extern void cp_genericize			(tree);
 #else
 #define ATTRIBUTE_GCC_CXXDIAG(m, n) ATTRIBUTE_NONNULL(m)
 #endif
+extern void cp_cpp_error			(cpp_reader *, int,
+						 const char *, va_list)
+     ATTRIBUTE_GCC_CXXDIAG(3,0);
 
 #endif /* ! GCC_CP_TREE_H */
diff -rupN GCC.orig/gcc/cp/error.c GCC/gcc/cp/error.c
--- GCC.orig/gcc/cp/error.c	2005-10-28 23:30:53.000000000 +0000
+++ GCC/gcc/cp/error.c	2005-11-02 00:02:31.000000000 +0000
@@ -2327,3 +2327,36 @@ cp_printer (pretty_printer *pp, text_inf
 #undef next_lang
 #undef next_int
 }
+
+/* Callback from cpp_error for PFILE to print diagnostics arising from
+   interpreting strings.  The diagnostic is of type LEVEL; MSG is the
+   translated message and AP the arguments.  */
+
+void
+cp_cpp_error (cpp_reader *pfile ATTRIBUTE_UNUSED, int level,
+	      const char *msg, va_list ap)
+{
+  diagnostic_info diagnostic;
+  diagnostic_t dlevel;
+  switch (level)
+    {
+    case CPP_DL_WARNING:
+    case CPP_DL_WARNING_SYSHDR:
+      dlevel = DK_WARNING;
+      break;
+    case CPP_DL_PEDWARN:
+      dlevel = pedantic_error_kind ();
+      break;
+    case CPP_DL_ERROR:
+      dlevel = DK_ERROR;
+      break;
+    case CPP_DL_ICE:
+      dlevel = DK_ICE;
+      break;
+    default:
+      gcc_unreachable ();
+    }
+  diagnostic_set_info_translated (&diagnostic, msg, &ap,
+				  input_location, dlevel);
+  report_diagnostic (&diagnostic);
+}
diff -rupN GCC.orig/gcc/cp/parser.c GCC/gcc/cp/parser.c
--- GCC.orig/gcc/cp/parser.c	2005-10-28 23:30:53.000000000 +0000
+++ GCC/gcc/cp/parser.c	2005-11-01 23:34:59.000000000 +0000
@@ -297,6 +297,11 @@ cp_lexer_new_main (void)
      string constant concatenation.  */
   c_lex_return_raw_strings = false;
 
+  /* Subsequent preprocessor diagnostics should use compiler
+     diagnostic functions to get the compiler source location.  */
+  cpp_get_options (parse_in)->client_diagnostic = true;
+  cpp_get_callbacks (parse_in)->error = cp_cpp_error;
+
   gcc_assert (lexer->next_token->type != CPP_PURGED);
   return lexer;
 }
diff -rupN GCC.orig/gcc/diagnostic.c GCC/gcc/diagnostic.c
--- GCC.orig/gcc/diagnostic.c	2005-10-28 23:34:10.000000000 +0000
+++ GCC/gcc/diagnostic.c	2005-11-01 22:27:18.000000000 +0000
@@ -112,19 +112,31 @@ diagnostic_initialize (diagnostic_contex
   context->lock = 0;
 }
 
+/* Initialize DIAGNOSTIC, where the message MSG has already been
+   translated.  */
 void
-diagnostic_set_info (diagnostic_info *diagnostic, const char *gmsgid,
-		     va_list *args, location_t location,
-		     diagnostic_t kind)
+diagnostic_set_info_translated (diagnostic_info *diagnostic, const char *msg,
+				va_list *args, location_t location,
+				diagnostic_t kind)
 {
   diagnostic->message.err_no = errno;
   diagnostic->message.args_ptr = args;
-  diagnostic->message.format_spec = _(gmsgid);
+  diagnostic->message.format_spec = msg;
   diagnostic->location = location;
   diagnostic->kind = kind;
   diagnostic->option_index = 0;
 }
 
+/* Initialize DIAGNOSTIC, where the message GMSGID has not yet been
+   translated.  */
+void
+diagnostic_set_info (diagnostic_info *diagnostic, const char *gmsgid,
+		     va_list *args, location_t location,
+		     diagnostic_t kind)
+{
+  diagnostic_set_info_translated (diagnostic, _(gmsgid), args, location, kind);
+}
+
 /* Return a malloc'd string describing a location.  The caller is
    responsible for freeing the memory.  */
 char *
diff -rupN GCC.orig/gcc/diagnostic.h GCC/gcc/diagnostic.h
--- GCC.orig/gcc/diagnostic.h	2005-10-28 23:34:10.000000000 +0000
+++ GCC/gcc/diagnostic.h	2005-11-01 22:25:17.000000000 +0000
@@ -184,6 +184,10 @@ extern void diagnostic_report_diagnostic
 #ifdef ATTRIBUTE_GCC_DIAG
 extern void diagnostic_set_info (diagnostic_info *, const char *, va_list *,
 				 location_t, diagnostic_t) ATTRIBUTE_GCC_DIAG(2,0);
+extern void diagnostic_set_info_translated (diagnostic_info *, const char *,
+					    va_list *, location_t,
+					    diagnostic_t)
+     ATTRIBUTE_GCC_DIAG(2,0);
 #endif
 extern char *diagnostic_build_prefix (diagnostic_info *);
 
diff -rupN GCC.orig/gcc/testsuite/g++.dg/cpp/string-1.C GCC/gcc/testsuite/g++.dg/cpp/string-1.C
--- GCC.orig/gcc/testsuite/g++.dg/cpp/string-1.C	1970-01-01 00:00:00.000000000 +0000
+++ GCC/gcc/testsuite/g++.dg/cpp/string-1.C	2005-11-01 22:00:29.000000000 +0000
@@ -0,0 +1,9 @@
+// Test location of diagnostics for interpreting strings.  Bug 17964.
+// Origin: Joseph Myers <joseph@codesourcery.com>
+// { dg-do compile }
+
+const char *s = "\q"; // { dg-error "unknown escape sequence" }
+
+const char *t = "\	"; // { dg-error "unknown escape sequence" }
+
+const char *u = "";
diff -rupN GCC.orig/libcpp/charset.c GCC/libcpp/charset.c
--- GCC.orig/libcpp/charset.c	2005-10-28 23:37:45.000000000 +0000
+++ GCC/libcpp/charset.c	2005-11-01 22:32:40.000000000 +0000
@@ -1277,8 +1277,14 @@ convert_escape (cpp_reader *pfile, const
 	cpp_error (pfile, CPP_DL_PEDWARN,
 		   "unknown escape sequence '\\%c'", (int) c);
       else
-	cpp_error (pfile, CPP_DL_PEDWARN,
-		   "unknown escape sequence: '\\%03o'", (int) c);
+	{
+	  /* diagnostic.c does not support "%03o".  When it does, this
+	     code can use %03o directly in the diagnostic again.  */
+	  char buf[32];
+	  sprintf(buf, "%03o", (int) c);
+	  cpp_error (pfile, CPP_DL_PEDWARN,
+		     "unknown escape sequence: '\\%s'", buf);
+	}
     }
 
   /* Now convert what we have to the execution character set.  */
diff -rupN GCC.orig/libcpp/errors.c GCC/libcpp/errors.c
--- GCC.orig/libcpp/errors.c	2005-10-28 23:37:45.000000000 +0000
+++ GCC/libcpp/errors.c	2005-11-01 22:08:27.000000000 +0000
@@ -140,20 +140,25 @@ cpp_error (cpp_reader * pfile, int level
   
   va_start (ap, msgid);
 
-  if (CPP_OPTION (pfile, traditional))
-    {
-      if (pfile->state.in_directive)
-	src_loc = pfile->directive_line;
-      else
-	src_loc = pfile->line_table->highest_line;
-    }
+  if (CPP_OPTION (pfile, client_diagnostic))
+    pfile->cb.error (pfile, level, _(msgid), ap);
   else
     {
-      src_loc = pfile->cur_token[-1].src_loc;
-    }
+      if (CPP_OPTION (pfile, traditional))
+	{
+	  if (pfile->state.in_directive)
+	    src_loc = pfile->directive_line;
+	  else
+	    src_loc = pfile->line_table->highest_line;
+	}
+      else
+	{
+	  src_loc = pfile->cur_token[-1].src_loc;
+	}
 
-  if (_cpp_begin_message (pfile, level, src_loc, 0))
-    v_message (msgid, ap);
+      if (_cpp_begin_message (pfile, level, src_loc, 0))
+	v_message (msgid, ap);
+    }
 
   va_end (ap);
 }
diff -rupN GCC.orig/libcpp/include/cpplib.h GCC/libcpp/include/cpplib.h
--- GCC.orig/libcpp/include/cpplib.h	2005-10-28 23:37:37.000000000 +0000
+++ GCC/libcpp/include/cpplib.h	2005-11-01 23:41:38.000000000 +0000
@@ -435,6 +435,9 @@ struct cpp_options
   /* True means return pragmas as tokens rather than processing
      them directly. */
   bool defer_pragmas;
+
+  /* True means error callback should be used for diagnostics.  */
+  bool client_diagnostic;
 };
 
 /* Callback for header lookup for HEADER, which is the name of a
@@ -467,6 +470,11 @@ struct cpp_callbacks
   int (*valid_pch) (cpp_reader *, const char *, int);
   void (*read_pch) (cpp_reader *, const char *, int, const char *);
   missing_header_cb missing_header;
+
+  /* Called to emit a diagnostic if client_diagnostic option is true.
+     This callback receives the translated message.  */
+  void (*error) (cpp_reader *, int, const char *, va_list)
+       ATTRIBUTE_PRINTF(3,0);
 };
 
 /* Chain of directories to look for include files in.  */

-- 
Joseph S. Myers               http://www.srcf.ucam.org/~jsm28/gcc/
    jsm@polyomino.org.uk (personal mail)
    joseph@codesourcery.com (CodeSourcery mail)
    jsm28@gcc.gnu.org (Bugzilla assignments and CCs)