Skip to content

Conversation

@smarter
Copy link
Member

@smarter smarter commented Nov 27, 2017

No description provided.

@smarter smarter requested a review from odersky November 27, 2017 13:55
@smarter smarter force-pushed the more-sip23 branch 2 times, most recently from 47009e0 to bce112f Compare November 27, 2017 15:22
@smarter
Copy link
Member Author

smarter commented Nov 27, 2017

@milessabin The narrowing trick from scalac does not work in Dotty:

scala> def narrow[T <: Singleton](x: T): T {} = x 
def narrow[T <: Singleton](x: T): T
scala> narrow(1) 
val res0: Int = 1

However, the following works in both scalac and dotty:

scala> type Identity[T] = T 
// defined alias type Identity = [T] => T
scala> def narrow[T <: Singleton](x: T): Identity[T] = x 
def narrow[T <: Singleton](x: T): ([T] => T)[T]
scala> narrow(1) 
val res1: ([T] => T)[1] = 1
scala> val b = res51
val b: ([T] => T)[1] = 1
scala> val c: 1 = b 
val c: 1 = 1

(The fact that the dotty repl prints ([T] => T)[T] instead of Identity[T] seems to be a pretty-printer issue, it shouldn't matter).

It might be useful to add a scala.Identity[T] as well as a definition of narrow to the Predef at some point.

@smarter smarter force-pushed the more-sip23 branch 3 times, most recently from 37cb879 to 2d7c60d Compare November 27, 2017 17:40
Copy link
Contributor

@odersky odersky left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM

final val STRINGconst = 77
final val IMPORTED = 78
final val RENAMED = 79
final val SCALASYMBOLconst = 80
Copy link
Contributor

Choose a reason for hiding this comment

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

Why add Scala? I'd prefer we just called it SYMBOLconst.

ConstantType(Constant(readType()))
case ENUMconst =>
ConstantType(Constant(readTermRef().termSymbol))
case SCALASYMBOLconst =>
Copy link
Contributor

@odersky odersky Nov 30, 2017

Choose a reason for hiding this comment

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

I don't think you need the duplication. SYMBOLconsts are simple types, so they should just occur in readSimpleType. I believe the duplication of CLASSconst and ENUMconst is also wrong and should be removed.

When a type variable is upper bounded by scala.Singleton, its
instantiation should not be widened
This brings us closer to fully supporting SIP-23.

Bump the version of TASTY to 1.1, adding support for symbol literals is
a backwards-compatible change.

The sip23-* tests are adapted from
scala/scala#5310, the only difference is that we
do not currently support the `T {}` trick to avoid widening when
inferring types.
@smarter smarter dismissed odersky’s stale review November 30, 2017 16:30

Comments addressed.

@smarter smarter merged commit 4fae88a into scala:master Nov 30, 2017
@allanrenucci allanrenucci deleted the more-sip23 branch December 14, 2017 19:24
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