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

Fix #13197: Deskolemize lifted named arguments #13590

Merged
merged 4 commits into from
Sep 24, 2021

Conversation

noti0na1
Copy link
Member

@noti0na1 noti0na1 commented Sep 22, 2021

Fix #13197

The lift named arguments contain skolem types, which causes checker in inliner to fail.

def foo(bar: Bar): Foo = {
  val b$1: (?1 : String | Int) & String = forceString[Int](bar.b)
  val a$1: String @uncheckedVariance = Foo.$lessinit$greater$default$1
  new Foo(a$1, b = b$1)
}

We add deskolemized to fix this issue.

@noti0na1
Copy link
Member Author

This bug only happens when the function is inlined.

@smarter Could you have a look of the code? I'm not sure whether this is the correct fix.

Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

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

I don't think widen should look inside types, this would be a big departure from its current behavior.
(?1 : String | Null) looks like a skolem and we have logic to widen skolem when ascribing a type to a val during typer: https://github.com/lampepfl/dotty/blob/aaac006a58c3f5d7fbd64d3a1a19261b51dfecf5/compiler/src/dotty/tools/dotc/typer/Namer.scala#L1707 It seems like the same logic should be used when we ascribe a type to a val during the inlining phase?

@noti0na1
Copy link
Member Author

@smarter This could also be a bug from named parameters.

extension [T](x: T | String) inline def forceString: x.type & String =
  x.asInstanceOf

trait Bar:
  def b: String | Int

class Foo(a: String = "", b: String)

object Foo:
  def foo(bar: Bar) = Foo(b = bar.b.forceString)

In typer, we can see the function foo becomes:

def foo(bar: Bar): Foo = {
  val b$1: (?1 : String | Int) & String = forceString[Int](bar.b)
  val a$1: String @uncheckedVariance = Foo.$lessinit$greater$default$1
  new Foo(a$1, b = b$1)
}

I think we need to widen the skolem type for b$1 first?

@smarter
Copy link
Member

smarter commented Sep 23, 2021

Yes that looks like the root of the problem indeed.

@noti0na1 noti0na1 marked this pull request as ready for review September 23, 2021 21:36
@noti0na1 noti0na1 changed the title Fix #13197: Handle AndType in widen Fix #13197: Deskolemize lifted named arguments Sep 23, 2021
@smarter smarter merged commit ee137ff into scala:master Sep 24, 2021
@smarter smarter deleted the fix-widen-singleton-in-and branch September 24, 2021 09:08
olsdavis pushed a commit to olsdavis/dotty that referenced this pull request Apr 4, 2022
@Kordyjan Kordyjan added this to the 3.1.1 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

broken Inlining type checks for proxied nn calls
3 participants