This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] libcpp: use better locations for _Pragma tokens (preprocessor/69126)


Our code for handling the "_Pragma" builtin macro is implemented in
libcpp/directives.c:destringize_and_run.  

It handles _Pragma by creating a one-line buffer containing the _Pragma
content, then sending it to do_pragma, which tokenizes it and handles
the input as if it were spelled as #pragma.

Unfortunately the tokens it generates have bogus location values; the
values are in the current highest ordinary map at the time of expansion,
and this determines the effective location of the synthesized #pragma.

Hence for PR preprocessor/69126:

VVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVV
#pragma GCC diagnostic push
#define MACRO \
    _Pragma("GCC diagnostic ignored \"-Wunused-variable\"") \
    int x;

int g()
{
    MACRO;
    return 0;
}
#pragma GCC diagnostic pop
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

although -save-temps looks sane:

VVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVV
#pragma GCC diagnostic push

int g()
{
# 19 "../../src/gcc/testsuite/c-c++-common/pr69126.c"
#pragma GCC diagnostic ignored "-Wunused-variable"
# 19 "../../src/gcc/testsuite/c-c++-common/pr69126.c"
    int x;;
    return 0;
}
#pragma GCC diagnostic pop
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

the "ignored" directive is treated as if it occured in this
bogus location:

(gdb) call inform (226930, "pre, the location of the ignore")
/tmp/test.cc:17:24: note: pre, the location of the ignore
     MACRO;
                        ^
which is effectively *after* the
    int x;;
line.

