Skip to content
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

SIP-66 - Implicit macro conversions #86

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Iltotore
Copy link

No description provided.

@soronpo
Copy link
Contributor

soronpo commented Jun 26, 2024

This should be sip 66

@bjornregnell bjornregnell changed the title SIP-65 - Implicit macro conversions SIP-66 - Implicit macro conversions Jun 26, 2024
@bishabosha
Copy link
Member

This is also required for Mill, which uses a lot of macro implicit conversions for its Task DSL

@Iltotore
Copy link
Author

Yes. That's why I included it in the Motivation section 👍

@lihaoyi mentioned it first in this thread

@iusildra
Copy link

iusildra commented Oct 3, 2024

Hello @smarter, do you have any news on it ? 🙏


or alternatively:

```scala
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the full class should look like this:

@java.lang.FunctionalInterface
abstract class InlineConversion[-T, +U]:
  inline def apply(x: T): U
  extension (x: T) inline def convert = this(x)

I prefer InlineConversion over MacroConversion since the implementations need not necessarily be macros; they could just be inline methods.

parameter, a "proxy given" can be defined:

```scala
inline given [T, U](using macroConv: MacroConversion[T, U]): Conversion[T, U] = new:
Copy link
Contributor

@odersky odersky Nov 15, 2024

Choose a reason for hiding this comment

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

I don't think this would do the right thing either. The apply method would pass a non-inlined x macroConv. I don't think that macroConv will be able to anything significant with x in this case.

I believe we just have to accept that InlineConversion and Conversion cannot be mixed.

def myMethod(using Conversion[Foo, Bar]) = ???
```

In this case, according to implicit rules and the `given` instance [previously defined](#support-using-conversion), the
Copy link
Contributor

Choose a reason for hiding this comment

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

As I wrote before, I don't think this given instance will work.

val x: Bar = Foo()
```

Therefore, the third option seems to be the most suited.
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree. I believe a good way to conceptualize this is that for finding a conversion between A and B we do an implicit search for Conversion[A, B] | InlineConversion[A, B]. And if there is one of each, we get an ambiguity.


### Abuse concerns

Implicit conversions, especially in Scala 2, were criticized for being prone to abuse, making code more confusing and
Copy link
Contributor

@odersky odersky Nov 15, 2024

Choose a reason for hiding this comment

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

I have come around to accept that macro conversions cannot be avoided in some existing use cases. So if we want to get rid of the old implicit def syntax, we need to provide a solution.

@chrisandrews-ms
Copy link

This proposal was discussed in today's SIP committee meeting. There was general agreement that inline/macro implicit conversions do have important use cases and will need to be supported going forwards.

However the conclusion was that we can't make a decision about this proposal without first deciding on the future syntax and restrictions for regular implicit conversions. That may take some time - I don't think there's even a SIP for it yet, although there have been various discussions on the contributors forum.

I believe the intention is to retain support for the current implicit inline def syntax until we have a suitable replacement (whether from SIP-66 or some future proposal). For now we intend to leave this SIP open in review state.

@odersky
Copy link
Contributor

odersky commented Nov 20, 2024

One problematic aspect in the proposal is to have two type classes for Conversion and InlineConversion side by side. This means we now need two implicit searches instead of one each time we search for a conversion. That looks like a steep price to pay.

But there is an alternative, which I discovered when playing with another potential SIP. We can make InlineConversion an inline type class which is a superclass of Conversion, like this:

abstract class InlineConversion[-From, +To]:
  inline def applyInline(inline x: From): To
  extension (x: From) inline def convertInline: To = applyInline(x)

abstract class Conversion[-From, +To] extends InlineConversion[From, To]:
  def apply(x: From): To

  inline def applyInline(inline x: From): To = apply(x)

  extension (x: From) def convert: To = apply(x)

When searching for an implicit conversion from A to B the compiler will search for an InlineConversion[A, B]. If the actual found instance is an InlineConversion, it will use its applyInline method to convert. If the actual found instance is a regular Conversion it can just call the apply method instead. It does not really matter since applyInline anyway forwards to apply in this case.

Another advantage of this scheme is that any questions how to disambiguate between inline and regular conversions are already answered by the standard given priority rules. That is, from 3.7 on inline conversion would win over conversion since it is more general.

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

Successfully merging this pull request may close these issues.

8 participants