Skip to content

[Sema][SR-14408] Improvements on enum equality diagnostics #39039

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

For some cases when attempting an == overload choice for enum and a dot member argument, for some choices solver ends trying to record a missing member for the argument. This causes the end result to be a set of solutions where although the score is better, given the context are not exactly. This causes diagnose ambiguity with fixes to work with a set of "best score" solutions that would not give the best diagnostics.
So the idea here is to look at the context in which this fix is being recorded and increase the impact if in this context a missing member fix less likely to be an useful diagnostic on this case.

Resolves SR-14408.

@LucianoPAlmeida LucianoPAlmeida requested a review from xedin August 25, 2021 12:00
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.

I think it would make sense to increase an impact of missing member fix in general (at least for argument position), so it doesn't interfere in situations where there is more than one specific fix. What to give it a try?

@LucianoPAlmeida
Copy link
Contributor Author

I think it would make sense to increase an impact of missing member fix in general (at least for argument position), so it doesn't interfere in situations where there is more than one specific fix. What to give it a try?

Sure! Let's try this :)

@LucianoPAlmeida LucianoPAlmeida force-pushed the SR-14408-ambiguity-enum branch from aed3259 to 93425b3 Compare August 25, 2021 22:58
@LucianoPAlmeida
Copy link
Contributor Author

I think it would make sense to increase an impact of missing member fix in general (at least for argument position), so it doesn't interfere in situations where there is more than one specific fix. What to give it a try?

Adjustments made @xedin
Run tests locally and I'm surprised that this change didn't affect any other diagnostic

@LucianoPAlmeida LucianoPAlmeida force-pushed the SR-14408-ambiguity-enum branch from 93425b3 to 3a9f2f5 Compare August 25, 2021 23:06
@LucianoPAlmeida LucianoPAlmeida requested a review from xedin August 25, 2021 23:06
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.

Looks good!

@xedin
Copy link
Contributor

xedin commented Aug 25, 2021

I expected that not much is going to get effected there since missing member is de-prioritized from the get go.

@LucianoPAlmeida LucianoPAlmeida force-pushed the SR-14408-ambiguity-enum branch from 3a9f2f5 to 41e31e4 Compare August 25, 2021 23:14
@xedin
Copy link
Contributor

xedin commented Aug 25, 2021

:shipit:

@LucianoPAlmeida
Copy link
Contributor Author

I expected that not much is going to get effected there since missing member is de-prioritized from the get go.

Ah true, makes sense. Thank you!

@LucianoPAlmeida
Copy link
Contributor Author

@swift-ci Please smoke test

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