bug-bison
[Top][All Lists]
Advanced

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

Re: data/variant.hh relies on undefined behaviour


From: Akim Demaille
Subject: Re: data/variant.hh relies on undefined behaviour
Date: Wed, 7 Jan 2015 15:38:26 +0100

Hi Thomas,

> Le 12 mars 2014 à 14:54, Thomas Jahns <address@hidden> a écrit :
> 
> Hello,
> 
> I'm trying to build bison-3.0.2 on IBM AIX 6.1 with xlc/xlC 11.1.0.8 (I've 
> also
> tried later versions) but I'm running into problems on "make check".
> 
> If I understand the code generated from variant.hh correctly it assumes that 
> for
> 
> someClass a;
> 
> typeid(a).name() == typeid(a).name()
> 
> Because it stores the result of calling name() in the yytname_ member.
> Unfortunately this assumption is incorrect

Bummer...  Yes, of course.

> and only
> 
> typeid(a) == typeid(a)
> 
> is strictly conformant. Which I wouldn't care about, but IBM xlC does generate
> different results for calling name() and therefore the calc++ test fails on 
> me.
> 
> Given the above I guess the cleanest solution would be to store the result of
> typeid as yytname_ and use typeid((void *)0) or similar instead of YY_NULLPTR.
> Asserting strcmp(yytname_, other.yytname_) == 0 instead of comparing the
> pointers would probably also work.

Actually I think the safest approach is to store pointers
to the type_info structure, we allows to use nullptr.  And
then to dereference to compare them.  That's roughly the
same as using type_index, but sticking to C++98.

I'm about to push the following commit for 3.0.3 (the "maint"
branch).  WDYT?

Thanks (both late, and in advance :).

commit 7cf84b13a079740a638fed6b46634bc888bd2622
Author: Akim Demaille <address@hidden>
Date:   Wed Jan 7 10:24:53 2015 +0100

    c++: variants: comparing addresses of typeid.name() is undefined
    
    Instead of storing and comparing pointers to names of types, store
    pointers to the typeids, and compares the typeids.
    Reported by Thomas Jahns.
    <http://lists.gnu.org/archive/html/bug-bison/2014-03/msg00001.html>
    
    * data/variant.hh (yytname_): Replace with...
    (yytypeid_): this.

diff --git a/THANKS b/THANKS
index 2436f00..908498c 100644
--- a/THANKS
+++ b/THANKS
@@ -132,6 +132,7 @@ Steve Murphy              address@hidden
 Sum Wu                    address@hidden
 Théophile Ranquet         address@hidden
 Thiru Ramakrishnan        address@hidden
+Thomas Jahns              address@hidden
 Tim Josling               address@hidden
 Tim Landscheidt           address@hidden
 Tim Van Holder            address@hidden
diff --git a/data/variant.hh b/data/variant.hh
index 78c2f94..f918e34 100644
--- a/data/variant.hh
+++ b/data/variant.hh
@@ -95,13 +95,13 @@ m4_define([b4_variant_define],
 
     /// Empty construction.
     variant ()]b4_parse_assert_if([
-      : yytname_ (YY_NULLPTR)])[
+      : yytypeid_ (YY_NULLPTR)])[
     {}
 
     /// Construct and fill.
     template <typename T>
     variant (const T& t)]b4_parse_assert_if([
-      : yytname_ (typeid (T).name ())])[
+      : yytypeid_ (&typeid (T))])[
     {
       YYASSERT (sizeof (T) <= S);
       new (yyas_<T> ()) T (t);
@@ -110,7 +110,7 @@ m4_define([b4_variant_define],
     /// Destruction, allowed only if empty.
     ~variant ()
     {]b4_parse_assert_if([
-      YYASSERT (!yytname_);
+      YYASSERT (!yytypeid_);
     ])[}
 
     /// Instantiate an empty \a T in here.
@@ -118,9 +118,9 @@ m4_define([b4_variant_define],
     T&
     build ()
     {]b4_parse_assert_if([
-      YYASSERT (!yytname_);
+      YYASSERT (!yytypeid_);
       YYASSERT (sizeof (T) <= S);
-      yytname_ = typeid (T).name ();])[
+      yytypeid_ = & typeid (T);])[
       return *new (yyas_<T> ()) T;
     }
 
@@ -129,9 +129,9 @@ m4_define([b4_variant_define],
     T&
     build (const T& t)
     {]b4_parse_assert_if([
-      YYASSERT (!yytname_);
+      YYASSERT (!yytypeid_);
       YYASSERT (sizeof (T) <= S);
-      yytname_ = typeid (T).name ();])[
+      yytypeid_ = & typeid (T);])[
       return *new (yyas_<T> ()) T (t);
     }
 
@@ -140,7 +140,7 @@ m4_define([b4_variant_define],
     T&
     as ()
     {]b4_parse_assert_if([
-      YYASSERT (yytname_ == typeid (T).name ());
+      YYASSERT (*yytypeid_ == typeid (T));
       YYASSERT (sizeof (T) <= S);])[
       return *yyas_<T> ();
     }
@@ -150,7 +150,7 @@ m4_define([b4_variant_define],
     const T&
     as () const
     {]b4_parse_assert_if([
-      YYASSERT (yytname_ == typeid (T).name ());
+      YYASSERT (*yytypeid_ == typeid (T));
       YYASSERT (sizeof (T) <= S);])[
       return *yyas_<T> ();
     }
@@ -167,8 +167,8 @@ m4_define([b4_variant_define],
     void
     swap (self_type& other)
     {]b4_parse_assert_if([
-      YYASSERT (yytname_);
-      YYASSERT (yytname_ == other.yytname_);])[
+      YYASSERT (yytypeid_);
+      YYASSERT (*yytypeid_ == *other.yytypeid_);])[
       std::swap (as<T> (), other.as<T> ());
     }
 
@@ -198,7 +198,7 @@ m4_define([b4_variant_define],
     destroy ()
     {
       as<T> ().~T ();]b4_parse_assert_if([
-      yytname_ = YY_NULLPTR;])[
+      yytypeid_ = YY_NULLPTR;])[
     }
 
   private:
@@ -233,7 +233,7 @@ m4_define([b4_variant_define],
     } yybuffer_;]b4_parse_assert_if([
 
     /// Whether the content is built: if defined, the name of the stored type.
-    const char *yytname_;])[
+    const std::type_info *yytypeid_;])[
   };
 ]])
 





reply via email to

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