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

Structural types regression in 3.3.2-RC1 #18535

Closed
YulyaSp opened this issue Sep 10, 2023 · 7 comments
Closed

Structural types regression in 3.3.2-RC1 #18535

YulyaSp opened this issue Sep 10, 2023 · 7 comments
Labels
itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label

Comments

@YulyaSp
Copy link

YulyaSp commented Sep 10, 2023

Compiler version

Last good version: >= 3.3.2-RC1-bin-20230530-28915c4-NIGHTLY
First bad version: <= 3.3.2-RC1-bin-20230724-ce1ce99-NIGHTLY

Minimized code

type FirstHelper[A] = { val _1: A }
case class Foo(i: Int)
val forced: FirstHelper[Int] = Foo(0)

Output

Fails to compile with

Found:    Foo
Required: FirstHelper[Int]

Expectation

Compiles

Comment

The quite brilliant idea to use structural types like that for use in match types is due to /raghar from Reddit. Hope it's not unintended that it used to work as one of my projects depends on this functionality.

Motivating example:

type FirstHelper[A] = { val _1: A }

type FirstOf[A <: Product] = A match
    case FirstHelper[a] => a

trait KeyExtractor[Row]:
    type Key
    def key(row: Row): Key

object KeyExtractor:
    type Aux[Row, _Key] = KeyExtractor[Row] { type Key = _Key }

given [_Key, Row <: Product & FirstHelper[_Key]]: KeyExtractor.Aux[Row, FirstOf[Row]] = new KeyExtractor[Row]:
    import reflect.Selectable.reflectiveSelectable
    type Key = _Key
    def key(row: Row): Key = row._1

def key[K, R](r: R)(using ext: KeyExtractor.Aux[R, K]) = ext.key(r)

case class Foo(i: Int)
key(Foo(0))

EDIT: Changed named a previously underscore-named variable, added a motivating example

@YulyaSp YulyaSp added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Sep 10, 2023
@YulyaSp YulyaSp changed the title Match type regression in 3.3.2-RC1 Structural types regression in 3.3.2-RC1 Sep 10, 2023
@SethTisue
Copy link
Member

SethTisue commented Sep 10, 2023

Note that val _: SomeType = ... is a pattern match, but val x: SomeType = ... isn't. Or, is that even correct? SLS 4.2 isn't completely clear to me on this point. But scala/bug#10384 seems to settle it?

Is it meaningful to you, in your use case, that it's val _: FirstHelper[Int] = Foo(0) specifically, rather than the non-pattern-matching variant val x: FirstHelper[Int] = Foo(0), or the match based variant Foo(0) match { case _: FirstHelper[Int] => ... }? I started testing all these variants in 2.13.11, 3.3.1, and 3.3.nightly and quickly became confused by the variety of resulting behaviors. (But I only have a few minutes at the moment; perhaps someone else would like to dig deeper.)

The smart money says that @som-snytt will want to weigh in on this.

@som-snytt
Copy link
Contributor

I'm still sensitive about my pandemic pounds, so I won't be weighing in.

But the spec (4.1 of what is now "ye olde spec") says if p isn't a name (and underscore is not one), and the p has no bound variables, which underscore hasn't, then you get rhs match { case _: T => () }.

Welcome to Scala 3.4.0-RC1-bin-SNAPSHOT-git-64c3138 (20.0.1, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.

scala> type V[A] = { val _1: A }
// defined alias type V[A] = Object{val _1: A}

scala> val x: V[Int] = (42, "hike")
-- [E007] Type Mismatch Error: -----------------------------------------------------------------------------------------
1 |val x: V[Int] = (42, "hike")
  |                ^^^^^^^^^^^^
  |                Found:    (Int, String)
  |                Required: V[Int]
  |
  | longer explanation available when compiling with `-explain`
1 error found

scala> (42, "hike") match { case _: (Int, String) => }

scala> (42, "hike") match { case _: V[Int] => }
1 warning found
-- [E092] Pattern Match Unchecked Warning: -----------------------------------------------------------------------------
1 |(42, "hike") match { case _: V[Int] => }
  |                          ^
  |                          the type test for V[Int] cannot be checked at runtime because it's a refinement type
  |
  | longer explanation available when compiling with `-explain`

compare

scala 2.13.11> val _: V[Int] = (42, "hike")
                      ^
               warning: a pattern match on a refinement type is unchecked

@YulyaSp
Copy link
Author

YulyaSp commented Sep 11, 2023

I wasn't aware of these subtleties between named and underscore variables. Thanks for the explanations. It does not however change this issue. I changed the code to a named variable, and added a motivating example for the feature

@SethTisue
Copy link
Member

I admit ignorance: why does this compile in Scala 3?

scala> case class Foo(i: Int)
// defined case class Foo
                                                                                                    
scala> Foo(0)._1
val res0: Int = 0

It doesn't compile in Scala 2:

scala> Foo(0)._1
              ^
       error: value _1 is not a member of Foo

@WojciechMazur
Copy link
Contributor

Bisect points to f29013c which now makes the typer distinguish structural types using val from the one using def.
The _1 accessor is in fact defined as a synthetic def so it makes sense here that it fails to compile now and I don't think we should treat it as regression

@WojciechMazur WojciechMazur closed this as not planned Won't fix, can't repro, duplicate, stale Sep 11, 2023
@YulyaSp
Copy link
Author

YulyaSp commented Sep 11, 2023

Oh! The whole code works when we do type FirstHelper[A] = { def _1: A } so all's well. Thanks!

@som-snytt
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label
Projects
None yet
Development

No branches or pull requests

4 participants