bug-bison
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Akim Demaille
Subject: Re: [Bison-Announce] Bison 3.4.91 released [beta]
Date: Sun, 1 Dec 2019 09:49:21 +0100

Hi Frank,

> Le 1 déc. 2019 à 09:15, Frank Heckenbach <address@hidden> 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 <address@hidden>
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([




reply via email to

[Prev in Thread] Current Thread [Next in Thread]