Basic implementation of 42.type #45

folone · 2014-09-12T12:45:50Z

Hey folks,

Sorry this took so long: got swamped with all the things that happened while I was away from the internet.
As discussed in #18, sending the first part of 42.type – without a way to materialize inhabitants of singleton types.

Please take a look @milessabin @puffnfresh @non

propensive · 2014-09-12T12:54:40Z

Thanks, George! :)

puffnfresh · 2014-09-12T13:28:56Z

Thank you! Looks pretty great, I'll take a close look today.

milessabin · 2014-09-12T18:16:23Z

Good stuff ... I'll take a look at it over the weekend.

milessabin · 2014-09-13T07:56:10Z

@paulp are there any specific issues that you'd like to highlight?

paulp · 2014-09-13T08:00:55Z

That's what I was doing. Unless this iPad client has betrayed me I left the preceding comment on the line I meant. Look for the var.

milessabin · 2014-09-13T08:06:16Z

@paulp It looks like it's just linked to the whole commit ... I might be missing something, but I don't see a comment on any particular lines.

relrod · 2014-09-13T09:33:02Z

@milessabin If you go to the issue view that has all the comments on it (including this one), go to @paulp's comment and it says "paulp commented on 7e59605 src/reflect/scala/reflect/internal/Types.scala 12 hours ago" - click on the filename and it will bring you to the file he commented on, though it doesn't seem to auto-scroll to the specific line.

Anyway, because I'm a nice person (at least once in a while :P) here's a direct link to the line (and comment beneath it). ;)

milessabin · 2014-09-13T09:52:31Z

@codeblock Thanks. @paulp assuming the mutable state were eliminated, would you be happy?

folone · 2014-09-13T13:31:34Z

@paulp Agree, will look into this. There's a single place where asDeclaredSingleton is used: scala.tools.nsc.typechecker.Typers, line 5117. I tried making isDeclaredSingleton a def, and changing that block to

tree setType {
  if (tree.isLiteral) new SimpleTypeProxy {
    override def underlying = refTyped.tpe.resultType
    override def isDeclaredSingleton = true
  }
  else refTyped.tpe.resultType
}

That didn't work though. Will continue looking.

paulp · 2014-09-13T14:27:55Z

It won't be that easy, that's how a var ended up there in the first place. You can't just attach state to a Type and expect things to work. That's just as true of a def in a subclass as it is of a naked var. Everywhere that types are propagated, duplicated, created, everything must propagate that state. The fact that it kind of works because the state survives a while just makes it worse, because it plants all kinds of bugs for the next guy to enjoy.

I would suggest annotating the type except the compiler arbitrarily discards annotations on the way through TypeMaps. That is the cause of untold bugs. I fixed it in policy actually. I'm not sure what to suggest for reliably attaching information to a Type. The fact that something so basic admits no direct solution is why I don't work on other peoples' terms (or their types) anymore.

milessabin · 2014-09-17T06:51:08Z

See also #51.

typelevel-bot · 2014-09-23T20:30:03Z

Can one of the admins verify this patch?

puffnfresh · 2014-09-23T22:56:04Z

add to whitelist

puffnfresh · 2014-09-24T16:52:12Z

Build is green, what can we do to make this ready for merging?

milessabin · 2014-09-25T06:14:05Z

I think the main thing was the mutable state issue ... @folone, have you been able to make any progress on that?

folone · 2014-09-25T07:36:59Z

Did not get around to exploring what @paulp suggested. Will try to do that over the weekend.

folone · 2014-09-25T08:34:24Z

Rebased onto the latest 2.11.x in the meantime.

retronym · 2014-10-02T05:26:44Z

Here's what we have today:

SingletonType marks a term as stable. For a term f with a stable type, the program { g(f); h(f) } can be rewritten as { val temp = f; g(temp); g(temp); } (well, approximately, if g or h have prefixes you need to evaluate them first).

ConstantType means that a term need not be evaluated at all, the value in its type can be used instead. This has been to support "constant expressions" / "constant value definitions". Note that "constant types" themselves are not specced in Types, they currently an internal implementation detail of constant expressions.

I had previously thought it would be sufficient to change our constant folder to act not on the types, but rather on the terms. so Literal(Const(42)) + Literal(Const(42)) could fold to 84, but q"foo"{42.type} + q"foo"{42.type} would not. That's what I prototyped in topic/const-type-fold.

But on second thoughts, I now wonder if we instead will need to introduce an alternative to ConstantType that doesn't imply inlinability.

I think it would be really clarifiying to work out a few of these design issues by considering how they would be specced.

