1626149 - Use PlainOldDataSerializer again for some types

Status: RESOLVED INVALID

(Core :: IPC, enhancement)

Description

[Masatoshi Kimura [:emk]]

Bug 1620400 had to stop using PlainOldDataSerializer for some types because Maybe was not trivially copyable.

But bug 1613363 made Maybe<trivially copyable type> trivially copyable. So we can use PlainOldDataSerializer again now.

Comment 1

[Masatoshi Kimura [:emk]]

Ah, is_trivially_copyable != is_trivially_copy_constructible. PlainOldDataSerializer requires the prior.

Comment 2

[Simon Giesecke [:sg] [he/him]]

PlainOldDataSerializer checks for is_trivially_copyable, but I think is_trivially_destructible plus is_trivially_copy_constructible (and maybe is_trivially_copy_assignable) is sufficient for its purposes, so maybe the check should be relaxed. The reason Maybe<trivially copyable type> is not trivially copyable itself is that its move operations are not trivial, to guarantee a moved-from Maybe is always empty/nothing. (Though I am not sure if this guarantee really should be given, I didn't implement it like that in order to preserve the existing behaviour)

Comment 3

[Masatoshi Kimura [:emk]]

No, is_trivially_copy_constructible is not sufficient. The C++ spec requires is_trivially_copyable.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

I don't have any opinion here, will defer to the more knowledgeable C++ hackers.

Comment 5

[Masatoshi Kimura [:emk]]

Why I can't access bug 1620719 (7935cf13d83f) and bug 1613009 (ead2e7c7696c and e798886970d1)? I thought that they are security sensitive.

Comment 6

[Botond Ballo [:botond]]

(In reply to Masatoshi Kimura [:emk] from comment #5)

Why I can't access bug 1620719 (7935cf13d83f) and bug 1613009 (ead2e7c7696c and e798886970d1)? I thought that they are security sensitive.

Yep. I cc'ed you on them.

Comment 7

[Botond Ballo [:botond]]

A few thoughts here:

• C++ does indeed require is_trivially_copyable to copy the bytes of an object around. The requirement is stated here.
• It's not clear to me why this stricter requirement is in place. The more relaxed requirement Simon suggests in comment 2 seems sensible to me.
• std::optional<T> made the design choice that if you move from a populated optional, the moved-from optional remains populated (with its contained object in a moved-from state) rather than empty. As a result, std::optional<T> can be implemented in a way that it is trivially copyable if T is.