Skip to content
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 #43411, wrapped NamedTuple can be bitstype more often #44311

Merged
merged 1 commit into from
Feb 24, 2022

Conversation

JeffBezanson
Copy link
Member

fix #43411

@JeffBezanson JeffBezanson added bugfix This change fixes an existing bug backport 1.7 backport 1.8 Change should be backported to release-1.8 labels Feb 22, 2022
Copy link
Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like jl_get_fieldtypes is sometimes invalid for NamedTuple, as a special case here:

ndt->types = jl_emptysvec; // XXX: this is essentially always false
and we need to avoid using it, as this PR does

But if jl_get_fieldtypes was returning svec() here, how did we end up returning 1 from this function? Ah, I see we were incorrectly returning 0 here earlier, since the fieldtypes we considered were incorrect.

@vtjnash
Copy link
Member

vtjnash commented Feb 22, 2022

Is this comment unnecessary now?

julia/src/jltypes.c

Lines 228 to 229 in afd9926

if (jl_is_tuple_type(dt) || jl_is_namedtuple_type(dt))
return 0; // TODO: relax more?

@KristofferC KristofferC mentioned this pull request Feb 24, 2022
47 tasks
@JeffBezanson JeffBezanson merged commit f20d5de into master Feb 24, 2022
@JeffBezanson JeffBezanson deleted the jb/fix43411 branch February 24, 2022 22:08
staticfloat pushed a commit to JuliaCI/julia-buildkite-testing that referenced this pull request Mar 2, 2022
KristofferC pushed a commit that referenced this pull request Mar 3, 2022
KristofferC pushed a commit that referenced this pull request Mar 7, 2022
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
@KristofferC KristofferC removed the backport 1.8 Change should be backported to release-1.8 label Mar 15, 2022
KristofferC pushed a commit that referenced this pull request Apr 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This change fixes an existing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrapping NamedTuple in struct is no longer bitstype in 1.7
3 participants