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

Strange behaviour related to type test of path-dependent types and divergence from Scala 2 #20354

Open
liufengyun opened this issue May 7, 2024 · 4 comments

Comments

@liufengyun
Copy link
Contributor

liufengyun commented May 7, 2024

Compiler version

3.5.0-RC1-bin-20240506-1cdf99f-NIGHTLY

Minimized code

trait CmdLineParser { outer =>

  trait Opt[+T] {
    val default: T
    val names: Set[String]
    val help: String
  }

  trait IntOpt extends Opt[Int] {
    val parser = outer                          //       <=== comment out this line, we get "true true"
  }
}

object FirstParser extends CmdLineParser {
  object OptMinSuccess extends IntOpt {
    val default = 100
    val names = Set("bla")
    val help = "bla"
  }

  val opts = List(OptMinSuccess)
}

object SecondParser extends CmdLineParser {
  object OptMinSuccess extends IntOpt {
    val default = 50
    val names = Set("bla")
    val help = "help"
  }
}


val a = SecondParser.OptMinSuccess.isInstanceOf[FirstParser.IntOpt]

println(a)

(SecondParser.OptMinSuccess: SecondParser.IntOpt)  match {
  case _: FirstParser.IntOpt => println("true")
  case _ => println("false")
}

Output

true
false

If we comment out the following line in trait IntOpt:

  trait IntOpt extends Opt[Int] {
    // val parser = outer                          //       <=== comment out this line, we get "true true"
  }

We get

true
true

In contrast, Scala 2 reports a compilation error for the pattern match in both versions:

pattern type is incompatible with expected type;
 found   : Playground.FirstParser.IntOpt
 required: Playground.SecondParser.IntOpt

Expectation

  • We would expect the output to be always the same, ideally false false
  • Change code in a trait should not change type test result
@liufengyun liufengyun added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label area:spec labels May 7, 2024
@odersky
Copy link
Contributor

odersky commented May 8, 2024

What does Scla 2 give for the isInstanceOf?

@odersky
Copy link
Contributor

odersky commented May 8, 2024

The problem that IntOpt with the member commented out does not get an outer pointer, and we cannot change that.

We could restrict the type system to prevent the match as Scala 2 does. But it could be easily circumvented by widening:

(SecondParser.OptMinSuccess: Any)  match {
  case _: FirstParser.IntOpt => println("true")
  case _ => println("false")
}

So I think it's best to verify that we have an outer pointer and if that's not the case we issue a warning that the full type cannot be tested at run time.

@odersky odersky added area:pattern-matching and removed stat:needs triage Every issue needs to have an "area" and "itype" label area:spec labels May 8, 2024
@liufengyun
Copy link
Contributor Author

What does Scala 2 give for the isInstanceOf?

It returns true.

The problem that IntOpt with the member commented out does not get an outer pointer, and we cannot change that.

I was surprised to find my understanding of outers in Scala incorrect. Is this behavior specified somewhere? Or it's just an optimization that has become de facto standard? /cc: @sjrd

@odersky
Copy link
Contributor

odersky commented May 8, 2024

It would be quite inefficient if every inner trait needed an outer pointer. So it gets one only if the body references an outer member. I don't know whether that's specced.

odersky added a commit to dotty-staging/dotty that referenced this issue May 8, 2024
This reverts one part of scala#20261. When we fail with both an ambiguity on one implicit
argument and another error on another we prefer the other error. I added a comment
why this is needed.

Fixes scala#20354
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.

2 participants