Skip to content

WIP SIP-23 #12

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

Closed
wants to merge 19 commits into from
Closed

WIP SIP-23 #12

wants to merge 19 commits into from

Conversation

adriaanm
Copy link
Owner

WIP PR to centralize discussion, accept improvements/tests/polish/docs/...

Everyone is welcome to look for the checkboxes/TODO items here and in the discussion below and take them on! If you're not sure where/how to start, just ask!

Testing

  • syntax (infix notation, backquotes, quasiquotes, string interpolation?,...)
  • subtyping
  • implicits
  • dependent method types
  • type inference: not inferred for signatures, inferred when <: Singleton

Spec/docs

  • include negated literal types in InfixType BNF (like prefixExpr)
  • update syntax overview in spec
  • spec the subtyping relationship between ConstantType / LiteralType and scala.Singleton.
  • include SIP doc under spec/

Type inference / type checking

  • When inferring signatures, treat literal types like singleton types (i.e., don't infer them -- widen instead)
  • treat singleton type bound on type param same as Singleton bound (stopWidening when bounds exists _.isStable)
  • include fix for https://issues.scala-lang.org/browse/SI-8564 (idm.deconst)
  • lub/glb
  • instanceOf checks: x.isInstanceOf[3] should expand to x == 3, and x.asInstanceOf[3] should be if (x != 3) throw new ClassCastException(..)

Corner cases

  • val t: 1 = t (NOTE: t has value 0...)

Design space

  • Support negative literals
  • Support Symbol literals --> challenge: desugars into Symbol.apply("symbol")
  • Support class literals? (probably not)
  • Model () as LiteralType? (I say 'no': () is not a syntactic literal, it's also too deeply ingrained to incorporate here)

Feature interaction

  • Predicate on language flag (can't do this in parser, though -- use -Xsip:23 instead?)
  • Update quasiquotes
  • Inconsistent representation of parse trees in different contexts
  • scalap

@adriaanm
Copy link
Owner Author

scala/ (sip23 $=) $ qs -Xexperimental
Welcome to Scala version 2.11.5-20141028-095954-500f1ab300 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0).
Type in expressions to have them evaluated.
Type :help for more information.

scala> val x = 1
x: 1 = 1

scala> val y = 2
y: 2 = 2

scala> (x + 1) match { case _ : y.type => 3 }
res1: 3 = 3

scala> def foo = { println(x); y }
foo: Int

scala> def stable[T <: Singleton](x: T): T = x
stable: [T <: Singleton](x: T)T

scala> stable(y)
res3: 2 = 2

scala> identity(y)
res4: Int = 2

@adriaanm
Copy link
Owner Author

scala> x.isInstanceOf[y.type]
res6: Boolean = false

scala> x.isInstanceOf[1]
res7: Boolean = true

@@ -1160,6 +1160,7 @@ trait Implicits {
// necessary only to compile typetags used inside the Universe cake
case ThisType(thisSym) =>
gen.mkAttributedThis(thisSym)
// TODO SIP-23: TypeTag for LiteralType
Copy link
Owner Author

Choose a reason for hiding this comment

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

  • This could serve as the vehicle for summoning the type's inhabitant.

Copy link

Choose a reason for hiding this comment

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

Why not have a macro for this?

@folone
Copy link

folone commented Oct 28, 2014

This looks amazing, @adriaanm! I'm playing with it at the moment. Got a few questions:

  1. Is the plan to allow .type syntax only on non-literals? Or will we have an optional .type on literals as well?

    scala> val test: 10 = 10
    test: 10 = 10
    
    scala> val test: 10.type = 10
    <console>:1: error: ';' expected but '.' found.
           val test: 10.type = 10
                       ^
  2. The example I was struggling with the most is now working (yay!):

      scala> def ok: 7 = {
         | println("yay")
         | 7
         | }
    ok: 7
    
    scala> ok
    yay
    res10: 7 = 7
  3. This is just awesome:

    scala> val x = 1
    x: 1 = 1
    
    scala> val y: x.type = 1
    y: x.type = 1
    
    scala> implicitly[x.type =:= y.type]
    res16: =:=[x.type,y.type] = <function1>
    
    scala> implicitly[1 =:= y.type]
    res17: =:=[1,y.type] = <function1>
  4. The issue that you've fixed in SI-8564 does not work in this branch though:

    scala> def noisyIdentity(x: Any): x.type = {
         |     println("got " + x)
         |     x
         |   }
    noisyIdentity: (x: Any)x.type
    
    scala> noisyIdentity("test")
    res11: "test" = test
    
    scala> noisyIdentity(new java.lang.String("test"))
    got test
    res12: String = test
  5. Parsing unary minus is not working:

    scala> val test: -4 = -4
    <console>:1: error: ';' expected but integer literal found.
           val test: -4 = -4
                      ^
    
    scala> val test = -4
    test: -4 = -4
  6. Some more random rough edges I could find:

    scala> val t: 1 = t
    <console>:9: warning: value t does nothing other than call itself recursively
           val t: 1 = t
                      ^
    t: 1 = 0 // jvm default value?
    scala> trait Succ[T] {
         |     type Out
         |     def apply(x: T): Out
         |   }
    defined trait Succ
    
    scala> implicit object One extends Succ[1] {
         |     type Out = 2
         | def apply(x: 1) = 2
         | }
    defined object One
    
    scala> def f[T](x: T)(implicit succ: Succ[T]) = succ(x)
    f: [T](x: T)(implicit succ: Succ[T])succ.Out
    
    scala> implicitly[Succ[1]]
    res13: Succ[1] = One$@735d537e
    
    scala> f(1)
    <console>:12: error: could not find implicit value for parameter succ: Succ[Int]
                  f(1)
                   ^

Hope this is helpful. I'm super excited to see this moving forward! 💯 👍

@adriaanm
Copy link
Owner Author

Thanks for the feedback -- very helpful! I have to work on my devoxx talk for the next couple of days, but will get back to coding on this soon. In the mean time, PRs accepted on my sip23 branch ;-)

@folone
Copy link

folone commented Oct 28, 2014

I'll definitely spend quite some time with it!

@adriaanm
Copy link
Owner Author

Regarding the last rough edge, it's probably related to lack of type equality between ConstantType and LiteralType. Still mulling that one over.

@retronym
Copy link

Type inference for vals becomes more specific, is this intended?

scala> val y = 1
y: 1 = 1

scala> val y = if (true) 1 else 1
y: 1 = 1

scala> val y = if ("".isEmpty) 1 else 1
y: 1 = 1

@adriaanm
Copy link
Owner Author

It is, though it's up for debate. Under the hood, a ConstantType now deconsts to a LiteralType, which only then widens to the underlying type. Before, deconst would also widen (because there was no more precise type that wasn't a ConstantType).

@retronym
Copy link

What's the plan for class literals? The corresponding LiteralType can be inferred, but not expressed in syntax:

scala> val y = classOf[String]
y: classOf[java.lang.String] = class java.lang.String

Again, I think tend to think we should not infer these types unless guided by a bound of Singleton.

@adriaanm
Copy link
Owner Author

I was planning to only support syntactic literals. Right now, Symbols are not supported.

@retronym
Copy link

  • One small thing that is missing at the moment is a spec for the subtyping relationship between ConstantType / LiteralType and scala.Singleton.

@adriaanm
Copy link
Owner Author

The problem with Symbols is similar to classOf, they are method calls underneath:

scala> val x: 's = 's
<console>:7: error: stable identifier required, but scala.Symbol.apply("s") found.
       val x: 's = 's

@adriaanm
Copy link
Owner Author

Again, I think tend to think we should not infer these types unless guided by a bound of Singleton.

Do you have any snags in mind if we do?

@retronym
Copy link

Just the general problem of accidentally inferring too specific a type for some public API that ties your hands down the track. IIUC, that's why we deconst in type inference today, rather than just to ensure side effects aren't folded away.

I'd expect var and val infer the same type.

scala> val x = if ("".isEmpty) 1 else 1
x: 1 = 1

scala> var x = if ("".isEmpty) 1 else 1
x: Int = 1

@adriaanm
Copy link
Owner Author

As a compromise, we could widen for non-local vals. I think it's useful to preserve precise types locally.

@retronym
Copy link

Different rules for different contexts doesn't sound ideal to me. But we can give it some thought.

@adriaanm
Copy link
Owner Author

It feels desirable to me. widenIfNecessary already implements different rules for vars (shouldWiden) and (final) vals.

@adriaanm
Copy link
Owner Author

@odersky, what do you think? Is inferring x: 1 for val x = 1 desirable?

@retronym
Copy link

While we ponder that one, here's a curious wrinkle:

scala> List[1](1)
<console>:8: error: type mismatch;
 found   : Array[1]
 required: Array[Int]
Note: 1 <: Int, but class Array is invariant in type T.
You may wish to investigate a wildcard type such as `_ <: Int`. (SLS 3.2.10)
              List[1](1)
                     ^

@adriaanm
Copy link
Owner Author

Nice... covariant array ftw

@adriaanm
Copy link
Owner Author

Probably a widen in varargs code?

@retronym
Copy link

#13

@adriaanm
Copy link
Owner Author

Thanks!

@retronym
Copy link

Here's a failing test case with Java varargs.

https://github.com/retronym/scala/compare/topic/sip23-java-varargs?expand=1

In foundReqMessage, we find:

found = Array[<LiteralType(1)>]
req = Array[<RefinedType(LiteralType(1), Object)>]

The latter type is incorrectly printed as Array[1].

@retronym
Copy link

This works as expected, but is the sort of thing we'll have to add test cases for:

scala> class C { def foo(x: 1) = 0; def foo(x: Int) = 0 }
<console>:33: error: double definition:
def foo(x: 1): Int at line 33 and
def foo(x: Int): Int at line 33
have same type after erasure: (x: Int)Int
       class C { def foo(x: 1) = 0; def foo(x: Int) = 0 }
                                        ^

@retronym
Copy link

This seems inconsistent:

scala> class C[X <: Singleton](x: X)
defined class C

scala> new C(1)
res10: C[1] = C@7cd3860

vs

scala> class C[X <: 1](x: X)
defined class C

scala> new C(1)
<console>:35: error: inferred type arguments [Int] do not conform to class C's type parameter bounds [X <: 1]
              new C(1)
              ^
<console>:35: error: type mismatch;
 found   : Int(1)
 required: X
              new C(1)
                    ^

@retronym
Copy link

The spec for erasure (in "03. Types") currently says:

- The erasure of a singleton type `$p$.type` is the
  erasure of the type of $p$.

ConstantType-s aren't directly mentioned. Today, they are left intact by erasure, but later widened in the backend. I'm planning to change things to so they are widened during erasure; this is to fix:

scala> object O { override final val toString = "" }
java.lang.ClassFormatError: Duplicate method name&signature in class file O$

We should review that area of the spec and implementation holistically for that change and SIP-23.

folone and others added 11 commits November 18, 2014 14:10
TODO: deriving this from type param bounds is not the best way,
should consider all constraints that we encounter

scala> def stable[T <: 1](x: T): T = x
stable: [T <: 1](x: T)T

scala> stable(1)
res0: 1 = 1

scala> stable(2)
<console>:9: error: inferred type arguments [2] do not conform to method stable's type parameter bounds [T <: 1]
              stable(2)
              ^
<console>:9: error: type mismatch;
 found   : Int(2)
 required: T
              stable(2)
                     ^

scala> def stable[T <: Singleton](x: T): T = x
stable: [T <: Singleton](x: T)T

scala> val x: Int = 2
x: Int = 2

scala> val y: x.type = x
y: x.type = 2

scala> stable(y)
res3: x.type = 2
this bootstraps
```
scala> val x = "a"
x: "a" = a

scala> x.asInstanceOf["a"]
res1: "a" = a

scala> "1".asInstanceOf["2"]
x asInstanceOf Throwable
java.lang.ClassCastException
  ... 33 elided

scala> "1".asInstanceOf["1"]
res3: "1" = 1

scala> 1.asInstanceOf[1]
res4: 1 = 1

scala> 1.asInstanceOf[2]
x asInstanceOf Throwable
java.lang.ClassCastException
  ... 33 elided

```
@adriaanm adriaanm force-pushed the sip23 branch 4 times, most recently from f67a7bc to 61a16f5 Compare November 18, 2014 22:20
@soc
Copy link

soc commented Dec 10, 2014

What's the perspective here regarding union types?

If we have something like if (bool) 1 else 2 would the return type be 1 | 2?

@adriaanm
Copy link
Owner Author

Likely, but union types are still too far in the future to know for sure how they'll interact with this feature. They are primarily about class types -- you could already experiment in dotty and see what p.type | q.type means, where p and q refer to strings...

@adriaanm adriaanm force-pushed the 2.11.x branch 5 times, most recently from 93cb02e to 3410d25 Compare December 29, 2014 22:43
@adriaanm adriaanm force-pushed the 2.11.x branch 2 times, most recently from 7d6747a to 43289bc Compare January 10, 2015 04:56
@adriaanm
Copy link
Owner Author

(Will reopen when I get a chance to work on this again!)

@adriaanm adriaanm closed this Jan 16, 2015
adriaanm pushed a commit that referenced this pull request Aug 19, 2016
  - Avoid boxing the {Object, Int, ...}Ref itself by storing it in
    a val, not a var
  - Avoid box/unbox of primitive lazy expressions due which are added
    to "adapt" it to the erased type of the fictional syncronized
    method, by using a `return`. We have to add a dummy throw after
    the synchronized block, but this is elimnated by the always-on
    DCE in the code generator.

```
⚡  qscalac -Xprint:mixin $(f "class C { def foo = { lazy val x = 42; x }}"); javap -private -c -cp . C
[[syntax trees at end of                     mixin]] // a.scala
package <empty> {
  class C extends Object {
    def foo(): Int = {
      lazy <artifact> val x$lzy: scala.runtime.LazyInt = new scala.runtime.LazyInt();
      C.this.x$1(x$lzy)
    };
    final private[this] def x$1(x$lzy$1: scala.runtime.LazyInt): Int = {
      x$lzy$1.synchronized({
        if (x$lzy$1.initialized().unary_!())
          {
            x$lzy$1.initialized_=(true);
            x$lzy$1.value_=(42)
          };
        return x$lzy$1.value()
      });
      throw null
    };
    def <init>(): C = {
      C.super.<init>();
      ()
    }
  }
}

Compiled from "a.scala"
public class C {
  public int foo();
    Code:
       0: new           #12                 // class scala/runtime/LazyInt
       3: dup
       4: invokespecial #16                 // Method scala/runtime/LazyInt."<init>":()V
       7: astore_1
       8: aload_1
       9: invokestatic  #20                 // Method x$1:(Lscala/runtime/LazyInt;)I
      12: ireturn

  private static final int x$1(scala.runtime.LazyInt);
    Code:
       0: aload_0
       1: dup
       2: astore_1
       3: monitorenter
       4: aload_0
       5: invokevirtual scala#31                 // Method scala/runtime/LazyInt.initialized:()Z
       8: ifne          22
      11: aload_0
      12: iconst_1
      13: invokevirtual scala#35                 // Method scala/runtime/LazyInt.initialized_$eq:(Z)V
      16: aload_0
      17: bipush        42
      19: invokevirtual scala#39                 // Method scala/runtime/LazyInt.value_$eq:(I)V
      22: aload_0
      23: invokevirtual scala#42                 // Method scala/runtime/LazyInt.value:()I
      26: istore_2
      27: goto          33
      30: aload_1
      31: monitorexit
      32: athrow
      33: aload_1
      34: monitorexit
      35: iload_2
      36: ireturn
    Exception table:
       from    to  target type
           4    30    30   Class java/lang/Throwable

  public C();
    Code:
       0: aload_0
       1: invokespecial scala#43                 // Method java/lang/Object."<init>":()V
       4: return
}
```
adriaanm pushed a commit that referenced this pull request Aug 22, 2016
Top level modules in Scala currently desugar as:

```
class C; object O extends C { toString }
```

```
public final class O$ extends C {
  public static final O$ MODULE$;

  public static {};
    Code:
       0: new           #2                  // class O$
       3: invokespecial #12                 // Method "<init>":()V
       6: return

  private O$();
    Code:
       0: aload_0
       1: invokespecial #13                 // Method C."<init>":()V
       4: aload_0
       5: putstatic     #15                 // Field MODULE$:LO$;
       8: aload_0
       9: invokevirtual #21                 // Method java/lang/Object.toString:()Ljava/lang/String;
      12: pop
      13: return
}
```

The static initalizer `<clinit>` calls the constructor `<init>`, which
invokes superclass constructor, assigns `MODULE$= this`, and then runs
the remainder of the object's constructor (`toString` in the example
above.)

