Disallow subtypes of Function1 acting as implicit conversions

[odersky]

The new test falseView.scala shows the problem. We might create
an implicit value of some type that happens to be a subtype of Function1.
We might not expect that this gives us an implicit conversion, this
is most often unintended and surprising.

See the comment in Implicits#discardForView for a discussion why
we picked the particular scheme implemented here.

[smarter]

Yes! This is something I've wanted ever since I saw http://scalapuzzlers.com/#pzzlr-054

[odersky]

Feels good to slain another puzzler. I have added it as a test.

[lihaoyi]

Can we ban all implicit vals acting as conversions, even Function1 itself? It seems to me that if someone wants an implicit conversion, they can define a method with def. Having two ways to write an implicit (val and def) seems unnecessary to me

[smarter]

@lihaoyi The common usecase that this allow is def foo(...)(implicit ev: A => B) = ..., you could always do:

def foo(...)(implicit ev: A => B) = {
  implicit def conv(x: A): B = ev(x)
  ...
}

But that would be pretty cumbersome.

[lihaoyi]

To satisfy that common use case, could we do something like

trait Converter[A, B]{
  def apply(a: A): B
}
implicit def convertable[A, B](a: A)(implicit c: Converter[A, B]): B = c.apply(a)

In the standard library, and then let people write

def foo[A, B](a: A)(implicit ev: Converter[A, B]) = {
  val b: B = a // convertable(a)(ev)
}

That should (I think?) have the same implicit conversion effect, while also replacing the use of Function1[A, B] (which is a very common type!) with a Converter[A, B] type, which you're much less likely to unintentionally include in your inheritance hierarchy.

EDIT: Perhaps another alternative, if we want to keep using Function1 but remove it from the "implicit conversion" logic in the compiler, is defining convertable like:

implicit def convertable[A, B](a: A)(implicit c: A => B): B = c.apply(a)

Then implicit conversions can always be method-driven as far as the compiler is concerned.; Function1[A, B] becomes just another implicit parameter/value, and it just happens there's an implicit def method in the predef that uses that to convert things.

[odersky]

@lihaoyi

EDIT: Perhaps another alternative, if we want to keep using Function1 but remove it from the "implicit conversion" logic in the compiler, is defining convertable like:

 implicit def convertable[A, B](a: A)(implicit c: A => B): B = c.apply(a)

That's covered in the comment explaining the scheme under "throws out the baby with the bathwater". It would just revert to the status quo where every implicit value of some subtype of Function1 creates an implicit conversion. I like the Converter idea better except that with our current implicit search algorithm it would slow down implicit search significantly. One can get aorund it, at the limit by special casing like we already do for <:<.

[odersky]

@lihaoyi The latest commit follows your suggestion. Thanks for making it!

[nafg]

I think it's more elegant not to distinguish between implicit def or val. The way I see it, there are only implicit values. When something doesn't match the expected type, the compiler searches the implicit scope for an A => B. The fact that the def syntax works follows the same principle as eta expansion.

[vladap]

Another reason why puzzler-054 happens is that implicit val is used for a common type.

What would we lost to enforce that implicit val has to be a custom type unrelated to any common type defined by stdlib?

In the puzzler it would require to define custom Deck type:

case class Deck(cards: List[Card])

To make Deck a subtype of List[Card] I could define implicit conversion to select List[Card] from Deck. Which is insipired by embedded types in Golang minus inheritance.

object Deck {
 implicit def toListCard(deck: Deck): List[Card] = deck.cards
}

Now it works just like type substitution by nominal subtyping. I can use Deck anywhere List[Card] is expected.

Puzzler-54 changes to this and is gone. It is still convinient to use Deck just like it would be List[Card] itself or extend List[Card]:

case class Card(number: Int, suit: String = "clubs") {
  val value = (number % 13) + 1 // ace = 1, king = 13
  def isInDeck(implicit deck: Deck) = deck contains this
}

case class Deck(cards: List[Card])
object Deck {
  implicit def toListCard(deck: Deck): List[Card] = deck.cards
}

