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. > > Thanks, > Eric
