Re: [lmi] Working with and around strict aliasing

[Greg Chicares]

On 2015-09-19 22:36, Vadim Zeitlin wrote:
> On Sat, 19 Sep 2015 15:46:05 +0000 Greg Chicares <address@hidden> wrote:
[...]
>  Actually I'm pretty sure the union trick is safe to use as it's used in
> just too much existing code to be broken now, see e.g. this answer from
> Pete Becker (who is something I'd trust on anything
> C/C++-standard-related): http://stackoverflow.com/a/16637183/15275

There's hardly anyone I'd trust more than him. Yet in that post what he's
really saying AIUI is that the union trick and reinterpret_cast *should* DTRT,
and I already agreed with that. Here, we want to get from "should" to "must".

>  Also, I don't have the C11 standard here, but apparently it enshrined such
> use of the unions and even if it didn't make it into C++14, I hope it will
> happen sooner or later (and definitely by the time of lmi compiler
> upgrade).
> 
>  So the use of the union doesn't really seem wrong to me, it's just that
> memcpy() works even better in our particular case.

And I would suppose that it's easier for a compiler to optimize memcpy();
but, without optimization, memcpy() is probably less efficient.

> GC>  - Use memcpy(). AIUI, this is guaranteed to work.
> 
>  Yes, there is no doubt about this, the only question is whether it works
> as efficiently as the cast or union trick because it's definitely not
> _guaranteed_ to do it. But apparently in practice it does, at least I
> couldn't manage to make any of the compilers I tested (several versions of
> g++, clang 3.7 and msvc 14) generate any calls to memcpy() when they could
> be avoided.
> 
>  In fact, in a rather amusing -- at least if you're not affected by a crash
> due to it in your code -- turn of events, it seems that g++ is now so good
> at detecting memcpy()-like functions in the code that it automatically
> optimizes them to calls to std::memcpy() which are then in turn optimized
> away (see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56888).

That's hilarious.

I wonder whether 'mpatrol' uses the right flags to prevent its instrumented
mem*() from being replaced. But I'm not interested enough to check--I hardly
ever use it, and there are more modern alternatives.

> GC> I tried to come up with another name that describes what it does,
> GC> rather than how it works. "type_punning_cast" seems like an awful name
> GC> because it describes what it does not do. I like "punny_cast", playing
> GC> on the usual retort to an awful pun ("That's not punny!", a pun on 
> "funny")
> GC> but perhaps that's too precious. What do you think of "convert_cast" or
> GC> "type_conversion_cast"?
> 
>  I like them better than my initial idea and the other alternatives but
> IMHO this is still not ideal and I finally settled on something else:
> deserialize_cast().

Voilà le mot juste!

Committed 20150920T1758Z, revision 6292.

> I think it describes exactly what the function does and
> is much more specific than the two latter variants above which seem to
> imply that they can convert between arbitrary types and not just convert
> from "char*"

I was strongly tempted to change that to "unsigned char*", to guard
against trap representations, but exquisite refinements can wait.
I even resisted, for now, the urge to add a static assertion that the
type converted to is a POD. But I did change this, in 'soa_helpers.hpp':

-        rec.index = *reinterpret_cast<boost::int32_t*>(index_record);
+        rec.index = deserialize_cast<boost::int32_t>(index_record);

because of an extremely similar change in 'actuarial_table.cpp':

        int index_table_number =
-            *reinterpret_cast<boost::int32_t*>(index_record)
+            deserialize_cast<boost::int32_t>(index_record)

figuring that if either is problematic, then both are. The question,
if I'm not missing something, is whether or not dereferencing this:
  reinterpret_cast<boost::int32_t*>(index_record)
comes under the shadow of the strict-aliasing prohibition, and I
suppose it's an accident that g++ diagnosed one but not the other.