RFR: universal type variables: initial prototype

Maurizio Cimadamore mcimadamore at openjdk.java.net
Thu Aug 5 13:18:45 UTC 2021


On Thu, 5 Aug 2021 04:23:29 GMT, Vicente Romero <vromero at openjdk.org> wrote:

>> src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java line 3615:
>> 
>>> 3613:                     return to.head.withTypeVar(t);
>>> 3614:                 }
>>> 3615:                 if (allowUniversalTVars &&
>> 
>> Not sure about this - the main issue here seems that `t.equalsIgnoreMetadata` is too weak and would return `false` for `ref` vs.`val` mismatches. But again I'm uncertain about the need of overriding `withTypeVar` (see comment above).
>
> not sure I follow you here, `t.equalsIgnoreMetadata` is already being used in the method, not sure I see in what case it can give an incorrect answer

What I mean is that the newly added code seems to workaround the fact that in some cases we do not call `withTypeVar`. I'm saying that because you added another test which, if satisfied, ends up calling `to.head.withTypeVar(t);` again, which seems to suggest that you found cases where this call was needed, but was not triggered - probably because of a mismatch between T and T.ref?

>> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java line 825:
>> 
>>> 823:     }
>>> 824: 
>>> 825:     List<Type> attribTypes(List<JCExpression> trees, Env<AttrContext> env, Predicate<JCExpression> valueOK) {
>> 
>> Do you really need a predicate here? Wouldn't a boolean be ok? Or perhaps a flag in env, or some other means?
>
> the thing is that one value only doesn't apply to all cases, what if some type variables are universal but others aren't?

But you have only one predicate? Anyway - see the other related comment - you really need this predicate when you call this later on:


attribTypes(tree.arguments, env, arg -> valueOKSet.contains(arg));


But it feels like the code in there can be simplified so that you use a single `attribType` inside a loop, by passing the right boolean flag for that parameter.

-------------

PR: https://git.openjdk.java.net/valhalla/pull/521



More information about the valhalla-dev mailing list