Skip to content

Output type mismatch in SAM #18315

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
prolativ opened this issue Jul 31, 2023 · 3 comments · Fixed by #18317
Closed

Output type mismatch in SAM #18315

prolativ opened this issue Jul 31, 2023 · 3 comments · Fixed by #18317
Assignees
Labels
area:typer itype:bug regression This worked in a previous version but doesn't anymore
Milestone

Comments

@prolativ
Copy link
Contributor

Compiler version

3.4.0-RC1-bin-20230729-a73316a-NIGHTLY - failing, current nightly
3.3.2-RC1-bin-20230715-4851278-NIGHTLY - first bad release (first bad commit: 18f90d9)
3.3.2-RC1-bin-20230714-04eae14-NIGHTLY - last good release

Minimized code

trait Zippable[-A, -B] {
  type Out
  def zip(left: A, right: B): Out
}

object Zippable extends ZippableLowPrio:
  given append[A <: Tuple, B]: (Zippable[A, B] { type Out = Tuple.Append[A, B] }) =
    (left, right) => left :* right

trait ZippableLowPrio:
  given pair[A, B]: (Zippable[A, B] { type Out = (A, B) }) =
    (left, right) => (left, right)

def zip[A, B](a: A, b: B)(using z: Zippable[A, B]) = z.zip(a, b)

object Test:
  val x1: (Int, Int)           = zip(1, 2)
  val x2: (Int, Int, Int)      = zip((1, 2), 3)
  val x3: (Int, Int, Int, Int) = zip((1, 2, 3), 4)

Output

-- [E008] Not Found Error: Zippable.scala:8:26 
8 |    (left, right) => left :* right
  |                     ^^^^^^^
  |                     value :* is not a member of A
-- [E007] Type Mismatch Error: Zippable.scala:12:21 
12 |    (left, right) => (left, right)
   |                     ^^^^^^^^^^^^^
   |                     Found:    (A, B)
   |                     Required: Zippable.this.Out
   |
   | longer explanation available when compiling with `-explain`

With -explain enabled:

[error] Zippable.scala:8:22
[error] value :* is not a member of A
[error]     (left, right) => left :* right
[error]                      ^^^^^^^
[error] Zippable.scala:12:22
[error] Found:    (A, B)
[error] Required: Zippable.this.Out
[error] 
[error] Explanation
[error] ===========
[error] 
[error] Tree: Tuple2.apply[A, B](left, right)
[error] I tried to show that
[error]   (A, B)
[error] conforms to
[error]   Zippable.this.Out
[error] but the comparison trace ended with `false`:
[error] 
[error]   ==> (A, B)  <:  Zippable.this.Out
[error]   <== (A, B)  <:  Zippable.this.Out = false
[error] 
[error] The tests were made under the empty constraint
[error]     (left, right) => (left, right)
[error]                      ^^^^^^^^^^^^^

Expectation

The code should compile successfully

@prolativ prolativ added itype:bug area:typer regression This worked in a previous version but doesn't anymore labels Jul 31, 2023
@prolativ prolativ added this to the 3.3.2 milestone Jul 31, 2023
@prolativ
Copy link
Contributor Author

It looks like it's not really only about the output type. The following code also stopped compiling because of the regression:

trait Foo {
  type XXX
  def foo(x: Int): Int
}

def makeFoo: Foo { type XXX = String } =
  x => x

Compilation error (with -explain):

[error] Foo.scala:7:3
[error] Found:    <notype>
[error] Required: Foo{type XXX = String}
[error] 
[error] Explanation
[error] ===========
[error] 
[error] Tree: {
[error]   def $anonfun(x: Int): Int =
[error]     {
[error]       x
[error]     }
[error]   closure($anonfun:<notype>)
[error] }
[error] I tried to show that
[error]   <notype>
[error] conforms to
[error]   Foo{type XXX = String}
[error] but the comparison trace ended with `false`:
[error] 
[error]   ==> <notype>  <:  Foo{type XXX = String}
[error]     ==> <notype>  <:  Foo
[error]     <== <notype>  <:  Foo = false
[error]   <== <notype>  <:  Foo{type XXX = String} = false
[error] 
[error] The tests were made under the empty constraint
[error]   x => x
[error]   ^^^^^^

The error disappears when Foo { type XXX = String } gets replaced with just Foo

@smarter
Copy link
Member

smarter commented Jul 31, 2023

It looks like this only worked by chance. Scala 2 doesn't support refinements in SAM types, and supporting them properly when a SAM type closure is expanded into an anonymous class is non-trivial (the refinement has to be moved inside the class). For example, Scala 3.3.0 compiles the following code, but it crashes with -Ycheck:all because of the malformed tree after phase ExpandSAMs:

trait Foo {
  var x: Int = 1
  type XXX
  def foo(x: Int): Int
}

object Test {
  def makeFoo: Foo { type XXX = String } =
    x => x
}
java.lang.AssertionError: assertion failed:
Found:    Object with Foo {...}
Required: Foo{type XXX = String}

I see you've put this in the 3.3.2 milestone, but there is currently no release-3.3.2 branch and main is now on 3.4.x, so I don't know if #18201 is in what will become 3.3.2 /cc @Kordyjan .

@prolativ
Copy link
Contributor Author

If we can't make this work, we should at least make the error message clear, warning users that SAM conversions don't work with refinements

smarter added a commit to dotty-staging/dotty that referenced this issue Jul 31, 2023
This was dropped in scala#18201 which restricted SAM types to valid parent types,
but it turns out that there is code in the wild that relies on refinements
being allowed here.

To support this properly, we had to enhance ExpandSAMs to move refinements into
type members to pass Ycheck (previous Scala 3 releases would accept the code in
tests/run/i18315.scala but fail Ycheck).

Fixes scala#18315.
smarter added a commit to dotty-staging/dotty that referenced this issue Jul 31, 2023
This was dropped in scala#18201 which restricted SAM types to valid parent types,
but it turns out that there is code in the wild that relies on refinements
being allowed here.

To support this properly, we had to enhance ExpandSAMs to move refinements into
type members to pass Ycheck (previous Scala 3 releases would accept the code in
tests/run/i18315.scala but fail Ycheck).

Fixes scala#18315.
@Kordyjan Kordyjan modified the milestones: 3.3.2, 3.4.0 Aug 1, 2023
smarter added a commit that referenced this issue Aug 1, 2023
This was dropped in #18201 which restricted SAM types to valid parent
types, but it turns out that there is code in the wild that relies on
refinements being allowed here.

To support this properly, we had to enhance ExpandSAMs to move
refinements into type members to pass Ycheck (previous Scala 3 releases
would accept the code in tests/run/i18315.scala but fail Ycheck).

Fixes #18315.

This should be backported to any branch where #18201 is backported.
@Kordyjan Kordyjan modified the milestones: 3.3.2, 3.4.0 Aug 1, 2023
WojciechMazur pushed a commit to WojciechMazur/dotty that referenced this issue Sep 6, 2024
This was dropped in scala#18201 which restricted SAM types to valid parent types,
but it turns out that there is code in the wild that relies on refinements
being allowed here.

To support this properly, we had to enhance ExpandSAMs to move refinements into
type members to pass Ycheck (previous Scala 3 releases would accept the code in
tests/run/i18315.scala but fail Ycheck).

Fixes scala#18315.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:typer itype:bug regression This worked in a previous version but doesn't anymore
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants