[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] variants: avoid type punning issue.
From: |
Akim Demaille |
Subject: |
Re: [PATCH] variants: avoid type punning issue. |
Date: |
Thu, 31 Jan 2013 20:47:04 +0100 |
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)
> ])