Ping: https://gcc.gnu.org/ml/gcc-patches/2017-10/msg01834.html
On 10/25/17, Eric Gallager <eg...@gwmail.gwu.edu> wrote: > On Sat, Sep 30, 2017 at 8:05 PM, Eric Gallager <eg...@gwmail.gwu.edu> > wrote: >> On Fri, Sep 29, 2017 at 11:15 AM, David Malcolm <dmalc...@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 <eg...@gwmail.gwu.edu> >>>> >>>> * macro.c (check_trad_stringification): Have warning be >>>> controlled by >>>> -Wtraditional. >>>> >>>> gcc/testsuite/ChangeLog: >>>> >>>> 2017-09-17 Eric Gallager <eg...@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 <eg...@gwmail.gwu.edu> >>>> wrote: >>>> > Pinging this: https://gcc.gnu.org/ml/gcc-patches/2017-03/msg01325.h >>>> > tml >>>> > >>>> > On 3/24/17, Eric Gallager <eg...@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 <eg...@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 >
