Variable size field descriptors #13147
Conversation
| _jl_field_accessors(size) | ||
| _jl_field_accessors(isptr) | ||
|
|
||
| static inline uint32_t _jl_fielddesc_size(int8_t fielddesc_type) |
There was a problem hiding this comment.
We should only use jl_ as a prefix, not _jl_.
|
This is great, I think it's better to start with this more minimal subset of the change. The size estimation code is pretty complex and I hadn't finished reviewing it. |
yuyichao
force-pushed
the
yyc/type_size-api
branch
from
cd42c84
to
32b2d86
Compare
|
Done.
Totally agree. |
yuyichao
force-pushed
the
yyc/type_size-api
branch
from
32b2d86
to
9220451
Compare
| @@ -274,13 +283,18 @@ typedef struct _jl_datatype_t { | |||
| uint8_t pointerfree; | |||
| int32_t ninitialized; | |||
| // hidden fields: | |||
| uint8_t fielddesc_type : 2; // 0 -> 8, 1 -> 16, 2 -> 32 | |||
There was a problem hiding this comment.
maybe steal a few bits from the alignment field (which should never need to be larger than a byte) instead of adding another 32-bit word here?
There was a problem hiding this comment.
I'll do this after waiting to see if the AppVeyor failure is repeatable.
yuyichao
force-pushed
the
yyc/type_size-api
branch
from
9220451
to
a0a1946
Compare
Variable size field descriptors
| uint32_t uid; | ||
| void *struct_decl; //llvm::Value* | ||
| void *ditype; // llvm::MDNode* to be used as llvm::DIType(ditype) | ||
| jl_fielddesc_t fields[]; | ||
| union { |
There was a problem hiding this comment.
C++ doesn't like this unknown union
There was a problem hiding this comment.
I guess this union can just be removed since the members are all zero sized.
There was a problem hiding this comment.
it's not valid to have an unsized field that is not at the end of the struct, if that's what you are thinking
There was a problem hiding this comment.
zero-sized arrays are a GCC-extension anyways (https://msdn.microsoft.com/en-us/library/79wf64bc.aspx). ISO C99 requires that you don't specify the size (https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html)
This is the part of #11888 that breaks the C api (but not julia API). We need this (or maybe part of this) before 0.4 final if we want any part of #11888 to be backported to the 0.4 series.