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
Make "PrimitiveParameterizedClass.default" a poly expression. #369
base: lworld
Are you sure you want to change the base?
Changes from all commits
e9289cf
0c08826
8ce4dce
131b9c7
f4073c4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -50,6 +50,7 @@ | ||
import com.sun.tools.javac.comp.Check.CheckContext; | ||
import com.sun.tools.javac.comp.DeferredAttr.AttrMode; | ||
import com.sun.tools.javac.comp.MatchBindingsComputer.MatchBindings; | ||
import com.sun.tools.javac.comp.Resolve.MethodResolutionPhase; | ||
import com.sun.tools.javac.jvm.*; | ||
import static com.sun.tools.javac.resources.CompilerProperties.Fragments.Diamond; | ||
import static com.sun.tools.javac.resources.CompilerProperties.Fragments.DiamondInvalidArg; | ||
@@ -672,7 +673,7 @@ KindSelector pkind() { | ||
* @param env The environment visitor argument. | ||
* @param resultInfo The result info visitor argument. | ||
*/ | ||
Type attribTree(JCTree tree, Env<AttrContext> env, ResultInfo resultInfo) { | ||
Type attribTree (JCTree tree, Env<AttrContext> env, ResultInfo resultInfo) { | ||
Env<AttrContext> prevEnv = this.env; | ||
ResultInfo prevResult = this.resultInfo; | ||
try { | ||
@@ -5020,25 +5021,62 @@ public void visitDefaultValue(JCDefaultValue tree) { | ||
} | ||
|
||
// Attribute the qualifier expression, and determine its symbol (if any). | ||
Type site = attribTree(tree.clazz, env, new ResultInfo(KindSelector.TYP_PCK, Type.noType)); | ||
Type clazztype = attribTree(tree.clazz, env, new ResultInfo(KindSelector.TYP, Type.noType)); | ||
if (!pkind().contains(KindSelector.TYP_PCK)) | ||
site = capture(site); // Capture field access | ||
clazztype = capture(clazztype); // Capture field access | ||
|
||
Symbol sym = switch (site.getTag()) { | ||
case WILDCARD -> throw new AssertionError(tree); | ||
case PACKAGE -> { | ||
log.error(tree.pos, Errors.CantResolveLocation(Kinds.KindName.CLASS, site.tsym.getQualifiedName(), null, null, | ||
Fragments.Location(Kinds.typeKindName(env.enclClass.type), env.enclClass.type, null))); | ||
yield syms.errSymbol; | ||
} | ||
case ERROR -> types.createErrorType(names._default, site.tsym, site).tsym; | ||
default -> new VarSymbol(STATIC, names._default, site, site.tsym); | ||
}; | ||
if (TreeInfo.isPossiblePolyDefault(tree, clazztype)) { | ||
var flavor = clazztype.getTag() == CLASS ? clazztype.getFlavor() : Flavor.X_Typeof_X; | ||
ClassType site = new ClassType(clazztype.getEnclosingType(), | ||
clazztype.tsym.type.getTypeArguments(), | ||
clazztype.tsym, | ||
clazztype.getMetadata(), | ||
flavor); | ||
|
||
if (site.hasTag(TYPEVAR) && sym.kind != ERR) { | ||
site = types.skipTypeVars(site, true); | ||
Env<AttrContext> diamondEnv = env.dup(tree); | ||
diamondEnv.info.selectSuper = false; | ||
diamondEnv.info.pendingResolutionPhase = MethodResolutionPhase.BASIC; | ||
|
||
var clazzTypeArgs = site.getTypeArguments(); | ||
Type constrType = tree.defaultValueConstructor = new ForAll(clazzTypeArgs, | ||
new MethodType(List.nil(), site, List.nil(), syms.methodClass)); | ||
|
||
MethodSymbol constructor = new MethodSymbol(Flags.SYNTHETIC, names.init, constrType, site.tsym); | ||
ResultInfo diamondResult = new ResultInfo(KindSelector.VAL, newMethodTemplate(resultInfo.pt, List.nil(), List.nil()), | ||
defaultPolyContext(tree, clazztype.tsym, resultInfo.checkContext), CheckMode.NO_TREE_UPDATE); | ||
|
||
Type defaultType = checkId(tree, site, constructor, diamondEnv, diamondResult); | ||
|
||
if (defaultType.isErroneous()) { | ||
tree.clazz.type = types.createErrorType(clazztype); | ||
} else { | ||
tree.defaultValueConstructor = types.createMethodTypeWithReturn(defaultType, syms.voidType); | ||
tree.clazz.type = defaultType.getReturnType(); | ||
} | ||
clazztype = chk.checkClassType(tree.clazz, tree.clazz.type, true); | ||
result = check(tree, clazztype, KindSelector.VAL, resultInfo); | ||
} else { | ||
chk.validate(tree.clazz, env); | ||
Symbol sym = switch (clazztype.getTag()) { | ||
case WILDCARD -> throw new AssertionError(tree); | ||
case ERROR -> types.createErrorType(names._default, clazztype.tsym, clazztype).tsym; | ||
default -> new VarSymbol(STATIC, names._default, clazztype, clazztype.tsym); | ||
}; | ||
if (clazztype.hasTag(TYPEVAR) && sym.kind != ERR) { | ||
clazztype = types.skipTypeVars(clazztype, true); | ||
} | ||
result = tree.type = checkId(tree, clazztype, sym, env, resultInfo); | ||
} | ||
result = checkId(tree, site, sym, env, resultInfo); | ||
} | ||
// where | ||
CheckContext defaultPolyContext(JCDefaultValue tree, TypeSymbol tsym, CheckContext checkContext) { | ||
return new Check.NestedCheckContext(checkContext) { | ||
@Override | ||
public void report(DiagnosticPosition _unused, JCDiagnostic details) { | ||
enclosingContext.report(tree.clazz, | ||
diags.fragment(Fragments.CantApplyDiamond1(Fragments.Diamond(tsym), details))); | ||
} | ||
}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see a test that triggers this diagnostic. |
||
} | ||
|
||
public void visitLiteral(JCLiteral tree) { | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -276,6 +276,27 @@ public static boolean isDiamond(JCTree tree) { | ||
} | ||
} | ||
|
||
/** Return true if a default expression may represent a poly expression (with elided types) */ | ||
public static boolean isPossiblePolyDefault(JCDefaultValue tree, Type clazztype) { | ||
JCTypeApply applyTree = TreeInfo.getTypeApplication(tree.clazz); | ||
if (applyTree != null) { | ||
return applyTree.arguments.isEmpty(); | ||
} else { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The return statement is not reachable and so the block is not useful. This is because, Foo<>.default is a syntax error. Given a primitive class Foo we will only support Foo.default involving implicit diamond like inference and will not support Foo<>.default which is what this code is handling. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (I think I meant to say return applyTree.arguments.isEmpty(); will never be true.) |
||
// No type arguments before .default - Consider if the type is generic or not | ||
return clazztype == null || clazztype.tsym.type.isParameterized(); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This method is problematic - This will always treat Foo.default as a poly expression in an invocation context even when the concerned primitive type is not a generic class. For instance the Foo.default expression in the following snippet becomes a poly expression when it is a standalone expression in reality:
|
||
} | ||
|
||
/** Return true if a tree directly or indirectly represents a type application. */ | ||
public static JCTypeApply getTypeApplication(JCTree tree) { | ||
switch(tree.getTag()) { | ||
case TYPEAPPLY: return (JCTypeApply)tree; | ||
case NEWCLASS: return getTypeApplication(((JCNewClass)tree).clazz); | ||
case ANNOTATED_TYPE: return getTypeApplication(((JCAnnotatedType)tree).underlyingType); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think there is a copy + paste problem. We can never see a NEWCLASS here. The LHS of .default is just a type node and cannot be an rvalue expression ==> new X().default is illegal. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think perhaps what we really need to do here is to attribute the clazz node of JCDefaultValue first - perhaps by first making a copy of the node and then see if it is a generic class but lacks type arguments. Such as this:
|
||
default: return null; | ||
} | ||
} | ||
|
||
public static boolean isEnumInit(JCTree tree) { | ||
switch (tree.getTag()) { | ||
case VARDEF: | ||
@@ -294,6 +315,9 @@ public static void setPolyKind(JCTree tree, PolyKind pkind) { | ||
case NEWCLASS: | ||
((JCNewClass)tree).polyKind = pkind; | ||
break; | ||
case DEFAULT_VALUE: | ||
((JCDefaultValue)tree).polyKind = pkind; | ||
break; | ||
case REFERENCE: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is surprising. I would expected polyKind to be set in a similar fashion to switch and conditional expressions. |
||
((JCMemberReference)tree).refPolyKind = pkind; | ||
break; | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
/* | ||
* @test /nodynamiccopyright/ | ||
* @bug 8210906 | ||
* @summary [lworld] default value creation should not impose raw types on users. | ||
* @compile/fail/ref=PolyDefault.out -Xlint:all -Werror -XDrawDiagnostics -XDdev PolyDefault.java | ||
*/ | ||
import java.util.concurrent.Callable; | ||
import java.util.LinkedList; | ||
import java.util.Collection; | ||
import java.util.List; | ||
|
||
public primitive class PolyDefault<E> implements Callable<E> { | ||
E value; | ||
protected PolyDefault() { this.value = E.default; } | ||
PolyDefault(E value) { this.value = value; } | ||
|
||
@Override | ||
public E call() throws Exception { | ||
return value; | ||
} | ||
|
||
@FunctionalInterface | ||
interface PolyProducer { | ||
PolyDefault<String> produce(); | ||
} | ||
|
||
interface Foo<X extends Number> extends List<X> { | ||
} | ||
|
||
public static <T extends Boolean> String overload(List<T> nums) { | ||
return ""; | ||
} | ||
|
||
public static <T extends Number> T overload(Collection<T> nums) { | ||
return T.default; | ||
} | ||
|
||
public static void main(String [] args) throws Exception { | ||
|
||
List<Integer> il = LinkedList.default; | ||
Integer i0 = il.get(0); | ||
|
||
var a = overload(Foo.default); | ||
var b = a.intValue(); | ||
|
||
// Things which should just work | ||
PolyDefault<LinkedList<Long>> foo = PolyDefault.default; // Poly expression | ||
LinkedList<Long> c = foo.call(); // This should be fine, inferred above | ||
var genericDefault1 = PolyDefault<LinkedList<Long>>.default; | ||
|
||
PolyProducer genericDefault = () -> PolyDefault.default; | ||
|
||
// Problems | ||
List<String> boing = new PolyDefault<>(); // Error: Can't make a PolyDefault into a list | ||
List<String> boom = PolyDefault.default; // Error: Can't make a PolyDefault into a list | ||
List<String> tschak = Foo.default; // Error: Can make a Foo into a list, but then must be List<? extends Number> | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
PolyDefault.java:54:45: compiler.err.prob.found.req: (compiler.misc.cant.apply.diamond.1: (compiler.misc.diamond: PolyDefault), (compiler.misc.infer.no.conforming.instance.exists: E, PolyDefault<E>, java.util.List<java.lang.String>)) | ||
PolyDefault.java:55:29: compiler.err.prob.found.req: (compiler.misc.cant.apply.diamond.1: (compiler.misc.diamond: PolyDefault), (compiler.misc.infer.no.conforming.instance.exists: E, PolyDefault<E>, java.util.List<java.lang.String>)) | ||
PolyDefault.java:56:31: compiler.err.prob.found.req: (compiler.misc.cant.apply.diamond.1: (compiler.misc.diamond: PolyDefault.Foo), (compiler.misc.incompatible.bounds: X, (compiler.misc.eq.bounds: java.lang.String), (compiler.misc.upper.bounds: java.lang.Number))) | ||
3 errors |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
/* | ||
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved. | ||
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. | ||
* | ||
* This code is free software; you can redistribute it and/or modify it | ||
* under the terms of the GNU General Public License version 2 only, as | ||
* published by the Free Software Foundation. Oracle designates this | ||
* particular file as subject to the "Classpath" exception as provided | ||
* by Oracle in the LICENSE file that accompanied this code. | ||
* | ||
* This code is distributed in the hope that it will be useful, but WITHOUT | ||
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or | ||
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License | ||
* version 2 for more details (a copy is included in the LICENSE file that | ||
* accompanied this code). | ||
* | ||
* You should have received a copy of the GNU General Public License version | ||
* 2 along with this work; if not, write to the Free Software Foundation, | ||
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. | ||
* | ||
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA | ||
* or visit www.oracle.com if you need additional information or have any | ||
* questions. | ||
*/ | ||
|
||
/* | ||
* @test | ||
* @bug 8210906 | ||
* @summary [lworld] default value creation should not impose raw types on users. | ||
* @run main PolyDefaultArgs | ||
*/ | ||
public primitive class PolyDefaultArgs<T> { | ||
|
||
static void m(PolyDefaultArgs<String> p) { } | ||
|
||
static <T>void check(PolyDefaultArgs.ref<T> rp) { | ||
if (rp != null) { | ||
throw new RuntimeException("reference projection must be null"); | ||
} | ||
} | ||
|
||
public static void main(String [] args) { | ||
// Ensure that the arguments to m() can be inferred | ||
m(new PolyDefaultArgs<>()); // OK | ||
m(PolyDefaultArgs<String>.default); // OK | ||
m(PolyDefaultArgs.default); // Now also OK | ||
|
||
// Ensure that the default of a reference type is null | ||
check(PolyDefaultArgs.ref<String>.default); | ||
check(PolyDefaultArgs.ref.default); | ||
} | ||
} |
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
UnknownTypeDefault.java:11:22: compiler.err.cant.resolve.location: kindname.class, Y, null, null, (compiler.misc.location: kindname.class, UnknownTypeDefault, null) | ||
UnknownTypeDefault.java:12:24: compiler.err.cant.resolve.location: kindname.class, y.Z, null, null, (compiler.misc.location: kindname.class, UnknownTypeDefault, null) | ||
UnknownTypeDefault.java:11:21: compiler.err.cant.resolve.location: kindname.class, Y, , , (compiler.misc.location: kindname.class, UnknownTypeDefault, null) | ||
UnknownTypeDefault.java:12:22: compiler.err.doesnt.exist: y | ||
2 errors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am surprised that ResolvedDefaultType extends ResolvedMemberType rather than ArgumentType directly. While the Inference behavior could be indirectly modelled it as if the class has a generic factory method named 'default':
this implementation detail could be pushed deep down and not manifest here and in elsewhere (setPolyKind())