Skip to content

[ConstraintSystem] Don't warn about some redundant coercions #27877

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

Closed
wants to merge 1 commit into from

Conversation

xedin
Copy link
Contributor

@xedin xedin commented Oct 25, 2019

Let's not warn about possible redundant coercions if the
"coerced type" is not known upfront, because that would
mean that solver would infer it from coercion.

Resolves: rdar://problem/56608085

Let's not warn about possible redundant coercions if the
"coerced type" is not known upfront, because that would
mean that solver would infer it from coercion.

Resolves: rdar://problem/56608085
@xedin
Copy link
Contributor Author

xedin commented Oct 25, 2019

/cc @LucianoPAlmeida. Unfortunately looks like some warnings are too eager. This is showing up in validation-test/Sema/type_checker_perf/fast/rdar17077404.swift which is only run in --no-assertions build at the moment.

@xedin
Copy link
Contributor Author

xedin commented Oct 25, 2019

@swift-ci please smoke test

@xedin
Copy link
Contributor Author

xedin commented Oct 25, 2019

If this breaks something else I'll have to revert your original PR :/

@xedin
Copy link
Contributor Author

xedin commented Oct 25, 2019

Looks like this breaks cases like let _: (Float) -> Void = { $0 as Float } :(

@LucianoPAlmeida
Copy link
Contributor

LucianoPAlmeida commented Oct 25, 2019

Right, that makes sense.
I see you've opened a PR to revert, thank you @xedin :)
Is that the only case broken? I think I can open a PR fixing that right now so we don't have to revert everything. But also can reopen the PR with the fixes 👍

@LucianoPAlmeida
Copy link
Contributor

@xedin Just took continuation of this in another PR, hope you don't mind :)
Also, which should be the test we run on the CI to make sure everything is ok?

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