-
Notifications
You must be signed in to change notification settings - Fork 8
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
Preparation for Scala 3 migration #201
base: main
Are you sure you want to change the base?
Conversation
// for { p <- this.copy(boundT = boundT + NatKind.IDWrapper(x)).`type`(e)} | ||
// yield (p._1, NatToDataLambda(x, e)) | ||
val pp: Pure[((OrderedSet[Identifier], OrderedSet[Kind.Identifier]), DataType)] = | ||
this.copy(boundT = boundT + NatKind.IDWrapper(x)).`type`(e) | ||
pp.map(p => (p._1, NatToDataLambda(x, e))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the for
syntax not supported anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, of course it is, but here the system got confused somehow (and I with it ...).
I think that maybe the type inference has changed, and therefore the type annotation helps here, but I am not sure.
The explicit version with the map
should be the correct one - from my point of view at least.
@@ -414,7 +414,7 @@ object NamedRewriteDSL { | |||
|
|||
import scala.language.implicitConversions | |||
|
|||
val `_`: rct.TypePlaceholder.type = rct.TypePlaceholder | |||
val `__`: rct.TypePlaceholder.type = rct.TypePlaceholder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we can also just remove that syntax at that point and always use names ..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is completely up to you, I just renamed it in a way that Scala 3 does not compain about ...
object DepLambda { | ||
def apply[T, I](kind: Kind[T, I], x: I): Object { | ||
def apply[U <: PhraseType](body: Phrase[U]): DepLambda[T, I, U] | ||
} = new { | ||
def apply[T, I](kind: Kind[T, I], x: I): SyntaxHelper[T, I] = SyntaxHelper(kind, x) | ||
|
||
case class SyntaxHelper[T, I](kind: Kind[T, I], x: I) { | ||
def apply[U <: PhraseType](body: Phrase[U]): DepLambda[T, I, U] = DepLambda(kind, x, body) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use a case class DepLambda
to avoid the helper?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course, there is a class DepLambda
that represents a dependent lambda phrase.
We usually keep those classes free of any syntactic sugar (which I think is a good idea) and even for this here it is not clear if this syntax should be defined here or elsewhere in a DSL package.
Anyway, I think the purpose of this is only to infer the type U
and not have to annotate this, while still allowing the annotation of T
and I
.
We might want to look into this after having migrated to Scala 3 again, I supose ...
This PR introduces changes that will help to ease the migration to Scala 3, but the code in this PR remains valid Scala 2 code.
In detail the changes are:
()
is replaced withUnit
(as Scala 3 seems to no longer accept()
as a type)T#U
) in theFunctionHelper
class is avoided as these have been dropped in Scala 3._1
and._2
notations on Rise and DPIA expressions do not work in Scala 3 (I don't know why) I have replaced them with`1`
and`2`
`_`
notation is also not accepted by Scala 3, I have replaced it with`__`
unapply
method generated for case classes.