folone · 2014-10-02T19:16:57Z

Hey folks,

Just letting you know that I'm still here. I have a working solution (no vars) along the lines of what @retronym suggests: a type similar to ConstantType, that does not imply constant folding. It still has some rough edges though. I'm also on the road at the moment, and have limited internet access. Will push what I've got when I'm back on Monday, so this can be further discussed. By the way, guess this change will also require changes to the spec. Will work on that too.

puffnfresh · 2014-10-02T20:33:43Z

🎆

non · 2014-10-02T21:02:27Z

Awesome! This sounds great, thanks!

paulp · 2014-10-02T21:39:41Z

If you add a new subclass of Type, you may soon see why they call it the expression "problem".

folone · 2014-10-06T14:57:27Z

Pushed that fix. Here are the rough edges I was talking about:

scala> var t: 7.type = 7
<console>:7: error: assignment to non variable
       var t: 7.type = 7
           ^

and

scala> def test: 23.type = {
     | println("PANDA!")
     | 23
     | }
<console>:7: error: type mismatch;
 found   : Int
 required: 23.type
       def test: 23.type = {
                           ^

milessabin · 2014-10-06T20:38:00Z

I have no problem with the first being an error: I'd just rather it read "error: assignment to variable" ;-)

The second case is of a piece with SI-5103 ... if we explicitly ask for a singleton type then the compiler shouldn't widen values out from under us.

Am I right that the latter can be worked around with a type ascription on the literal, ie. (23: 23.type)?

folone · 2014-10-06T20:52:12Z

@milessabin right, something like that would work:

scala> def test: 23.type = {
     | println("panda!")
     | val res: 23.type = 23
     | res
     | }
test: 23.type

folone · 2014-10-16T14:29:18Z

Fixed the var rough edge in 4a4c88e

non · 2014-10-16T14:39:34Z

What do you make of this Jenkins failure?

http://typelevel-ci.orexio.org/job/typelevel-scala-pr/34/console

folone · 2014-10-16T14:40:33Z

@non Seems strange, I've just rebuilt ant clean && ant build locally. Worked fine.

folone · 2014-10-16T14:41:32Z

This last commit broke a few tests though, will look into that. Reverting until I can find a fix. One of the failing tests is related to SI-2984

- Implements mandatory `.type` for identifiers - Implements optional `.type` for literals - Fixes a few problems with it, updates .check files

- Remove mutable state by introducing `DeclaredSingletonType` - Update the spec

folone · 2014-10-28T13:58:19Z

There's a better implementation in adriaanm#12, which addresses most of the issues discussed here. I'll look into porting it into this repo over the weekend.

folone · 2014-11-17T16:00:37Z

Closing this, as it is superseded by #86, #87, and #88.

Rather than in implementation of the abstract method in the expanded anonymous class. This leads to more more efficient use of the constant pool, code shapes more amenable to SAM inlining, and is compatible with the old behaviour of `-Xexperimental` in Scala 2.11, which ScalaJS now relies upon. Manual test: ``` scala> :paste -raw // Entering paste mode (ctrl-D to finish) package p1; trait T { val x = 0; def apply(): Any }; class DelambdafyInline { def t: T = (() => "") } // Exiting paste mode, now interpreting. scala> :javap -c p1.DelambdafyInline Compiled from "<pastie>" public class p1.DelambdafyInline { public p1.T t(); Code: 0: new scala#10 // class p1/DelambdafyInline$$anonfun$t$1 3: dup 4: aload_0 5: invokespecial scala#16 // Method p1/DelambdafyInline$$anonfun$t$1."<init>":(Lp1/DelambdafyInline;)V 8: areturn public final java.lang.Object p1$DelambdafyInline$$$anonfun$1(); Code: 0: ldc scala#22 // String 2: areturn public p1.DelambdafyInline(); Code: 0: aload_0 1: invokespecial scala#25 // Method java/lang/Object."<init>":()V 4: return } scala> :javap -c p1.DelambdafyInline$$anonfun$t$1 Compiled from "<pastie>" public final class p1.DelambdafyInline$$anonfun$t$1 implements p1.T,scala.Serializable { public static final long serialVersionUID; public int x(); Code: 0: aload_0 1: getfield scala#25 // Field x:I 4: ireturn public void p1$T$_setter_$x_$eq(int); Code: 0: aload_0 1: iload_1 2: putfield scala#25 // Field x:I 5: return public final java.lang.Object apply(); Code: 0: aload_0 1: getfield scala#34 // Field $outer:Lp1/DelambdafyInline; 4: invokevirtual scala#37 // Method p1/DelambdafyInline.p1$DelambdafyInline$$$anonfun$1:()Ljava/lang/Object; 7: areturn public p1.DelambdafyInline$$anonfun$t$1(p1.DelambdafyInline); Code: 0: aload_1 1: ifnonnull 6 4: aconst_null 5: athrow 6: aload_0 7: aload_1 8: putfield scala#34 // Field $outer:Lp1/DelambdafyInline; 11: aload_0 12: invokespecial scala#42 // Method java/lang/Object."<init>":()V 15: aload_0 16: invokespecial scala#45 // Method p1/T.$init$:()V 19: return } scala> :quit ``` Adriaan is to `git blame` for `reflection-mem-typecheck.scala`.

puffnfresh mentioned this pull request Sep 12, 2014

42.type #18

Closed

folone force-pushed the 42.type-typelevel branch from 132c0e5 to 6e8fa15 Compare September 25, 2014 08:33

folone force-pushed the 42.type-typelevel branch from 21697cb to b7f4946 Compare October 6, 2014 14:56

folone force-pushed the 42.type-typelevel branch from 4a4c88e to b7f4946 Compare October 17, 2014 08:30

folone added 3 commits October 22, 2014 10:13

Basic implementation of 42.type:

b11d7ba

- Implements mandatory `.type` for identifiers - Implements optional `.type` for literals - Fixes a few problems with it, updates .check files

Get rid of unneeded var.

0c521ca

Address the review:

0c0cd8a

- Remove mutable state by introducing `DeclaredSingletonType` - Update the spec

folone force-pushed the 42.type-typelevel branch from b7f4946 to 0c0cd8a Compare October 24, 2014 07:57

folone mentioned this pull request Nov 14, 2014

SIP-23 #86

Merged

folone closed this Nov 17, 2014

folone deleted the 42.type-typelevel branch November 17, 2014 16:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Basic implementation of 42.type #45

Basic implementation of 42.type #45

folone commented Sep 12, 2014

propensive commented Sep 12, 2014

puffnfresh commented Sep 12, 2014

milessabin commented Sep 12, 2014

milessabin commented Sep 13, 2014

paulp commented Sep 13, 2014

milessabin commented Sep 13, 2014

relrod commented Sep 13, 2014

milessabin commented Sep 13, 2014

folone commented Sep 13, 2014

paulp commented Sep 13, 2014

milessabin commented Sep 17, 2014

typelevel-bot commented Sep 23, 2014

puffnfresh commented Sep 23, 2014

puffnfresh commented Sep 24, 2014

milessabin commented Sep 25, 2014

folone commented Sep 25, 2014

folone commented Sep 25, 2014

retronym commented Oct 2, 2014

folone commented Oct 2, 2014

puffnfresh commented Oct 2, 2014

non commented Oct 2, 2014

paulp commented Oct 2, 2014

folone commented Oct 6, 2014

milessabin commented Oct 6, 2014

folone commented Oct 6, 2014

folone commented Oct 16, 2014

non commented Oct 16, 2014

folone commented Oct 16, 2014

folone commented Oct 16, 2014

folone commented Oct 28, 2014

folone commented Nov 17, 2014

Basic implementation of 42.type #45

Basic implementation of 42.type #45

Conversation

folone commented Sep 12, 2014

propensive commented Sep 12, 2014

puffnfresh commented Sep 12, 2014

milessabin commented Sep 12, 2014

milessabin commented Sep 13, 2014

paulp commented Sep 13, 2014

milessabin commented Sep 13, 2014

relrod commented Sep 13, 2014

milessabin commented Sep 13, 2014

folone commented Sep 13, 2014

paulp commented Sep 13, 2014

milessabin commented Sep 17, 2014

typelevel-bot commented Sep 23, 2014

puffnfresh commented Sep 23, 2014

puffnfresh commented Sep 24, 2014

milessabin commented Sep 25, 2014

folone commented Sep 25, 2014

folone commented Sep 25, 2014

retronym commented Oct 2, 2014

folone commented Oct 2, 2014

puffnfresh commented Oct 2, 2014

non commented Oct 2, 2014

paulp commented Oct 2, 2014

folone commented Oct 6, 2014

milessabin commented Oct 6, 2014

folone commented Oct 6, 2014

folone commented Oct 16, 2014

non commented Oct 16, 2014

folone commented Oct 16, 2014

folone commented Oct 16, 2014

folone commented Oct 28, 2014

folone commented Nov 17, 2014