-Wclass-memaccess warning should be in -Wextra, not -Wall

[Marc Glisse]

On Sun, 8 Jul 2018, Jason Merrill wrote:

> On Sun, Jul 8, 2018 at 6:40 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>> On Fri, 6 Jul 2018, Martin Sebor wrote:
>>> On 07/05/2018 05:14 PM, Soul Studios wrote:
>>>>
>>>> Simply because a struct has a constructor does not mean it isn't a
>>>> viable target/source for use with memcpy/memmove/memset.
>>>
>>>
>>> As the documentation that Segher quoted explains, it does
>>> mean exactly that.
>>>
>>> Some classes have user-defined copy and default ctors with
>>> the same effect as memcpy/memset.  In modern C++ those ctors
>>> should be defaulted (= default) and GCC should emit optimal
>>> code for them.
>>
>> What if I want to memcpy a std::pair<int,int>?
>
> That's fine, since the pair copy constructor is defaulted, and trivial
> for pair<int,int>.

G++ does currently warn for

#include <cstring>
#include <utility>
typedef std::pair<int,int> P;
void f(P*d, P const*s){ std::memcpy(d,s,sizeof(P)); }

because copy-assignment is not trivial.

IIRC std::pair and std::tuple are not as trivial as they could be for ABI 
reasons.

Boost.Container chose to disable the warning ( 
https://github.com/boostorg/container/commit/62a8b6666eb0f12242fb1e99daa98533ce74e735 
) instead of making their version of pair trivial. I don't know why, but 
maybe that was to avoid a mess of #ifdef to maintain a C++03 version of 
the code.

>> Some classes may have several states, some that are memcpy-safe, and some
>> that are not. A user may know that at some point in their program, all the
>> objects in a given array are safe, and want to memcpy the whole array
>> somewhere.
>
> The user may know that, but the language only defines the semantics of
> memcpy for trivially copyable classes.  If you want to assume that the
> compiler will do what you expect with this instance of undefined
> behavior, you can turn off the warning.  You may well be right, but I
> don't think it follows that putting this warning about undefined
> behavior in -Wall is wrong.

(note that I am not the original reporter, I am only trying to help find 
examples)
I don't mind the warning so much, I am more scared of the optimizations 
that may follow.

>> memcpy can also be used to work around the lack of a destructive move in
>> C++.
>
> I wonder what you mean by "the lack of a destructive move in C++",
> given that much of C++11 was about supporting destructive move
> semantics.

There is a misunderstanding here. C++11 added move semantics that one 
might call "conservative", i.e. the moved-from object is still alive and 
one should eventually run its destructor. "destructive move" is used in 
some papers / blogs to refer to a move that also destructs the original 
object. For some types that are not trivially default constructible (like 
libstdc++'s std::deque IIRC), a conservative move is still expensive while 
a destructive move is trivial (memcpy). Libstdc++'s std::string is one of 
the rare types that are not trivially destructively movable (it can 
contain a pointer to itself). Most operations in std::vector<V> could use 
a destructive move of V very naturally.

The denomination conservative/destructive is certainly not canonical, I 
don't know if there are better words to describe it.

>> For instance, vector<vector<T>>::resize could safely use memcpy (and
>> skip destroy before deallocate). In this particular case, we could imagine
>> at some point in the future that the compiler would notice it is equivalent
>> to memcpy+bzero, and then that the bzero is dead, but there are more
>> complicated use cases for destructive move.
>
> Indeed, resizing a vector<vector<T>> will loop over the outer vector
> calling the move constructor for each inner vector, which will copy
> the pointer and zero out the moved-from object, which the optimizer
> could then coalesce into memcpy/bzero.  This sort of pattern is common
> enough in C++11 containers that this seems like an attractive
> optimization, if we don't already perform it.
>
> What more complicated uses don't reduce to memcpy/bzero, but you would
> still want to use memcpy for somehow?

Noticing that it reduces to memcpy can be hard. For std::deque, you have 
to cancel a new/delete pair (which we still do not handle), and for that 
you may first need some loop fusion to put the new and delete next to each 
other. For GMP's mpz_class, the allocation is hidden in opaque mpz_init / 
mpz_clear functions, so the compiler cannot simplify move+destruct into 
memcpy.

I would certainly welcome optimizer improvements that make it less useful 
to specialize the library, but some things are easier to do at the level 
of the library.

-- 
Marc Glisse