Skip to content

Decouple ImplicitFunctionN from FunctionN. #2008

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

Conversation

nicolasstucki
Copy link
Contributor

This removes an illegal method override mentioned in #2000.

This removes an illegal method override mentioned in scala#2000.
@nicolasstucki nicolasstucki force-pushed the implicit-function-decouple branch from 41b1b97 to ebf7fd4 Compare February 21, 2017 13:32
case Closure(Nil, id @ Ident(nme.ANON_FUN), _) =>
val adaptToPrototype =
if (defn.isFunctionType(pt)) {
// convert implicit function to function
Copy link
Contributor

@odersky odersky Feb 21, 2017

Choose a reason for hiding this comment

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

Not sure about the change in this logic. Questions:

  • Do we want to convert implicit functions to functions, or to other SAM types? Does this case even arise? I was assuming that an implicit function is always immediately applied.
  • What would happen if we kept the previous code here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is from implicit function to function, previously this worked because implicit functions where a subtype of functions.

A concrete case that fails without this change is

-- [E007] Type Mismatch Error: /Users/nicolasstucki/GitHub/dotty/tests/partest-generated/run/implicitFuns.scala --------
148 |        println(if (thisTransaction.isAborted) "aborted" else s"result: $res")
    |                                                                              ^
    |                                                                        found:    implicit Transaction => Unit
    |                                                                        required: Transaction => Any

where

  def transaction[T](op: Transaction => T) = {
    ...
  }

  def main(args: Array[String]) = {
    transaction {
      implicit thisTransaction =>
        val res = f1(args.length)
        println(if (thisTransaction.isAborted) "aborted" else s"result: $res")
    }
  }

Is there a better way to implement this coercion from implicit function to function?

@odersky
Copy link
Contributor

odersky commented Feb 22, 2017

@nicolasstucki Thanks for explaining! So we need to make implicit functions compatible with normal functions after all. This means we have the following choices:

  • add a specific conversion, as in this PR. This is conceptually heavy. Every new kind of conversion adds considerable complexity to the spec.
  • drop implicit modifier on apply method of implicit function, but treat it as an implicit anyway. This is also irregular.
  • Make implicit functions subtypes of functions, but omit the check on implicit overrides. This is the status quo. To keep it bullet proof we have to keep implicit functions effectively final (which they are, as of now, because of the impossibility to define apply).

To come to a resolution here I think it's best to answer first the question whether implicit functions should be final or not. Making them non-final gives us a lot of new power. For instance:

 class Dummy
 implicit val d: Dummy = new Dummy
 class Ref[T](var elem: T) extends implicit Dummy => T {
    def apply(implicit d: Dummy): T = elem
 }

 val r = new Ref(22)
 val y = r    // y has type Int, not Ref[Int]!

Super powerful. But do we want it?

@nicolasstucki
Copy link
Contributor Author

I do not have any use cases for this. We should still double check that with #2014 we can not override the apply of an implicit function with def apply(d: Dummy) (in the example above), this was perviously possible.

@odersky
Copy link
Contributor

odersky commented Feb 23, 2017

We should still double check that with #2014 we can not override the apply of an implicit function with def apply(d: Dummy) (in the example above), this was perviously possible.

I tried it in the REPL: Now it's no longer possible.

@nicolasstucki
Copy link
Contributor Author

Then this PR is irrelevant now.

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