variants: avoid type punning issue

This is based on what is recommended by both Scott Meyers, in 'Effective
C++', and Andrei Alexandrescu and Herb Sutter in 'C++ Coding Standards'.

Use a static_cast on void* rather than directly use a reinterpret_cast,
which can have nefarious effects on objects.  However, even though following
this guideline is good practice in general, I am not quite sure how relevant
it is when applied to conversions from POD to objects.  Actually, it might
very well be the opposite: isn't this exactly what reinterpret_cast is for?
What we really want *is* to transmit the memory map as a series of bytes,
which, if I am correct, falls into the kind of "low level" hack for which
this cast is meant.

In any case, this silences the warning, which will be greatly appreciated by
anyone using variants with a compiler supporting -fstrict-aliasing.

* data/variant.hh (as): Here.
* tests/c++.at (Exception safety, C++ Variant-based Symbols, Variants):
Don't use NO_STRICT_ALIAS_CXXFLAGS (revert commit ddb9db15), as type punning
is no longer an issue.
* tests/atlocal.in, configure.ac (NO_STRICT_ALIAS_CXXFLAGS): Remove
definition.
* examples/local.mk (NO_STRICT_ALIAS_CXXFLAGS): Remove from AM_CXXFLAGS.
* doc/bison.texi: Don't mention type punning issues.

diff --git a/configure.ac b/configure.ac
index f10dabc1ea9fc7daefc1ecbf5d9c10d656dcf82c..05a8d9209523a28c616668194bd6e156c29013d9 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -144,8 +144,6 @@ if test "$enable_gcc_warnings" = yes; then
   done
   # Clang++ 3.2+ reject C code generated by Flex.
   gl_WARN_ADD([-Wno-null-conversion], [WARN_NO_NULL_CONVERSION_CXXFLAGS])
-  # Variants break strict aliasing analysis.
-  gl_WARN_ADD([-fno-strict-aliasing], [NO_STRICT_ALIAS_CXXFLAGS])
   CXXFLAGS=$save_CXXFLAGS
   AC_LANG_POP([C++])
 fi

diff --git a/data/variant.hh b/data/variant.hh
index 047e641ba7ae37d96d848f152d598d81d9b4be6d..e2a537ad3d694d7e0f19bda3f63c48e2596b9067 100644 (file)
--- a/data/variant.hh
+++ b/data/variant.hh
@@ -142,7 +142,10 @@ m4_define([b4_variant_define],
     {]b4_parse_assert_if([
       YYASSERT (tname == typeid (T).name ());
       YYASSERT (sizeof (T) <= S);])[
-      return reinterpret_cast<T&> (buffer.raw);
+      {
+        void *dummy = buffer.raw;
+        return *static_cast<T*> (dummy);
+      }
     }
 
     /// Const accessor to a built \a T (for %printer).
@@ -152,7 +155,10 @@ m4_define([b4_variant_define],
     {]b4_parse_assert_if([
       YYASSERT (tname == typeid (T).name ());
       YYASSERT (sizeof (T) <= S);])[
-      return reinterpret_cast<const T&> (buffer.raw);
+      {
+        const void *dummy = buffer.raw;
+        return *static_cast<const T*> (dummy);
+      }
     }
 
     /// Swap the content with \a other, of same type.

diff --git a/doc/bison.texi b/doc/bison.texi
index 7a36f854e2794e8a846f8141a9928f3afef5d587..1218b5837165e37dc78715eb6864a49375880642 100644 (file)
--- a/doc/bison.texi
+++ b/doc/bison.texi
@@ -10268,14 +10268,6 @@ therefore, since, as far as we know, @code{double} is the most demanding
 type on all platforms, alignments are enforced for @code{double} whatever
 types are actually used.  This may waste space in some cases.
 
-@item
-Our implementation is not conforming with strict aliasing rules.  Alias
-analysis is a technique used in optimizing compilers to detect when two
-pointers are disjoint (they cannot ``meet'').  Our implementation breaks
-some of the rules that G++ 4.4 uses in its alias analysis, so @emph{strict
-alias analysis must be disabled}.  Use the option
-@option{-fno-strict-aliasing} to compile the generated parser.
-
 @item
 There might be portability issues we are not aware of.
 @end itemize

diff --git a/examples/local.mk b/examples/local.mk
index 3185976ac21379a3c73abb05b6d65b3a7fb97591..05e28e1b9fe8966ffe27f42672cc9be917cccd21 100644 (file)
--- a/examples/local.mk
+++ b/examples/local.mk
@@ -17,7 +17,6 @@ dist_noinst_SCRIPTS = examples/extexi examples/test
 TEST_LOG_COMPILER = $(top_srcdir)/examples/test
 
 AM_CXXFLAGS =                                                  \
-  $(NO_STRICT_ALIAS_CXXFLAGS)                                  \
   $(WARN_CXXFLAGS) $(WARN_CXXFLAGS_TEST) $(WERROR_CXXFLAGS)
 
 ## ------------ ##

diff --git a/tests/atlocal.in b/tests/atlocal.in
index 439a2615acc91d75c91e13a3f3e462eec0ac14e1..3a9f873b24f6e4372ecd618a0d7b4aa1a45e0d1c 100644 (file)
--- a/tests/atlocal.in
+++ b/tests/atlocal.in
@@ -47,9 +47,6 @@ NO_WERROR_CXXFLAGS='@CXXFLAGS@ @WARN_CXXFLAGS@ @WARN_CXXFLAGS_TEST@'
   CFLAGS="$NO_WERROR_CFLAGS   @WERROR_CFLAGS@"
 CXXFLAGS="$NO_WERROR_CXXFLAGS @WERROR_CXXFLAGS@"
 
-# C++ variants break strict aliasing analysis.
-NO_STRICT_ALIAS_CXXFLAGS='@NO_STRICT_ALIAS_CXXFLAGS@'
-
 # If 'exit 77'; skip all C++ tests; otherwise ':'.
 BISON_CXX_WORKS='@BISON_CXX_WORKS@'
 

diff --git a/tests/c++.at b/tests/c++.at
index a21fb4eb2f736afef8671ed1128da183d92ff8ae..d36e3225d74f530e028315109a8e9b9a3a5101d2 100644 (file)
--- a/tests/c++.at
+++ b/tests/c++.at
@@ -93,7 +93,7 @@ int main()
 ]])
 
 AT_BISON_CHECK([-o list.cc list.yy])
-AT_COMPILE_CXX([list], [$NO_STRICT_ALIAS_CXXFLAGS list.cc])
+AT_COMPILE_CXX([list], [list.cc])
 AT_PARSER_CHECK([./list], 0, [],
 [12
 123
@@ -251,7 +251,7 @@ namespace yy
 ]])
 
 AT_BISON_CHECK([-o list.cc list.yy])
-AT_COMPILE_CXX([list], [$NO_STRICT_ALIAS_CXXFLAGS list.cc])
+AT_COMPILE_CXX([list], [list.cc])
 AT_PARSER_CHECK([./list], 0,
 [(0, 1, 2, 4)
 ])
@@ -804,7 +804,7 @@ main (int argc, const char *argv[])
 }
 ]])
 AT_BISON_CHECK([[-o input.cc --report=all input.yy]])
-AT_COMPILE_CXX([input], [[$NO_STRICT_ALIAS_CXXFLAGS input.cc]])
+AT_COMPILE_CXX([[input]])
 
 AT_PARSER_CHECK([[./input aaaas]], [[2]], [[]],
 [[exception caught: reduction