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

new "cyclic aliasing or subtype" error in 2.13 #12622

Closed
retronym opened this issue Jul 21, 2022 · 5 comments · Fixed by scala/scala#10626
Closed

new "cyclic aliasing or subtype" error in 2.13 #12622

retronym opened this issue Jul 21, 2022 · 5 comments · Fixed by scala/scala#10626

Comments

@retronym
Copy link
Member

retronym commented Jul 21, 2022

Reproduction steps

object repro {
  trait ScenarioParam {
    type Builder <: Type
  }

  trait ScenarioParamBuilder

  trait Type {
    type Builder <: ScenarioParamBuilder
  }

  trait Types[H <: ScenarioParam, T <: Type] extends Type {
    type Builder = H#Builder with T#Builder
  }

  trait Nil extends Type {
    type Builder = ScenarioParamBuilder
  }

  trait ScenarioTarget {
    type FilterParam <: Type
  }

  class P1 extends ScenarioParam
  class P2 extends ScenarioParam

  object someTarget extends ScenarioTarget {
    type FilterParam = Types[P1, Types[P2, Nil]]
  }

  class WhereClauseBuilder1[T <: ScenarioTarget] {
    type FilterBuilderType = T#FilterParam#Builder
    def m1(f: FilterBuilderType => Any): Any = null
    def m2(f: T#FilterParam#Builder => Any): Any = null
  }
  
  // triggers cyclic aliasing error on 2.13 only
  (null: WhereClauseBuilder1[someTarget.type]).m1(x => null)

  // triggers the cyclic aliasing error on 2.12 and 2.13
  // this corresponds to the desugaring of the above with a with PR #5999
  val stabilizer: WhereClauseBuilder1[someTarget.type] = null
  stabilizer.m1(x => null)
  
  // also triggers the cyclic aliasing error on 2.12 and 2.13
  (null: WhereClauseBuilder1[someTarget.type]).m2(x => null)

}
$ scalac  --scala-version 2.13.8 -d /tmp test/files/pos/stabilizer-cyclic.scala
test/files/pos/stabilizer-cyclic.scala:38: error: cyclic aliasing or subtyping involving type Builder
  (null: WhereClauseBuilder1[someTarget.type]).m1(x => null)
                                                  ^
test/files/pos/stabilizer-cyclic.scala:43: error: cyclic aliasing or subtyping involving type Builder
  stabilizer.m1(x => null)
                ^
test/files/pos/stabilizer-cyclic.scala:46: error: cyclic aliasing or subtyping involving type Builder
  (null: WhereClauseBuilder1[someTarget.type]).m2(x => null)
                                                  ^
3 errors

$ scalac  --scala-version 2.12.15 -d /tmp test/files/pos/stabilizer-cyclic.scala
test/files/pos/stabilizer-cyclic.scala:43: error: cyclic aliasing or subtyping involving type Builder
  stabilizer.m1(x => null)
                ^
test/files/pos/stabilizer-cyclic.scala:46: error: cyclic aliasing or subtyping involving type Builder
  (null: WhereClauseBuilder1[someTarget.type]).m2(x => null)
                                                  ^

The relevant change in 2.13 appears to be scala/scala#5999 (stabilising vals)

scalac -Yrecursion 1 avoids all errors in both 2.12 and 2.13.

@SethTisue
Copy link
Member

Putting on "Backlog" since there hasn't been any activity in quite some time — @retronym let us know if this should be prioritized for 2.13.11

@SethTisue SethTisue added this to the Backlog milestone Jan 13, 2023
@guizmaii
Copy link

guizmaii commented Oct 9, 2023

I wonder if this bug is related to this other one zio/zio-prelude#1183

@som-snytt
Copy link

@abebeos probably it's not necessary to at-mention, especially on a stale ticket.

I think (or assume) this ticket is a balance between stabilization (a feature) and possible limitations (regressions).

A feature can be worth a regression, even if the regression is not a quick fix.

To your other questions, I think retronym (and others) have custom tooling (with custom args).

Sometimes they demonstrate them in tickets, just to maintain an aura of mystery.

scala-cli improves that situation, or scala -Dscala.repl.info at least makes plain which version was used.

@som-snytt
Copy link

som-snytt commented Nov 21, 2023

The reproduction is as shown. I tested a year ago locally with no options. In showing a reproduction, it's necessary to indicate which version was used. 2.13.12 has the same result.

latest scala 2 scalac -Y says

  -Yrecursion <n>                       Set recursion depth used when locking symbols.

It works for me -Yrecursion 1 or -Yrecursion:1. As Seth would say, what did you try, and how did you try it?

scala 3 rejects the reproduction outright because of projections. One may say projectile projections.

As Seth would ask, why are you asking? Do you have a problem with your project, or did you think this looks like a fun issue to solve?

@SethTisue
Copy link
Member

SethTisue commented Nov 21, 2023

In general, I second Som's remarks.

I would not describe our attitude towards regressions as "casual".

Shouldn't regressions be catched by the test-suite?

Ideally, yes. In practice, things slip through sometimes, even though we have an enormous test suite in scala/scala, and even though we have also put substantial engineering effort into creating the Scala 2 community build to catch regressions.

if something slips through, then it must be fixed immediately and a regression-test added

In a perfect world, sure. In practice, sometimes a big step forward is worth a small step backward, especially if the step backwards is 1) small in the sense that we expect not many users will be affected, and 2) it isn't clear how the problem can be addressed.

In simple enough projects, it might be feasible to have a stricter policy, as you would like. But the complexity of the Scala 2 type checker well exceeds any plausible definition of "simple enough".

In short, we don't regret merging scala/scala#5999. Despite this ticket, in our judgment the benefit still substantially exceeds the cost. Note that 5999 was merged in 2018, and the regression wasn't even noticed until 3.5 years later.

Nor we do regret leaving the regression provisionally unfixed, though we would certainly welcome a fix. Perhaps you'd like to look into it yourself...?

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

Successfully merging a pull request may close this issue.

5 participants