Re: [PATCH] variants: avoid type punning issue.

[Akim Demaille]

Le 31 janv. 2013 à 17:23, Théophile Ranquet <address@hidden> a écrit :

> 2013/1/30 Akim Demaille <address@hidden>:
>> 
>> Le 29 janv. 2013 à 22:59, Theophile Ranquet <address@hidden> a écrit :
>> 
>>> This is based on what Scott Meyers recommends in 'Effective C++'.
>> 
>> Actually it's Alexandrescu and Sutter in "C++ Coding Standards"
>> that I showed to you :)
> 
> Oh, right, but both are true. Should I reference both? I do believe
> "Effective C++" outdates "C++ Coding Standards".

As you wish.

> 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 e2a537a..78aa876 100644
> --- a/data/variant.hh
> +++ b/data/variant.hh
> @@ -143,7 +143,7 @@ m4_define([b4_variant_define],
>      YYASSERT (tname == typeid (T).name ());
>      YYASSERT (sizeof (T) <= S);])[
>      {
> -        void *dummy = buffer.raw;
> +        void* dummy = buffer.raw;

Actually it was right in the first place.  This is the style
promoted by the GNU Coding Standards.

>        return *static_cast<T*> (dummy);
>      }
>    }
> @@ -156,7 +156,7 @@ m4_define([b4_variant_define],
>      YYASSERT (tname == typeid (T).name ());
>      YYASSERT (sizeof (T) <= S);])[
>      {
> -        const void *dummy = buffer.raw;
> +        const void* dummy = buffer.raw;
>        return *static_cast<const T*> (dummy);
>      }
>    }
> diff --git a/doc/bison.texi b/doc/bison.texi
> index d2d3da3..4046b69 100644
> --- a/doc/bison.texi
> +++ b/doc/bison.texi
> @@ -10208,14 +10208,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
> address@hidden to compile the generated parser.
> -
> address@hidden
> 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 874d640..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])

So I believe both can be AT_FULL_COMPILE now.

Fine job, push whenever you want.

> AT_PARSER_CHECK([./list], 0,
> [(0, 1, 2, 4)
> ])