It turns out that this relies on a bug in the JVM's verifier: assignment to a
static final must occur lexically within the <clinit>, not from within `<init>`
(even if the latter is happens to be called by the former).

I'd like to move the assignment to <clinit> but that would
change behaviour of "benign" cyclic references between modules.

Example:

```
package p1; class CC { def foo = O.bar}; object O {new CC().foo; def bar = println(1)};

// Exiting paste mode, now interpreting.

scala> p1.O
1
```

This relies on the way that we assign MODULE$ field after the super class constructors
are finished, but before the rest of the module constructor is called.

Instead, this commit removes the ACC_FINAL bit from the field. It actually wasn't
behaving as final at all, precisely the issue that the stricter verifier
now alerts us to.

```
scala> :paste -raw
// Entering paste mode (ctrl-D to finish)

package p1; object O

// Exiting paste mode, now interpreting.

scala> val O1 = p1.O
O1: p1.O.type = p1.O$@ee7d9f1

scala> scala.reflect.ensureAccessible(p1.O.getClass.getDeclaredConstructor()).newInstance()
res0: p1.O.type = p1.O$@64cee07

scala> O1 eq p1.O
res1: Boolean = false
```

We will still achieve safe publication of the assignment to other threads
by virtue of the fact that `<clinit>` is executed within the scope of
an initlization lock, as specified by:

  https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-5.html#jvms-5.5

Fixes scala/scala-dev#SD-194
SethTisue pushed a commit that referenced this pull request Apr 25, 2018
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.

8 participants