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

feat: support for traits as type arguments with variance on datatypes in Java #3072

Merged
merged 21 commits into from
Nov 29, 2022

Conversation

RustanLeino
Copy link
Collaborator

@RustanLeino RustanLeino commented Nov 18, 2022

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 expensive DowncastClone 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 if A<X, Y> is a supertype of B<U, V, W>, first keep converting the latter up the trait parents of B until it gets to A (if ever). This is already done for all the compilers. Call the result A<X', Y'>. For the other compilers, one would now start to see if the parameters X, Y are target supertypes of X', Y' with variance in mind (for example, if the first type parameter is contravariant, we would actually check if X' is a supertype of X). For Java, however, we continue with variance in mind only if A 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 if X, Y are target-type equal to X', Y'. If they are not, the compiler calls EmitDowncast. (To read these in the code, look at EmitDowncastIfNecessary in SinglePassCompiler.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.

@RustanLeino RustanLeino marked this pull request as ready for review November 18, 2022 01:22
fabiomadge
fabiomadge previously approved these changes Nov 29, 2022
Copy link
Collaborator

@fabiomadge fabiomadge left a 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!

Source/DafnyCore/Compilers/Compiler-Csharp.cs Outdated Show resolved Hide resolved
Source/DafnyCore/Compilers/SinglePassCompiler.cs Outdated Show resolved Hide resolved
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})");
Copy link
Collaborator

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.

Copy link
Collaborator Author

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.

Source/DafnyCore/Compilers/Compiler-java.cs Outdated Show resolved Hide resolved
Test/git-issues/git-issue-2013.dfy Outdated Show resolved Hide resolved
var t: set<nat> := s;
var u: set<Even> := s;
t := u;
u := t;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
u := t;
s := t;

Both seem like a decent idea.

Copy link
Collaborator Author

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.

Copy link
Collaborator Author

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).

@@ -0,0 +1 @@
feat: support for traits as type arguments with variance on datatypes in Java
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
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

Copy link
Collaborator Author

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"

@RustanLeino RustanLeino merged commit 50c2c99 into dafny-lang:master Nov 29, 2022
@RustanLeino RustanLeino deleted the issue-2013 branch November 29, 2022 16:46
robin-aws added a commit that referenced this pull request Nov 30, 2022
…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>
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.

Feature request: support for traits as type arguments with variance on datatypes in Java
2 participants