question about JDK-8063054 (incorrect raw type warnings for method references)

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

question about JDK-8063054 (incorrect raw type warnings for method references)

Liam Miller-Cushon
I've seen JDK-8063054 [1] crop up a few times, and did some investigation that I thought I'd share in case it's useful.

To summarize the bug, javac emits spurious rawtypes warnings for some method references:

class Test {
  void test(Box<? extends Box<Number>> b) { 
    Number n = b.<Number>map(Box::get).get(); 
  } 
  interface Func<S,T> { T apply(S arg); } 
  interface Box<T> { 
    T get(); 
    <R> Box<R> map(Func<T,R> f); 
  } 
}
...
javac -fullversion -Xlint:rawtypes Test.java
javac full version "9-ea+156"
Test.java:3: warning: [rawtypes] found raw type: Box
      Number n = b.<Number>map(Box::get).get(); 

I think the cause is related to the fix for JDK-8012685 [2]. Logic was added to skip rawtype warnings on method references if the qualifier can be inferred. This is done by testing if the inferred type is parameterized: http://hg.openjdk.java.net/jdk9/dev/langtools/annotate/33d1937af1a3/src/share/classes/com/sun/tools/javac/comp/Attr.java#l2738

In the example above, the inferred type of Box is `capture of ? extends Test.Box<java.lang.Number>`. It isn't raw, but it also isn't trivially parameterized, so it fails the check in Attr and still results in a rawtypes warning.

I think the fix might be as simple as replacing `!desc.getParameterTypes().head.isParameterized()` with something that handles capture variables.

Thanks,
Liam

Reply | Threaded
Open this post in threaded view
|

Re: question about JDK-8063054 (incorrect raw type warnings for method references)

Maurizio Cimadamore


On 16/02/17 02:33, Liam Miller-Cushon wrote:
> I think the fix might be as simple as replacing
> `!desc.getParameterTypes().head.isParameterized()` with something that
> handles capture variables.
Hi Liam,
I think your analysis is correct - isParameterized would not work for
non-class types, so something else is required here. I believe
intersection types like Serializable & Comparable<Foo> might also end up
with the same problem - and, presumably, synthetic inner class types.

Maurizio
Reply | Threaded
Open this post in threaded view
|

Re: question about JDK-8063054 (incorrect raw type warnings for method references)

B. Blaser
Hi,

On 16 February 2017 at 11:35, Maurizio Cimadamore
<[hidden email]> wrote:

>
>
> On 16/02/17 02:33, Liam Miller-Cushon wrote:
>>
>> I think the fix might be as simple as replacing
>> `!desc.getParameterTypes().head.isParameterized()` with something that
>> handles capture variables.
>
> Hi Liam,
> I think your analysis is correct - isParameterized would not work for
> non-class types, so something else is required here. I believe intersection
> types like Serializable & Comparable<Foo> might also end up with the same
> problem - and, presumably, synthetic inner class types.
>
> Maurizio

Regarding Daniel's JBS comment, I think the condition should be made
on isRaw() instead of isParameterized().
The following example wouldn't produce any warning:

class Test2 {
    interface Consumer<T> { void accept(T arg); }
    interface Parent<P> { void foo(); }
    interface Child extends Parent<String> {}
    static <T> void m(T arg, Consumer<T> f) {}
    public void test(Child c) { m(c, Parent::foo); }
}

Then, we could add a check on the bound of the captured type variable,
as below. Liam's example wouldn't produce any warning but the
following one would:

class Test3 {
    void test(Box<Box> b) {
        Object o = b.<Object>map(Box::get).get();
    }
    interface Func<S,T> { T apply(S arg); }
    interface Box<T> {
        T get();
        <R> Box<R> map(Func<T,R> f);
    }
}

Any comment is welcome.
Thanks,
Bernard

diff -r 43a83431f19d
src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
   Wed Mar 15 15:46:43 2017 +0100
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
   Sat Mar 18 19:01:02 2017 +0100
