Skip to content
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

Always use baseType when constraining patternTp with scrutineeTp #20032

Merged
merged 1 commit into from
Apr 6, 2024

Conversation

EugeneFlesselle
Copy link
Contributor

@EugeneFlesselle EugeneFlesselle commented Mar 27, 2024

In the following example:

type Cond[B <: Boolean] <: Tuple2[String, String] = B match ...
type Decoded[B <: Boolean] = Cond[B] match
  case (h1, _) => Int

When constraining the (h1, _) pattern with Cond[B], we incorrectly assumed we could constrain h1 with B, because Cond[B] is an applied type of which the baseType is Tuple2.

The issue can be fixed in constrainSimplePatternType by obtaining the baseType for both the patternTp and scrutineeTp, with the most general base of the two.

So in the above example, we wound constrain B with String by obtaining (String, String) from Cond[B].

Fixes #19706

In the following example:
```
type Cond[B <: Boolean] <: Tuple2[String, String] = B match ...
type Decoded[B <: Boolean] = Cond[B] match
  case (h1, _) => Int
```
When constraining the `(h1, _)` pattern with `Cond[B]`,
we incorrectly assumed we could constrain h1 with B,
because `Cond[B]` is an applied type of which the baseType is Tuple2.

The issue can be fixed in constrainSimplePatternType
by obtaining the baseType for both the patternTp and scrutineeTp,
with the most general base of the two.

So in the above example, we wound constrain `B` with String
by obtaining `(String, String)` from `Cond[B]`.
@EugeneFlesselle EugeneFlesselle marked this pull request as ready for review March 27, 2024 12:20
@EugeneFlesselle EugeneFlesselle requested a review from smarter March 27, 2024 12:20
@EugeneFlesselle EugeneFlesselle enabled auto-merge (rebase) March 27, 2024 12:52
Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

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

LGTM, that was not obvious at all!

@EugeneFlesselle EugeneFlesselle merged commit 521ce95 into scala:main Apr 6, 2024
19 checks passed
@Kordyjan Kordyjan added this to the 3.5.0 milestone May 10, 2024
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.

Match type reduction doesn't terminate
3 participants