RFR 8144903: JShell: determine incorrectly the type of the expression which is array type of captured type
Maurizio Cimadamore
maurizio.cimadamore at oracle.com
Wed Dec 9 22:12:45 UTC 2015
What we can do to improve this is to use something similar to the
analysis used in JLS 9.9 for functional types:
* if the bound of the corresponding parameter (the captured var bound
field) contains any other formal parameter (hard case), just use the
'wildcard' type
Example:
class Foo<X extends Comparable<X>> {
X get();
}
Foo<? extends String> fs = ...
fs.get() --> String
* if the bound B of the corresponding parameter does not contain any
other formal (non fbound case) then:
- if wildcard is '? extends T', the inferred type is glb(B, T) (**)
- if wildcard is '? super T', the inferred type is T
- if wildcard is '?', the inferred type is B
This should take care of most of the nasty cases, in a way that's
compatible with how things are handled for lambdas.
In your CharsSequence case, since the bound is CharSequence and the
wildcard type is Object, you are in the happy case (#2), so the inferred
type is glb(CharSequence, Object) = Object & CharSequence.
(**) as this example demonstrates, sometimes glb can return intersection
types, so I'm assuming you already have some logic to 'normalize'
intersection types away - and of course this logic should not throw info
away - i.e. if you normalize Object & Serializable, I would expect the
logic to retain Serializable, and discard the non-interesting bound.
Opinions?
Maurizio
On 09/12/15 20:46, ShinyaYoshida wrote:
> Hi Maurizio and Jan,
> Thank you for your advice.
> I've tried to unroll it in wildcard to take the advice.
> http://cr.openjdk.java.net/~shinyafox/kulla/8144903/webrev.02/
> <http://cr.openjdk.java.net/%7Eshinyafox/kulla/8144903/webrev.02/>
>
> But I've met a problem.
> In this case, we get the Object instead of CharSequence, this is not
> acceptable for user:
>
> In TypeNameTest#testBounds
> assertEval("class Test1<X extends CharSequence> { public X
> get() { return null; } }");
> Snippet x = varKey(assertEval("Test1<?> test1 = new Test1<>();"));
> VarSnippet sn4 = (VarSnippet) varKey(assertEval("test1.get()"));
> assertEquals(sn4.typeName(), "CharSequence");
>
> Compiler determines the upper bound of wildcard"?" as CharSequence
> from the declaration.
> But WildcardType doesn't have that information at all.
>
> Do you have any idea for this?
>
> Regards,
> shinyafox(Shinya Yoshida)
>
>
> 2015-12-10 0:00 GMT+09:00 Jan Lahoda <jan.lahoda at oracle.com
> <mailto:jan.lahoda at oracle.com>>:
>
> I am looking at:
> http://hg.netbeans.org/jet-main/file/default/java.source.base/src/org/netbeans/api/java/source/SourceUtils.java#l1067
>
> (which is solving a similar problem; and while I found out there
> may be a bug related to arrays in that.)
>
> I concur with Maurizio that CapturedType.wildcard should
> (presumably) be used always for CapturedTypes. When the resulting
> wildcard is not suitable for the given place (i.e. top-level
> type), then we should use getExtendsBound()/getSuperBound() to
> unroll the wildcard.
>
> Out of the recent two patches, I like webrev.01_0 much more.
> Presumably just override visitWildcardType and unroll the wildcard
> if needed, and change the visitCapturedType to just:
> return visit(t.wildcard, locale);
>
> Would that work?
>
> Jan
>
>
> On 9.12.2015 13:39, Maurizio Cimadamore wrote:
>
> Hi Shinya,
> In my experience, the best way to get to an usable declared
> type from a
> capture type is to always follow the wildcard - i.e. the upper
> bound of
> a captured type is more of an internal type system info (the
> result of
> capture conversion) and I don't think it should be used.
>
> That said, I think there's a problem here: captured vars can
> appear in
> two places, type position or type-argument position:
>
> 1) Type position
>
> class Foo<X> {
> X get() { .. }
> }
>
> Foo<? extends String> fs = ...
> fs.get(); // you get a #CAP in a type position - i.e. the
> return type of
> get()
>
> 2) Type-argument position
>
> class Foo2<X> {
> List<X> get() { .. }
> }
>
> Foo<? extends String> fs = ...
> fs.get(); // you get a List<#CAP>, so #CAP is in a
> type-argument position
>
> I think the TypePrinter should have some logic to detect as to
> whethe
> you are in case #1 or #2, and if in #1 it should drop
> wildcards to the
> floor; if it's in #2 it is ok to retain wildcards. So, I think
> a good
> solution would emit String for #1 and List<? extends String>
> for #2.
>
> As usual - this is my 0.02$ :-)
>
> [This is essentially an inference problem - we are trying to
> infer a
> denotable type from the type of an expression which might contain
> non-denotable types - the problem is not too different from
> what we have
> to do when the functional interfaces that are the target of a
> lambda
> conversion contains wildcards].
>
> Maurizio
>
> On 09/12/15 00:34, Robert Field wrote:
>
> Exactly, Shinya. Still curious of Jan's thinking.
>
> -Robert
>
>
>
> On December 8, 2015 15:13:42 ShinyaYoshida
> <bitterfoxc at gmail.com <mailto:bitterfoxc at gmail.com>> wrote:
>
> Hi Robert,
> Thank you for your review and comments.
>
> 2015-12-09 5:28 GMT+09:00 Robert Field
> <robert.field at oracle.com
> <mailto:robert.field at oracle.com>>:
>
> [Jan, I'd like your opinion]
>
> Thank you for seeing this and drilling into where
> the problem is.
> This is
> important to fix.
>
> I see that this would address the problem.
> However, I am squeamish
> about
> the solution as it unwraps to match the result of
> layers of underlying
> super class behavior. Looking at it further, I
> believe the problem
> is in
> the pre-existing typeToPrint, my bad! I don't
> think the approach I
> took is
> right. Stepping back, we want upper bound unless
> the type is
> embedded in a
> type that takes generic types, class types. That
> suggests to me
> replacing
> typeToPrint with a boolean useWildCard = false,
> which would be
> changed by
> visitClassType(). But alas this is recursive and
> this visitor does not
> have state that passes through the recursion, so
> it would be global
> -- in
> this case that should work, but ugly.
>
> Certainly, my fix is too depend on the super
> implementation.
> The fix which you mentioned is like this, right?
> http://cr.openjdk.java.net/~shinyafox/kulla/8144903/webrev.01_0/
> <http://cr.openjdk.java.net/%7Eshinyafox/kulla/8144903/webrev.01_0/>
>
>
>
> Ah! Really the problem is that we want the
> top-level type to be erased.
> But that means holding an instance of Types.
> Deeper integration with
> javac. Hmmmm....
>
> Another possible fix is making TypePrinter general(no
> recursive
> state) and
> adding the print method what is erase top-level
> capture to TypePrinter,
> using the print method at callsite:
> http://cr.openjdk.java.net/~shinyafox/kulla/8144903/webrev.01_1/
> <http://cr.openjdk.java.net/%7Eshinyafox/kulla/8144903/webrev.01_1/>
>
> Regards,
> shinyafox(Shinya Yoshida)
>
>
>
>
> -Robert
>
> On Dec 8, 2015, at 1:10 AM, ShinyaYoshida
> <bitterfoxc at gmail.com
> <mailto:bitterfoxc at gmail.com>> wrote:
>
> Hi Robert,
> Could you review my patch for
> "JShell: determine incorrectly the type of the
> expression which is
> array
> type of captured type"?
> https://bugs.openjdk.java.net/browse/JDK-8144903
>
> webrev:
> http://cr.openjdk.java.net/~shinyafox/kulla/8144903/webrev.00/
> <http://cr.openjdk.java.net/%7Eshinyafox/kulla/8144903/webrev.00/>
>
> Regards,
> shinyafox(Shinya Yoshida)
>
>
>
>
>
More information about the kulla-dev
mailing list