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

Backport Scala 3 signature polymorphic method tests #12678

Closed
SethTisue opened this issue Oct 26, 2022 · 6 comments
Closed

Backport Scala 3 signature polymorphic method tests #12678

SethTisue opened this issue Oct 26, 2022 · 6 comments
Assignees
Milestone

Comments

@SethTisue
Copy link
Member

SethTisue commented Oct 26, 2022

once scala/scala3#16225 lands in Scala 3, I'll look into this

perhaps the new tests will pass, perhaps adjustments will be needed

including perhaps making the spec more explicit (about void? about Object fallback? about casts vs type ascription? see https://docs.oracle.com/javase/specs/jls/se8/html/jls-15.html#jls-15.12.3)

@SethTisue
Copy link
Member Author

SethTisue commented Nov 30, 2022

Dale pointed out that in my backport (scala/scala#10195) I added this comment:

    // sanity check that the non-signature-polymorphic invocations work as expected

but that's wrong, invoke and invokeExact are both signature polymorphic

I should fix that before closing this.

@SethTisue
Copy link
Member Author

as for the spec,

about Object fallback?

I think this is covered by this spec language: "If the expected type is undefined then $U$ is scala.AnyRef"

about casts vs type ascription?

I think our existing spec language is clear in its use of the phrase "expected type". Type ascription controls the expected type. There's no need to support using asInstanceOf instead.

about void?

this one required more thought. I'll comment separately

@SethTisue
Copy link
Member Author

SethTisue commented Nov 30, 2022

One passage in the JLS about void states:

If the method invocation expression is an expression statement, the compile-time result is void

Scala does not have the concept of expression statements. dotty-i11332.scala tests handling of Unit (which becomes void in bytecode when used as the result type of a method). So I think we're good there.

@SethTisue
Copy link
Member Author

SethTisue commented Nov 30, 2022

And another passage in the JLS states:

An argument expression which is the null literal null is treated as having the static type Void.

I was puzzled by this passage. To test it, I wrote the following Java program:

import java.lang.invoke.*;
import static java.lang.invoke.MethodType.methodType;

public class Foo {
  public String foo(Void v) {
    return "void";
  }
  public static void main(String[] argv) {
    try {
      MethodHandles.Lookup l = MethodHandles.lookup();
      MethodHandle mh = l.findVirtual(Foo.class, "foo", methodType(String.class, Void.class));
      String result = (String) mh.invokeExact(new Foo(), null);
      if (!result.equals("void")) {
        throw new IllegalStateException();
      }
    }
    catch (Throwable t) {
      throw new IllegalStateException(t);
    }
  }
}

This succeeds as expected; the null in (String) mh.invokeExact(new Foo(), null) is taken as having type Void.

However, the same program also works if we make this change:

-      String result = (String) mh.invokeExact(new Foo(), null);
+      String result = (String) mh.invokeExact(new Foo(), (Void) null);

so that leaves me mystified as to why this passage was included in the Java spec.

Do we have a problem on the Scala side? It doesn't seem to me that we do, since the same code in Scala works fine with type ascription: null: Void.

scala/scala#10232 adds that as a test.

Regardless, I'm nagged by a feeling that I've misunderstood something, as it would be out of character for the Java spec authors to include unnecessary language. It's rare for a method to take a parameter of type Void, so it doesn't seem likely that allowing null in place of (Void) null would have been included as a mere convenience.

Anyway, unless someone wants to come forward and argue that something is still missing in Scala implementation, let's assume we're good.

@dwijnand
Copy link
Member

Yeah. I had written the implementation and test for that in Scala 3, but dropped it later because Guillaume didn't think we needed to implement the JLS on that in Scala. The 2-3 lines it was to support stuck out disproportionally to the value it provided.

@SethTisue
Copy link
Member Author

fixed by the combination of scala/scala#10195 and scala/scala#10232

@SethTisue SethTisue changed the title Backport Scala 3 signature polymorphic method tests (and make fixes, if needed) Backport Scala 3 signature polymorphic method tests Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants