-
-
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
fix #39804, ABI handling of structs with String references #39821
Conversation
@@ -151,7 +151,7 @@ void classifyType(Classification& accum, jl_datatype_t *dt, uint64_t offset) con | |||
size_t i; | |||
for (i = 0; i < jl_datatype_nfields(dt); ++i) { | |||
jl_value_t *ty = jl_field_type(dt, i); | |||
if (!jl_is_datatype(ty) || ((jl_datatype_t*)ty)->layout == NULL || jl_is_array_type(ty)) | |||
if (jl_field_isptr(dt, i)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think ->layout
is necessarily defined. I'm unsure the best way to match _julia_struct_to_llvm
here, as it seems many other abi files might similarly be trying to access it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated a couple other cases; see what you think.
ccb517d
to
571064a
Compare
It looks to me like the other ABIs don't need this, but somebody else should check to be sure.
Unfortunately does not help #28669.
fix #39804