I believe this is a long-standing bug, but the cleanup of the
C++ FE's impl of -Wunused-variable in r226234 (to avoid using
%q+D) has exposed this as a regression (since the -Wunused-variable
warning is now emitted for this location:
    int x;
         ^
rather than at input_location:
     return 0;
             ^
and hence isn't suppressed by the _Pragma.

The function destringize_and_run is full of comments like
  /* Ugh; an awful kludge.  We are really not set up to be lexing
and:
  /* ??? Antique Disgusting Hack.  What does this do?  */
and similar "???" lines.

I believe it predates macro maps (I believe they were added in
r180081 on 2011-10-15); as far as I can tell, the code is set
up for *line* maps, i.e. it may even predate column information
(it gets the line right, but not the column).

To minimize the change to destringize_and_run at this stage, the
following patch papers over the problem by fixing up the locations
for the tokens synthesized for _Pragma, setting them all to be at the
expansion point of the _Pragma directive, rather than some columns
after it.

This fixes the location of the synthesized #pragma and hence
fixes the regression seen in the PR.  The -save-temps output (which
was already correct) is unaffected.

Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu;
adds 3 PASS results to g++.sum and 1 to gcc.sum
(although the regression currently only affects C++, I put the new
test case into c-c++-common for good measure).

OK for trunk in stage 3?

gcc/testsuite/ChangeLog:
	PR preprocessor/69126
	* c-c++-common/pr69126.c: New test case.

libcpp/ChangeLog:
	PR preprocessor/69126
	* directives.c (destringize_and_run): Add expansion_loc param; use
	it when handling unexpanded pragmas to fixup the locations of the
	synthesized tokens.
	(_cpp_do__Pragma): Add expansion_loc param and use it when calling
	destringize_and_run.
	* internal.h (_cpp_do__Pragma): Add expansion_loc param.
	* macro.c (builtin_macro): Pass expansion location of _Pragma to
	_cpp_do__Pragma.
---
 gcc/testsuite/c-c++-common/pr69126.c | 22 ++++++++++++++++++++++
 libcpp/directives.c                  | 13 ++++++++++---
 libcpp/internal.h                    |  2 +-
 libcpp/macro.c                       |  2 +-
 4 files changed, 34 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/pr69126.c

diff --git a/gcc/testsuite/c-c++-common/pr69126.c b/gcc/testsuite/c-c++-common/pr69126.c
new file mode 100644
index 0000000..fb4dcfb
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr69126.c
@@ -0,0 +1,22 @@
+/* { dg-options "-Wunused-variable" } */
+
+#pragma GCC diagnostic push
+int f()
+{
+    _Pragma("GCC diagnostic ignored \"-Wunused-variable\"")
+    int x;
+    return 0;
+}
+#pragma GCC diagnostic pop
+
+#pragma GCC diagnostic push
+#define MACRO \
+    _Pragma("GCC diagnostic ignored \"-Wunused-variable\"") \
+    int x;
+
+int g()
+{
+    MACRO;
+    return 0;
+}
+#pragma GCC diagnostic pop
diff --git a/libcpp/directives.c b/libcpp/directives.c
index eff1861..a1e1239 100644
--- a/libcpp/directives.c
+++ b/libcpp/directives.c
@@ -1753,7 +1753,8 @@ get__Pragma_string (cpp_reader *pfile)
 /* Destringize IN into a temporary buffer, by removing the first \ of
    \" and \\ sequences, and process the result as a #pragma directive.  */
 static void
-destringize_and_run (cpp_reader *pfile, const cpp_string *in)
+destringize_and_run (cpp_reader *pfile, const cpp_string *in,
+		     source_location expansion_loc)
 {
   const unsigned char *src, *limit;
   char *dest, *result;
@@ -1833,6 +1834,12 @@ destringize_and_run (cpp_reader *pfile, const cpp_string *in)
 	      toks = XRESIZEVEC (cpp_token, toks, maxcount);
 	    }
 	  toks[count] = *cpp_get_token (pfile);
+	  /* _Pragma is a builtin, so we're not within a macro-map, and so
+	     the token locations are set to bogus ordinary locations
+	     near to, but after that of the "_Pragma".
+	     Paper over this by setting them equal to the location of the
+	     _Pragma itself (PR preprocessor/69126).  */
+	  toks[count].src_loc = expansion_loc;
 	  /* Macros have been already expanded by cpp_get_token
 	     if the pragma allowed expansion.  */
 	  toks[count++].flags |= NO_EXPAND;
@@ -1867,14 +1874,14 @@ destringize_and_run (cpp_reader *pfile, const cpp_string *in)
 
 /* Handle the _Pragma operator.  Return 0 on error, 1 if ok.  */
 int
-_cpp_do__Pragma (cpp_reader *pfile)
+_cpp_do__Pragma (cpp_reader *pfile, source_location expansion_loc)
 {
   const cpp_token *string = get__Pragma_string (pfile);
   pfile->directive_result.type = CPP_PADDING;
 
   if (string)
     {
-      destringize_and_run (pfile, &string->val.str);
+      destringize_and_run (pfile, &string->val.str, expansion_loc);
       return 1;
     }
   cpp_error (pfile, CPP_DL_ERROR,
diff --git a/libcpp/internal.h b/libcpp/internal.h
index e04ae68..bafd480 100644
--- a/libcpp/internal.h
+++ b/libcpp/internal.h
@@ -688,7 +688,7 @@ extern int _cpp_handle_directive (cpp_reader *, int);
 extern void _cpp_define_builtin (cpp_reader *, const char *);
 extern char ** _cpp_save_pragma_names (cpp_reader *);
 extern void _cpp_restore_pragma_names (cpp_reader *, char **);
-extern int _cpp_do__Pragma (cpp_reader *);
+extern int _cpp_do__Pragma (cpp_reader *, source_location);
 extern void _cpp_init_directives (cpp_reader *);
 extern void _cpp_init_internal_pragmas (cpp_reader *);
 extern void _cpp_do_file_change (cpp_reader *, enum lc_reason, const char *,
diff --git a/libcpp/macro.c b/libcpp/macro.c
index ca1d1d6..cfb09ce 100644
--- a/libcpp/macro.c
+++ b/libcpp/macro.c
@@ -430,7 +430,7 @@ builtin_macro (cpp_reader *pfile, cpp_hashnode *node, source_location loc)
       if (pfile->state.in_directive)
 	return 0;
 
-      return _cpp_do__Pragma (pfile);
+      return _cpp_do__Pragma (pfile, loc);
     }
 
   buf = _cpp_builtin_macro_text (pfile, node);
-- 
1.8.5.3


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]