Re: [Bison-Announce] Bison 3.4.91 released [beta]

Akim Demaille <[email protected]>
Newsgroups gmane.comp.parsers.bison.bugs
Message-ID <[email protected]>
Hi Frank,

> Le 1 déc. 2019 à 09:15, Frank Heckenbach <[email protected]> a écrit :
> 
> No sweat about the warnings, but if the empty_state comparison is
> actually wrong (if my test was correct), this might be more urgent.

Yes, I'll have a look at it.  I thought I had started with something
simple, but there are plenty of ramifications all over the place...

> AFAICS, it would then access yystos_[0xff], out of bounds (usually).
> 
>> --- a/data/skeletons/c++.m4
>> +++ b/data/skeletons/c++.m4
>> @@ -545,7 +545,7 @@ m4_define([b4_yytranslate_define],
>>     const int user_token_number_max_ = ]b4_user_token_number_max[;
>>     const token_number_type undef_token_ = ]b4_undef_token_number[;
>> 
>> -    if (static_cast<int> (t) <= yyeof_)
>> +    if (t <= 0)
>>       return yyeof_;
>>     else if (static_cast<int> (t) <= user_token_number_max_)
>>       return translate_table[t];
> 
> You did notice there's another "static_cast<int> (t)" in the
> "else if" line?
> 
> If you can't use "int { t }" as I suggested (doesn't work with older
> C++ standards, does it?), maybe you can use a temporary like
> "int tt = t;" to avoid both casts at once.

Yes, I did notice this cast/warning too.  And it took me a while to
be sure how to address it.  See below.  Cheers!

commit 478cb5cf12fb6ac0f3b34077fed70a29c5eefca8
Author: Akim Demaille <[email protected]>
Date:   Sat Nov 30 13:20:48 2019 +0100

    c++: remove useless cast about user_token_number_max_
    
    Reported by Frank Heckenbach.
    https://lists.gnu.org/archive/html/bug-bison/2019-11/msg00016.html
    
    The cast is needed when yytranslate_'s argument type is token_type,
    i.e., when api.token.constructor is defined.
    
        373. types.at:138: testing lalr1.cc api.value.type=variant api.token.constructor ...
        ======== Testing with C++ standard flags: ''
        ../../tests/types.at:138: bison --color=no -fno-caret  -o test.cc test.y
        ../../tests/types.at:138: $CXX $CXXFLAGS $CPPFLAGS  $LDFLAGS -o test test.cc $LIBS
        stderr:
        test.cc:966:16: error: result of comparison of constant 257 with
                        expression of type 'yy::parser::token_type'
                       (aka 'yy::parser::token::yytokentype') is always true
                       [-Werror,-Wtautological-constant-out-of-range-compare]
            else if (t <= user_token_number_max_)
                     ~ ^  ~~~~~~~~~~~~~~~~~~~~~~
        1 error generated.
    
    It is because it is expected that when api.token.constructor is
    defined, only symbol constructors will be used, that yytranslate_ then
    takes a token_type.  But it is wrong: we still allow literal
    characters in this case, as demonstrated by test 373 for instance.
    
        %define api.value.type variant
        %define api.token.constructor
        %token <std::pair<int, int>> '1' '2';
        [...]
        static yy::parser::symbol_type yylex ()
        {
          static char const input[] = "12";
          int res = input[toknum++];
          typedef yy::parser::symbol_type symbol;
          if (res)
            return symbol (res, std::make_pair (res - '0', res - '0' + 1));
          else
            return symbol (res);
        }
    
    So let yytranslate_ always take an int, which makes the cast truly
    useless.
    
    * data/skeletons/c++.m4, data/skeletons/lalr1.cc (yytranslate_): here.

diff --git a/data/skeletons/c++.m4 b/data/skeletons/c++.m4
index c78ebed6..6e825ff3 100644
--- a/data/skeletons/c++.m4
+++ b/data/skeletons/c++.m4
@@ -529,8 +529,7 @@ m4_define([b4_token_constructor_define], [])
 # sometimes in the cc file.
 m4_define([b4_yytranslate_define],
 [  b4_inline([$1])b4_parser_class[::token_number_type
-  ]b4_parser_class[::yytranslate_ (]b4_token_ctor_if([token_type],
-                                                          [int])[ t)
+  ]b4_parser_class[::yytranslate_ (int t)
   {
 ]b4_api_token_raw_if(
 [[    return static_cast<yy::parser::token_number_type> (t);]],
@@ -546,7 +545,7 @@ m4_define([b4_yytranslate_define],
 
     if (t <= 0)
       return yyeof_;
-    else if (static_cast<int> (t) <= user_token_number_max_)
+    else if (t <= user_token_number_max_)
       return translate_table[t];
     else
       return yy_undef_token_;]])[
diff --git a/data/skeletons/lalr1.cc b/data/skeletons/lalr1.cc
index 5c8a67df..8a2bee04 100644
--- a/data/skeletons/lalr1.cc
+++ b/data/skeletons/lalr1.cc
@@ -268,7 +268,9 @@ m4_define([b4_shared_declarations],
     static const ]b4_int_type(b4_table_ninf, b4_table_ninf)[ yytable_ninf_;
 
     /// Convert a scanner token number \a t to a symbol number.
-    static token_number_type yytranslate_ (]b4_token_ctor_if([token_type], [int])[ t);
+    /// In theory \a t should be a token_type, but character literals
+    /// are valid, yet not member of the token_type enum.
+    static token_number_type yytranslate_ (int t);
 
     // Tables.
 ]b4_parser_tables_declare[]b4_error_verbose_if([
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.