Disallow UPCXX_SERIALIZED_FIELDS on bitfield members
Example code:
#include <upcxx/upcxx.hpp>
#include <iostream>
#include <string>
struct T {
std::string s;
int64_t x :3;
int64_t y :3;
int64_t z :2;
UPCXX_SERIALIZED_FIELDS(s,x,y,z)
};
int main() {
upcxx::init();
upcxx::rpc(0,[](T t) { }, T()).wait();
upcxx::finalize();
return 0;
}
This code currently fails to compile, because our implementation of UPCXX_SERIALIZED_FIELDS
makes a fundamental assumption that it can create (non-const) references to the named fields, however C++ [class.bit] prohibits non-const references to bitfields (and also pointers to bitfield members).
I suspect that it's impossible to portably implement UPCXX_SERIALIZED_FIELDS
to handle bit field members (at least without additional introspection support we don't have in C++11). While I cannot prove that suspicion, our current solution approach definitely won't work for bitfields.
I suggest we update the spec to prohibit bitfield member arguments to UPCXX_SERIALIZED_FIELDS
. Classes such as the one above will need to use one of the other serialization mechanisms.
CC: @Rob Egan
Comments (4)
-
-
I am pretty sure (but short of 100%) based on my knowledge C, that something like the following has sufficiently well-defined semantics be used:
struct T { std::string s; union { struct { int64_t x :3; int64_t y :3; int64_t z :2; } fields; int64_t xyz; } u; UPCXX_SERIALIZED_FIELDS(s, u.xyz) };
If I am correct that this is well-defined, is it worth documenting this somewhere (other than here in the issue) as a viable work around? Of course this does significant "damage" to existing code using the
x
,y
,z
fields, but might be acceptable for classes being developed from scratch for UPC++. -
reporter @Rob Egan said:
A related question for the spec might also mention restrictions on unions and how they are treated.
AFAIK unions should "work" with
UPCXX_SERIALIZED_FIELDS
, but they are probably not what you usually want. In particular, passing more than one union member toUPCXX_SERIALIZED_FIELDS
will result in the overlapping reads for the data being serialized and (more importantly) overlapping writes of different types during deserialization, which could easily result in UB and data corruption. Unions are not "easy mode" serialization and should probably use one of the other more capable serialization mechanisms.@Paul Hargrove said:
is it worth documenting this somewhere (other than here in the issue) as a viable work around?
IMO bitfield members should be specified to not qualify for "easy mode" serialization. This is just one more of four existing restrictions on "easy mode" serialization. IMO the suggested workaround should be the obvious one - use
UPCXX_SERIALIZED_VALUES
or Custom Serialization instead. It's a few more lines of typing, but is transparent to clients of the class. -
- changed status to resolved
Prohibit bit fields in UPCXX_SERIALIZED_FIELDS. Fixes
#175.→ <<cset 93047e9850e6>>
- Log in to comment
I think that is perfectly okay. imo, bitfields only need to be used to compact data members into a standard type and so should composed into structs that are trivial to serialize only.
A related question for the spec might also mention restrictions on unions and how they are treated.