Skip to content

[SR-12626] Fix crash when non WritableKeyPath to a set subscript assign #31140

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

Conversation

LucianoPAlmeida
Copy link
Contributor

This fixes SR-12626 where applying a non-WritableKeyPath to a set subscript crashes the compiler.
Also, partially fixes the SR-12425, fixing the crash, but diagnostics have to be improved. So just added the test cases

cc @xedin @hamishknight :)

Resolves SR-12626.

@LucianoPAlmeida LucianoPAlmeida force-pushed the SR-12425-crash-dynamic-member-key-path branch from bbf0432 to 655bc92 Compare April 19, 2020 22:11
Copy link
Contributor

@hamishknight hamishknight left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

@LucianoPAlmeida LucianoPAlmeida force-pushed the SR-12425-crash-dynamic-member-key-path branch from 655bc92 to 4dfbf6f Compare April 20, 2020 03:39
@LucianoPAlmeida LucianoPAlmeida force-pushed the SR-12425-crash-dynamic-member-key-path branch from 4dfbf6f to ab81236 Compare April 20, 2020 16:47
@xedin
Copy link
Contributor

xedin commented Apr 20, 2020

@swift-ci please smoke test

Copy link
Contributor

@hamishknight hamishknight left a comment

Choose a reason for hiding this comment

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

LGTM!

@xedin
Copy link
Contributor

xedin commented Apr 20, 2020

@swift-ci please smoke test Linux platform

@xedin xedin merged commit 72559bd into swiftlang:master Apr 20, 2020
@LucianoPAlmeida LucianoPAlmeida deleted the SR-12425-crash-dynamic-member-key-path branch April 20, 2020 22:45
@LucianoPAlmeida
Copy link
Contributor Author

Thanks @xedin @hamishknight :)
I'll try to send a follow up with diagnostics improvements 👍

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