implicit val deck = Deck(List(Card(1, "clubs")))
implicit def intToCard(n: Int) = Card(n)

println(1.isInDeck)

Interesting note is that this implicit conversion is completely invisible in Golang.

implicit def toListCard(deck: Deck): List[Card] = deck.cards

[vladap]

What would we lost to enforce that implicit val has to be a custom type unrelated to any common type defined by stdlib?

It is probably not good enough rule as a general one because it would need to cover libraries. I have to think more how suggested limitations relates to it.

[vladap]

In puzzler-54 I would expect compiler to fail with ambiguous resolution. What is the reason that it prefers implicit conversion defined by val over def?

[vladap]

@lihaoyi, @smarter

The common usecase that this allow is def foo(...)(implicit ev: A => B) = ...

I'm not sure I follow. lihaoyi was suggesting to disallow vals acting as conversions and allow only defs.

How it impacts the above use case? Currently it accepts def conversion directly as ev, no need to define it as implicit val. It would be disallowed by lihaoyi suggestion as well? As I understand it wouldn't be allowed to ETA expand def to function val during implicit search? Or however it works behind the wheels.

If I understand it correctly at all. Why such restriction has to be there? Aren't function vals which are obtained by compiler from user defined implicit defs fine for implicit conversions?

I can't figure out how it can go bad if only implicit defs (never vals) are considered during implicit conversion search and the code below would continue to work just like it works now.

class Foo { def m = 1 }
class Bar { def m = 2 }

implicit def fooToBar(foo: Foo): Bar = new Bar

def baz()(implicit ev: Foo => Bar): Int = {
  val foo = new Foo
  val bar: Bar = ev(foo)
  
  bar.m // returns 2
}

One can forget to actually convert using ev inside baz() method body. It will still work though. Evidence is a needless and confusing declaration in this case.

// ... the same as before

def baz()(implicit ev: Foo => Bar): Int = {
  val foo = new Foo
  val bar: Bar = foo
  
  bar.m // returns 2
}

EDIT:

Aren't function vals which are obtained by compiler from user defined implicit defs fine for implicit conversions?

They are not implicit conversion, they have to be used explicitly.

[smarter]

@vladap This works in Scala 2 currently, without any supporting code:

object Test {
  def foo[A,B](a: A)(implicit ev: A => B) = {
    val b: B = a
  }
}

This works because ev is an implicit val in scope whose type is a function from A to B, so the compiler will add an implicit conversion: val b: B = ev(a).

[vladap]

@smarter. You use type parameters, hence it is cleaner. Otherwise B would be unknown without A => B evidence. With a bit of thinking I can infer that it will probably use ev, still it is not that obvious.

I used concrete types Foo, Bar and in this case it is not obvious if it is rewritten using ev or fooToBar directly. Maybe the rule is that it uses the option closest to the call scope - in this case ev which is input argument. But if I would remove implicit block it would still work as far as implicit def fooToBar is around.

It could be better if it would fail at compilation, enforcing to explicitly use ev(a).

Interesting point to realize is that the code below doesn't use implicit conversion. The conversion itself is actually explicit.

def baz(foo: Foo)(implicit ev: Foo => Bar) = {
  val bar: Bar = ev(foo) // this is explicit conversion
}

[vladap]

What I think confuses me is if this suggested change to disallow subtypes of Function1 actually disallows Function1 itself as well to act as implicit conversion.

EDIT: I think this doesn't make sense. Function1 can't act itself as implicit conversion.

[vladap]

I have confirmed by decompilation that it indeed replaces foo with ev(foo). Even it is not what I have said in my source code. The only way how to read source code correctly is that it takes ev as an input argument but it is not used. Then implicit conversion fooToBar is applied to foo. Any other explanation feels magical to me. I have clearly made a mistake, it is not what I intended.

Is it possible to create some puzzler based on it? At first sight it seems that end result is the same in both cases and it can boil down to a cosmetic. But I just can't get rid of a code smell feeling.

[vladap]

@smarter

def foo(...)(implicit ev: A => B) = {
  implicit def conv(x: A): B = ev(x)
  ...
}

