Re: Bison 3.2.90 released [beta]

[Derek Clegg]

Thanks for this fix. Minor: I don’t usually see the “goto x; x:” idiom as a way 
to “fall through” a case statement. I would probably keep the /*Fallthrough*/ 
comment (or add some equivalent comment) with an eye toward making this clear 
what’s going on.

In any case, thanks!
Derek

> On Jan 14, 2019, at 10:46 PM, Akim Demaille <address@hidden> wrote:
> 
> 
> 
>> Le 13 janv. 2019 à 22:20, Derek Clegg <address@hidden> a écrit :
>> 
>> Here’s another one:
>> 
>> aux/parser.cc:228:13: error: unannotated fall-through between switch labels
>>     [-Werror,-Wimplicit-fallthrough]
>>           default:
>>           ^
>> aux/parser.cc:228:13: note: insert '[[fallthrough]];' to silence this warning
>>           default:
>>           ^
>>           [[fallthrough]]; 
>> aux/parser.cc:228:13: note: insert 'break;' to avoid fall-through
>>           default:
>>           ^
>>           break; 
>> 1 error generated.
>> 
>> I fixed this like so:
>> 
>> diff -ur bison-3.2.90/data/skeletons/lalr1.cc bison/data/skeletons/lalr1.cc
>> --- bison-3.2.90/data/skeletons/lalr1.cc        2019-01-10 
>> 21:59:08.000000000 -\
>> 0800
>> +++ bison/data/skeletons/lalr1.cc       2019-01-13 13:15:26.000000000 -0800
>> @@ -533,6 +533,11 @@
>>              if (*++yyp != '\\')
>>                goto do_not_strip_quotes;
>>              // Fall through.
>> +#ifdef __has_feature
>> +#if __has_feature(cxx_attributes)
>> +              [[fallthrough]];
>> +#endif
>> +#endif
>>            default:
>>              yyr += *yyp;
>>              break;
>> 
>> There may be a more canonical way to do this; I don’ know.
> 
> Thanks for the report!  I'll install the following patch, once peer-reviewed 
> by the CI.
> 
> commit 547fe5a0fb50ab5f5a4e576b588fe5064b8002a9
> Author: Akim Demaille <address@hidden>
> Date:   Tue Jan 15 07:43:16 2019 +0100
> 
>    c, c++: avoid implicit fall-throw
> 
>    Reported by Derek Clegg.
>    http://lists.gnu.org/archive/html/bug-bison/2019-01/msg00004.html
> 
>    * configure.ac (warn_common): Add -Wimplicit-fallthrough.
>    This does trigger failures in the test suite.
>    * data/skeletons/glr.c, data/skeletons/lalr1.cc, tests/c++.at:
>    Make fall-throws explicit.
> 
> diff --git a/configure.ac b/configure.ac
> index b7ba45dc..23aeb86d 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -96,7 +96,8 @@ AM_CONDITIONAL([ENABLE_GCC_WARNINGS], [test 
> "$enable_gcc_warnings" = yes])
> if test "$enable_gcc_warnings" = yes; then
>   warn_common='-Wall -Wextra -Wno-sign-compare -Wcast-align
>     -fparse-all-comments -Wdocumentation
> -    -Wformat -Wnull-dereference -Wpointer-arith -Wshadow
> +    -Wformat -Wimplicit-fallthrough -Wnull-dereference
> +    -Wpointer-arith -Wshadow
>     -Wundefined-func-template -Wwrite-strings'
>   warn_c='-Wbad-function-cast -Wstrict-prototypes'
>   warn_cxx='-Wextra-semi -Wnoexcept'
> diff --git a/data/skeletons/glr.c b/data/skeletons/glr.c
> index bc491d51..87f2830b 100644
> --- a/data/skeletons/glr.c
> +++ b/data/skeletons/glr.c
> @@ -590,7 +590,10 @@ yytnamerr (char *yyres, const char *yystr)
>           case '\\':
>             if (*++yyp != '\\')
>               goto do_not_strip_quotes;
> -            /* Fall through.  */
> +            else
> +              goto append;
> +
> +          append:
>           default:
>             if (yyres)
>               yyres[yyn] = *yyp;
> diff --git a/data/skeletons/lalr1.cc b/data/skeletons/lalr1.cc
> index f3ad181c..053ba3be 100644
> --- a/data/skeletons/lalr1.cc
> +++ b/data/skeletons/lalr1.cc
> @@ -532,7 +532,10 @@ m4_if(b4_prefix, [yy], [],
>             case '\\':
>               if (*++yyp != '\\')
>                 goto do_not_strip_quotes;
> -              // Fall through.
> +              else
> +                goto append;
> +
> +            append:
>             default:
>               yyr += *yyp;
>               break;
> diff --git a/tests/c++.at b/tests/c++.at
> index 215c032c..bb1d0dab 100644
> --- a/tests/c++.at
> +++ b/tests/c++.at
> @@ -1242,7 +1242,8 @@ yylex (yy::parser::semantic_type *lvalp)
>   default:
>     lvalp->]AT_VARIANT_IF([build<Object> (res)],
>                           [obj = new Object (res)])[;
> -    // Fall through.
> +    goto zero;
> +  zero:
>   case 0:
>     return res;
>   }
>