[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: |
Wed, 30 Jan 2013 09:05:48 +0100 |
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 :) And there is quite a few threads about
this on StackOverflow.
http://stackoverflow.com/questions/573294/when-to-use-reinterpret-cast
http://stackoverflow.com/questions/1863069/casting-via-void-instead-of-using-reinterpret-cast?lq=1
http://stackoverflow.com/questions/5025843/why-do-we-have-reinterpret-cast-in-c-when-two-chained-static-cast-can-do-its
etc.
> 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.
I agree.
> In any case, this silences the warning, which will be greatly appreciated by
> anyone using variants with a compiler supporting -fstrict-aliasing.
Well, the point is not to silence the warning, but really
to stop worrying the compiler about what we are doing here.
The reinterpret_cast on the raw memory was fine, except
that G++ could lose track of what we were doing with our
object, which is bad. I'm looking for any means to avoid this.
I would have preferred to keep the reinterpret_cast, which is
more faithful here (we are really reading the memory with the
different pattern of interpretation, as you noted above) and
with the GCC attribute "may_alias"
(http://gcc.gnu.org/onlinedocs/gcc-4.7.0/gcc/Type-Attributes.html)
but we failed to use it to fix the issue, right?
> * data/variant.hh (as): Here.
> * tests/c++.at (Exception safety): Revert commit ddb9db15, type punning
> is no longer an issue.
This is not the only place where NO_STRICT_ALIAS_CXXFLAGS is
used: please remove:
- all its uses
- all the code that defines it
- references to this type punning issue in the documentation
> ---
> data/variant.hh | 10 ++++++++--
> tests/c++.at | 2 +-
> 2 files changed, 9 insertions(+), 3 deletions(-)
>
> 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<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/tests/c++.at b/tests/c++.at
> index a21fb4e..874d640 100644
> --- a/tests/c++.at
> +++ b/tests/c++.at
> @@ -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
> --
> 1.8.1.2
>
>