Skip to content

[TypeCheckPattern] Attempt ExprPattern conversion before failing pattern coercion to optional #67479

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 1 commit into from
Aug 11, 2023

Conversation

Jumhyn
Copy link
Member

@Jumhyn Jumhyn commented Jul 23, 2023

See previous PR[1] and previous forum threads[2][3] regarding similar behavior and the intended semantics of expression pattern matching.

Expected behavior is for structural pattern matching and expression pattern matching to be transparent for the user. Currently, when matching an EnumElementPattern (of the form .foo) against an expression of type T, if we fail to find an enum element T.foo then we will attempt to fall back to matching .foo as an expression, which can find static members .foo.

However, if T is an optional type U?, then if lookup of Optional.foo fails to find a matching enum element member, we will look into the wrapped type for an enum element U.foo. If this fails, we (presently) do not take the fallback path and instead immediately fail with the error "enum case 'foo' not found in type 'U?'" even though lookup for UnresolvedMemberExprs would also look through optional types when attempting expression matching.

This PR allows us to match against U.foo expressions by delaying the failure diagnostic in the optional case until after we've attempted the expression fallback. This will have the effect of allowing us to match the pattern .foo against values of type U? when U has a static member foo: Self. It will also potentially change the diagnostic in failure cases from "enum case 'foo' not found in type 'U?'" to "type 'U?' has no member 'foo'," which is IMO more accurate since 'foo' is indeed permitted to be any (static) member, not just an enum case.

[1]: #22486
[2]: https://forums.swift.org/t/matching-optionals-in-a-switch-statement/12905
[3]: https://forums.swift.org/t/allow-enum-case-matching-without/20544

@Jumhyn Jumhyn force-pushed the enum-expr-optional-matching branch from 57de0bc to d13e997 Compare July 23, 2023 16:04
@Jumhyn Jumhyn requested a review from theblixguy July 23, 2023 16:10
@Jumhyn
Copy link
Member Author

Jumhyn commented Aug 2, 2023

@swift-ci please test

1 similar comment
@Jumhyn
Copy link
Member Author

Jumhyn commented Aug 2, 2023

@swift-ci please test

@Jumhyn
Copy link
Member Author

Jumhyn commented Aug 2, 2023

@swift-ci please test source compatibility

@hamishknight hamishknight self-requested a review August 2, 2023 17:09
@Jumhyn
Copy link
Member Author

Jumhyn commented Aug 2, 2023

@swift-ci please test

@Jumhyn
Copy link
Member Author

Jumhyn commented Aug 2, 2023

@swift-ci please test windows platform

1 similar comment
@Jumhyn
Copy link
Member Author

Jumhyn commented Aug 2, 2023

@swift-ci please test windows platform

@Jumhyn
Copy link
Member Author

Jumhyn commented Aug 2, 2023

@swift-ci please test source compatibility release

@Jumhyn
Copy link
Member Author

Jumhyn commented Aug 2, 2023

@swift-ci please test windows platform

Copy link
Contributor

@hamishknight hamishknight left a comment

Choose a reason for hiding this comment

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

Thanks! This LGTM

@Jumhyn
Copy link
Member Author

Jumhyn commented Aug 10, 2023

@swift-ci please test windows platform

@Jumhyn
Copy link
Member Author

Jumhyn commented Aug 10, 2023

@swift-ci please test source compatibility debug

@Jumhyn
Copy link
Member Author

Jumhyn commented Aug 11, 2023

Thanks @hamishknight!

@Jumhyn Jumhyn merged commit b2b768f into swiftlang:main Aug 11, 2023
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