Re: [PING][patch] PR81794: have "would be stringified in traditional C" warning in libcpp/macro.c be controlled by -Wtraditional

Re: [PING][patch] PR81794: have "would be stringified in traditional C" warning in libcpp/macro.c be controlled by -Wtraditional

[Eric Gallager]

[-- Attachment #1: Type: text/plain, Size: 2029 bytes --]

Attached is a version of
https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00481.html that contains
a combination of both the fix and the testcase update, as requested in
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81794#c2

I had to use a different computer than I usually use to send this
email, as the hard drive that originally had this patch is currently
unresponsive. Since it's also the one with my ssh keys on it, I can't
commit with it. Sorry if the ChangeLogs get mangled.

libcpp/ChangeLog:

2017-03-24  Eric Gallager  <egall@gwmail.gwu.edu>

     * macro.c (check_trad_stringification): Have warning be controlled by
     -Wtraditional.

gcc/testsuite/ChangeLog:

2017-09-17  Eric Gallager  <egall@gwmail.gwu.edu>

    PR preprocessor/81794
    * gcc.dg/pragma-diag-7.c: Update to include check for
    stringification.

On Sat, May 6, 2017 at 11:33 AM, Eric Gallager <egall@gwmail.gwu.edu> wrote:
> Pinging this: https://gcc.gnu.org/ml/gcc-patches/2017-03/msg01325.html
>
> On 3/24/17, Eric Gallager <egall@gwmail.gwu.edu> wrote:
>> It seemed odd to me that gcc was issuing a warning about compatibility
>> with traditional C that I couldn't turn off by pushing/popping
>> -Wtraditional over the problem area, so I made the attached (minor)
>> patch to fix it. Survives bootstrap, but the only testing I've done
>> with it has been compiling the one file that was giving me issues
>> previously, which I'd need to reduce further to turn it into a proper
>> test case.
>>
>> Thanks,
>> Eric Gallager
>>
>> libcpp/ChangeLog:
>>
>> 2017-03-24  Eric Gallager  <egall@gwmail.gwu.edu>
>>
>>       * macro.c (check_trad_stringification): Have warning be controlled by
>>       -Wtraditional.
>>
>
> So I did the reducing I mentioned above and now have a testcase for
> it; it was pretty similar to the one from here:
> https://gcc.gnu.org/ml/gcc-patches/2017-03/msg01319.html
> so I combined them into a single testcase and have attached the
> combined version. I can confirm that the testcase passes with my patch
> applied.

[-- Attachment #2: patch-pragma-diag-traditional.diff --]
[-- Type: text/plain, Size: 1572 bytes --]

 gcc/testsuite/gcc.dg/pragma-diag-7.c |   13 +++++++++++++
 libcpp/macro.c                       |    2 +-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.dg/pragma-diag-7.c b/gcc/testsuite/gcc.dg/pragma-diag-7.c
index 402ee56..e06c410 100644
--- a/gcc/testsuite/gcc.dg/pragma-diag-7.c
+++ b/gcc/testsuite/gcc.dg/pragma-diag-7.c
@@ -7,3 +7,16 @@ unsigned long bad = 1UL; /* { dg-warning "suffix" } */
 /* Note the extra space before the pragma on this next line: */
  #pragma GCC diagnostic pop
 unsigned long ok_again = 2UL; /* { dg-bogus "suffix" } */
+
+/* Redundant with the previous pop, but just shows that it fails to stop the
+ * following warning with an unpatched GCC: */
+#pragma GCC diagnostic ignored "-Wtraditional"
+
+/* { dg-bogus "would be stringified" .+1 } */
+#define UNW_DEC_PROLOGUE(fmt, body, rlen, arg) \
+  do {									\
+      unw_rlen = rlen;							\
+      *(int *)arg = body;						\
+      printf("    %s:%s(rlen=%lu)\n",					\
+             fmt, (body ? "body" : "prologue"), (unsigned long)rlen);	\
+  } while (0)
diff --git a/libcpp/macro.c b/libcpp/macro.c
index de18c22..71363b5 100644
--- a/libcpp/macro.c
+++ b/libcpp/macro.c
@@ -3316,7 +3316,7 @@ check_trad_stringification (cpp_reader *pfile, const cpp_macro *macro,
 	  if (NODE_LEN (node) == len
 	      && !memcmp (p, NODE_NAME (node), len))
 	    {
-	      cpp_error (pfile, CPP_DL_WARNING,
+	      cpp_warning (pfile, CPP_W_TRADITIONAL,
 	   "macro argument \"%s\" would be stringified in traditional C",
 			 NODE_NAME (node));
 	      break;

Re: [PING][patch] PR81794: have "would be stringified in traditional C" warning in libcpp/macro.c be controlled by -Wtraditional

[Eric Gallager]

Ping: https://gcc.gnu.org/ml/gcc-patches/2017-09/msg01107.html

cc-ing additional libcpp (i.e. preprocessor) maintainers and
diagnostic messages maintainers

On Sun, Sep 17, 2017 at 8:00 PM, Eric Gallager <egall@gwmail.gwu.edu> wrote:
> Attached is a version of
> https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00481.html that contains
> a combination of both the fix and the testcase update, as requested in
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81794#c2
>
> I had to use a different computer than I usually use to send this
> email, as the hard drive that originally had this patch is currently
> unresponsive. Since it's also the one with my ssh keys on it, I can't
> commit with it. Sorry if the ChangeLogs get mangled.
>
> libcpp/ChangeLog:
>
> 2017-03-24  Eric Gallager  <egall@gwmail.gwu.edu>
>
>      * macro.c (check_trad_stringification): Have warning be controlled by
>      -Wtraditional.
>
> gcc/testsuite/ChangeLog:
>
> 2017-09-17  Eric Gallager  <egall@gwmail.gwu.edu>
>
>     PR preprocessor/81794
>     * gcc.dg/pragma-diag-7.c: Update to include check for
>     stringification.
>
> On Sat, May 6, 2017 at 11:33 AM, Eric Gallager <egall@gwmail.gwu.edu> wrote:
>> Pinging this: https://gcc.gnu.org/ml/gcc-patches/2017-03/msg01325.html
>>
>> On 3/24/17, Eric Gallager <egall@gwmail.gwu.edu> wrote:
>>> It seemed odd to me that gcc was issuing a warning about compatibility
>>> with traditional C that I couldn't turn off by pushing/popping
>>> -Wtraditional over the problem area, so I made the attached (minor)
>>> patch to fix it. Survives bootstrap, but the only testing I've done
>>> with it has been compiling the one file that was giving me issues
>>> previously, which I'd need to reduce further to turn it into a proper
>>> test case.
>>>
>>> Thanks,
>>> Eric Gallager
>>>
>>> libcpp/ChangeLog:
>>>
>>> 2017-03-24  Eric Gallager  <egall@gwmail.gwu.edu>
>>>
>>>       * macro.c (check_trad_stringification): Have warning be controlled by
>>>       -Wtraditional.
>>>
>>
>> So I did the reducing I mentioned above and now have a testcase for
>> it; it was pretty similar to the one from here:
>> https://gcc.gnu.org/ml/gcc-patches/2017-03/msg01319.html
>> so I combined them into a single testcase and have attached the
>> combined version. I can confirm that the testcase passes with my patch
>> applied.

Re: [PING][patch] PR81794: have "would be stringified in traditional C" warning in libcpp/macro.c be controlled by -Wtraditional

[David Malcolm]

On Sun, 2017-09-17 at 20:00 -0400, Eric Gallager wrote:
> Attached is a version of
> https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00481.html that
> contains
> a combination of both the fix and the testcase update, as requested
> in
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81794#c2
> 
> I had to use a different computer than I usually use to send this
> email, as the hard drive that originally had this patch is currently
> unresponsive. Since it's also the one with my ssh keys on it, I can't
> commit with it. Sorry if the ChangeLogs get mangled.

Thanks for putting this together; sorry about the delay in reviewing
it.

The patch mostly looks good.

Did you perform a full bootstrap and run of the testsuite with this
patch?  If so, it's best to state this in the email, so that we know
that the patch has survived this level of testing.

Some nits below:

> libcpp/ChangeLog:
> 
> 2017-03-24  Eric Gallager  <egall@gwmail.gwu.edu>
> 
>      * macro.c (check_trad_stringification): Have warning be
> controlled by
>      -Wtraditional.
> 
> gcc/testsuite/ChangeLog:
> 
> 2017-09-17  Eric Gallager  <egall@gwmail.gwu.edu>
> 
>     PR preprocessor/81794
>     * gcc.dg/pragma-diag-7.c: Update to include check for
>     stringification.
> 
> On Sat, May 6, 2017 at 11:33 AM, Eric Gallager <egall@gwmail.gwu.edu>
> wrote:
> > Pinging this: https://gcc.gnu.org/ml/gcc-patches/2017-03/msg01325.h
> > tml
> > 
> > On 3/24/17, Eric Gallager <egall@gwmail.gwu.edu> wrote:
> > > It seemed odd to me that gcc was issuing a warning about
> > > compatibility
> > > with traditional C that I couldn't turn off by pushing/popping
> > > -Wtraditional over the problem area, so I made the attached
> > > (minor)
> > > patch to fix it. Survives bootstrap, but the only testing I've
> > > done
> > > with it has been compiling the one file that was giving me issues
> > > previously, which I'd need to reduce further to turn it into a
> > > proper
> > > test case.
> > > 
> > > Thanks,
> > > Eric Gallager
> > > 
> > > libcpp/ChangeLog:
> > > 
> > > 2017-03-24  Eric Gallager  <egall@gwmail.gwu.edu>
> > > 
> > >       * macro.c (check_trad_stringification): Have warning be
> > > controlled by
> > >       -Wtraditional.
> > > 
> > 
> > So I did the reducing I mentioned above and now have a testcase for
> > it; it was pretty similar to the one from here:
> > https://gcc.gnu.org/ml/gcc-patches/2017-03/msg01319.html
> > so I combined them into a single testcase and have attached the
> > combined version. I can confirm that the testcase passes with my
> > patch
> > applied.

[...]

> diff --git a/gcc/testsuite/gcc.dg/pragma-diag-7.c b/gcc/testsuite/gcc.dg/pragma-diag-7.c
> index 402ee56..e06c410 100644
> --- a/gcc/testsuite/gcc.dg/pragma-diag-7.c
> +++ b/gcc/testsuite/gcc.dg/pragma-diag-7.c
> @@ -7,3 +7,16 @@ unsigned long bad = 1UL; /* { dg-warning "suffix" } */
>  /* Note the extra space before the pragma on this next line: */
>   #pragma GCC diagnostic pop
>  unsigned long ok_again = 2UL; /* { dg-bogus "suffix" } */
> +
> +/* Redundant with the previous pop, but just shows that it fails to stop the
> + * following warning with an unpatched GCC: */
> +#pragma GCC diagnostic ignored "-Wtraditional"
> +
> +/* { dg-bogus "would be stringified" .+1 } */

As far as I can tell, this dg-bogus line doesn't actually get matched;
when I run the testsuite without the libcpp fix, I get:

  FAIL: gcc.dg/pragma-diag-7.c (test for excess errors)

If I update the dg-bogus line to read:

  /* { dg-bogus "would be stringified" "" { target *-*-* } .+1 } */

then it's matched, and I get:

  FAIL: gcc.dg/pragma-diag-7.c  (test for bogus messages, line 16)

I believe that as written the ".+1" 2nd argument is interpreted as a
human-readable description of the problem, rather than as a line
offset; I believe you would need to add positional args for the
description and filter so that the line offset is argument 4.

That said, I think the dg-bogus here is unnecessary: if the warning is
erroneously emitted, we get:

  FAIL: gcc.dg/pragma-diag-7.c (test for excess errors)

(where "errors" really means "excess errors, warnings and extraneous
gunk that isn't a note").

So this testcase hunk is good without the dg-bogus line.

> +#define UNW_DEC_PROLOGUE(fmt, body, rlen, arg) \
> +  do {									\
> +      unw_rlen = rlen;							\
> +      *(int *)arg = body;						\
> +      printf("    %s:%s(rlen=%lu)\n",					\
> +             fmt, (body ? "body" : "prologue"), (unsigned long)rlen);	\
> +  } while (0)
> diff --git a/libcpp/macro.c b/libcpp/macro.c
> index de18c22..71363b5 100644
> --- a/libcpp/macro.c
> +++ b/libcpp/macro.c
> @@ -3316,7 +3316,7 @@ check_trad_stringification (cpp_reader *pfile, const cpp_macro *macro,
>  	  if (NODE_LEN (node) == len
>  	      && !memcmp (p, NODE_NAME (node), len))
>  	    {
> -	      cpp_error (pfile, CPP_DL_WARNING,
> +	      cpp_warning (pfile, CPP_W_TRADITIONAL,
>  	   "macro argument \"%s\" would be stringified in traditional C",
>  			 NODE_NAME (node));
>  	      break;

This hunk looks good.

So the patch is good if you drop the bogus "dg-bogus" line, provided
that it's bootstrapped and regression-tested.

Did you complete the FSF copyright assignment paperwork, and do you
have access to the GCC compile farm? (that could help with testing)

Thanks again for working on this (and for the work you've been doing in
Bugzilla); I hope you get your hard drive back...

Dave

Re: [PING][patch] PR81794: have "would be stringified in traditional C" warning in libcpp/macro.c be controlled by -Wtraditional

[David Malcolm]

On Fri, 2017-09-29 at 11:15 -0400, David Malcolm wrote:
> On Sun, 2017-09-17 at 20:00 -0400, Eric Gallager wrote:
> > Attached is a version of
> > https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00481.html that
> > contains
> > a combination of both the fix and the testcase update, as requested
> > in
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81794#c2
> > 
> > I had to use a different computer than I usually use to send this
> > email, as the hard drive that originally had this patch is
> > currently
> > unresponsive. Since it's also the one with my ssh keys on it, I
> > can't
> > commit with it. Sorry if the ChangeLogs get mangled.
> 
> Thanks for putting this together; sorry about the delay in reviewing
> it.
> 
> The patch mostly looks good.
> 
> Did you perform a full bootstrap and run of the testsuite with this
> patch?  If so, it's best to state this in the email, so that we know
> that the patch has survived this level of testing.
> 
> Some nits below:
> 
> > libcpp/ChangeLog:
> > 
> > 2017-03-24  Eric Gallager  <egall@gwmail.gwu.edu>
> > 
> >      * macro.c (check_trad_stringification): Have warning be
> > controlled by
> >      -Wtraditional.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 2017-09-17  Eric Gallager  <egall@gwmail.gwu.edu>
> > 
> >     PR preprocessor/81794
> >     * gcc.dg/pragma-diag-7.c: Update to include check for
> >     stringification.

Another nit: both ChangeLog entries ought to have the "PR
preprocessor/81794" heading (currently only the testsuite one does).

Re: [PING][patch] PR81794: have "would be stringified in traditional C" warning in libcpp/macro.c be controlled by -Wtraditional

[Eric Gallager]

On Fri, Sep 29, 2017 at 11:15 AM, David Malcolm <dmalcolm@redhat.com> wrote:
> On Sun, 2017-09-17 at 20:00 -0400, Eric Gallager wrote:
>> Attached is a version of
>> https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00481.html that
>> contains
>> a combination of both the fix and the testcase update, as requested
>> in
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81794#c2
>>
>> I had to use a different computer than I usually use to send this
>> email, as the hard drive that originally had this patch is currently
>> unresponsive. Since it's also the one with my ssh keys on it, I can't
>> commit with it. Sorry if the ChangeLogs get mangled.
>
> Thanks for putting this together; sorry about the delay in reviewing
> it.
>
> The patch mostly looks good.
>
> Did you perform a full bootstrap and run of the testsuite with this
> patch?  If so, it's best to state this in the email, so that we know
> that the patch has survived this level of testing.

Yes, I bootstrapped with it, but I haven't done a full run of the
testsuite with it yet; just the one testcase I updated.

>
> Some nits below:
>
>> libcpp/ChangeLog:
>>
>> 2017-03-24  Eric Gallager  <egall@gwmail.gwu.edu>
>>
>>      * macro.c (check_trad_stringification): Have warning be
>> controlled by
>>      -Wtraditional.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2017-09-17  Eric Gallager  <egall@gwmail.gwu.edu>
>>
>>     PR preprocessor/81794
>>     * gcc.dg/pragma-diag-7.c: Update to include check for
>>     stringification.
>>
>> On Sat, May 6, 2017 at 11:33 AM, Eric Gallager <egall@gwmail.gwu.edu>
>> wrote:
>> > Pinging this: https://gcc.gnu.org/ml/gcc-patches/2017-03/msg01325.h
>> > tml
>> >
>> > On 3/24/17, Eric Gallager <egall@gwmail.gwu.edu> wrote:
>> > > It seemed odd to me that gcc was issuing a warning about
>> > > compatibility
>> > > with traditional C that I couldn't turn off by pushing/popping
>> > > -Wtraditional over the problem area, so I made the attached
>> > > (minor)
>> > > patch to fix it. Survives bootstrap, but the only testing I've
>> > > done
>> > > with it has been compiling the one file that was giving me issues
>> > > previously, which I'd need to reduce further to turn it into a
>> > > proper
>> > > test case.
>> > >
>> > > Thanks,
>> > > Eric Gallager
>> > >
>> > > libcpp/ChangeLog:
>> > >
>> > > 2017-03-24  Eric Gallager  <egall@gwmail.gwu.edu>
>> > >
>> > >       * macro.c (check_trad_stringification): Have warning be
>> > > controlled by
>> > >       -Wtraditional.
>> > >
>> >
>> > So I did the reducing I mentioned above and now have a testcase for
>> > it; it was pretty similar to the one from here:
>> > https://gcc.gnu.org/ml/gcc-patches/2017-03/msg01319.html
>> > so I combined them into a single testcase and have attached the
>> > combined version. I can confirm that the testcase passes with my
>> > patch
>> > applied.
>
> [...]
>
>> diff --git a/gcc/testsuite/gcc.dg/pragma-diag-7.c b/gcc/testsuite/gcc.dg/pragma-diag-7.c
>> index 402ee56..e06c410 100644
>> --- a/gcc/testsuite/gcc.dg/pragma-diag-7.c
>> +++ b/gcc/testsuite/gcc.dg/pragma-diag-7.c
>> @@ -7,3 +7,16 @@ unsigned long bad = 1UL; /* { dg-warning "suffix" } */
>>  /* Note the extra space before the pragma on this next line: */
>>   #pragma GCC diagnostic pop
>>  unsigned long ok_again = 2UL; /* { dg-bogus "suffix" } */
>> +
>> +/* Redundant with the previous pop, but just shows that it fails to stop the
>> + * following warning with an unpatched GCC: */
>> +#pragma GCC diagnostic ignored "-Wtraditional"
>> +
>> +/* { dg-bogus "would be stringified" .+1 } */
>
> As far as I can tell, this dg-bogus line doesn't actually get matched;
> when I run the testsuite without the libcpp fix, I get:
>
>   FAIL: gcc.dg/pragma-diag-7.c (test for excess errors)
>
> If I update the dg-bogus line to read:
>
>   /* { dg-bogus "would be stringified" "" { target *-*-* } .+1 } */
>
> then it's matched, and I get:
>
>   FAIL: gcc.dg/pragma-diag-7.c  (test for bogus messages, line 16)
>
> I believe that as written the ".+1" 2nd argument is interpreted as a
> human-readable description of the problem, rather than as a line
> offset; I believe you would need to add positional args for the
> description and filter so that the line offset is argument 4.
>
> That said, I think the dg-bogus here is unnecessary: if the warning is
> erroneously emitted, we get:
>
>   FAIL: gcc.dg/pragma-diag-7.c (test for excess errors)
>
> (where "errors" really means "excess errors, warnings and extraneous
> gunk that isn't a note").
>
> So this testcase hunk is good without the dg-bogus line.

OK, the line can be removed when committing.

>
>> +#define UNW_DEC_PROLOGUE(fmt, body, rlen, arg) \
>> +  do {                                                                       \
>> +      unw_rlen = rlen;                                                       \
>> +      *(int *)arg = body;                                            \
>> +      printf("    %s:%s(rlen=%lu)\n",                                        \
>> +             fmt, (body ? "body" : "prologue"), (unsigned long)rlen);        \
>> +  } while (0)
>> diff --git a/libcpp/macro.c b/libcpp/macro.c
>> index de18c22..71363b5 100644
>> --- a/libcpp/macro.c
>> +++ b/libcpp/macro.c
>> @@ -3316,7 +3316,7 @@ check_trad_stringification (cpp_reader *pfile, const cpp_macro *macro,
>>         if (NODE_LEN (node) == len
>>             && !memcmp (p, NODE_NAME (node), len))
>>           {
>> -           cpp_error (pfile, CPP_DL_WARNING,
>> +           cpp_warning (pfile, CPP_W_TRADITIONAL,
>>          "macro argument \"%s\" would be stringified in traditional C",
>>                        NODE_NAME (node));
>>             break;
>
> This hunk looks good.
>
> So the patch is good if you drop the bogus "dg-bogus" line, provided
> that it's bootstrapped and regression-tested.
>
> Did you complete the FSF copyright assignment paperwork, and do you
> have access to the GCC compile farm? (that could help with testing)

Yes, I have a copyright assignment on file (it was a prerequisite for
putting my name in the MAINTAINERS file), but no, I don't think I have
access to the GCC compile farm. I agree, it'd be much better to test
on the compile farm than on my own computer, since running the
testsuite on my own computer usually takes an entire day (it's really
old and slow).

>
> Thanks again for working on this (and for the work you've been doing in
> Bugzilla); I hope you get your hard drive back...
>
> Dave

And thank you for all your work on improving gcc's diagnostics! If
you'd like to send me a new hard drive, feel free to contact me
off-list for my mailing address.

Thanks,
Eric

Re: [PING][patch] PR81794: have "would be stringified in traditional C" warning in libcpp/macro.c be controlled by -Wtraditional

[Eric Gallager]

On Sat, Sep 30, 2017 at 8:05 PM, Eric Gallager <egall@gwmail.gwu.edu> wrote:
> On Fri, Sep 29, 2017 at 11:15 AM, David Malcolm <dmalcolm@redhat.com> wrote:
>> On Sun, 2017-09-17 at 20:00 -0400, Eric Gallager wrote:
>>> Attached is a version of
>>> https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00481.html that
>>> contains
>>> a combination of both the fix and the testcase update, as requested
>>> in
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81794#c2
>>>
>>> I had to use a different computer than I usually use to send this
>>> email, as the hard drive that originally had this patch is currently
>>> unresponsive. Since it's also the one with my ssh keys on it, I can't
>>> commit with it. Sorry if the ChangeLogs get mangled.
>>
>> Thanks for putting this together; sorry about the delay in reviewing
>> it.
>>
>> The patch mostly looks good.
>>
>> Did you perform a full bootstrap and run of the testsuite with this
>> patch?  If so, it's best to state this in the email, so that we know
>> that the patch has survived this level of testing.
>
> Yes, I bootstrapped with it, but I haven't done a full run of the
> testsuite with it yet; just the one testcase I updated.

Update: I've now run the testsuite with it; test results are here:
https://gcc.gnu.org/ml/gcc-testresults/2017-10/msg01751.html
I'm pretty sure all the FAILs are unrelated to this patch.

>
>>
>> Some nits below:
>>
>>> libcpp/ChangeLog:
>>>
>>> 2017-03-24  Eric Gallager  <egall@gwmail.gwu.edu>
>>>
>>>      * macro.c (check_trad_stringification): Have warning be
>>> controlled by
>>>      -Wtraditional.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 2017-09-17  Eric Gallager  <egall@gwmail.gwu.edu>
>>>
>>>     PR preprocessor/81794
>>>     * gcc.dg/pragma-diag-7.c: Update to include check for
>>>     stringification.
>>>
>>> On Sat, May 6, 2017 at 11:33 AM, Eric Gallager <egall@gwmail.gwu.edu>
>>> wrote:
>>> > Pinging this: https://gcc.gnu.org/ml/gcc-patches/2017-03/msg01325.h
>>> > tml
>>> >
>>> > On 3/24/17, Eric Gallager <egall@gwmail.gwu.edu> wrote:
>>> > > It seemed odd to me that gcc was issuing a warning about
>>> > > compatibility
>>> > > with traditional C that I couldn't turn off by pushing/popping
>>> > > -Wtraditional over the problem area, so I made the attached
>>> > > (minor)
>>> > > patch to fix it. Survives bootstrap, but the only testing I've
>>> > > done
>>> > > with it has been compiling the one file that was giving me issues
>>> > > previously, which I'd need to reduce further to turn it into a
>>> > > proper
>>> > > test case.
>>> > >
>>> > > Thanks,
>>> > > Eric Gallager
>>> > >
>>> > > libcpp/ChangeLog:
>>> > >
>>> > > 2017-03-24  Eric Gallager  <egall@gwmail.gwu.edu>
>>> > >
>>> > >       * macro.c (check_trad_stringification): Have warning be
>>> > > controlled by
>>> > >       -Wtraditional.
>>> > >
>>> >
>>> > So I did the reducing I mentioned above and now have a testcase for
>>> > it; it was pretty similar to the one from here:
>>> > https://gcc.gnu.org/ml/gcc-patches/2017-03/msg01319.html
>>> > so I combined them into a single testcase and have attached the
>>> > combined version. I can confirm that the testcase passes with my
>>> > patch
>>> > applied.
>>
>> [...]
>>
>>> diff --git a/gcc/testsuite/gcc.dg/pragma-diag-7.c b/gcc/testsuite/gcc.dg/pragma-diag-7.c
>>> index 402ee56..e06c410 100644
>>> --- a/gcc/testsuite/gcc.dg/pragma-diag-7.c
>>> +++ b/gcc/testsuite/gcc.dg/pragma-diag-7.c
>>> @@ -7,3 +7,16 @@ unsigned long bad = 1UL; /* { dg-warning "suffix" } */
>>>  /* Note the extra space before the pragma on this next line: */
>>>   #pragma GCC diagnostic pop
>>>  unsigned long ok_again = 2UL; /* { dg-bogus "suffix" } */
>>> +
>>> +/* Redundant with the previous pop, but just shows that it fails to stop the
>>> + * following warning with an unpatched GCC: */
>>> +#pragma GCC diagnostic ignored "-Wtraditional"
>>> +
>>> +/* { dg-bogus "would be stringified" .+1 } */
>>
>> As far as I can tell, this dg-bogus line doesn't actually get matched;
>> when I run the testsuite without the libcpp fix, I get:
>>
>>   FAIL: gcc.dg/pragma-diag-7.c (test for excess errors)
>>
>> If I update the dg-bogus line to read:
>>
>>   /* { dg-bogus "would be stringified" "" { target *-*-* } .+1 } */
>>
>> then it's matched, and I get:
>>
>>   FAIL: gcc.dg/pragma-diag-7.c  (test for bogus messages, line 16)
>>
>> I believe that as written the ".+1" 2nd argument is interpreted as a
>> human-readable description of the problem, rather than as a line
>> offset; I believe you would need to add positional args for the
>> description and filter so that the line offset is argument 4.
>>
>> That said, I think the dg-bogus here is unnecessary: if the warning is
>> erroneously emitted, we get:
>>
>>   FAIL: gcc.dg/pragma-diag-7.c (test for excess errors)
>>
>> (where "errors" really means "excess errors, warnings and extraneous
>> gunk that isn't a note").
>>
>> So this testcase hunk is good without the dg-bogus line.
>
> OK, the line can be removed when committing.
>
>>
>>> +#define UNW_DEC_PROLOGUE(fmt, body, rlen, arg) \
>>> +  do {                                                                       \
>>> +      unw_rlen = rlen;                                                       \
>>> +      *(int *)arg = body;                                            \
>>> +      printf("    %s:%s(rlen=%lu)\n",                                        \
>>> +             fmt, (body ? "body" : "prologue"), (unsigned long)rlen);        \
>>> +  } while (0)
>>> diff --git a/libcpp/macro.c b/libcpp/macro.c
>>> index de18c22..71363b5 100644
>>> --- a/libcpp/macro.c
>>> +++ b/libcpp/macro.c
>>> @@ -3316,7 +3316,7 @@ check_trad_stringification (cpp_reader *pfile, const cpp_macro *macro,
>>>         if (NODE_LEN (node) == len
>>>             && !memcmp (p, NODE_NAME (node), len))
>>>           {
>>> -           cpp_error (pfile, CPP_DL_WARNING,
>>> +           cpp_warning (pfile, CPP_W_TRADITIONAL,
>>>          "macro argument \"%s\" would be stringified in traditional C",
>>>                        NODE_NAME (node));
>>>             break;
>>
>> This hunk looks good.
>>
>> So the patch is good if you drop the bogus "dg-bogus" line, provided
>> that it's bootstrapped and regression-tested.
>>
>> Did you complete the FSF copyright assignment paperwork, and do you
>> have access to the GCC compile farm? (that could help with testing)
>
> Yes, I have a copyright assignment on file (it was a prerequisite for
> putting my name in the MAINTAINERS file), but no, I don't think I have
> access to the GCC compile farm. I agree, it'd be much better to test
> on the compile farm than on my own computer, since running the
> testsuite on my own computer usually takes an entire day (it's really
> old and slow).

Update: I now have access to the GCC compile farm; that's where I
produced the test results linked above.

>
>>
>> Thanks again for working on this (and for the work you've been doing in
>> Bugzilla); I hope you get your hard drive back...
>>
>> Dave
>
> And thank you for all your work on improving gcc's diagnostics! If
> you'd like to send me a new hard drive, feel free to contact me
> off-list for my mailing address.

Update on hard drive status: I've ordered and received a new one, I
just need to salvage data off the old one and then install the new one
and then I should be good for committing again. I'd still appreciate
if someone else could commit for me while I'm still working on fixing
it though.

>
> Thanks,
> Eric

Re: [PING][patch] PR81794: have "would be stringified in traditional C" warning in libcpp/macro.c be controlled by -Wtraditional

[Eric Gallager]

Ping: https://gcc.gnu.org/ml/gcc-patches/2017-10/msg01834.html

On 10/25/17, Eric Gallager <egall@gwmail.gwu.edu> wrote:
> On Sat, Sep 30, 2017 at 8:05 PM, Eric Gallager <egall@gwmail.gwu.edu>
> wrote:
>> On Fri, Sep 29, 2017 at 11:15 AM, David Malcolm <dmalcolm@redhat.com>
>> wrote:
>>> On Sun, 2017-09-17 at 20:00 -0400, Eric Gallager wrote:
>>>> Attached is a version of
>>>> https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00481.html that
>>>> contains
>>>> a combination of both the fix and the testcase update, as requested
>>>> in
>>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81794#c2
>>>>
>>>> I had to use a different computer than I usually use to send this
>>>> email, as the hard drive that originally had this patch is currently
>>>> unresponsive. Since it's also the one with my ssh keys on it, I can't
>>>> commit with it. Sorry if the ChangeLogs get mangled.
>>>
>>> Thanks for putting this together; sorry about the delay in reviewing
>>> it.
>>>
>>> The patch mostly looks good.
>>>
>>> Did you perform a full bootstrap and run of the testsuite with this
>>> patch?  If so, it's best to state this in the email, so that we know
>>> that the patch has survived this level of testing.
>>
>> Yes, I bootstrapped with it, but I haven't done a full run of the
>> testsuite with it yet; just the one testcase I updated.
>
> Update: I've now run the testsuite with it; test results are here:
> https://gcc.gnu.org/ml/gcc-testresults/2017-10/msg01751.html
> I'm pretty sure all the FAILs are unrelated to this patch.
>
>>
>>>
>>> Some nits below:
>>>
>>>> libcpp/ChangeLog:
>>>>
>>>> 2017-03-24  Eric Gallager  <egall@gwmail.gwu.edu>
>>>>
>>>>      * macro.c (check_trad_stringification): Have warning be
>>>> controlled by
>>>>      -Wtraditional.
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>> 2017-09-17  Eric Gallager  <egall@gwmail.gwu.edu>
>>>>
>>>>     PR preprocessor/81794
>>>>     * gcc.dg/pragma-diag-7.c: Update to include check for
>>>>     stringification.
>>>>
>>>> On Sat, May 6, 2017 at 11:33 AM, Eric Gallager <egall@gwmail.gwu.edu>
>>>> wrote:
>>>> > Pinging this: https://gcc.gnu.org/ml/gcc-patches/2017-03/msg01325.h
>>>> > tml
>>>> >
>>>> > On 3/24/17, Eric Gallager <egall@gwmail.gwu.edu> wrote:
>>>> > > It seemed odd to me that gcc was issuing a warning about
>>>> > > compatibility
>>>> > > with traditional C that I couldn't turn off by pushing/popping
>>>> > > -Wtraditional over the problem area, so I made the attached
>>>> > > (minor)
>>>> > > patch to fix it. Survives bootstrap, but the only testing I've
>>>> > > done
>>>> > > with it has been compiling the one file that was giving me issues
>>>> > > previously, which I'd need to reduce further to turn it into a
>>>> > > proper
>>>> > > test case.
>>>> > >
>>>> > > Thanks,
>>>> > > Eric Gallager
>>>> > >
>>>> > > libcpp/ChangeLog:
>>>> > >
>>>> > > 2017-03-24  Eric Gallager  <egall@gwmail.gwu.edu>
>>>> > >
>>>> > >       * macro.c (check_trad_stringification): Have warning be
>>>> > > controlled by
>>>> > >       -Wtraditional.
>>>> > >
>>>> >
>>>> > So I did the reducing I mentioned above and now have a testcase for
>>>> > it; it was pretty similar to the one from here:
>>>> > https://gcc.gnu.org/ml/gcc-patches/2017-03/msg01319.html
>>>> > so I combined them into a single testcase and have attached the
>>>> > combined version. I can confirm that the testcase passes with my
>>>> > patch
>>>> > applied.
>>>
>>> [...]
>>>
>>>> diff --git a/gcc/testsuite/gcc.dg/pragma-diag-7.c
>>>> b/gcc/testsuite/gcc.dg/pragma-diag-7.c
>>>> index 402ee56..e06c410 100644
>>>> --- a/gcc/testsuite/gcc.dg/pragma-diag-7.c
>>>> +++ b/gcc/testsuite/gcc.dg/pragma-diag-7.c
>>>> @@ -7,3 +7,16 @@ unsigned long bad = 1UL; /* { dg-warning "suffix" } */
>>>>  /* Note the extra space before the pragma on this next line: */
>>>>   #pragma GCC diagnostic pop
>>>>  unsigned long ok_again = 2UL; /* { dg-bogus "suffix" } */
>>>> +
>>>> +/* Redundant with the previous pop, but just shows that it fails to
>>>> stop the
>>>> + * following warning with an unpatched GCC: */
>>>> +#pragma GCC diagnostic ignored "-Wtraditional"
>>>> +
>>>> +/* { dg-bogus "would be stringified" .+1 } */
>>>
>>> As far as I can tell, this dg-bogus line doesn't actually get matched;
>>> when I run the testsuite without the libcpp fix, I get:
>>>
>>>   FAIL: gcc.dg/pragma-diag-7.c (test for excess errors)
>>>
>>> If I update the dg-bogus line to read:
>>>
>>>   /* { dg-bogus "would be stringified" "" { target *-*-* } .+1 } */
>>>
>>> then it's matched, and I get:
>>>
>>>   FAIL: gcc.dg/pragma-diag-7.c  (test for bogus messages, line 16)
>>>
>>> I believe that as written the ".+1" 2nd argument is interpreted as a
>>> human-readable description of the problem, rather than as a line
>>> offset; I believe you would need to add positional args for the
>>> description and filter so that the line offset is argument 4.
>>>
>>> That said, I think the dg-bogus here is unnecessary: if the warning is
>>> erroneously emitted, we get:
>>>
>>>   FAIL: gcc.dg/pragma-diag-7.c (test for excess errors)
>>>
>>> (where "errors" really means "excess errors, warnings and extraneous
>>> gunk that isn't a note").
>>>
>>> So this testcase hunk is good without the dg-bogus line.
>>
>> OK, the line can be removed when committing.
>>
>>>
>>>> +#define UNW_DEC_PROLOGUE(fmt, body, rlen, arg) \
>>>> +  do {
>>>>      \
>>>> +      unw_rlen = rlen;
>>>>      \
>>>> +      *(int *)arg = body;                                            \
>>>> +      printf("    %s:%s(rlen=%lu)\n",
>>>>      \
>>>> +             fmt, (body ? "body" : "prologue"), (unsigned long)rlen);
>>>>      \
>>>> +  } while (0)
>>>> diff --git a/libcpp/macro.c b/libcpp/macro.c
>>>> index de18c22..71363b5 100644
>>>> --- a/libcpp/macro.c
>>>> +++ b/libcpp/macro.c
>>>> @@ -3316,7 +3316,7 @@ check_trad_stringification (cpp_reader *pfile,
>>>> const cpp_macro *macro,
>>>>         if (NODE_LEN (node) == len
>>>>             && !memcmp (p, NODE_NAME (node), len))
>>>>           {
>>>> -           cpp_error (pfile, CPP_DL_WARNING,
>>>> +           cpp_warning (pfile, CPP_W_TRADITIONAL,
>>>>          "macro argument \"%s\" would be stringified in traditional C",
>>>>                        NODE_NAME (node));
>>>>             break;
>>>
>>> This hunk looks good.
>>>
>>> So the patch is good if you drop the bogus "dg-bogus" line, provided
>>> that it's bootstrapped and regression-tested.
>>>
>>> Did you complete the FSF copyright assignment paperwork, and do you
>>> have access to the GCC compile farm? (that could help with testing)
>>
>> Yes, I have a copyright assignment on file (it was a prerequisite for
>> putting my name in the MAINTAINERS file), but no, I don't think I have
>> access to the GCC compile farm. I agree, it'd be much better to test
>> on the compile farm than on my own computer, since running the
>> testsuite on my own computer usually takes an entire day (it's really
>> old and slow).
>
> Update: I now have access to the GCC compile farm; that's where I
> produced the test results linked above.
>
>>
>>>
>>> Thanks again for working on this (and for the work you've been doing in
>>> Bugzilla); I hope you get your hard drive back...
>>>
>>> Dave
>>
>> And thank you for all your work on improving gcc's diagnostics! If
>> you'd like to send me a new hard drive, feel free to contact me
>> off-list for my mailing address.
>
> Update on hard drive status: I've ordered and received a new one, I
> just need to salvage data off the old one and then install the new one
> and then I should be good for committing again. I'd still appreciate
> if someone else could commit for me while I'm still working on fixing
> it though.

New update on hard drive status: All data is salvaged and transferred,
but installing the new hard drive into the internal hard drive slot
didn't actually fixing the disk errors I was experiencing. It works
perfectly fine when I boot from it as an external hard drive, though,
which is what I'm doing now. So, I'm pretty sure I can commit again
now. So, OK to commit this patch?

>
>>
>> Thanks,
>> Eric
>

Re: [PING][patch] PR81794: have "would be stringified in traditional C" warning in libcpp/macro.c be controlled by -Wtraditional

[Eric Gallager]

Ping again: https://gcc.gnu.org/ml/gcc-patches/2017-11/msg00123.html

On 11/2/17, Eric Gallager <egall@gwmail.gwu.edu> wrote:
> Ping: https://gcc.gnu.org/ml/gcc-patches/2017-10/msg01834.html
>
> On 10/25/17, Eric Gallager <egall@gwmail.gwu.edu> wrote:
>> On Sat, Sep 30, 2017 at 8:05 PM, Eric Gallager <egall@gwmail.gwu.edu>
>> wrote:
>>> On Fri, Sep 29, 2017 at 11:15 AM, David Malcolm <dmalcolm@redhat.com>
>>> wrote:
>>>> On Sun, 2017-09-17 at 20:00 -0400, Eric Gallager wrote:
>>>>> Attached is a version of
>>>>> https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00481.html that
>>>>> contains
>>>>> a combination of both the fix and the testcase update, as requested
>>>>> in
>>>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81794#c2
>>>>>
>>>>> I had to use a different computer than I usually use to send this
>>>>> email, as the hard drive that originally had this patch is currently
>>>>> unresponsive. Since it's also the one with my ssh keys on it, I can't
>>>>> commit with it. Sorry if the ChangeLogs get mangled.
>>>>
>>>> Thanks for putting this together; sorry about the delay in reviewing
>>>> it.
>>>>
>>>> The patch mostly looks good.
>>>>
>>>> Did you perform a full bootstrap and run of the testsuite with this
>>>> patch?  If so, it's best to state this in the email, so that we know
>>>> that the patch has survived this level of testing.
>>>
>>> Yes, I bootstrapped with it, but I haven't done a full run of the
>>> testsuite with it yet; just the one testcase I updated.
>>
>> Update: I've now run the testsuite with it; test results are here:
>> https://gcc.gnu.org/ml/gcc-testresults/2017-10/msg01751.html
>> I'm pretty sure all the FAILs are unrelated to this patch.
>>
>>>
>>>>
>>>> Some nits below:
>>>>
>>>>> libcpp/ChangeLog:
>>>>>
>>>>> 2017-03-24  Eric Gallager  <egall@gwmail.gwu.edu>
>>>>>
>>>>>      * macro.c (check_trad_stringification): Have warning be
>>>>> controlled by
>>>>>      -Wtraditional.
>>>>>
>>>>> gcc/testsuite/ChangeLog:
>>>>>
>>>>> 2017-09-17  Eric Gallager  <egall@gwmail.gwu.edu>
>>>>>
>>>>>     PR preprocessor/81794
>>>>>     * gcc.dg/pragma-diag-7.c: Update to include check for
>>>>>     stringification.
>>>>>
>>>>> On Sat, May 6, 2017 at 11:33 AM, Eric Gallager <egall@gwmail.gwu.edu>
>>>>> wrote:
>>>>> > Pinging this: https://gcc.gnu.org/ml/gcc-patches/2017-03/msg01325.h
>>>>> > tml
>>>>> >
>>>>> > On 3/24/17, Eric Gallager <egall@gwmail.gwu.edu> wrote:
>>>>> > > It seemed odd to me that gcc was issuing a warning about
>>>>> > > compatibility
>>>>> > > with traditional C that I couldn't turn off by pushing/popping
>>>>> > > -Wtraditional over the problem area, so I made the attached
>>>>> > > (minor)
>>>>> > > patch to fix it. Survives bootstrap, but the only testing I've
>>>>> > > done
>>>>> > > with it has been compiling the one file that was giving me issues
>>>>> > > previously, which I'd need to reduce further to turn it into a
>>>>> > > proper
>>>>> > > test case.
>>>>> > >
>>>>> > > Thanks,
>>>>> > > Eric Gallager
>>>>> > >
>>>>> > > libcpp/ChangeLog:
>>>>> > >
>>>>> > > 2017-03-24  Eric Gallager  <egall@gwmail.gwu.edu>
>>>>> > >
>>>>> > >       * macro.c (check_trad_stringification): Have warning be
>>>>> > > controlled by
>>>>> > >       -Wtraditional.
>>>>> > >
>>>>> >
>>>>> > So I did the reducing I mentioned above and now have a testcase for
>>>>> > it; it was pretty similar to the one from here:
>>>>> > https://gcc.gnu.org/ml/gcc-patches/2017-03/msg01319.html
>>>>> > so I combined them into a single testcase and have attached the
>>>>> > combined version. I can confirm that the testcase passes with my
>>>>> > patch
>>>>> > applied.
>>>>
>>>> [...]
>>>>
>>>>> diff --git a/gcc/testsuite/gcc.dg/pragma-diag-7.c
>>>>> b/gcc/testsuite/gcc.dg/pragma-diag-7.c
>>>>> index 402ee56..e06c410 100644
>>>>> --- a/gcc/testsuite/gcc.dg/pragma-diag-7.c
>>>>> +++ b/gcc/testsuite/gcc.dg/pragma-diag-7.c
>>>>> @@ -7,3 +7,16 @@ unsigned long bad = 1UL; /* { dg-warning "suffix" }
>>>>> */
>>>>>  /* Note the extra space before the pragma on this next line: */
>>>>>   #pragma GCC diagnostic pop
>>>>>  unsigned long ok_again = 2UL; /* { dg-bogus "suffix" } */
>>>>> +
>>>>> +/* Redundant with the previous pop, but just shows that it fails to
>>>>> stop the
>>>>> + * following warning with an unpatched GCC: */
>>>>> +#pragma GCC diagnostic ignored "-Wtraditional"
>>>>> +
>>>>> +/* { dg-bogus "would be stringified" .+1 } */
>>>>
>>>> As far as I can tell, this dg-bogus line doesn't actually get matched;
>>>> when I run the testsuite without the libcpp fix, I get:
>>>>
>>>>   FAIL: gcc.dg/pragma-diag-7.c (test for excess errors)
>>>>
>>>> If I update the dg-bogus line to read:
>>>>
>>>>   /* { dg-bogus "would be stringified" "" { target *-*-* } .+1 } */
>>>>
>>>> then it's matched, and I get:
>>>>
>>>>   FAIL: gcc.dg/pragma-diag-7.c  (test for bogus messages, line 16)
>>>>
>>>> I believe that as written the ".+1" 2nd argument is interpreted as a
>>>> human-readable description of the problem, rather than as a line
>>>> offset; I believe you would need to add positional args for the
>>>> description and filter so that the line offset is argument 4.
>>>>
>>>> That said, I think the dg-bogus here is unnecessary: if the warning is
>>>> erroneously emitted, we get:
>>>>
>>>>   FAIL: gcc.dg/pragma-diag-7.c (test for excess errors)
>>>>
>>>> (where "errors" really means "excess errors, warnings and extraneous
>>>> gunk that isn't a note").
>>>>
>>>> So this testcase hunk is good without the dg-bogus line.
>>>
>>> OK, the line can be removed when committing.
>>>
>>>>
>>>>> +#define UNW_DEC_PROLOGUE(fmt, body, rlen, arg) \
>>>>> +  do {
>>>>>      \
>>>>> +      unw_rlen = rlen;
>>>>>      \
>>>>> +      *(int *)arg = body;
>>>>> \
>>>>> +      printf("    %s:%s(rlen=%lu)\n",
>>>>>      \
>>>>> +             fmt, (body ? "body" : "prologue"), (unsigned long)rlen);
>>>>>      \
>>>>> +  } while (0)
>>>>> diff --git a/libcpp/macro.c b/libcpp/macro.c
>>>>> index de18c22..71363b5 100644
>>>>> --- a/libcpp/macro.c
>>>>> +++ b/libcpp/macro.c
>>>>> @@ -3316,7 +3316,7 @@ check_trad_stringification (cpp_reader *pfile,
>>>>> const cpp_macro *macro,
>>>>>         if (NODE_LEN (node) == len
>>>>>             && !memcmp (p, NODE_NAME (node), len))
>>>>>           {
>>>>> -           cpp_error (pfile, CPP_DL_WARNING,
>>>>> +           cpp_warning (pfile, CPP_W_TRADITIONAL,
>>>>>          "macro argument \"%s\" would be stringified in traditional
>>>>> C",
>>>>>                        NODE_NAME (node));
>>>>>             break;
>>>>
>>>> This hunk looks good.
>>>>
>>>> So the patch is good if you drop the bogus "dg-bogus" line, provided
>>>> that it's bootstrapped and regression-tested.
>>>>
>>>> Did you complete the FSF copyright assignment paperwork, and do you
>>>> have access to the GCC compile farm? (that could help with testing)
>>>
>>> Yes, I have a copyright assignment on file (it was a prerequisite for
>>> putting my name in the MAINTAINERS file), but no, I don't think I have
>>> access to the GCC compile farm. I agree, it'd be much better to test
>>> on the compile farm than on my own computer, since running the
>>> testsuite on my own computer usually takes an entire day (it's really
>>> old and slow).
>>
>> Update: I now have access to the GCC compile farm; that's where I
>> produced the test results linked above.
>>
>>>
>>>>
>>>> Thanks again for working on this (and for the work you've been doing in
>>>> Bugzilla); I hope you get your hard drive back...
>>>>
>>>> Dave
>>>
>>> And thank you for all your work on improving gcc's diagnostics! If
>>> you'd like to send me a new hard drive, feel free to contact me
>>> off-list for my mailing address.
>>
>> Update on hard drive status: I've ordered and received a new one, I
>> just need to salvage data off the old one and then install the new one
>> and then I should be good for committing again. I'd still appreciate
>> if someone else could commit for me while I'm still working on fixing
>> it though.
>
> New update on hard drive status: All data is salvaged and transferred,
> but installing the new hard drive into the internal hard drive slot
> didn't actually fixing the disk errors I was experiencing. It works
> perfectly fine when I boot from it as an external hard drive, though,
> which is what I'm doing now. So, I'm pretty sure I can commit again
> now. So, OK to commit this patch?
>
>>
>>>
>>> Thanks,
>>> Eric
>>
>

Re: [PING][patch] PR81794: have "would be stringified in traditional C" warning in libcpp/macro.c be controlled by -Wtraditional

[Eric Gallager]

Ping yet again: https://gcc.gnu.org/ml/gcc-patches/2017-11/msg00123.html

On 11/9/17, Eric Gallager <egall@gwmail.gwu.edu> wrote:
> Ping again: https://gcc.gnu.org/ml/gcc-patches/2017-11/msg00123.html
>
> On 11/2/17, Eric Gallager <egall@gwmail.gwu.edu> wrote:
>> Ping: https://gcc.gnu.org/ml/gcc-patches/2017-10/msg01834.html
>>
>> On 10/25/17, Eric Gallager <egall@gwmail.gwu.edu> wrote:
>>> On Sat, Sep 30, 2017 at 8:05 PM, Eric Gallager <egall@gwmail.gwu.edu>
>>> wrote:
>>>> On Fri, Sep 29, 2017 at 11:15 AM, David Malcolm <dmalcolm@redhat.com>
>>>> wrote:
>>>>> On Sun, 2017-09-17 at 20:00 -0400, Eric Gallager wrote:
>>>>>> Attached is a version of
>>>>>> https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00481.html that
>>>>>> contains
>>>>>> a combination of both the fix and the testcase update, as requested
>>>>>> in
>>>>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81794#c2
>>>>>>
>>>>>> I had to use a different computer than I usually use to send this
>>>>>> email, as the hard drive that originally had this patch is currently
>>>>>> unresponsive. Since it's also the one with my ssh keys on it, I can't
>>>>>> commit with it. Sorry if the ChangeLogs get mangled.
>>>>>
>>>>> Thanks for putting this together; sorry about the delay in reviewing
>>>>> it.
>>>>>
>>>>> The patch mostly looks good.
>>>>>
>>>>> Did you perform a full bootstrap and run of the testsuite with this
>>>>> patch?  If so, it's best to state this in the email, so that we know
>>>>> that the patch has survived this level of testing.
>>>>
>>>> Yes, I bootstrapped with it, but I haven't done a full run of the
>>>> testsuite with it yet; just the one testcase I updated.
>>>
>>> Update: I've now run the testsuite with it; test results are here:
>>> https://gcc.gnu.org/ml/gcc-testresults/2017-10/msg01751.html
>>> I'm pretty sure all the FAILs are unrelated to this patch.
>>>
>>>>
>>>>>
>>>>> Some nits below:
>>>>>
>>>>>> libcpp/ChangeLog:
>>>>>>
>>>>>> 2017-03-24  Eric Gallager  <egall@gwmail.gwu.edu>
>>>>>>
>>>>>>      * macro.c (check_trad_stringification): Have warning be
>>>>>> controlled by
>>>>>>      -Wtraditional.
>>>>>>
>>>>>> gcc/testsuite/ChangeLog:
>>>>>>
>>>>>> 2017-09-17  Eric Gallager  <egall@gwmail.gwu.edu>
>>>>>>
>>>>>>     PR preprocessor/81794
>>>>>>     * gcc.dg/pragma-diag-7.c: Update to include check for
>>>>>>     stringification.
>>>>>>
>>>>>> On Sat, May 6, 2017 at 11:33 AM, Eric Gallager <egall@gwmail.gwu.edu>
>>>>>> wrote:
>>>>>> > Pinging this: https://gcc.gnu.org/ml/gcc-patches/2017-03/msg01325.h
>>>>>> > tml
>>>>>> >
>>>>>> > On 3/24/17, Eric Gallager <egall@gwmail.gwu.edu> wrote:
>>>>>> > > It seemed odd to me that gcc was issuing a warning about
>>>>>> > > compatibility
>>>>>> > > with traditional C that I couldn't turn off by pushing/popping
>>>>>> > > -Wtraditional over the problem area, so I made the attached
>>>>>> > > (minor)
>>>>>> > > patch to fix it. Survives bootstrap, but the only testing I've
>>>>>> > > done
>>>>>> > > with it has been compiling the one file that was giving me issues
>>>>>> > > previously, which I'd need to reduce further to turn it into a
>>>>>> > > proper
>>>>>> > > test case.
>>>>>> > >
>>>>>> > > Thanks,
>>>>>> > > Eric Gallager
>>>>>> > >
>>>>>> > > libcpp/ChangeLog:
>>>>>> > >
>>>>>> > > 2017-03-24  Eric Gallager  <egall@gwmail.gwu.edu>
>>>>>> > >
>>>>>> > >       * macro.c (check_trad_stringification): Have warning be
>>>>>> > > controlled by
>>>>>> > >       -Wtraditional.
>>>>>> > >
>>>>>> >
>>>>>> > So I did the reducing I mentioned above and now have a testcase for
>>>>>> > it; it was pretty similar to the one from here:
>>>>>> > https://gcc.gnu.org/ml/gcc-patches/2017-03/msg01319.html
>>>>>> > so I combined them into a single testcase and have attached the
>>>>>> > combined version. I can confirm that the testcase passes with my
>>>>>> > patch
>>>>>> > applied.
>>>>>
>>>>> [...]
>>>>>
>>>>>> diff --git a/gcc/testsuite/gcc.dg/pragma-diag-7.c
>>>>>> b/gcc/testsuite/gcc.dg/pragma-diag-7.c
>>>>>> index 402ee56..e06c410 100644
>>>>>> --- a/gcc/testsuite/gcc.dg/pragma-diag-7.c
>>>>>> +++ b/gcc/testsuite/gcc.dg/pragma-diag-7.c
>>>>>> @@ -7,3 +7,16 @@ unsigned long bad = 1UL; /* { dg-warning "suffix" }
>>>>>> */
>>>>>>  /* Note the extra space before the pragma on this next line: */
>>>>>>   #pragma GCC diagnostic pop
>>>>>>  unsigned long ok_again = 2UL; /* { dg-bogus "suffix" } */
>>>>>> +
>>>>>> +/* Redundant with the previous pop, but just shows that it fails to
>>>>>> stop the
>>>>>> + * following warning with an unpatched GCC: */
>>>>>> +#pragma GCC diagnostic ignored "-Wtraditional"
>>>>>> +
>>>>>> +/* { dg-bogus "would be stringified" .+1 } */
>>>>>
>>>>> As far as I can tell, this dg-bogus line doesn't actually get matched;
>>>>> when I run the testsuite without the libcpp fix, I get:
>>>>>
>>>>>   FAIL: gcc.dg/pragma-diag-7.c (test for excess errors)
>>>>>
>>>>> If I update the dg-bogus line to read:
>>>>>
>>>>>   /* { dg-bogus "would be stringified" "" { target *-*-* } .+1 } */
>>>>>
>>>>> then it's matched, and I get:
>>>>>
>>>>>   FAIL: gcc.dg/pragma-diag-7.c  (test for bogus messages, line 16)
>>>>>
>>>>> I believe that as written the ".+1" 2nd argument is interpreted as a
>>>>> human-readable description of the problem, rather than as a line
>>>>> offset; I believe you would need to add positional args for the
>>>>> description and filter so that the line offset is argument 4.
>>>>>
>>>>> That said, I think the dg-bogus here is unnecessary: if the warning is
>>>>> erroneously emitted, we get:
>>>>>
>>>>>   FAIL: gcc.dg/pragma-diag-7.c (test for excess errors)
>>>>>
>>>>> (where "errors" really means "excess errors, warnings and extraneous
>>>>> gunk that isn't a note").
>>>>>
>>>>> So this testcase hunk is good without the dg-bogus line.
>>>>
>>>> OK, the line can be removed when committing.
>>>>
>>>>>
>>>>>> +#define UNW_DEC_PROLOGUE(fmt, body, rlen, arg) \
>>>>>> +  do {
>>>>>>      \
>>>>>> +      unw_rlen = rlen;
>>>>>>      \
>>>>>> +      *(int *)arg = body;
>>>>>> \
>>>>>> +      printf("    %s:%s(rlen=%lu)\n",
>>>>>>      \
>>>>>> +             fmt, (body ? "body" : "prologue"), (unsigned
>>>>>> long)rlen);
>>>>>>      \
>>>>>> +  } while (0)
>>>>>> diff --git a/libcpp/macro.c b/libcpp/macro.c
>>>>>> index de18c22..71363b5 100644
>>>>>> --- a/libcpp/macro.c
>>>>>> +++ b/libcpp/macro.c
>>>>>> @@ -3316,7 +3316,7 @@ check_trad_stringification (cpp_reader *pfile,
>>>>>> const cpp_macro *macro,
>>>>>>         if (NODE_LEN (node) == len
>>>>>>             && !memcmp (p, NODE_NAME (node), len))
>>>>>>           {
>>>>>> -           cpp_error (pfile, CPP_DL_WARNING,
>>>>>> +           cpp_warning (pfile, CPP_W_TRADITIONAL,
>>>>>>          "macro argument \"%s\" would be stringified in traditional
>>>>>> C",
>>>>>>                        NODE_NAME (node));
>>>>>>             break;
>>>>>
>>>>> This hunk looks good.
>>>>>
>>>>> So the patch is good if you drop the bogus "dg-bogus" line, provided
>>>>> that it's bootstrapped and regression-tested.
>>>>>
>>>>> Did you complete the FSF copyright assignment paperwork, and do you
>>>>> have access to the GCC compile farm? (that could help with testing)
>>>>
>>>> Yes, I have a copyright assignment on file (it was a prerequisite for
>>>> putting my name in the MAINTAINERS file), but no, I don't think I have
>>>> access to the GCC compile farm. I agree, it'd be much better to test
>>>> on the compile farm than on my own computer, since running the
>>>> testsuite on my own computer usually takes an entire day (it's really
>>>> old and slow).
>>>
>>> Update: I now have access to the GCC compile farm; that's where I
>>> produced the test results linked above.
>>>
>>>>
>>>>>
>>>>> Thanks again for working on this (and for the work you've been doing
>>>>> in
>>>>> Bugzilla); I hope you get your hard drive back...
>>>>>
>>>>> Dave
>>>>
>>>> And thank you for all your work on improving gcc's diagnostics! If
>>>> you'd like to send me a new hard drive, feel free to contact me
>>>> off-list for my mailing address.
>>>
>>> Update on hard drive status: I've ordered and received a new one, I
>>> just need to salvage data off the old one and then install the new one
>>> and then I should be good for committing again. I'd still appreciate
>>> if someone else could commit for me while I'm still working on fixing
>>> it though.
>>
>> New update on hard drive status: All data is salvaged and transferred,
>> but installing the new hard drive into the internal hard drive slot
>> didn't actually fixing the disk errors I was experiencing. It works
>> perfectly fine when I boot from it as an external hard drive, though,
>> which is what I'm doing now. So, I'm pretty sure I can commit again
>> now. So, OK to commit this patch?
>>
>>>
>>>>
>>>> Thanks,
>>>> Eric
>>>
>>
>

Re: [PING][patch] PR81794: have "would be stringified in traditional C" warning in libcpp/macro.c be controlled by -Wtraditional

[David Malcolm]

On Wed, 2017-10-25 at 14:45 -0400, Eric Gallager wrote:
> On Sat, Sep 30, 2017 at 8:05 PM, Eric Gallager <egall@gwmail.gwu.edu>
> wrote:
> > On Fri, Sep 29, 2017 at 11:15 AM, David Malcolm <dmalcolm@redhat.co
> > m> wrote:
> > > On Sun, 2017-09-17 at 20:00 -0400, Eric Gallager wrote:
> > > > Attached is a version of
> > > > https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00481.html that
> > > > contains
> > > > a combination of both the fix and the testcase update, as
> > > > requested
> > > > in
> > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81794#c2
> > > > 
> > > > I had to use a different computer than I usually use to send
> > > > this
> > > > email, as the hard drive that originally had this patch is
> > > > currently
> > > > unresponsive. Since it's also the one with my ssh keys on it, I
> > > > can't
> > > > commit with it. Sorry if the ChangeLogs get mangled.
> > > 
> > > Thanks for putting this together; sorry about the delay in
> > > reviewing
> > > it.
> > > 
> > > The patch mostly looks good.
> > > 
> > > Did you perform a full bootstrap and run of the testsuite with
> > > this
> > > patch?  If so, it's best to state this in the email, so that we
> > > know
> > > that the patch has survived this level of testing.
> > 
> > Yes, I bootstrapped with it, but I haven't done a full run of the
> > testsuite with it yet; just the one testcase I updated.
> 
> Update: I've now run the testsuite with it; test results are here:
> https://gcc.gnu.org/ml/gcc-testresults/2017-10/msg01751.html
> I'm pretty sure all the FAILs are unrelated to this patch.
> 
> > 
> > > 
> > > Some nits below:
> > > 
> > > > libcpp/ChangeLog:
> > > > 
> > > > 2017-03-24  Eric Gallager  <egall@gwmail.gwu.edu>
> > > > 
> > > >      * macro.c (check_trad_stringification): Have warning be
> > > > controlled by
> > > >      -Wtraditional.
> > > > 
> > > > gcc/testsuite/ChangeLog:
> > > > 
> > > > 2017-09-17  Eric Gallager  <egall@gwmail.gwu.edu>
> > > > 
> > > >     PR preprocessor/81794
> > > >     * gcc.dg/pragma-diag-7.c: Update to include check for
> > > >     stringification.
> > > > 
> > > > On Sat, May 6, 2017 at 11:33 AM, Eric Gallager <egall@gwmail.gw
> > > > u.edu>
> > > > wrote:
> > > > > Pinging this: https://gcc.gnu.org/ml/gcc-patches/2017-03/msg0
> > > > > 1325.h
> > > > > tml
> > > > > 
> > > > > On 3/24/17, Eric Gallager <egall@gwmail.gwu.edu> wrote:
> > > > > > It seemed odd to me that gcc was issuing a warning about
> > > > > > compatibility
> > > > > > with traditional C that I couldn't turn off by
> > > > > > pushing/popping
> > > > > > -Wtraditional over the problem area, so I made the attached
> > > > > > (minor)
> > > > > > patch to fix it. Survives bootstrap, but the only testing
> > > > > > I've
> > > > > > done
> > > > > > with it has been compiling the one file that was giving me
> > > > > > issues
> > > > > > previously, which I'd need to reduce further to turn it
> > > > > > into a
> > > > > > proper
> > > > > > test case.
> > > > > > 
> > > > > > Thanks,
> > > > > > Eric Gallager
> > > > > > 
> > > > > > libcpp/ChangeLog:
> > > > > > 
> > > > > > 2017-03-24  Eric Gallager  <egall@gwmail.gwu.edu>
> > > > > > 
> > > > > >       * macro.c (check_trad_stringification): Have warning
> > > > > > be
> > > > > > controlled by
> > > > > >       -Wtraditional.
> > > > > > 
> > > > > 
> > > > > So I did the reducing I mentioned above and now have a
> > > > > testcase for
> > > > > it; it was pretty similar to the one from here:
> > > > > https://gcc.gnu.org/ml/gcc-patches/2017-03/msg01319.html
> > > > > so I combined them into a single testcase and have attached
> > > > > the
> > > > > combined version. I can confirm that the testcase passes with
> > > > > my
> > > > > patch
> > > > > applied.
> > > 
> > > [...]
> > > 
> > > > diff --git a/gcc/testsuite/gcc.dg/pragma-diag-7.c
> > > > b/gcc/testsuite/gcc.dg/pragma-diag-7.c
> > > > index 402ee56..e06c410 100644
> > > > --- a/gcc/testsuite/gcc.dg/pragma-diag-7.c
> > > > +++ b/gcc/testsuite/gcc.dg/pragma-diag-7.c
> > > > @@ -7,3 +7,16 @@ unsigned long bad = 1UL; /* { dg-warning
> > > > "suffix" } */
> > > >  /* Note the extra space before the pragma on this next line:
> > > > */
> > > >   #pragma GCC diagnostic pop
> > > >  unsigned long ok_again = 2UL; /* { dg-bogus "suffix" } */
> > > > +
> > > > +/* Redundant with the previous pop, but just shows that it
> > > > fails to stop the
> > > > + * following warning with an unpatched GCC: */
> > > > +#pragma GCC diagnostic ignored "-Wtraditional"
> > > > +
> > > > +/* { dg-bogus "would be stringified" .+1 } */
> > > 
> > > As far as I can tell, this dg-bogus line doesn't actually get
> > > matched;
> > > when I run the testsuite without the libcpp fix, I get:
> > > 
> > >   FAIL: gcc.dg/pragma-diag-7.c (test for excess errors)
> > > 
> > > If I update the dg-bogus line to read:
> > > 
> > >   /* { dg-bogus "would be stringified" "" { target *-*-* } .+1 }
> > > */
> > > 
> > > then it's matched, and I get:
> > > 
> > >   FAIL: gcc.dg/pragma-diag-7.c  (test for bogus messages, line
> > > 16)
> > > 
> > > I believe that as written the ".+1" 2nd argument is interpreted
> > > as a
> > > human-readable description of the problem, rather than as a line
> > > offset; I believe you would need to add positional args for the
> > > description and filter so that the line offset is argument 4.
> > > 
> > > That said, I think the dg-bogus here is unnecessary: if the
> > > warning is
> > > erroneously emitted, we get:
> > > 
> > >   FAIL: gcc.dg/pragma-diag-7.c (test for excess errors)
> > > 
> > > (where "errors" really means "excess errors, warnings and
> > > extraneous
> > > gunk that isn't a note").
> > > 
> > > So this testcase hunk is good without the dg-bogus line.
> > 
> > OK, the line can be removed when committing.
> > 
> > > 
> > > > +#define UNW_DEC_PROLOGUE(fmt, body, rlen, arg) \
> > > > +  do
> > > > {                                                              
> > > >          \
> > > > +      unw_rlen =
> > > > rlen;                                                       \
> > > > +      *(int *)arg =
> > > > body;                                            \
> > > > +      printf("    %s:%s(rlen=%lu)\n",                         
> > > >                \
> > > > +             fmt, (body ? "body" : "prologue"), (unsigned
> > > > long)rlen);        \
> > > > +  } while (0)
> > > > diff --git a/libcpp/macro.c b/libcpp/macro.c
> > > > index de18c22..71363b5 100644
> > > > --- a/libcpp/macro.c
> > > > +++ b/libcpp/macro.c
> > > > @@ -3316,7 +3316,7 @@ check_trad_stringification (cpp_reader
> > > > *pfile, const cpp_macro *macro,
> > > >         if (NODE_LEN (node) == len
> > > >             && !memcmp (p, NODE_NAME (node), len))
> > > >           {
> > > > -           cpp_error (pfile, CPP_DL_WARNING,
> > > > +           cpp_warning (pfile, CPP_W_TRADITIONAL,
> > > >          "macro argument \"%s\" would be stringified in
> > > > traditional C",
> > > >                        NODE_NAME (node));
> > > >             break;
> > > 
> > > This hunk looks good.
> > > 
> > > So the patch is good if you drop the bogus "dg-bogus" line,
> > > provided
> > > that it's bootstrapped and regression-tested.
> > > 
> > > Did you complete the FSF copyright assignment paperwork, and do
> > > you
> > > have access to the GCC compile farm? (that could help with
> > > testing)
> > 
> > Yes, I have a copyright assignment on file (it was a prerequisite
> > for
> > putting my name in the MAINTAINERS file), but no, I don't think I
> > have
> > access to the GCC compile farm. I agree, it'd be much better to
> > test
> > on the compile farm than on my own computer, since running the
> > testsuite on my own computer usually takes an entire day (it's
> > really
> > old and slow).
> 
> Update: I now have access to the GCC compile farm; that's where I
> produced the test results linked above.
> 
> > 
> > > 
> > > Thanks again for working on this (and for the work you've been
> > > doing in
> > > Bugzilla); I hope you get your hard drive back...
> > > 
> > > Dave
> > 
> > And thank you for all your work on improving gcc's diagnostics! If
> > you'd like to send me a new hard drive, feel free to contact me
> > off-list for my mailing address.
> 
> Update on hard drive status: I've ordered and received a new one, I
> just need to salvage data off the old one and then install the new
> one
> and then I should be good for committing again. I'd still appreciate
> if someone else could commit for me while I'm still working on fixing
> it though.
> 
> > 
> > Thanks,
> > Eric

Sorry about the delay in dealing with this.

I verified the patch (bootstrap and regression testing), and there were
no regressions against a "control" build (on x86_64-pc-linux-gnu) [1]. 
I decided to add the fixed version of the "dg-bogus", as I felt it
helped document the intent of the test case.

I've committed it to trunk on your behalf as r254981.

Dave

[1] FWIW the script I use for this is here:
  https://github.com/davidmalcolm/gcc-build
which uses this comparison script:
  https://github.com/davidmalcolm/jamais-vu
Takes a few hours and a lot of disk space.

Re: [PING][patch] PR81794: have "would be stringified in traditional C" warning in libcpp/macro.c be controlled by -Wtraditional

[Bernhard Reutner-Fischer]

On Mon, Nov 20, 2017 at 08:03:23PM -0500, David Malcolm wrote:
 
> [1] FWIW the script I use for this is here:
>   https://github.com/davidmalcolm/gcc-build

NUM_CORES=$(getconf _NPROCESSORS_ONLN || echo 1)

You usually don't want to count offline processors.

and in create_src, instead of fiddling you user settings per tree copy i'd
cat >> ~/.gitconfig <<EOF

[includeIf "gitdir:gcc*"]
    path = ~/.gitconfig.gcc
EOF

and set user.email therein once.

an re: the in-tree isl
---8<---
# would have --with-isl=$DEPDIR
# but conmfigure fails; the embedded isl in cloog seems to return
# isl_version() of "UNKNOWN"
--8<---

Yea, I ran into this too, see
https://gcc.gnu.org/ml/gcc-patches/2015-04/msg00021.html
https://gcc.gnu.org/ml/gcc-patches/2015-04/msg00024.html

In the end it was April 1st and it was not funny enough to complete so i
just gave up installed it separately ;)

HTH,