From be6fa942acae21a4a025bac5e339451be6ad136d Mon Sep 17 00:00:00 2001 From: Theophile Ranquet Date: Tue, 29 Jan 2013 22:35:04 +0100 Subject: [PATCH] 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. --- configure.ac | 2 -- data/variant.hh | 10 ++++++++-- doc/bison.texi | 8 -------- examples/local.mk | 1 - tests/atlocal.in | 3 --- tests/c++.at | 6 +++--- 6 files changed, 11 insertions(+), 19 deletions(-) diff --git a/configure.ac b/configure.ac index f10dabc..05a8d92 100644 --- 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 047e641..e2a537a 100644 --- 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 (buffer.raw); + { + void *dummy = buffer.raw; + return *static_cast (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 (buffer.raw); + { + const void *dummy = buffer.raw; + return *static_cast (dummy); + } } /// Swap the content with \a other, of same type. diff --git a/doc/bison.texi b/doc/bison.texi index 7a36f85..1218b58 100644 --- a/doc/bison.texi +++ b/doc/bison.texi @@ -10269,14 +10269,6 @@ 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 3185976..05e28e1 100644 --- 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 439a261..3a9f873 100644 --- 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 a21fb4e..d36e322 100644 --- 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 -- 2.7.4