@@ -2962,9 +2962,15 @@

                 if (that.getMode() == ReferenceMode.INVOKE &&
                         TreeInfo.isStaticSelector(that.expr, names) &&
-                        that.kind.isUnbound() &&
-                        !desc.getParameterTypes().head.isParameterized()) {
-                    chk.checkRaw(that.expr, localEnv);
+                        that.kind.isUnbound()) {
+
+                    Type t = desc.getParameterTypes().head;
+                    if (t.isRaw() ||
+                            t.getTag() == TYPEVAR &&
+                            ((TypeVar)t).isCaptured() &&
+                            ((TypeVar)t).bound.isRaw()) {
+                        chk.checkRaw(that.expr, localEnv);
+                    }
                 }

                 if (that.sym.isStatic() &&
TreeInfo.isStaticSelector(that.expr, names) &&
Reply | Threaded
Open this post in threaded view
|

Re: question about JDK-8063054 (incorrect raw type warnings for method references)

B. Blaser
On 18 March 2017 at 19:32, B. Blaser <[hidden email]> wrote:

> Hi,
>
> On 16 February 2017 at 11:35, Maurizio Cimadamore
> <[hidden email]> wrote:
>>
>>
>> On 16/02/17 02:33, Liam Miller-Cushon wrote:
>>>
>>> I think the fix might be as simple as replacing
>>> `!desc.getParameterTypes().head.isParameterized()` with something that
>>> handles capture variables.
>>
>> Hi Liam,
>> I think your analysis is correct - isParameterized would not work for
>> non-class types, so something else is required here. I believe intersection
>> types like Serializable & Comparable<Foo> might also end up with the same
>> problem - and, presumably, synthetic inner class types.
>>
>> Maurizio

Here are two more examples producing a warning that should probably be
avoided (am I right?). The first one involves an intersection type and
the second one an anonymous class:

class Test5 {
    <B extends A & Box<Number>> void test(Box<B> b) {
        Number n = b.<Number>map(Box::get).get();
    }
    interface Func<S,T> { T apply(S arg); }
    interface Box<T> {
        T get();
        <R> Box<R> map(Func<T,R> f);
    }
    interface A {}
}

class Test6 {
    interface Consumer<T> { void accept(T arg); }
    interface Parent<P> { void foo(); }
    interface Child extends Parent<String> {}
    static <T> void m(T arg, Consumer<T> f) {}
    public void test(Child c) { m(new Child() { public void foo() {}
}, Parent::foo); }
}

I had to write (here under) a method Types.isRaw() using a type
visitor to handle captured type variables, intersection types and
anonymous classes.

How does this look, did I miss anything?

Thanks,
Bernard

diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java
b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java
@@ -227,6 +227,52 @@
         };
     // </editor-fold>

