Skip to content

Allow cold values in static methods using parametricity #14751

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
Closed
wants to merge 1 commit into from
Closed

Allow cold values in static methods using parametricity #14751

wants to merge 1 commit into from

Conversation

Xavientois
Copy link
Contributor

@Xavientois Xavientois commented Mar 22, 2022 •
edited
Loading

Closes #14460

Allow cold arguments to be passed if the parameter is !Matchable and the method is global static.

Review by @liufengyun

@Xavientois
Allow cold values in static methods using parametricity
d719739 
Closes scala#14460

Allow cold arguments to be passed if the parameter is `!Matchable` and the method is global static.
@Xavientois Xavientois force-pushed the allow-cold-args-on-static-matchable branch from 9a48ebe to d719739 Compare March 22, 2022 21:54
olhotak
olhotak reviewed Mar 23, 2022
View reviewed changes
compiler/src/dotty/tools/dotc/transform/init/Semantic.scala
val (argErrors, args) = evalArgs(argss.flatten, thisV, klass)
// Allow cold args for static methods with non-matchable params
val methodType = ref.symbol.info.stripPoly
val allMatchable = methodType.paramInfoss.flatten.forall { (info) => info <:< defn.MatchableType }
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 we wanted to do the check per-argument rather than for all the arguments. That is, in def call, we have

      def checkArgs = args.flatMap(_.promote)

There we want to promote only those args that correspond to non-matchable params. Even if some params are matchable, the call is still fine as long as we can promote the args that correspond to those params.

tests/init/neg/enum-desugared.scala
@@ -1,35 +0,0 @@
package example
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we move these tests to pos instead of deleting them entirely?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Definitely

@olhotak
Copy link
Contributor

olhotak commented Mar 23, 2022 •
edited
Loading

In cases SeqLiteral, don't esnureHot, but eval the arguments and .join their values to get the value for the SeqLiteral.

When checking if info <:< Matchable, first check whether info is T*. If it is, strip the *, to test T <: Matchable instead of T* <: Matchable. To strip the *, call repeatedToSingle on the Type.

@olhotak
Copy link
Contributor

olhotak commented Mar 23, 2022

In def call, return Cold if any of the arguments are not Hot, including arguments passed for non-Matchable parameters.

Xavientois added a commit that referenced this pull request Mar 27, 2022
@Xavientois
Allow non-hot values in SeqLiterals
2a85f53 
Closes #14460, #14751

Do not ensure that elements in a SeqLiteral are Hot.

Review by @liufengyun
@Xavientois Xavientois mentioned this pull request Mar 27, 2022
Merged
Xavientois added a commit that referenced this pull request Mar 28, 2022
@Xavientois
Allow non-hot values in SeqLiterals
7a79695 
Closes #14460, #14751

Do not ensure that elements in a SeqLiteral are Hot.

Review by @liufengyun
Xavientois added a commit that referenced this pull request Apr 4, 2022
@Xavientois
Allow non-hot values in SeqLiterals
c01c30a 
Closes #14460, #14751

Do not ensure that elements in a SeqLiteral are Hot.

Review by @liufengyun
Xavientois added a commit that referenced this pull request Apr 4, 2022
@Xavientois
Allow non-hot values in SeqLiterals
9ddeda3 
Closes #14460, #14751

Do not ensure that elements in a SeqLiteral are Hot.

Review by @liufengyun
Xavientois added a commit that referenced this pull request Apr 4, 2022
@Xavientois
Allow non-hot values in SeqLiterals
9858f5f 
Closes #14460, #14751

Do not ensure that elements in a SeqLiteral are Hot.

Review by @liufengyun
Xavientois added a commit that referenced this pull request Apr 4, 2022
@Xavientois
Allow non-hot values in SeqLiterals
5aa4a79 
Closes #14460, #14751

Do not ensure that elements in a SeqLiteral are Hot.

Review by @liufengyun
Xavientois added a commit that referenced this pull request Apr 5, 2022
@Xavientois
Allow non-hot values in SeqLiterals
ff64ce8 
Closes #14460, #14751

Do not ensure that elements in a SeqLiteral are Hot.

Review by @liufengyun
Xavientois added a commit that referenced this pull request Apr 5, 2022
@Xavientois
Allow non-hot values in SeqLiterals
da09d3a 
Closes #14460, #14751

Do not ensure that elements in a SeqLiteral are Hot.

Review by @liufengyun
@liufengyun liufengyun mentioned this pull request Apr 6, 2022
Closed
@Xavientois Xavientois mentioned this pull request Apr 12, 2022
Merged
@olhotak
Copy link
Contributor

olhotak commented May 9, 2022

Closing in favour of #14916.

@olhotak olhotak closed this May 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@olhotak olhotak olhotak left review comments

Assignees

@Xavientois Xavientois

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Inner enum declarations cause initialization-checker error
2 participants
@Xavientois @olhotak