Re: Add "-Wimplicit-fallthrough" to default flags

From: Andy Fan <zhihui(dot)fan1213(at)gmail(dot)com>
To: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Julien Rouhaud <rjuju123(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Add "-Wimplicit-fallthrough" to default flags
Date: 2020-05-13 08:17:50
Message-ID: CAKU4AWopttFQzHFcz+aZk_2EOVMGa_-12j7RsakHHLJ5bRZdaQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

On Wed, May 13, 2020 at 4:13 PM Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
wrote:

> At Tue, 12 May 2020 17:12:51 -0400, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote in
> > Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> writes:
> > > Fixed one straggler in contrib, and while testing it I realized why
> > > ccache doesn't pay attention to the changes I was doing in the file:
> > > ccache compares the *preprocessed* version of the file and only if that
> > > differs from the version that was cached last, ccache sends the new one
> > > to the compiler; and of course these comments are not present in the
> > > preprocessed version, so changing only the comment accomplishes
> nothing.
> > > You have to touch one byte outside of any comments.
> >
> > Ugh. So the only way ccache could avoid this is to drop the
> > preprocessed-file comparison check if -Wimplicit-fallthrough is on.
> > Doesn't really sound like something we'd want to ask them to do.
> >
> > > I bet this is going to bite someone ... maybe we'd be better off going
> > > all the way to -Wimplicit-fallthrough=5 and use the
> > > __attribute__((fallthrough)) stuff instead.
> >
> > I'm not really in favor of the __attribute__ solution --- seems too
> > gcc-specific. FALLTHROUGH-type comments are understood by other
> > sorts of tools besides gcc.
> >
> > In practice, it doesn't seem like this'll be a huge problem once
> > we're past the initial fixup stage. We can revisit it later if
> > that prediction proves wrong, of course.
>
> FWIW, I got a warning for jsonpath_gram.c.
>
> > jsonpath_gram.c:1026:16: warning: this statement may fall through
> [-Wimplicit-fallthrough=]
> > if (*++yyp != '\\')
> > ^
> > jsonpath_gram.c:1029:11: note: here
> > default:
> > ^~~~~~~
>
> jsonpath_gram.c:1025
> > case '\\':
> > if (*++yyp != '\\')
> > goto do_not_strip_quotes;
> > /* Fall through. */
> > default:
>
> It is generated code by bison.
>
> $ bison --version
> bison (GNU Bison) 3.0.4
>
>
I just found this just serval minutes ago. Upgrading your bison to the
latest
version (3.6) is ok. I'd like we have a better way to share this knowledge
through.
I spend ~30 minutes to troubleshooting this issue.

Best Regards
Andy Fan

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Kyotaro Horiguchi 2020-05-13 10:15:58 Re: Add "-Wimplicit-fallthrough" to default flags
Previous Message Kyotaro Horiguchi 2020-05-13 08:13:07 Re: Add "-Wimplicit-fallthrough" to default flags

Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2020-05-13 08:21:59 Re: SLRU statistics
Previous Message Kyotaro Horiguchi 2020-05-13 08:13:07 Re: Add "-Wimplicit-fallthrough" to default flags