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

Possible regression in typer in 2.13.10 (only with Xsource:3) #12671

Closed
adampauls opened this issue Oct 22, 2022 · 7 comments · Fixed by scala/scala#10198
Closed

Possible regression in typer in 2.13.10 (only with Xsource:3) #12671

adampauls opened this issue Oct 22, 2022 · 7 comments · Fixed by scala/scala#10198
Assignees
Labels
Milestone

Comments

@adampauls
Copy link

adampauls commented Oct 22, 2022

Compiler version

2.13.10 (with -Xsource:3).

Tested with Scastie using options:

scalacOptions ++= Seq(
  "-deprecation",
  "-Xsource:3",
  "-encoding", "UTF-8",
  "-feature",
  "-unchecked"
)

Minimized code

import scala.collection.{mutable, IterableOnce}
import scala.collection.immutable.{AbstractSet, Set, SetOps}

final case class Foo[-T](components: IndexedSeq[Int])

sealed trait FooTrie[T]
    extends AbstractSet[Foo[T]]
    with SetOps[Foo[T], Set, FooTrie[T]] {

  override def fromSpecific(
      coll: IterableOnce[Foo[T]]
  ): FooTrie[T] = {
    coll.iterator.foldLeft(empty)(_ incl _) // error here
  }

  override def newSpecificBuilder
      : mutable.Builder[Foo[T], FooTrie[T]] = ???

  override def incl(elem: Foo[T]): FooTrie[T] = ???

  override def empty = FooTrie.empty[T]
}

object FooTrie {
  def empty[T]: FooTrie[T] = ???
}

Output

type mismatch;
 found   : scala.collection.immutable.Set[Playground.Foo[T]]
 required: Playground.FooTrie[T]

Expectation

This compiles without -Xsource:3 and it compiled under 2.13.8 with -Xsource:3. It will also compile fine override def empty = FooTrie.empty[T] is changed to override def empty: FooTrie[T] = FooTrie.empty[T]

scala/scala#10160 seems like a possible candidate based on the fact that it is specific to -Xsource:3.

@som-snytt
Copy link

The instance def empty is inferred by Scala 2 as

override def empty: scala.collection.immutable.Set[Foo[T]] = FooTrie.empty[T]

and by Scala 3 as

override def empty: FooTrie[T] = FooTrie.empty[FooTrie.this.T]

Workaround is explicit

override def empty: FooTrie[T] = FooTrie.empty[T]

@SethTisue
Copy link
Member

But the code compiles in Scala 3? (3.2.1-RC4, 3.2.0, currently nightly)

@som-snytt
Copy link

Dotty uses all the overrides:

    /** A type for this definition that might be inherited from elsewhere:
     *  If this is a setter parameter, the corresponding getter type.
     *  If this is a class member, the conjunction of all result types
     *  of overridden methods.
     *  NoType if neither case holds.
     */

though there is a shadow of a doubt

// TODO: Look only at member of supertype instead?

so it uses the intersection of both inherited empties

constraint now:  uninstantiated variables: B
 constrained types: [B](z: B)(op: (B, Foo[T]) => B): B
 bounds:
     B
 ordering:
typed ident empty in method fromSpecific
using inherited type for empty; from: trait IterableFactoryDefaults; raw: => CC[A @uncheckedVariance], inherited: Set[Foo[T] @uncheckedVariance]
using inherited type for empty; from: trait IterableOps; raw: => C, inherited: FooTrie[T]
approx B, from below = true, inst = (FooTrie.this.empty : => FooTrie[T])
instantiating FooTrie[T] with FooTrie[T]
forced instantiation of B = FooTrie[T]

because

trait Iterable[+A] extends IterableOnce[A]
  with IterableOps[A, Iterable, Iterable[A]]
  with IterableFactoryDefaults[A, Iterable] {

@adampauls
Copy link
Author

Yes, compiles fine under 3.2.0 and 3.2.1-RC2 (tested with Scastie) and from dotty/main.

@adampauls
Copy link
Author

adampauls commented Oct 23, 2022

I think it's almost certainly scala/scala#9891. Apparently, we are that one person in a billion.

@adampauls
Copy link
Author

adampauls commented Oct 23, 2022

Just to summarize my understanding:

  • Scala 2 will happily infer a type for an overridden member from the body if there is not explicit type.
  • The above code compiled under 2.13.8, relying on that behavior.
  • That leads to undesirable surprises where overrides from sub-subclasses might not work as expected because the inferred type is "too tight."
  • Scala 3 changed that behavior and instead uses the conjunction of all inherited types.
  • Since one of the inherited types of empty is the collection type itself, this code should also compile with the above Scala 3 behavior.
  • Prefer type of overridden member when inferring (under -Xsource:3) scala#9891 attempted to port that Scala 3 behavior for -Xsource:3, except that it appears not to correctly take the conjunction over all inherited types. Instead, it picks one of them (possibly the first in linear order or something).

Does that sound right?

@som-snytt
Copy link

@adampauls yes, that sounds about right. It's a lot of words.

The difference between Scala 2 and 3 is that Scala 2 (-Xsource:3) assumes the type that is overridden, whereas Scala 3 takes the intersection of all the overrides. One might think that covariant overrides only narrow the type, but in the example, type params in the result make a difference.

I think this example shows that the Scala 3 approach is useful. I'll attempt to replicate it.

I also noticed that the base class order seems inverted: Defaults should not be last, but first or second, since anything more specific should override the "default". That would also solve this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants