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.

Sign up for GitHub

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

Jump to bottom

Use PolyFunction instead of ErasedFunction #18295

Merged
merged 2 commits into from
Jul 28, 2023
Merged

Use PolyFunction instead of ErasedFunction #18295

merged 2 commits into from
Jul 28, 2023

Conversation

nicolasstucki
Copy link
Contributor

We generalize the meaning of PolyFunction to mean any kind of refined
lambda encoding. These refinements support any type with the following
shape as a lambda type:

PolyFunction {
  def apply[[T1, ..., Tn]]([given] [erased] x1: X1, ..., [erased] xn: Xn): R
}

@nicolasstucki nicolasstucki self-assigned this Jul 26, 2023
@nicolasstucki nicolasstucki force-pushed the remove-scala-runtime-ErasedFunction branch 3 times, most recently from def880d to f701f28 Compare July 26, 2023 15:02
@bishabosha
Copy link
Member

bishabosha commented Jul 26, 2023 •
edited
Loading

does this include monomorphic dependent function types? (main point being raised here is if this is TASTy backwards compatible)

@nicolasstucki nicolasstucki force-pushed the remove-scala-runtime-ErasedFunction branch 2 times, most recently from ce06c7c to 8e8bd34 Compare July 26, 2023 15:12
@nicolasstucki
Copy link
Contributor Author

does this include monomorphic dependent function types?

No, monomorphic dependent function types will not change with this PR. However, we might want to revisit them in the future to simplify refined function types.

There is no change in binary compact in this PR. Everything should be TASTy backward compatible.

@nicolasstucki nicolasstucki force-pushed the remove-scala-runtime-ErasedFunction branch 3 times, most recently from 9cd35cb to 6db6f70 Compare July 27, 2023 07:33
@nicolasstucki nicolasstucki marked this pull request as ready for review July 27, 2023 12:33
@nicolasstucki nicolasstucki requested a review from smarter July 27, 2023 12:34
smarter
smarter approved these changes Jul 27, 2023
View reviewed changes
Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

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

I think isNonRefinedFunction could also be renamed isNonPolyFunction

compiler/src/dotty/tools/dotc/cc/Setup.scala Outdated
@@ -197,7 +197,7 @@ extends tpd.TreeTraverser:
val mt = ContextualMethodType(paramName :: Nil)(
_ => paramType :: Nil,
mt => if isLast then res else expandThrowsAlias(res, mt :: encl))
val fntpe = RefinedType(defn.ErasedFunctionClass.typeRef, nme.apply, mt)
val fntpe = RefinedType(defn.PolyFunctionClass.typeRef, nme.apply, mt)
Copy link
Member

Choose a reason for hiding this comment

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

We could add an apply method to defn.PolyFunctionOf that constructs refinements like this.

library/src/scala/PolyFunction.scala Outdated
*
* Marker trait for many (poly) function types.
*
* This is the only trait that can be refined with a method or polymorphic method,
Copy link
Member

Choose a reason for hiding this comment

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

This isn't the only trait that can be refined with a method (any trait can as long as it's not polymorphic) so the documentation needs to be reworded.

@smarter smarter assigned nicolasstucki and unassigned smarter Jul 27, 2023
@nicolasstucki nicolasstucki force-pushed the remove-scala-runtime-ErasedFunction branch 2 times, most recently from 3856aa1 to 749b07c Compare July 27, 2023 14:54
@nicolasstucki
Copy link
Contributor Author

I think isNonRefinedFunction could also be renamed isNonPolyFunction

Does this function return true for dependent refinements?

@nicolasstucki
Copy link
Contributor Author

I will have a deeper look at the uses of isNonRefinedFunction. It might deserve a separate PR.

@smarter
Copy link
Member

smarter commented Jul 27, 2023

Does this function return true for dependent refinements?

Ah no that's isFunctionNType

@nicolasstucki nicolasstucki force-pushed the remove-scala-runtime-ErasedFunction branch 3 times, most recently from 7061e48 to 749b07c Compare July 28, 2023 09:44
@nicolasstucki
Copy link
Contributor Author

I will follow up on isNonRefinedFcuntion in #18305

smarter
smarter approved these changes Jul 28, 2023
View reviewed changes
library/src/scala/PolyFunction.scala Outdated Show resolved Hide resolved
@nicolasstucki
Use PolyFunction instead of ErasedFunction
8a6ef64 
We generalize the meaning of `PolyFunction` to mean any kind of refined
lambda encoding. These refinements support any type with the following
shape as a lambda type:

```scala
PolyFunction {
  def apply[[T1, ..., Tn]]([given] [erased] x1: X1, ..., [erased] xn: Xn): R
}
```
@nicolasstucki
Add defn.PolyFunctionOf.apply
abdc2ca
@nicolasstucki nicolasstucki force-pushed the remove-scala-runtime-ErasedFunction branch from 749b07c to abdc2ca Compare July 28, 2023 10:10
@nicolasstucki nicolasstucki enabled auto-merge July 28, 2023 10:26
@nicolasstucki nicolasstucki merged commit 16de9eb into scala:main Jul 28, 2023
@nicolasstucki nicolasstucki deleted the remove-scala-runtime-ErasedFunction branch July 28, 2023 13:49
@Kordyjan Kordyjan added this to the 3.4.0 milestone Aug 1, 2023
@nicolasstucki nicolasstucki mentioned this pull request Aug 30, 2023
Closed
nicolasstucki added a commit to dotty-staging/dotty that referenced this pull request Feb 23, 2024
@nicolasstucki
Remove ErasedFunction
8ae80fe 
This can be removed now that the reference compiler (3.4.0) no longer
tries to load the symbol of `ErasedFunction`.

Follow up of scala#18295
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smarter smarter smarter approved these changes

Assignees

@nicolasstucki nicolasstucki

Labels
None yet
Projects
None yet
Milestone
3.4.0
Development

Successfully merging this pull request may close these issues.
4 participants
@nicolasstucki @bishabosha @smarter @Kordyjan