Vavr: Vector.of() is ambiguous with GWT compiler

[nfekete]

There are two static methods overloading Vector.of(). One is accepting a single T element, the other a varargs T... elements. The GWT compiler creates a method reference to the wrong method (T element instead of the varargs version) when inferring the method reference from the body of Vector.tabulate. This way, under GWT, the tabulate method will create a Vector of arrays instead of Vector of T.

Since AFAIK GWT uses the eclipse compiler, this probably causes problems with older version of eclipse too (haven't tested).

I think it's a bit unfortunate that the two methods share the same name since ambiguity can happen. What if I want a Vector of arrays? What if I want a vector of elements of the array. I think renaming the varargs version to ofAll would make sense and would align it to the other methods which accept either primitive varargs or iterables or streams.

Another solution could be to remove the Vector.of(T element) version altogether, since the varargs version should be enough, that's what varargs was invented for in the first place.

Personally, I like the first (rename) solution more , but the second one (remove) could be backwards source-compatible, while the rename will not.

[nfekete]

There might be other collections with the same ambiguity, I haven't looked thoroughly, but List has the same issue.

[danieldietrich]

All collections will have that issue. We aligned the naming to standard Java.

Example:

• java.util.stream.Stream.of(T... values)
• java.util.stream.Stream.of(T value)

We reworked already the factory method API because of ambiguities, see #721.
For special cases, like array of arrays, an explicit cast or type hint behind the '.' has to be provided.

Maybe it is an Eclipse Compiler bug. Java's Stream has the same problem.

[danieldietrich]

@ruslansennov could you please add the following gwt/javascript tests to javaslang-gwt project:

Factory method tests:

• assertThat(Set.of(1).contains(1)).isTrue()
• assertThat(Set.of(1, 2).contains(2)).isTrue()

API alias tests:

• assertThat(Set(1).contains(1)).isTrue()
• assertThat(Set(1, 2).contains(2)).isTrue()

These should ensure that the generated javascript code calls the right methods.

[ruslansennov]

sure

[nfekete]

You could add this as well (if you want to check if the bug manifests or not)
assertThat(Vector.tabulate(2, Function.identity()).contains(1)).isTrue()

[ruslansennov]

@nfekete, thank you I confirm the problem. This test passes in GWT and of course fails in Java

// GWT scope
public void testTabulate() {
    Vector<?> v = Vector.tabulate(2, Function.identity());
    // actually v is not {0, 1} but is {{0, 1}}
    assertTrue(v.size() == 1);  // true in GWT, false in Java
    Object[] head = (Object[]) v.get(0);
    assertEquals(head[1], 1);
}

[danieldietrich]

@ruslansennov isn't this a general problem for javascript? How to choose the right method in an untyped language:

function of(element) { ... }

function of(elementArray) { ... }

In Javascript these are the same. I guess the GWT Java to Javascript compiler will disambiguate the names by adding param type information to the function names...

[ruslansennov]

Daniel, you're right, JavaScript is funny...
We can change signature of internal Collections.tabulate(...) from

C tabulate(int n, Function<? super Integer, ? extends T> f, C empty, Function<T[], C> of)

to something like

C tabulate(int n, Function<? super Integer, ? extends T> f, C empty, Function<java.util.List<T>, C> of)

and this solves the problem.

[danieldietrich]

@ruslansennov thank you, that sounds good!

of will then be ofAll. It should be sufficient if it takes an Iterable (does not matter much here):

C tabulate(int n, Function<? super Integer, ? extends T> f, C empty, Function<Iterable<T>, C> ofAll)

I would stay with the array:

final T[] elements = (T[]) new Object[n];

and finally wrap the array as List:

return ofAll.apply(Arrays.asList(elements));

[ruslansennov]

... and we should check all other factory methods in GWT

[nfekete]

Are you sure it's a JS problem? Somehow I assumed that the GWT compiler is there for us to work around JS funkyness (I'm using a mild word here), and was totally convinced that this bug is the GWT compiler's fault, more precisely the Eclipse compiler embedded and used by GWT.

[nfekete]

GWT compiler actually creates differently named js methods for overloaded java methods, so I really think this is not a JS problem. GWT (relying on info from the ECJ) just generates a reference to the wrong method.

[danieldietrich]

@nfekete To report a GWT bug we need a minimal example. S.th. like this:

import java.util.function.Function;

public class Test {

    public static void main(String[] args) {
        final String result = test(Test::of);
        if ("one".equals(result)) {
            throw new Error("'many' expected but found 'one'");
        }
    }

    static <T> String of(T t) {
        return "one";
    }

    static <T> String of(T[] ts) {
        return "many";
    }

    static <T> String test(Function<T[], String> of) {
        @SuppressWarnings("unchecked")
        final T[] ts = (T[]) new Object[] { null };
        return of.apply(ts);
    }

}

@ruslansennov could you run that code please in a GWT test/example (on the client side)?

@ruslansennov I updated the example

[nfekete]

Now I'm really baffled. Something deeper is going on here. I tried your example by including it into the javaslang-gwt-example project and it works. The bug doesn't manifest itself. I also tried with method reference to static methods as in the tabulate case but it still works ok. Also with varargs instead of array,

[danieldietrich]

Interesting. Then we should strip down the Javaslang library to the classes and methods that are called (but keep the implementations as far as possible).

There are still differences, for example the Collections class is package private and so on.

I will create a stripped down example tomorrow, need some sleep.

[nfekete]

Okay, I figured out something. The method reference GWT generates is actually good. The Test.of(T...) gets referenced but the passed-in array of Ts is actually a one-element array consisting of the array of elements that should get passed wrapped in as the first element of the array. So, in practice, the array argument gets wrapped in another array.

[nfekete]

Here's a small test case to reproduce the behavior:

import java.util.function.Function;

public class Test {

    public static void main(String[] args) {
        String[] array = new String[] { "1", "2" };
        first(array);
        Function<String[], String> fn = Test::first;
        fn.apply(array);
    }

    static String first(String... strings) {
        return strings[0];
    }

}

It seems this is a GWT problem, not Eclipse Compiler for Java problem. I'm gonna file a bug for GWT based on this.

Sorry for wasting your time with this, it has nothing to do with Javaslang. I mean, we knew from the start that Javaslang code was correct, but it seemed that the overloading of the *::of method is confusing the compiler. But it's the varargs + method reference that is confusing the compiler.

[nfekete]

GWT issue filed at: https://github.com/gwtproject/gwt/issues/9497

[ruslansennov]

This blocks javaslang-gwt release. I'm not sure we should fix javaslang internals or should wait for next version of GWT, but I would reopen this issue for a while

[danieldietrich]

@ruslansennov it should not affect our 2.1.0 release. It will be fixed in GWT 2.8.1. If GWT 2.8.1 isn't out before Javaslang 2.1.0, we are able to release javaslang-gwt-2.1.1 without changing javaslang-2.1.0.

I would close this ticket.

[ruslansennov]

"The fix should be straightforward" - good news :)

[nfekete]

GWT fix landed in HEAD, which has snapshots built as maven version HEAD-SNAPSHOT, see here:
https://oss.sonatype.org/content/repositories/google-snapshots/com/google/gwt/gwt-servlet/HEAD-SNAPSHOT/
I tried the above repro snippet and it works correctly.

[nfekete]

Vector.tabulate() also works correctly with gwt-HEAD-SNAPSHOT.

[danieldietrich]

Great to hear!