Skip to content

Conversation

@nicolasstucki
Copy link
Contributor

Though final vals with constants are guarantieed to be inlined in Dotty,
we need to keep the definition in case these are used from Scala 2.
Scala 2 misses some inlining oportunities.

@nicolasstucki nicolasstucki force-pushed the keep-inlined-final-vals-in-generated-code branch 7 times, most recently from 77c1bb0 to b2353f3 Compare June 30, 2020 13:43
@nicolasstucki nicolasstucki marked this pull request as ready for review June 30, 2020 15:09
@nicolasstucki nicolasstucki requested a review from bishabosha June 30, 2020 15:10
Copy link
Member

@bishabosha bishabosha left a comment

Choose a reason for hiding this comment

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

it seems that a final val constant is not stable in TASTy unless a stable path is requested for it:

object Consts {
  final val msg = "Hello World" // not stable
}
object TestConsts {
  def foo = Consts.msg
}
object Consts {
  final val msg = "Hello World" // now it is stable because of a use site
}
object TestConsts {
  def foo: Consts.msg.type = Consts.msg
}

Though final vals with constants are guarantieed to be inlined in Dotty,
we need to keep the definition in case these are used from Scala 2.
Scala 2 misses some inlining oportunities.
@nicolasstucki nicolasstucki force-pushed the keep-inlined-final-vals-in-generated-code branch from b2353f3 to 2a8c275 Compare July 3, 2020 10:45
@nicolasstucki
Copy link
Contributor Author

@bishabosha that is not related to the changes in this PR. Could you open an issue for it?

@bishabosha
Copy link
Member

@nicolasstucki I made the issue, otherwise this looks good to me

@nicolasstucki nicolasstucki merged commit 2917bbb into scala:master Jul 3, 2020
@nicolasstucki nicolasstucki deleted the keep-inlined-final-vals-in-generated-code branch July 3, 2020 14:17
sjrd added a commit to sjrd/dotty that referenced this pull request Jul 16, 2020
sjrd added a commit to sjrd/dotty that referenced this pull request Jul 22, 2020
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.

2 participants