[resubmit] Experimental Single Abstract Method support (sammy meets world) #3037

Merged
merged 9 commits into from Oct 14, 2013

Projects

None yet

5 participants

@gkossakowski
The Scala Programming Language member

This is resubmission of #3018 after merge conflict has been resolved.

Under -Xexperimental, typedFunction invokes synthesizeSAMFunction when the expected type for the function literal (pt) is not the built-in FunctionN type of the expected arity, but pt does have a SAM with the expected number of arguments.

synthesizeSAMFunction expands the following tree:

{ (p1: T1, ..., pN: TN) => body } : S

to:

{
 def apply$body(p1: T1, ..., pN: TN): T = body

 new S { def apply(p1: T1, ..., pN: TN): T = apply$body(p1,..., pN) }
}

The expansion assumes S (the expected type) defines a single abstract method (let's call that method apply for simplicity).

  1. If T is not fully defined, it is inferred by type checking def apply$body without a result type before type checking the block. The method's inferred result type is used instead of T. See test case

  2. To more easily enforce that S's members are not in scope in body, that tree goes to the apply$body method that's outside the anonymous subclass of S. (The separate apply$body method simplifies the implementation of 1&2.) See test case

  3. The following restrictions apply to S:

    1. Its primary constructor (if any) must be public, no-args, not overloaded.
    2. S must have exactly one abstract member, its SAM
    3. SAM must take exactly one argument list
    4. SAM must be monomorphic

We may later relax these requirements to allow an implicit argument list, both on the constructor and the SAM. Could also let the SAM be polymorphic.

PS: We'll require import language.sam instead of -Xexperimental, as soon as the SIP is ready and there are more tests.

adriaanm and others added some commits Oct 4, 2013
@adriaanm adriaanm Simplify partest.task target, fix typo in comment. 8db838e
@adriaanm adriaanm Clarify findMembers, add reverse engineered docs
When looking for deferred members, it only makes sense
to retry when deferred members aren't excluded.
e864129
@adriaanm adriaanm Single Abstract Method support: synthesis helpers
`synthesizeSAMFunction` will be used to expand the following tree:

```
{ (p1: T1, ..., pN: TN) => body } : S
```

to:

```
{
 def apply$body(p1: T1, ..., pN: TN): T = body

 new S { def apply(p1: T1, ..., pN: TN): T = apply$body(p1,..., pN) }
}
```

The expansion assumes `S` (the expected type) defines a single abstract method
(let's call that method `apply` for simplicity).

1. If 'T' is not fully defined, it is inferred by type checking
`def apply$body` without a result type before type checking the block.
The method's inferred result type is used instead of T`.
[See test/files/pos/sammy_poly.scala]

2. To more easily enforce S's members are not in scope in `body`, that tree
goes to the `apply$body` method that's outside the anonymous subclass of S.
(The separate `apply$body` method simplifies the implementation of 1&2.)

3. The following restrictions apply to S:
  1. Its primary constructor (if any) must be public, no-args, not overloaded.
  2. S must have exactly one abstract member, its SAM
  3. SAM must take exactly one argument list
  4. SAM must be monomorphic

We may later relax these requirements to allow an implicit argument list,
both on the constructor and the SAM. Could also let the SAM be polymorphic.
657e85f
@adriaanm adriaanm Single Abstract Method support: synthesize SAMs
Under `-Xexperimental`, `typedFunction` invokes `synthesizeSAMFunction`
when the expected type for the function literal (`pt`) is not the built-in
`FunctionN` type of the expected arity, but `pt` does have a SAM
with the expected number of arguments.

PS: We'll require `import language.sam` instead of `-Xexperimental`,
as soon as the SIP is ready and there are more tests.
67062db
@retronym retronym Don't pursue SAM translation after an arity mismatch.
Before this change:

    scala> trait T { def apply(a: Int): Int }
    defined trait T

    scala> ((x: Int, y: Int) => 0): T
    <console>:9: error: object creation impossible, since method apply in trait T of type (a: Int)Int is not defined
                  ((x: Int, y: Int) => 0): T
                                    ^
After the change, these cases report the same errors as they do
*without* -Xexperimental.
00c9c16
@adriaanm adriaanm Extract SerialVersionUIDAnnotation. Make SAM body synthetic.
Addressing review feedback.
4265ab6
@adriaanm adriaanm Single Abstract Method support: java8 test
Inspired by test/files/run/t7398.scala and sammy_poly.
Added some notes to original tests.

Elaborating on that note: we don't yet desugar `f(a)` to `f.sam(a)`,
like we do for regular functions: `f(a)` becomes `f.apply(a)`.

It seems pleasingly symmetrical and is easy to implement,
but not sure it's a good idea...
58ba39c
@adriaanm adriaanm Remove stray debug comment 71d4e28
@gkossakowski gkossakowski Merge remote-tracking branch 'scala/master' into fix-merge-3018
Conflicts:
	src/compiler/scala/tools/nsc/typechecker/Typers.scala
b126e5c
@gkossakowski
The Scala Programming Language member

Review by @retronym. This is identical to #3018 apart from the last commit which resolves the merge conflict against current master.

@retronym retronym was assigned Oct 13, 2013
@gkossakowski
The Scala Programming Language member

@retronym: ping

The merge conflict was due to 83feb86.

@retronym
The Scala Programming Language member

The merge looks correct.

LGTM

@gkossakowski
The Scala Programming Language member

Thanks!

@gkossakowski gkossakowski merged commit 686fb48 into scala:master Oct 14, 2013

1 check passed

Details default pr-scala Took 77 min.
@gkossakowski gkossakowski deleted the gkossakowski:fix-merge-3018 branch Oct 14, 2013
@adriaanm
The Scala Programming Language member

Thanks!

@scottcarey

PS: We'll require import language.sam instead of -Xexperimental, as soon as the SIP is ready and there are more tests.

So, 'import language.sam' does not work on Scala 2.11. I suppose the -Xexperimental part was not changed? I can not find any info about this other than this PR, especially with respect to how to use it (via language import or experimental flag). Any idea when it will be available by import? 2.12 (a long time from now) or 2.11.future (not too distant)?

@adriaanm
The Scala Programming Language member

It's available in 2.11.5-SNAPSHOT under -Xexperimental. Happy to hear about bugs! It works for me with the Java 8 Streams API except for a couple of rough edges around type inference because we do not support wildcard capture. Will be available under language flag in 2.12 but with same functionality.

@scottcarey

Well, I'm not sure I'll enable -Xexperimental (and all the other undocumented stuff that may or may not turn on) on my builds for production code to test it out, and test whether or not it propagates through to everyone's development environments without issue. ScalaIDE is flaky enough as is.

That is a lot more work than adding an import statement.
I'd happily import 'scala.language.experimental.sam' instead, and be forced to change the import statement once it was no longer experimental.

So far, its been easier to add some implicits for the SAM types I want.

@adriaanm
The Scala Programming Language member

Good to know, thanks for sharing your perspective. We're not ready to declare it non-experimental yet, though. We don't add new features to minor releases in any case.

@adriaanm
The Scala Programming Language member

We could expose it under an individual flag, say -Xpreview-sip:24?

@som-snytt

+1. -Ysip:24 is briefer.

@scottcarey

That would work. I'll still have to go through all of the development environment testing to ensure the flag works its way from maven through to the IDEs without manual intervention before pushing a change, but at least I would not need to wonder what else it enabled and it would be easier to explain and justify as a stand-alone item.

@scottcarey

Sad news.
ScalaIDE does not pick up the additional scalac arguments from the maven build, so although adding -Xexperimental to the build configuration allows for SAM typing to work with a command line build, it does not propagate to the IDE. This means that every IDE developer would have to manually set -Xexperimental (or a new -Ysip:24) on all ~30 scala projects, one at a time, every time they re-imported the build configuration (~once a week).

As a result, until Scala IDE (and IntelliJ, which may or may not work as I have not tested) pick up the additional configuration flags automatically, setting this on non-sbt projects would probably get me hanged. Of course, the non-sbt projects happen to be the java-scala hybrids, where this feature would be particularly useful.

I'll bring this up with the Scala IDE folk tomorrow, perhaps I'm missing something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment