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

Patmat: Use less type variables in prefix inference #16827

Merged
merged 1 commit into from
Feb 16, 2023

Conversation

dwijnand
Copy link
Member

@dwijnand dwijnand commented Feb 3, 2023

In code like:

class Outer:
  sealed trait Foo
  case class Bar() extends Foo

  def mat(foo: Foo) = foo match
    case Bar() =>

When in the course of decomposing the scrutinee's type, which is
Outer.this.Foo, we're trying to instantiate subclass Outer.this.Bar,
the Outer.this is fixed - it needn't be inferred, via type variables
and type bounds. Cutting down on type variables, particularly when GADT
symbols are also present, can really speed up the operation, including
making code that used to hang forever compile speedily.

Fixes #16785

@dwijnand dwijnand marked this pull request as ready for review February 3, 2023 18:22
Copy link
Contributor

@liufengyun liufengyun left a comment

Choose a reason for hiding this comment

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

LGTM

compiler/src/dotty/tools/dotc/core/TypeOps.scala Outdated Show resolved Hide resolved
compiler/src/dotty/tools/dotc/core/TypeOps.scala Outdated Show resolved Hide resolved
In code like:

    class Outer:
      sealed trait Foo
      case class Bar() extends Foo

      def mat(foo: Foo) = foo match
        case Bar() =>

When in the course of decomposing the scrutinee's type, which is
`Outer.this.Foo`, we're trying to instantiate subclass `Outer.this.Bar`,
the `Outer.this` is fixed - it needn't be inferred, via type variables
and type bounds.  Cutting down on type variables, particularly when GADT
symbols are also present, can really speed up the operation, including
making code that used to hang forever compile speedily.
@dwijnand dwijnand merged commit 0416992 into scala:main Feb 16, 2023
@dwijnand dwijnand deleted the gadt-hang branch February 16, 2023 15:37
SethTisue added a commit to SethTisue/dotty that referenced this pull request Feb 23, 2023
this was fixed recently (by scala#16827), but the test case is simpler
than the one in the PR. minimized from user code on the Typelevel
Discord
@TomasMikula
Copy link
Contributor

Hi, will this not be backported to release 3.3.0?

My project now

  • compiles with 3.2.2
  • compilation hangs with 3.3.0-RC4 (i.e. a regression)
  • compiles with 3.3.1-RC1-bin-20230425-c9ca7ce-NIGHTLY (I assume thanks to this fix, but can't be sure).

Note that the example in #16785 also hanged in 3.2.2, so it was not classified as a regression, whereas now I'm hitting a regression.

@TomasMikula
Copy link
Contributor

Here's proof of regression:

git clone https://github.com/TomasMikula/libretto.git
cd libretto
git checkout b042b14bc4162488f2ad36c3727272212380b295

# compiles with the used Scala version 3.2.2
sbt lambda/compile

# Edit build.sbt, set
# ThisBuild / scalaVersion := "3.3.0-RC4"

# compilation hangs
sbt lambda/compile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:done This PR was successfully backported.
Projects
None yet
4 participants