Skip to content

[SR-15562] [Sema] Do not consider warning fixes for non-augmenting fix recording #40480

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 2 commits into from
Dec 9, 2021

Conversation

LucianoPAlmeida
Copy link
Contributor

The problem is that for non-augmenting fixes(TreatRValueAsLValue) if there is already a fix recorded at that anchor or subexpr, this fix is not recorded. The problem in this case is since we move the checked cast warning diagnostics to fix format in #34980, warning fixes were also considered so a failure was being skipped which seems incorrect and it was causing the issue. Warning fixes should not be considered in that logic because this will cause this issue where a warning fix recorded makes it skip the recording of any error fix which may lead the solver in this state of not able to produce a diagnostic.

Resolves SR-15562.

@LucianoPAlmeida LucianoPAlmeida requested a review from xedin December 9, 2021 02:56
@LucianoPAlmeida
Copy link
Contributor Author

@swift-ci Please smoke test

Copy link
Contributor

@xedin xedin left a comment

Choose a reason for hiding this comment

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

Makes sense, thank you!

@xedin xedin merged commit 57bfef0 into swiftlang:main Dec 9, 2021
@LucianoPAlmeida LucianoPAlmeida deleted the SR-15562-fail-diag branch December 10, 2021 01:01
@LucianoPAlmeida
Copy link
Contributor Author

LucianoPAlmeida commented Dec 10, 2021

Thanks @xedin!
Quick question, was 5.6 branch already? If it was does it make sense to cherry-pick this?
edit: Just saw there is a branch already, so can we cherry-pick?

@xedin
Copy link
Contributor

xedin commented Dec 10, 2021

Not yet, so this should make it no problem.

@xedin
Copy link
Contributor

xedin commented Dec 10, 2021

Also, I'm wrong - it was - https://github.com/apple/swift/tree/release/5.6 so you'd have to cherry-pick.

@LucianoPAlmeida
Copy link
Contributor Author

Also, I'm wrong - it was - https://github.com/apple/swift/tree/release/5.6 so you'd have to cherry-pick.

Ah ok, will do it then
Thank you :)

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