Skip to content

Fill in the field-offset vector and payload size for fixed-layout value metadata #15033

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

Merged
merged 1 commit into from
Mar 7, 2018

Conversation

rjmccall
Copy link
Contributor

@rjmccall rjmccall commented Mar 7, 2018

This regression wasn't caught by normal code-generation pattern testing because the emission pattern substantially changed anyway, breaking tests that were looking for the field-offset vector, and because normal execution testing doesn't actually use the field-offset vector and enum payload size fields.

Reflection, which does use these fields, was skating by for common types because metadata is typically allocated out of freshly zeroed pages and most such types have only one field. But that doesn't always happen, so there was a non-determinstic failure that caught it.

Also, don't emit a completion function for value metadata with fixed layout.

We really shouldn't have to emit field-offset vectors for fixed-layout types; the layout should just go in the type descriptor. But for now, this is what we have to do.

@rjmccall
Copy link
Contributor Author

rjmccall commented Mar 7, 2018

@swift-ci Please test and merge.

…ue metadata.

This regression wasn't caught by normal testing because the emission
pattern substantially changed anyway, breaking tests that were looking for
the field-offset vector, and because normal execution testing doesn't
actually use the field-offset vector and enum payload size fields.
Reflection, which does use these fields, was skating by for common types
because metadata is typically allocated out of freshly zeroed pages and
most such types have only one field.

Also, don't emit a completion function for value metadata with fixed layout.

We really shouldn't have to emit field-offset vectors for fixed-layout types;
the layout should just go in the type descriptor.  But for now, this is what
we have to do.
@rjmccall rjmccall force-pushed the fixed-field-offset-vectors branch from bdddce9 to 9c8bdf2 Compare March 7, 2018 06:20
@rjmccall
Copy link
Contributor Author

rjmccall commented Mar 7, 2018

@swift-ci Please test and merge.

2 similar comments
@rjmccall
Copy link
Contributor Author

rjmccall commented Mar 7, 2018

@swift-ci Please test and merge.

@rjmccall
Copy link
Contributor Author

rjmccall commented Mar 7, 2018

@swift-ci Please test and merge.

@swift-ci swift-ci merged commit ae0027a into swiftlang:master Mar 7, 2018
@rjmccall rjmccall deleted the fixed-field-offset-vectors branch March 7, 2018 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants