-
Notifications
You must be signed in to change notification settings - Fork 269
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
feat: support for traits as type arguments with variance on datatypes in Java #3072
Conversation
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.
I did not expect this to be as simple. Good for us!
if (!isCoCall) { | ||
// For an ordinary constructor (that is, one that does not guard any co-recursive calls), generate: | ||
// Dt.<T>create_Cons( args ) | ||
wr.Write($"{dtName}.{typeParams}{DtCreateName(ctor)}({arguments})"); | ||
} else { | ||
wr.Write($"new {dt.CompileName}__Lazy("); | ||
wr.Write("() -> { return "); | ||
wr.Write($"new {DtCtorName(ctor)}({arguments})"); | ||
wr.Write($"new {DtCtorName(ctor)}{typeParams}({arguments})"); |
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.
This looked fishy to me but is fine; Nevertheless, I found a bug along the way.
module M {
codatatype Stream<T> = Next(shead: T, stail: Stream)
}
function method CoUp(n: int, b: bool): M.Stream<int>
{
if b then
CoUp(n, false) // recursive, not co-recursive, call
else
M.Next(n, CoUp(n+1, true)) // CoUp is co-recursive call
}
method Main(){
print CoUp(0, false), "\n";
}
It's a one-liner. L1968 needs a public
.
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.
Good find! It was handled incorrectly in 3 compilers. I fixed it.
var t: set<nat> := s; | ||
var u: set<Even> := s; | ||
t := u; | ||
u := t; |
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.
u := t; | |
s := t; |
Both seem like a decent idea.
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.
For further testing, I prefer u
, since the types Even
and nat
are not subtypes of each other.
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.
I added your test, too (here and in a few more places).
docs/dev/news/3072.feat
Outdated
@@ -0,0 +1 @@ | |||
feat: support for traits as type arguments with variance on datatypes in Java |
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.
feat: support for traits as type arguments with variance on datatypes in Java | |
feat: support for traits as type arguments by allowing variance on datatypes in Java |
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.
I also add "fully allowing"
Co-authored-by: Fabio Madge <[email protected]>
Co-authored-by: Fabio Madge <[email protected]>
Co-authored-by: Fabio Madge <[email protected]>
Co-authored-by: Fabio Madge <[email protected]>
…t now (#3141) Ideally the feature would have been removed in the PR that implemented it in Java (#3072), but unfortunately the `Feature` mechanism only allows a compiler to cleanly opt-out of supporting a feature, and we don't yet have a complementary mechanism to test that a compiler that claims to not support a feature does not in fact support that feature! <small>By submitting this pull request, I confirm that my contribution is made under the terms of the [MIT license](https://github.com/dafny-lang/dafny/blob/master/LICENSE.txt).</small>
This PR removes that pesky "not supported" error for Java where it wouldn't allow a
trait
to be passed in as a variant type parameter of a (co)datatype.Fixes #2013
The solution is pretty simple. Basically, it just emits casts when needed. To be accepted by Java's static type checker, these casts first upcast to
Object
and then downcast to the desired type. Since the JVM doesn't keep track of type parameters, it'll never know what hit it. (In particular, this means that we don't need the expensiveDowncastClone
mechanism that we're using for C# to satisfy .NET's more informed runtime.)To make it work, this PR also makes more precise the
IsTargetSupertype
method for Java. In particular, to check ifA<X, Y>
is a supertype ofB<U, V, W>
, first keep converting the latter up thetrait
parents ofB
until it gets toA
(if ever). This is already done for all the compilers. Call the resultA<X', Y'>
. For the other compilers, one would now start to see if the parametersX, Y
are target supertypes ofX', Y'
with variance in mind (for example, if the first type parameter is contravariant, we would actually check ifX'
is a supertype ofX
). For Java, however, we continue with variance in mind only ifA
is a collection type (because those already have run-time support for variance, using Java's bounded existential types). For non-collection types, we instead check ifX, Y
are target-type equal toX', Y'
. If they are not, the compiler callsEmitDowncast
. (To read these in the code, look atEmitDowncastIfNecessary
inSinglePassCompiler.cs
.)We could have used simple casts like this for collection types, too. But since I already have those, I left those alone.
By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.