Re: Gcc 3.3 regressions

[Alexandre Oliva <aoliva at redhat dot com>]

On Sep 17, 2003, Alexandre Oliva <aoliva@redhat.com> wrote:

> On Sep 17, 2003, "H. J. Lu" <hjl@lucon.org> wrote:
>> FAIL: gcc.dg/pack-test-2.c  (test for errors, line 6)

>> Alexandre, they may have something to do with your patches.

> They do.  It's apparently the same problem that caused regressions on
> Darwin in mainline.  I thought -Wunknown-pragmas would have triggered
> the same path, and was convinced my patch would fix it, but it turns
> out I was wrong: pragmas for which a handler is defined don't get the
> pfile->directive_line that do_pragma() passes to def_pragma.
> def_pragma sets lineno to that line and then calls the diagnostic
> function, whereas none of the direct handlers do.

Here's a patch that fixes the problem, and that should fix the
pragma-darwin.c regression in mainline for real.

While trying to figure out what was going on, I started converting the
testcase I had for the previous patch into something suitable for the
testsuite.  Even though it turns out that it exercises a different
feature from that which had the problem, I thought I'd submit the
testcase anyway.

Ok to install?

> I'm currently inclined to reintroduce the cb_line_change, but passing
> it false, like destringize_and_run() in the same file, and to do so
> right after we read the first pragma token name, or even before doing
> so, such that we don't have to pass pfile->directive_line to the
> cb_def_pragma, and all pragma handlers will have the correct line
> numbers already set up.  How's that?

I found this patch would be safer, at least for 3.3.  For mainline, we
could try to get rid of all cases of passing `line' explicitly to
callbacks, and always use the line_change interface, or get all pragma
handlers to handle line changing on their own.  I don't quite like the
latter, but I could see its being justified if additional calls to
line_change would significantly affect performance.

Index: gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* cpplib.c (do_pragma): Reintroduce cb_line_change call in the
	code path that calls a handler.

Index: gcc/cpplib.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cpplib.c,v
retrieving revision 1.326.2.2
diff -u -p -r1.326.2.2 cpplib.c
--- gcc/cpplib.c 16 Sep 2003 00:20:41 -0000 1.326.2.2
+++ gcc/cpplib.c 18 Sep 2003 00:28:34 -0000
@@ -1094,7 +1094,7 @@ do_pragma (pfile)
      cpp_reader *pfile;
 {
   const struct pragma_entry *p = NULL;
-  const cpp_token *token;
+  const cpp_token *token, *pragma_token = pfile->cur_token;
   unsigned int count = 1;
 
   pfile->state.prevent_expansion++;
@@ -1115,7 +1115,17 @@ do_pragma (pfile)
     }
 
   if (p)
-    (*p->u.handler) (pfile);
+    {
+      /* Since the handler below doesn't get the line number, that it
+	 might need for diagnostics, make sure it has the right
+	 numbers in place.  */
+      if (pfile->cb.line_change)
+	(*pfile->cb.line_change) (pfile, pragma_token, false);
+      (*p->u.handler) (pfile);
+      if (pfile->cb.line_change)
+	(*pfile->cb.line_change) (pfile, pfile->cur_token, false);
+      
+    }
   else if (pfile->cb.def_pragma)
     {
       _cpp_backup_tokens (pfile, count);
Index: gcc/testsuite/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* gcc.dg/cpp/Wunknown-pragmas-1.c: New test.

Index: gcc/testsuite/gcc.dg/cpp/Wunknown-pragmas-1.c
===================================================================
RCS file: gcc/testsuite/gcc.dg/cpp/Wunknown-pragmas-1.c
diff -N gcc/testsuite/gcc.dg/cpp/Wunknown-pragmas-1.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gcc/testsuite/gcc.dg/cpp/Wunknown-pragmas-1.c 18 Sep 2003 00:28:42 -0000
@@ -0,0 +1,29 @@
+/* Copyright 2003 Free Software Foundation, Inc.  */
+
+/* { dg-do compile } */
+/* { dg-options "-Wunknown-pragmas" } */
+
+/* Make sure we get warnings in the expected lines.  */
+
+#pragma unknown1 /* { dg-warning "unknown1" "unknown1" } */
+
+#define COMMA ,
+#define FOO(x) x
+#define BAR(x) _Pragma("unknown_before") x
+#define BAZ(x) x _Pragma("unknown_after")
+
+int _Pragma("unknown2") bar1; /* { dg-warning "unknown2" "unknown2" } */
+
+FOO(int _Pragma("unknown3") bar2); /* { dg-warning "unknown3" "unknown3" } */
+
+int BAR(bar3); /* { dg-warning "unknown_before" "unknown_before 1" } */
+
+BAR(int bar4); /* { dg-warning "unknown_before" "unknown_before 2" } */
+
+int BAZ(bar5); /* { dg-warning "unknown_after" "unknown_after 1" } */
+
+int BAZ(bar6;) /* { dg-warning "unknown_after" "unknown_after 2" } */
+
+FOO(int bar7; _Pragma("unknown4")) /* { dg-warning "unknown4" "unknown4" } */
+
+#pragma unknown5 /* { dg-warning "unknown5" "unknown5" } */

-- 
Alexandre Oliva   Enjoy Guarana', see http://www.ic.unicamp.br/~oliva/
Red Hat GCC Developer                 aoliva@{redhat.com, gcc.gnu.org}
CS PhD student at IC-Unicamp        oliva@{lsd.ic.unicamp.br, gnu.org}
Free Software Evangelist                Professional serial bug killer