Skip to content

IRGen: Empty fields do have an entry in the field offset vector #13904

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

Conversation

aschwaighofer
Copy link
Contributor

This is an error introduced as the result of a refactoring a while ago
and means that we will store dependently typed stored properties at the
wrong offset in a generic struct if it has stored properties of empty
types before said property.

rdar://36384871

This is an error introduced as the result of a refactoring a while ago
and means that we will store dependently typed stored properties at the
wrong offset in a generic struct if it has stored properties of empty
types before said property.

rdar://36384871
@aschwaighofer
Copy link
Contributor Author

@eeckstein Can you review this for swift-4.1-branch?

@aschwaighofer
Copy link
Contributor Author

@swift-ci Please test

@aschwaighofer
Copy link
Contributor Author

@swift-ci Please test source compatibility

@aschwaighofer
Copy link
Contributor Author

The source compat failure in Kitura is the same that is also happening on master ATM here: https://ci.swift.org/job/swift-master-source-compat-suite/1068/artifact/swift-source-compat-suite/FAIL_Kitura_4.0_BuildSwiftPackage.log

Not related to this patch.

@slavapestov
Copy link
Contributor

Reflection also relies on an accurate field offset vector. Does dump() work on a type with empty fields?

@aschwaighofer
Copy link
Contributor Author

@eeckstein Ping?

@aschwaighofer
Copy link
Contributor Author

aschwaighofer commented Jan 13, 2018

@slavapestov swift displayed the contents wrong before this patch:

10> let s = GenericStruct<MyImpl>()
s: GenericStruct<MyImpl> = {
  dummy = 1
  opt = nil
}

@aschwaighofer
Copy link
Contributor Author

Oh wait that is because it was wrongly set.

@aschwaighofer
Copy link
Contributor Author

@swift-ci Please test source compatibility

@aschwaighofer aschwaighofer merged commit ddc2e29 into swiftlang:master Jan 13, 2018
Copy link
Contributor

@eeckstein eeckstein left a comment

Choose a reason for hiding this comment

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

lgtm
Thanks for fixing this!

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.

3 participants