saleyn / eixx

Erlang C++ Interface
Apache License 2.0
137 stars 26 forks source link

static assertion failed: sizeof(eterm) == 2*sizeof(uint64_t) #15

Closed matwey closed 8 years ago

matwey commented 8 years ago

Hello,

I am trying to compile eixx and see the following:

g++ -DHAVE_CONFIG_H -I. -I..  -g -O0 -I./../include -pthread -I/usr/include -I /usr/lib/erlang/lib/erl_interface-3.7.17/include   -MMD -Wall -Wno-unused-local-typedefs -fno-strict-aliasing -std=c++11 -DBOOST_SYSTEM_NO_DEPRECATED=1 -MT test_node-test_node.o -MD -MP -MF .deps/test_node-test_node.Tpo -c -o test_node-test_node.o `test -f 'test_node.cpp' || echo './'`test_node.cpp
In file included from /usr/include/boost/spirit/home/support/detail/endian/endian.hpp:45:0,
                 from /usr/include/boost/spirit/home/support/detail/endian.hpp:26,
                 from ./../include/eixx/marshal/endian.hpp:35,
                 from ./../include/eixx/marshal/atom.hpp:40,
                 from ./../include/eixx/marshal/eterm.hpp:45,
                 from ./../include/eixx/eterm.hpp:37,
                 from ./../include/eixx/eixx.hpp:65,
                 from test_node.cpp:2:
./../include/eixx/eterm.hpp:57:5: error: static assertion failed: sizeof(eterm) == 2*sizeof(uint64_t)
     BOOST_STATIC_ASSERT(sizeof(eterm)     == 2*sizeof(uint64_t));
     ^

I use gcc 4.8.3, and boost 1.58, my system arch is x86 (32-bit).

matwey commented 8 years ago

I suppose

enum eterm_type {

should be

class enum eterm_type: std::unit64_t {

or something like that.

saleyn commented 8 years ago

Never tested this project on 32bit platforms as this wasn't something I ever needed. You can try to patch it if it works. In your suggestion above it should be "enum eterm_type: uint64_t", so that it doesn't enforce type prefixing, but eterm_type must be 4 bytes long (or else var type won't work).

matwey commented 8 years ago

Is it really required that sizeof(eterm)==16? Maybe you meant sizeof(eterm)<=16?

matwey commented 8 years ago

https://github.com/saleyn/eixx/blob/master/include/eixx/marshal/var.hpp#L55 here you suppose that sizeof(eterm_type)<=4, because there is assert that sizeof(var)==8

saleyn commented 8 years ago

The eterm contains a union of all supported types including double and pointer, and the eterm_type indicating what type is stored. So the size of eterm is max(sizeof(double),sizeof(void*)) + aligned_size_of(eterm_type)), which is 8+4(alignment)+4.

Apparently the size of eterm_type is 4, and the size of var is 4 + sizeof(atom), which is 8.

matwey commented 8 years ago

Sure, but aligning is platform dependent. I have not yet checked on ARMv7, but on i586 sizeof(eterm) is 12.

matwey commented 8 years ago

At armv7hl sizeof(eterm)==16

saleyn commented 8 years ago

The main design assumption was that eterm's storage can hold primitive types and reference types without sacrificing performance. I believe it should work on 32bit as well as 64bit architectures, in cases when sizeof(eterm) == 12, and 16, though as previously indicated was only concerned with a 64bit architecture. So I welcome the patches as long as they don't modify the 64bit performance/behavior.

matwey commented 8 years ago

Please, check new version of PR #18. Would you fine with:

BOOST_STATIC_ASSERT(sizeof(eterm)     == (EIXX_ALIGNOF_UINT64_T > sizeof(int) ? EIXX_ALIGNOF_UINT64_T : sizeof(int)) + sizeof(uint64_t)); 

?

However, you already check in eterm that your union is 16-bytes long, so the rest is determined by compiler/architecture and depends on the size of enum and alignment rules. I would think that checking sizeof(eterm) is redundant.

saleyn commented 8 years ago

yes, that's fine.