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

[Sema] Fix name confusion in Decodable synthesis #23252

Merged
merged 3 commits into from Mar 15, 2019

Conversation

Projects
None yet
4 participants
@brentdax
Copy link
Collaborator

brentdax commented Mar 12, 2019

Decodable’s init(from:) synthesis sometimes mistook a static property for an identically-named instance property, which could cause it to skip a property or possibly make other mistakes. This change factors a common helper function from encode(to:) and init(from:) synthesis which implements the right logic for both.

Resolves SR-10045 a.k.a. rdar://problem/48702779.

Brent Royal-Gordon
Fix name confusion in Decodable synthesis
Decodable’s init(from:) synthesis sometimes mistook a static property for an identically-named instance property, which could cause it to skip a property or possibly make other mistakes. This change factors a common helper function from encode(to:) and init(from:) synthesis which implements the right logic for both.

@brentdax brentdax force-pushed the brentdax:the-name-blame-game branch from ab09832 to f2102fc Mar 12, 2019

Brent Royal-Gordon added some commits Mar 12, 2019

@brentdax brentdax requested a review from itaiferber Mar 12, 2019

@brentdax

This comment has been minimized.

Copy link
Collaborator Author

brentdax commented Mar 12, 2019

@swift-ci please smoke test

@itaiferber
Copy link
Contributor

itaiferber left a comment

Changes look reasonable overall, just two small questions:

Show resolved Hide resolved lib/Sema/DerivedConformanceCodable.cpp
Show resolved Hide resolved lib/Sema/DerivedConformanceCodable.cpp
@brentdax

This comment has been minimized.

Copy link
Collaborator Author

brentdax commented Mar 14, 2019

@swift-ci please smoke test and merge

1 similar comment
@brentdax

This comment has been minimized.

Copy link
Collaborator Author

brentdax commented Mar 15, 2019

@swift-ci please smoke test and merge

@swift-ci swift-ci merged commit 19015fa into apple:master Mar 15, 2019

2 of 3 checks passed

Test and Merge (smoke test) Build started.
Details
Swift Test Linux Platform (smoke test)
Details
Swift Test OS X Platform (smoke test)
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.