Skip to content

Conversation

hamishknight
Copy link
Contributor

@hamishknight hamishknight commented May 4, 2023

Split off a couple of commits from #64280 (91209d4 + b5e9bbb), plus a couple of other diagnostic fixes and cleanups.

rdar://107724970
rdar://107651291
Resolves #65650

This seems better suited as its own fix, rather
than as part of ContextualMismatch.
We shouldn't be allocating placeholders for
type variables in the permanent arena, and we
should be caching them such that equality works.
…`false`

Returning `true` is wrong here as we could have
the error diagnosed by another fix, which if not
handled, would lead to us crashing as we assume
we diagnosed the issue. Instead, return `false`,
allowing us to at least bail with a bad error
rather than a crash.

We still need to go through and update argument
list diagnostic logic to handle patterns, but I'm
leaving that as future work for now.

rdar://107724970
rdar://107651291
- Simplify the fix locator when looking for a
fix already present in a pattern match, this
avoids us emitting both a diagnostic for the
argument conversion, and for a conformance failure.
- Include tuples in the diagnostic logic where
we emit a generic "operator cannot be applied"
diagnostic, as a conformance diagnostic is
unlikely to be helpful in that case.
We no longer need to check for a type variable
here, it no longer regresses diagnostics. Also,
while here, let's bump the impact for an
Any/AnyObject missing conformance, as that's
unlikely going to be helpful since they cannot
conform to protocols even if the user wanted them
to.
We can produce a hole here now without
regressing diagnostics.
Rather than eagerly binding them to holes if the
sequence element type ends up being Any, let's
record the CollectionElementContextualMismatch fix,
and then if the patterns end up becoming holes,
skip penalizing them if we know the fix was
recorded. This avoids prematurely turning type
variables for ExprPatterns into holes, which
should be able to get better bindings from the
expression provided. Also this means we'll apply
the logic to non-Any sequence types, which
previously we would give a confusing diagnostic
to.
@hamishknight
Copy link
Contributor Author

@swift-ci please 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.

Awesome, thank you!

@hamishknight hamishknight merged commit 3087afc into swiftlang:main May 4, 2023
@hamishknight hamishknight deleted the placeholder branch May 4, 2023 18:50
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.

Bad diagnostic for attempting to match non-tuple in array literal as tuple
2 participants