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

tryCompiletimeConstantFold in disjointnessBoundary #20168

Merged
merged 1 commit into from
May 29, 2024

Conversation

EugeneFlesselle
Copy link
Contributor

@EugeneFlesselle EugeneFlesselle commented Apr 11, 2024

Fixes #20166

Alternatively placing the tryCompiletimeConstantFold in AppliedType#superType also seems to fix the issue and not break anything. But I'm not very sure about the spec here either way.

@sjrd

@soronpo
Copy link
Contributor

soronpo commented Apr 12, 2024

I believe this requires a spec change. Maybe @smarter can address this in scala/improvement-proposals#84

@soronpo
Copy link
Contributor

soronpo commented Apr 12, 2024

But I'm not entirely sure #20166 requires a fix.

@EugeneFlesselle EugeneFlesselle marked this pull request as ready for review April 12, 2024 06:43
@EugeneFlesselle EugeneFlesselle requested a review from sjrd April 12, 2024 06:44
@EugeneFlesselle
Copy link
Contributor Author

EugeneFlesselle commented Apr 12, 2024

But I'm not entirely sure #20166 requires a fix.

If not the match types, then the compiletime.ops documentation needs to be modified. Right now they are contradictory.

/** Successor of a natural number where zero is the type 0 and successors are reduced as if the definition was:
*
* ```scala
* //{
* import compiletime.ops.int.*
* //}
* type S[N <: Int] <: Int = N match {
* case 0 => 1
* case 1 => 2
* case 2 => 3
* // ...
* case 2147483646 => 2147483647
* }
* ```
* @syntax markdown
*/
type S[N <: Int] <: Int

@soronpo
Copy link
Contributor

soronpo commented Apr 12, 2024

I don't understand, where is the contradiction? Where does it state that compiletime.ops can generally appear in a match type case? S is special cased in the match-type spec. It has known properties that define the disjointness well. It's not true for all the compiletime.ops operators.

@EugeneFlesselle
Copy link
Contributor Author

I don't understand, where is the contradiction? Where does it state that compiletime.ops can generally appear in a match type case? S is special cased in the match-type spec. It has known properties that define the disjointness well. It's not true for all the compiletime.ops operators.

We observe different semantics when replacing usages of compiletime.ops.int.S by what the documentation sais it is equivalent too (the test has an example). Independently of where it is used, I would say that is a contradiction.

@EugeneFlesselle
Copy link
Contributor Author

I've dropped aaf2dc7 from this PR in favour of having it in #20257.

@mbovel mbovel assigned sjrd and unassigned sjrd May 29, 2024
Copy link
Member

@sjrd sjrd left a comment

Choose a reason for hiding this comment

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

Sorry for dropping this.

AFAICT, this is correct. When we fall in that case, it should be the case that tp =:= tp.tryCompiletimeConstantFold, so they should agree on being provably disjoint or not.

@sjrd sjrd merged commit 110600b into scala:main May 29, 2024
19 checks passed
@sjrd sjrd deleted the match-types-constFold branch May 29, 2024 15:03
@Kordyjan Kordyjan added this to the 3.5.1 milestone Jul 3, 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.

Match type pattern with compiletime.ops isn't simplified
5 participants