I beg to differ. It is exactly what I would enforce. I would make it inconvenient to discourage using it. If anybody passes in an argument ev: A => B, I expect that it will be used inside the body. It is totally confusing if it is not. It is basically implicitly used implicit. Trying to make it more concise by not having to write ev(x) explicitly is exactly the kind of confusing code. There is very little gain because we are defining a function which itself makes code more concise at some other level. I write it once but can read many times. If anybody thinks that in some corner case he needs to avoid writing ev(x), I would like to see a real code where it makes a difference, he can define its own local implicit conversion. It should be strongly discouraged for general code nevertheless.

Past 3 years I programmed in Scala, Java, Javascript. I like Scala the most out of them. And I always miss it when programming in other too. But there is one thing I'm getting tired after 3 years. I call it Scala forensics. Understanding a feature sometimes rises multiple questions how it interacts with other features. As you can see I had to decompile code to ensure how it works, now I can hope that it is deterministic. I don't have an idea where to find it in specs. Typically I find out that it is thought out well and it makes sense in the end. But I would aim to decrease number of rules one has to remember if there is not a clear gain.

EDIT: The code above says to me. I want to implement this function in terms of implicit value. The next line says, no I changed my mind I actually want to use implicit conversion. It itself suggests that it is confusing. The fact that input argument is implicit shouldn't convey I want to hide it in a function implementation. It should just say that it is passed in implicitly.

[vladap]

Can we ban all implicit vals acting as conversions, even Function1 itself? It seems to me that if someone wants an implicit conversion, they can define a method with def. Having two ways to write an implicit (val and def) seems unnecessary to me

@lihaoyi Doesn't this suggestion actually enforce it? Which val or object which is not subtype of Function1 can be still considered as implicit conversion? Function1 itself can't act as conversion itself because it is a trait with abstract apply, I would have to make it implicit val or object first I believe.

[smarter]

@vladap As has already been said to you, this is an issue tracker, not a discussion forum, you shouldn't be posting tons of long comments one after another. I'm not even sure what you're arguing for here, this merged PR does what you want: def foo[A,B](a: A)(implicit ev: A => B) = { val b: B = a } no longer works, you have to do def foo[A,B](a: A)(implicit ev: ImplicitConverter[A, B]) = { val b: B = a } instead.

[vladap]

I couldn't know that to understand impact of this issue I have to create separate topic on scala-contributors. Other communities don't have a problem to discuss on Github. I have been told but I already started here at the same time and didn't want to offload it in the middle. Maybe I went off-board, I admit. But maybe if community would answer my questions and kindly ask to use SC next time it would be much better. This way I feel quite discouraged.

For sure I could be more concise and up to a point if after 3 years in Scala I would know that there is a compiler "feature" which implicitly applies my implicit value if it is Function1. I always use implicit values explicitly. I don't see a reason I wouldn't in general code. If I want implicit conversion I define implicit def. I just don't believe that it is possible to make implicits any better by adding 2 or 3 layers of indirection which itself are implicit. (implicit ... Implicit*) how many implicit words do we need to spread around code? But it can be just me.

[vladap]

I think it's more elegant not to distinguish between implicit def or val. The way I see it, there are only implicit values. When something doesn't match the expected type, the compiler searches the implicit scope for an A => B. The fact that the def syntax works follows the same principle as eta expansion.

Implicit conversions and implicit values, e.g. used for DI, have very different requirements. It is why they don't work well together as implicits in Scala 2.x. Scala 2.x have them inter-connected. This issue solves one side that implicit val can be used as implicit conversion. And there is loop-hole in a compiler which implicitly uses implicit val as implicit conversion. The second one still remains. It is shielded through indirections to pass puzzler but I think it doesn't make code any easier.

EDIT: It can be seen in puzzler where deck is an injected dependency accidentally used for implicit conversion.

[felixmulder]

@vladap: this is a merged PR, while we appreciate your input - unless it is actionable we would rather you discuss it on the aforementioned forums meant for such discussion.

If at a later point, the discussion turns into something actionable please open an issue.