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

Extension method with dependent type bounds #19197

Open
nicolasstucki opened this issue Dec 5, 2023 · 3 comments
Open

Extension method with dependent type bounds #19197

nicolasstucki opened this issue Dec 5, 2023 · 3 comments

Comments

@nicolasstucki
Copy link
Contributor

Compiler version

3.3.1

Minimized code

extension (tuple: Tuple)
  def *:[T >: tuple.type <: Tuple, H](x: H): H *: T = ???

Output

2 |  def `*:`[T >: tuple.type <: Tuple, H](x: H): H *: T = ???
  |                ^^^^^
  |                Not found: tuple - did you mean Tuple?

The issue seems to come from how we handle type parameters

[[syntax trees at end of                     typer]]
    extension [T >: tuple.type <: Tuple, H >: Nothing <: Any](tuple: Tuple)
      def *:(x: H): *:[H, T] = ???

It should be the same as in

extension (tuple: Tuple)
  def ok[T >: tuple.type <: Tuple, H](x: H): H *: T = ???
[[syntax trees at end of                     typer]]
    extension (tuple: Tuple) def ok[T >: tuple.type <: Tuple,
      H >: Nothing <: Any](x: H): *:[H, T] = ???

Expectation

Should compile

@bishabosha
Copy link
Member

you would have to lift out type parameters when you do the swap

@nicolasstucki
Copy link
Contributor Author

The swap seems to be fundamentally broken. The scoping while swapping adds too many restrictions.

@nicolasstucki
Copy link
Contributor Author

It seems that this and other use cases start working if we do not swap the parameters. They are sapped correctly at the call site. I will investigate further.

@nicolasstucki nicolasstucki self-assigned this Dec 5, 2023
nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Dec 5, 2023
This patch removes the special treatment of extension methods while
desugaring. Instead of swapping the arguments of the extension method
at definition site, we let the usual desugaring take care of it.

```scala
extension (xs: List[Int]) def -:(x: Int) = xs.remove(x)

1 -: List(2, 3) // desugars into List(2, 3).-:(1)
1 :: List(2, 3) // desugars into List(2, 3).::(1)
```

This implies that the following code works now:
```scala
List(2, 3).-:(1) // equivalent to the one bellow
-:(List(2, 3))(1)
```
Note that the parameters are passed in order, as they should.
This used to break with the previous implementation.

Futhermore, this fixes parameter scoping and dependencies, as it trivially
respects the original signature. An example of this is in the following
code example, where `T` is bounded by `tuple.type`, which would have been
out of scope in the previous implementation due to the swapping of parameters.

```scala
extension (tuple: Tuple)
  def *:[T >: tuple.type <: Tuple, H](x: H): H *: T = ...
```

The downside is that this is a breaking change as we change the semantics
of the extension method definitions. We need to find a way to migrate from
one to the other without breaking code. One option might be to have a
different syntax for the new semantics, and deprecate the old one. One
possibility would be to use `infix def` extensions to mark to mark these
methods.

One issue encountered in the library is with the `IArray.{++:, +:}`
operations. Luckily, it seems that by rewriting them into the new
semantics we get a binary and TASTy compatible signature. But updating
them would be a source incompatible change as it would break non-infix
calls to those methods.

Fixes scala#19197
nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Dec 5, 2023
This patch removes the special treatment of extension methods while
desugaring. Instead of swapping the arguments of the extension method
at definition site, we let the usual desugaring take care of it.

```scala
extension (xs: List[Int]) def -:(x: Int) = xs.remove(x)

1 -: List(2, 3) // desugars into List(2, 3).-:(1)
1 :: List(2, 3) // desugars into List(2, 3).::(1)
```

This implies that the following makes sense now:
```scala
List(2, 3).-:(1) // equivalent to the one bellow
-:(List(2, 3))(1)
```
Note that the parameters are passed in order as in the signature,
as they should. This used to break intuition in the previous implementation.

Futhermore, this fixes parameter scoping and dependencies, as it trivially
respects the original signature. An example of this is in the following
code example, where `T` is bounded by `tuple.type`, which would have been
out of scope in the previous implementation due to the swapping of parameters.

```scala
extension (tuple: Tuple)
  def *:[T >: tuple.type <: Tuple, H](x: H): H *: T = ...
```

The downside is that this is a breaking change as we change the semantics
of the extension method definitions. We need to find a way to migrate from
one to the other without breaking code. One option might be to have a
different syntax for the new semantics, and deprecate the old one. One
possibility would be to use `infix def` extensions to mark to mark these
methods.

One issue encountered in the library is with the `IArray.{++:, +:}`
operations. Luckily, it seems that by rewriting them into the new
semantics we get a binary and TASTy compatible signature.

Fixes scala#19197
nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Dec 6, 2023
This patch removes the special treatment of extension methods while
desugaring. Instead of swapping the arguments of the extension method
at definition site, we let the usual desugaring take care of it.

```scala
extension (xs: List[Int]) def -:(x: Int) = xs.remove(x)

1 -: List(2, 3) // desugars into List(2, 3).-:(1)
1 :: List(2, 3) // desugars into List(2, 3).::(1)
```

This implies that the following makes sense now:
```scala
List(2, 3).-:(1) // equivalent to the one bellow
-:(List(2, 3))(1)
```
Note that the parameters are passed in order as in the signature,
as they should. This used to break intuition in the previous implementation.

Futhermore, this fixes parameter scoping and dependencies, as it trivially
respects the original signature. An example of this is in the following
code example, where `T` is bounded by `tuple.type`, which would have been
out of scope in the previous implementation due to the swapping of parameters.

```scala
extension (tuple: Tuple)
  def *:[T >: tuple.type <: Tuple, H](x: H): H *: T = ...
```

The downside is that this is a breaking change as we change the semantics
of the extension method definitions. We need to find a way to migrate from
one to the other without breaking code. One option might be to have a
different syntax for the new semantics, and deprecate the old one. One
possibility would be to use `infix def` extensions to mark to mark these
methods.

One issue encountered in the library is with the `IArray.{++:, +:}`
operations. Luckily, it seems that by rewriting them into the new
semantics we get a binary and TASTy compatible signature.

Fixes scala#19197
@nicolasstucki nicolasstucki removed their assignment Jan 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants