-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Improve warning for inferring an undesirable type #27797
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
Improve warning for inferring an undesirable type #27797
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks!
@swift-ci please smoke test |
You’ve got one failing SourceKit test which needs to be updated |
df239ce
to
6aa0634
Compare
@swift-ci please smoke test |
Thanks, sorry about that. I believe it should be fixed now, but I was just trying to verify locally before pinging you again. Is |
No worries... it runs primary tests and validation tests, so |
94ab6bf
to
bf0b84b
Compare
OK, I just used the version in ConstraintSystem since they're both in sema. If that seems ok, I think this is ready to merge. (cc @theblixguy) |
Hmm feels a little strange to import the ConstraintSystem just for a small check, but fine, we can change it when #27195 lands...or we can just duplicate the check here. What do others think? |
Yeah, it's true. I'm happy to change it when the other PR lands, or just copy-paste the code for now if that's preferable. |
I was going to say it's simple enough to just expand inline, but this is fine too. |
OK, then I think this is ready to merge after another smoke test run. |
@swift-ci please smoke test |
bf0b84b
to
502691f
Compare
Thanks, all. I just made the requested change, merged with master and ran check-swift-validation locally if someone (maybe @theblixguy or @xedin) could do a last check & merge. |
@swift-ci please smoke test |
Seems like it passed! Would you be able to merge it @theblixguy? |
Thank you for your contribution! Please don’t forget to mark the JIRA ticket as resolved :) |
Thank you as well! |
Awesome! |
Adds a fixit for the warning about inferring a probably-unexpected type, and adds another case for when inferring a type of
[()]
. It would be easy to also warn for arrays of any of the existing cases but this seemed like overkill and would require some additional messages.Taken together, this:
Now produces:
With a fixit to add
: [()]
aftery
.Resolves SR-11511.