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 >> >
