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

Only allow erased parameters in erased definitions #19686

Conversation

nicolasstucki
Copy link
Contributor

So far, we do not have any use case for them. We could enable them in a later version.

The current implementation does not handle correctly the non-erased arguments to erased definitions. These should always be evaluated, but in some cases we can dorp them by mistake.

Copy link
Contributor

@EugeneFlesselle EugeneFlesselle left a comment

Choose a reason for hiding this comment

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

I don't think the issues regarding the lifting/evaluation of arguments to erased parameters are specific to the case of erased definitions.

We have the same problem here for example:

def bar(erased x: Int) = true
bar:
  println("hello")
  1

which after pruneErasedDefs gives:

def bar(erased x: Int): Boolean = true
Test.bar(scala.compiletime.erasedValue[(1 : Int)])

@nicolasstucki nicolasstucki marked this pull request as ready for review February 14, 2024 14:50
@nicolasstucki
Copy link
Contributor Author

nicolasstucki commented Feb 14, 2024

don't think the issues regarding the lifting/evaluation of arguments to erased parameters are specific to the case of erased definitions.

Not completely. We still need to check the purity of the erased parameter before we prune it. This is just a first step.

@nicolasstucki nicolasstucki force-pushed the desallow-non-erased-params-in-erased-defs branch from 39eba3a to 61b8336 Compare March 6, 2024 11:01
@nicolasstucki nicolasstucki force-pushed the desallow-non-erased-params-in-erased-defs branch from 61b8336 to d335e93 Compare April 10, 2024 11:21
So far, we do not have any use case for them. We could enable them in a
later version.

The current implementation does not handle correctly the non-erased
arguments to erased definitions. These should always be evaluated, but
in some cases we can dorp them by mistake.
@nicolasstucki nicolasstucki force-pushed the desallow-non-erased-params-in-erased-defs branch from d335e93 to f4ff6e3 Compare April 10, 2024 14:53
Copy link
Contributor

@EugeneFlesselle EugeneFlesselle left a comment

Choose a reason for hiding this comment

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

We indeed still need to check purity for erased arguments of both kinds of definitions.

But I agree non-erased parameters in erased defs have little use-cases that we know of.
And having more restrictions sounds good to me.

@nicolasstucki nicolasstucki merged commit a14c79e into scala:main Apr 11, 2024
19 checks passed
@nicolasstucki nicolasstucki deleted the desallow-non-erased-params-in-erased-defs branch April 11, 2024 15:29
@Kordyjan Kordyjan added this to the 3.5.0 milestone May 10, 2024
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.

3 participants