Ping yet again: https://gcc.gnu.org/ml/gcc-patches/2017-11/msg00123.html
On 11/9/17, Eric Gallager <eg...@gwmail.gwu.edu> wrote: > Ping again: https://gcc.gnu.org/ml/gcc-patches/2017-11/msg00123.html > > On 11/2/17, Eric Gallager <eg...@gwmail.gwu.edu> wrote: >> 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 >>> >> >