+    public boolean isRaw(Type t) {
+        return t != null && isRaw.visit(t);
+    }
+    // where
+        private final UnaryVisitor<Boolean> isRaw = new
UnaryVisitor<Boolean>() {
+            @Override
+            public Boolean visitType(Type t, Void ignored) {
+                return t.isRaw();
+            }
+
+            @Override
+            public Boolean visitClassType(ClassType t, Void ignored) {
+                if (t.isIntersection()) {
+                    for (Type u: ((IntersectionClassType) t).getComponents())
+                        if (isRaw(u))
+                            return true;
+
+                    return false;
+                }
+                else if (t.tsym.name.isEmpty()) {
+                    // Anonymous
+                    if (t.interfaces_field != null) {
+                        for (Type i: t.interfaces_field)
+                            if (isRaw(i))
+                                return true;
+                    }
+                    else if (t.supertype_field != null &&
isRaw(t.supertype_field))
+                        return true;
+
+                    return false;
+                }
+
+                return t.isRaw();
+            }
+
+            @Override
+            public Boolean visitTypeVar(TypeVar t, Void ignored) {
+                return t.bound != null && isRaw(t.bound);
+            }
+
+            @Override
+            public Boolean visitCapturedType(CapturedType t, Void ignored) {
+                return t.bound != null && isRaw(t.bound);
+            }
+        };
+
     // <editor-fold defaultstate="collapsed" desc="asSub">
     /**
      * Return the least specific subtype of t that starts with symbol
diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
@@ -2963,7 +2963,7 @@
                 if (that.getMode() == ReferenceMode.INVOKE &&
                         TreeInfo.isStaticSelector(that.expr, names) &&
                         that.kind.isUnbound() &&
-                        !desc.getParameterTypes().head.isParameterized()) {
+                        types.isRaw(desc.getParameterTypes().head)) {
                     chk.checkRaw(that.expr, localEnv);
                 }


> Regarding Daniel's JBS comment, I think the condition should be made
> on isRaw() instead of isParameterized().
> The following example wouldn't produce any warning:
>
> class Test2 {
>     interface Consumer<T> { void accept(T arg); }
>     interface Parent<P> { void foo(); }
>     interface Child extends Parent<String> {}
>     static <T> void m(T arg, Consumer<T> f) {}
>     public void test(Child c) { m(c, Parent::foo); }
> }
>
> Then, we could add a check on the bound of the captured type variable,
> as below. Liam's example wouldn't produce any warning but the
> following one would:
>
> class Test3 {
>     void test(Box<Box> b) {
>         Object o = b.<Object>map(Box::get).get();
>     }
>     interface Func<S,T> { T apply(S arg); }
>     interface Box<T> {
>         T get();
>         <R> Box<R> map(Func<T,R> f);
>     }
> }
>
> Any comment is welcome.
> Thanks,
> Bernard
>
> diff -r 43a83431f19d
> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
> --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
>    Wed Mar 15 15:46:43 2017 +0100
> +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
>    Sat Mar 18 19:01:02 2017 +0100
> @@ -2962,9 +2962,15 @@
>
>                  if (that.getMode() == ReferenceMode.INVOKE &&
>                          TreeInfo.isStaticSelector(that.expr, names) &&
> -                        that.kind.isUnbound() &&
> -                        !desc.getParameterTypes().head.isParameterized()) {
> -                    chk.checkRaw(that.expr, localEnv);
> +                        that.kind.isUnbound()) {
> +
> +                    Type t = desc.getParameterTypes().head;
> +                    if (t.isRaw() ||
> +                            t.getTag() == TYPEVAR &&
> +                            ((TypeVar)t).isCaptured() &&
> +                            ((TypeVar)t).bound.isRaw()) {
> +                        chk.checkRaw(that.expr, localEnv);
> +                    }
>                  }
>
>                  if (that.sym.isStatic() &&
> TreeInfo.isStaticSelector(that.expr, names) &&
Reply | Threaded
Open this post in threaded view
|

Re: question about JDK-8063054 (incorrect raw type warnings for method references)

Maurizio Cimadamore

Hi Bernard,
your patch looks good, I think it handles all the problematic cases I can think of.

My only reservation is in making isRaw a member of Types, which would hint that that's the definition of a raw type as per JLS 4.8, which it is not.

Ultimately, this routine is only used by the logic for checking method references, as an heuristics to try and see if the signature of the functional descriptor contains enough information to allow inference of the parameters of the 'raw' method reference receiver type.

Reading closer from the spec (15.13.1), I see the following statement:

"In the second search, if P1, ..., Pn is not empty and P1 is a subtype of ReferenceType, then the method reference expression is treated as if it were a method invocation expression with argument expressions of types P2, ..., Pn. If ReferenceType is a raw type, and there exists a parameterization of this type, G<...>, that is a supertype of P1, the type to search is the result of capture conversion (§5.1.10) applied to G<...>; otherwise, the type to search is the same as the type of the first search. Again, the type arguments, if any, are given by the method reference expression."

I think the lines in bold are the ones we need to pay attention to; in other words, if the receiver type is raw and the type of the first search is identical to the type of the second search, then it means no inference occurred. I think something like this could easily be achieved w/o any visitors, with something like this:

                if (that.getMode() == ReferenceMode.INVOKE &&
                        TreeInfo.isStaticSelector(that.expr, names) &&
                        that.kind.isUnbound() &&
                        types.isSameTypes(lookupHelper.site, that.expr.type) {
                    chk.checkRaw(that.expr, localEnv);
                }

I did some quick tests and this simple patch seems to remove the undesired warnings in the examples we have seen so far, but retains them where they are truly deserved. What do you think?

Maurizio

On 22/03/17 22:04, B. Blaser wrote:
On 18 March 2017 at 19:32, B. Blaser [hidden email] wrote:
Hi,

On 16 February 2017 at 11:35, Maurizio Cimadamore
[hidden email] wrote:

On 16/02/17 02:33, Liam Miller-Cushon wrote:
I think the fix might be as simple as replacing
`!desc.getParameterTypes().head.isParameterized()` with something that
handles capture variables.
Hi Liam,
I think your analysis is correct - isParameterized would not work for
non-class types, so something else is required here. I believe intersection
types like Serializable & Comparable<Foo> might also end up with the same
problem - and, presumably, synthetic inner class types.

Maurizio
Here are two more examples producing a warning that should probably be
avoided (am I right?). The first one involves an intersection type and
the second one an anonymous class:

class Test5 {
    <B extends A & Box<Number>> void test(Box<B> b) {
        Number n = b.<Number>map(Box::get).get();
    }
    interface Func<S,T> { T apply(S arg); }
    interface Box<T> {
        T get();
        <R> Box<R> map(Func<T,R> f);
    }
    interface A {}
}

class Test6 {
    interface Consumer<T> { void accept(T arg); }
    interface Parent<P> { void foo(); }
    interface Child extends Parent<String> {}
    static <T> void m(T arg, Consumer<T> f) {}
    public void test(Child c) { m(new Child() { public void foo() {}
}, Parent::foo); }
}

I had to write (here under) a method Types.isRaw() using a type
visitor to handle captured type variables, intersection types and
anonymous classes.

How does this look, did I miss anything?

Thanks,
Bernard

diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java
b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java
@@ -227,6 +227,52 @@
         };
     // </editor-fold>

+    public boolean isRaw(Type t) {
+        return t != null && isRaw.visit(t);
+    }
+    // where
+        private final UnaryVisitor<Boolean> isRaw = new
UnaryVisitor<Boolean>() {
+            @Override
+            public Boolean visitType(Type t, Void ignored) {
+                return t.isRaw();
+            }
+
+            @Override
+            public Boolean visitClassType(ClassType t, Void ignored) {
+                if (t.isIntersection()) {
+                    for (Type u: ((IntersectionClassType) t).getComponents())
+                        if (isRaw(u))
+                            return true;
+
+                    return false;
+                }
+                else if (t.tsym.name.isEmpty()) {
+                    // Anonymous
+                    if (t.interfaces_field != null) {
+                        for (Type i: t.interfaces_field)
+                            if (isRaw(i))
+                                return true;
+                    }
+                    else if (t.supertype_field != null &&
isRaw(t.supertype_field))
+                        return true;
+
+                    return false;
+                }
+
+                return t.isRaw();
+            }
+
+            @Override
+            public Boolean visitTypeVar(TypeVar t, Void ignored) {
+                return t.bound != null && isRaw(t.bound);
+            }
+
+            @Override
+            public Boolean visitCapturedType(CapturedType t, Void ignored) {
+                return t.bound != null && isRaw(t.bound);
+            }
+        };
+
     // <editor-fold defaultstate="collapsed" desc="asSub">
     /**
      * Return the least specific subtype of t that starts with symbol
diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
@@ -2963,7 +2963,7 @@
                 if (that.getMode() == ReferenceMode.INVOKE &&
                         TreeInfo.isStaticSelector(that.expr, names) &&
                         that.kind.isUnbound() &&
-                        !desc.getParameterTypes().head.isParameterized()) {
+                        types.isRaw(desc.getParameterTypes().head)) {
                     chk.checkRaw(that.expr, localEnv);
                 }


Regarding Daniel's JBS comment, I think the condition should be made
on isRaw() instead of isParameterized().
The following example wouldn't produce any warning:

class Test2 {
    interface Consumer<T> { void accept(T arg); }
    interface Parent<P> { void foo(); }
    interface Child extends Parent<String> {}
    static <T> void m(T arg, Consumer<T> f) {}
    public void test(Child c) { m(c, Parent::foo); }
}

Then, we could add a check on the bound of the captured type variable,
as below. Liam's example wouldn't produce any warning but the
following one would:

class Test3 {
    void test(Box<Box> b) {
        Object o = b.<Object>map(Box::get).get();
    }
    interface Func<S,T> { T apply(S arg); }
    interface Box<T> {
        T get();
        <R> Box<R> map(Func<T,R> f);
    }
}

Any comment is welcome.
Thanks,
Bernard

diff -r 43a83431f19d
src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
   Wed Mar 15 15:46:43 2017 +0100
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
   Sat Mar 18 19:01:02 2017 +0100
@@ -2962,9 +2962,15 @@

                 if (that.getMode() == ReferenceMode.INVOKE &&
                         TreeInfo.isStaticSelector(that.expr, names) &&
-                        that.kind.isUnbound() &&
-                        !desc.getParameterTypes().head.isParameterized()) {
-                    chk.checkRaw(that.expr, localEnv);
+                        that.kind.isUnbound()) {
+
+                    Type t = desc.getParameterTypes().head;
+                    if (t.isRaw() ||
+                            t.getTag() == TYPEVAR &&
+                            ((TypeVar)t).isCaptured() &&
+                            ((TypeVar)t).bound.isRaw()) {
+                        chk.checkRaw(that.expr, localEnv);
+                    }
                 }

                 if (that.sym.isStatic() &&
TreeInfo.isStaticSelector(that.expr, names) &&

Reply | Threaded
Open this post in threaded view
|

Re: question about JDK-8063054 (incorrect raw type warnings for method references)

B. Blaser
Hi Maurizio,

On 23 March 2017 at 12:58, Maurizio Cimadamore
<[hidden email]> wrote:

> Hi Bernard,
>
> [...]
>
> I think the lines in bold are the ones we need to pay attention to; in other
> words, if the receiver type is raw and the type of the first search is
> identical to the type of the second search, then it means no inference
> occurred. I think something like this could easily be achieved w/o any
> visitors, with something like this:
>
>                 if (that.getMode() == ReferenceMode.INVOKE &&
>                         TreeInfo.isStaticSelector(that.expr, names) &&
>                         that.kind.isUnbound() &&
>                         types.isSameTypes(lookupHelper.site, that.expr.type)
> {
>                     chk.checkRaw(that.expr, localEnv);
>                 }
>
> I did some quick tests and this simple patch seems to remove the undesired
> warnings in the examples we have seen so far, but retains them where they
> are truly deserved. What do you think?
>
> Maurizio

Fine, nice solution... I would simply rewrite it as follows (
lookupHelper.site.isRaw() ):

                if (that.getMode() == ReferenceMode.INVOKE &&
                        TreeInfo.isStaticSelector(that.expr, names) &&
                        that.kind.isUnbound() &&
                        lookupHelper.site.isRaw()) {
                    chk.checkRaw(that.expr, localEnv);
                }

This would avoid some redundant checks if "ReferenceType" isn't raw (I
think), for example:

class Test10 {
    interface Consumer<T> { void accept(T arg); }
    interface Parent<P> { void foo(); }
    interface Child extends Parent<String> {}
    static <T> void m(T arg, Consumer<T> f) {}
    public void test(Child c) { m(c, Parent<String>::foo); }
}

Here, "types.isSameType(lookupHelper.site, that.expr.type)" would
verify if "Parent<String> == Parent<String>" and then "chk.checkRaw()"
would check if "Parent<String>" is raw...

But, at first sight, this looks good. I'll try to do some more testing...

What do you think of this small rewriting?

Bernard
Reply | Threaded
Open this post in threaded view
|

Re: question about JDK-8063054 (incorrect raw type warnings for method references)

Maurizio Cimadamore


On 24/03/17 15:56, B. Blaser wrote:

> Hi Maurizio,
>
> On 23 March 2017 at 12:58, Maurizio Cimadamore
> <[hidden email]> wrote:
>> Hi Bernard,
>>
>> [...]
>>
>> I think the lines in bold are the ones we need to pay attention to; in other
>> words, if the receiver type is raw and the type of the first search is
>> identical to the type of the second search, then it means no inference
>> occurred. I think something like this could easily be achieved w/o any
>> visitors, with something like this:
>>
>>                  if (that.getMode() == ReferenceMode.INVOKE &&
>>                          TreeInfo.isStaticSelector(that.expr, names) &&
>>                          that.kind.isUnbound() &&
>>                          types.isSameTypes(lookupHelper.site, that.expr.type)
>> {
>>                      chk.checkRaw(that.expr, localEnv);
>>                  }
>>
>> I did some quick tests and this simple patch seems to remove the undesired
>> warnings in the examples we have seen so far, but retains them where they
>> are truly deserved. What do you think?
>>
>> Maurizio
> Fine, nice solution... I would simply rewrite it as follows (
> lookupHelper.site.isRaw() ):
>
>                  if (that.getMode() == ReferenceMode.INVOKE &&
>                          TreeInfo.isStaticSelector(that.expr, names) &&
>                          that.kind.isUnbound() &&
>                          lookupHelper.site.isRaw()) {
>                      chk.checkRaw(that.expr, localEnv);
>                  }
>
> This would avoid some redundant checks if "ReferenceType" isn't raw (I
> think), for example:
>
> class Test10 {
>      interface Consumer<T> { void accept(T arg); }
>      interface Parent<P> { void foo(); }
>      interface Child extends Parent<String> {}
>      static <T> void m(T arg, Consumer<T> f) {}
>      public void test(Child c) { m(c, Parent<String>::foo); }
> }
>
> Here, "types.isSameType(lookupHelper.site, that.expr.type)" would
> verify if "Parent<String> == Parent<String>" and then "chk.checkRaw()"
> would check if "Parent<String>" is raw...
>
> But, at first sight, this looks good. I'll try to do some more testing...
>
> What do you think of this small rewriting?
Yeah - I think that could work too (not that I'm too worried about
performances here - we're outside overload resolution at this stage).

Maurizio
>
> Bernard

Reply | Threaded
Open this post in threaded view
|

Re: question about JDK-8063054 (incorrect raw type warnings for method references)

Liam Miller-Cushon
Friendly ping - it doesn't look like this fix ever got merged. Did you run into problems with the approach?

On Fri, Mar 24, 2017 at 9:26 AM, Maurizio Cimadamore <[hidden email]> wrote:


On 24/03/17 15:56, B. Blaser wrote:
Hi Maurizio,

On 23 March 2017 at 12:58, Maurizio Cimadamore
<[hidden email]> wrote:
Hi Bernard,

[...]

I think the lines in bold are the ones we need to pay attention to; in other
words, if the receiver type is raw and the type of the first search is
identical to the type of the second search, then it means no inference
occurred. I think something like this could easily be achieved w/o any
visitors, with something like this:

                 if (that.getMode() == ReferenceMode.INVOKE &&
                         TreeInfo.isStaticSelector(that.expr, names) &&
                         that.kind.isUnbound() &&
                         types.isSameTypes(lookupHelper.site, that.expr.type)
{
                     chk.checkRaw(that.expr, localEnv);
                 }

I did some quick tests and this simple patch seems to remove the undesired
warnings in the examples we have seen so far, but retains them where they
are truly deserved. What do you think?

Maurizio
Fine, nice solution... I would simply rewrite it as follows (
lookupHelper.site.isRaw() ):

                 if (that.getMode() == ReferenceMode.INVOKE &&
                         TreeInfo.isStaticSelector(that.expr, names) &&
                         that.kind.isUnbound() &&
                         lookupHelper.site.isRaw()) {
                     chk.checkRaw(that.expr, localEnv);
                 }

This would avoid some redundant checks if "ReferenceType" isn't raw (I
think), for example:

class Test10 {
     interface Consumer<T> { void accept(T arg); }
     interface Parent<P> { void foo(); }
     interface Child extends Parent<String> {}
     static <T> void m(T arg, Consumer<T> f) {}
     public void test(Child c) { m(c, Parent<String>::foo); }
}

Here, "types.isSameType(lookupHelper.site, that.expr.type)" would
verify if "Parent<String> == Parent<String>" and then "chk.checkRaw()"
would check if "Parent<String>" is raw...

But, at first sight, this looks good. I'll try to do some more testing...

What do you think of this small rewriting?
Yeah - I think that could work too (not that I'm too worried about performances here - we're outside overload resolution at this stage).

Maurizio

Bernard


Reply | Threaded
Open this post in threaded view
|

Re: question about JDK-8063054 (incorrect raw type warnings for method references)

Maurizio Cimadamore

You are right - I think we've got distracted by other things. I will make sure this makes into the jdk10 repo.

Cheers
Maurizio


On 04/08/17 18:16, Liam Miller-Cushon wrote:
Friendly ping - it doesn't look like this fix ever got merged. Did you run into problems with the approach?

On Fri, Mar 24, 2017 at 9:26 AM, Maurizio Cimadamore <[hidden email]> wrote:


On 24/03/17 15:56, B. Blaser wrote:
Hi Maurizio,

On 23 March 2017 at 12:58, Maurizio Cimadamore
<[hidden email]> wrote:
Hi Bernard,

[...]

I think the lines in bold are the ones we need to pay attention to; in other
words, if the receiver type is raw and the type of the first search is
identical to the type of the second search, then it means no inference
occurred. I think something like this could easily be achieved w/o any
visitors, with something like this:

                 if (that.getMode() == ReferenceMode.INVOKE &&
                         TreeInfo.isStaticSelector(that.expr, names) &&
                         that.kind.isUnbound() &&
                         types.isSameTypes(lookupHelper.site, that.expr.type)
{
                     chk.checkRaw(that.expr, localEnv);
                 }

I did some quick tests and this simple patch seems to remove the undesired
warnings in the examples we have seen so far, but retains them where they
are truly deserved. What do you think?

Maurizio
Fine, nice solution... I would simply rewrite it as follows (
lookupHelper.site.isRaw() ):

                 if (that.getMode() == ReferenceMode.INVOKE &&
                         TreeInfo.isStaticSelector(that.expr, names) &&
                         that.kind.isUnbound() &&
                         lookupHelper.site.isRaw()) {
                     chk.checkRaw(that.expr, localEnv);
                 }

This would avoid some redundant checks if "ReferenceType" isn't raw (I
think), for example:

class Test10 {
     interface Consumer<T> { void accept(T arg); }
     interface Parent<P> { void foo(); }
     interface Child extends Parent<String> {}
     static <T> void m(T arg, Consumer<T> f) {}
     public void test(Child c) { m(c, Parent<String>::foo); }
}

Here, "types.isSameType(lookupHelper.site, that.expr.type)" would
verify if "Parent<String> == Parent<String>" and then "chk.checkRaw()"
would check if "Parent<String>" is raw...

But, at first sight, this looks good. I'll try to do some more testing...

What do you think of this small rewriting?
Yeah - I think that could work too (not that I'm too worried about performances here - we're outside overload resolution at this stage).

Maurizio

Bernard