-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
types as C-structs [rebased] #10579
types as C-structs [rebased] #10579
Conversation
5239666
to
6792f06
Compare
this makes references to types allocated in Julia recursively forward compatible with C structs
rename jl_typetag_t => jl_taggedvalue_t rename jl_typetagof => jl_astaggedvalue
6792f06
to
cbf9da5
Compare
types as C-structs [rebased]
Does this deserve a NEWS entry? |
Also need to remove the reference to #2818 from the "Calling C and Fortran" docs. |
This utterly broke the windows build. Can we please let CI do its job? |
True – there's not much point in paying AppVeyor for CI as a service if we're going to ignore it. |
@vtjnash or @Keno, how do we deal with this
|
Ah, I was considering fixing that, but couldn't come up with a test case originally. On my phone now, but if someone wants to take a shot at this -- every pool we allocate in the gc should be shifted by (sizeof(jl_typetag_t)%16 bytes) before slicing it up |
AppVeyor's also idle right now so if someone can translate that into a patch it shouldn't take long to check. |
@StefanKarpinski is it possible to change some permissions to let more people cancel appveyor builds? I don't think I can. |
@JeffBezanson you have to log out then log back in as StefanKarpinski. Everyone with commit access should be allowed to do that. |
It's a really odd UI (cc @FeodorFitsner this should probably be made simpler / more obvious) - you log in with your own Github credentials, but then you should see a screen that looks like this: |
I don't have that option, I presume because my GitHub account was renamed and AppVeyor got confused. |
Sure, this will be changed though it's non-trivial change (API is affected) that requires careful planning. Perhaps, will be a good chance to re-write UI on Angular 2.0 and TypeScript :) |
So, aside from the MinGW failure which I was hoping someone could translate from Jameson English into commit-worthy code (I can't...), there's also this: https://msdn.microsoft.com/en-us/library/0scy7z2d.aspx |
i assume you mean that issue comes up when you try to do a msvc compile? you could probably just make this macro define empty: Line 60 in 0d8cec3
it seems that may be a C99 feature |
Will try that. Thanks for fixing the alignment issue. |
That helps a bit. How about this line Line 86 in efb878e
I've also been trying with the VS 2015 rc's that are available on AppVeyor, and it doesn't help fix these. |
I don't think the size-zero array is a C99 thing, it looks from https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html like it's a GNU extension and the MSVC error is strictly speaking true according to the C standard. Someone should check whether ICC allows this construct. |
If the intent is to have a variable-length array, then you should be able to replace Line 49 in e1f0310
|
No, that doesn't work https://ci.appveyor.com/project/tkelman/julia/build/1.0.644 The other struct types with C99 flexible array members aren't causing problems because there don't appear to be any arrays of them anywhere. |
Ah, got it, that's the recursive nature of these that's the problem. |
@JeffBezanson and I are in the process of looking at and hopefully merging #2818. This rebases that PR (with some unrelated changes already cherry-picked to master) hopefully to be merged soon.