8237067: Implementation of <anyType>.default - (lworld branch)
Srikanth
srikanth.adayapalam at oracle.com
Wed Jan 29 05:16:58 UTC 2020
Hi Jesper,
Thanks for the patch. Here is a first batch of review comments. I will
review it once more and share any additional comments on Friday, so
please hold off on sending a patch that incorporates these comments, but
this batch is shared early so you can mull over it already.
(1) The change in LocaleProvidersRun.java seems unconnected to the fix.
I am able to back it out and the test still passes.
(2) CheckBadSelector.java: it is customary to include the bug number
with a @bug xxxxxxxx tag
(3) CheckMakeDefault.java: I apologize for having left stale identifiers
in this file. But now that we are changing it,
could you (a) remove the comments "// NO: Sinner is not a value class."
in line 13, (b) drop the "// OK" from line 19 (c) Drop the comment "//
Allowed in the new 'State of Valhalla'" in line 41 (it was allowed
already, the name badFactory was stale)
(4) DefaultNonInlines.java: (a) Copyright mentions 2018. (b) I am
curious - why are the statements using while instead if ?? (c) The SOP
is to throw new AssertionError rather than just Exception. (d) The
second "// Array type - differnt syntactically" is inappropriate. (d)
'\0' shows up highlit in red in Vim - (why ?) It accepts '\u0000'
without gripes.
(5) Gen.java: Can we factor out the common code result =
items.makeStackItem(tree.type); from the 3 paths ??
(6) JavacParser.java: 1345 - Just curious - what necessitates this change ??
Thanks
Srikanth
On 21/01/20 10:44 pm, Jesper Steen Møller wrote:
> Hi again
>
> Patch appended in plain text.
>
>> On 21 Jan 2020, at 18.01, Srikanth <srikanth.adayapalam at oracle.com> wrote:
>>
>> Hi Jesper,
>>
>> It looks like the patch attachment got stripped. Don't know why. Could you inline the fix and send it ?
>>
>> TIA
>> Srikanth (Mere mortal, neither Norse, nor God :) )
>>
>> On 21/01/20 10:28 pm, Jesper Steen Møller wrote:
>>> Hi people (or Norse gods ?)
>>>
>>> Here's an attempted patch for JDK-8237067. It works with tier1 and 2 tests against the lworld tip, so I think it’s ready for review.
>>>
>>> The purpose of the change is described in the ticket. As per the discussion in the week-end [1], default values are NOT introduced as constant expressions.
>>>
>>> -Jesper
>>>
>>> [1]: https://mail.openjdk.java.net/pipermail/valhalla-dev/2020-January/006713.html
>>>
>>>
>>>
>>>
>
> diff -r 7fca6c5e0d99 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 Fri Nov 22 15:19:11 2019 +0100
> +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java Sun Jan 19 13:49:09 2020 +0100
> @@ -3832,10 +3832,15 @@
> while (elt.hasTag(ARRAY))
> elt = ((ArrayType)elt).elemtype;
> if (elt.hasTag(TYPEVAR)) {
> - log.error(tree.pos(), Errors.TypeVarCantBeDeref);
> - result = tree.type = types.createErrorType(tree.name, site.tsym, site);
> - tree.sym = tree.type.tsym;
> - return ;
> + if (tree.name == names._default) {
> + result = check(tree, litType(BOT).constType(null),
> + KindSelector.VAL, resultInfo);
> + } else {
> + log.error(tree.pos(), Errors.TypeVarCantBeDeref);
> + result = tree.type = types.createErrorType(tree.name, site.tsym, site);
> + tree.sym = tree.type.tsym;
> + return;
> + }
> }
> }
>
> @@ -3993,12 +3998,7 @@
> // visitSelect that qualifier expression is a type.
> return syms.getClassField(site, types);
> } else if (name == names._default) {
> - if (!types.isValue(site)) {
> - log.error(pos, Errors.MakeDefaultWithNonvalue);
> - return syms.errSymbol;
> - } else {
> - return new VarSymbol(STATIC, names._default, site, site.tsym);
> - }
> + return new VarSymbol(STATIC, names._default, site, site.tsym);
> } else {
> // We are seeing a plain identifier as selector.
> Symbol sym = rs.findIdentInType(pos, env, site, name, resultInfo.pkind);
> @@ -4014,6 +4014,11 @@
> // done before attributing the type variables. In
> // other words, we are seeing this illegal program:
> // class B<T> extends A<T.foo> {}
> +
> + if (name == names._default) {
> + // Be sure to return the default value before examining bounds
> + return new VarSymbol(STATIC, names._default, site, site.tsym);
> + }
> Symbol sym = (site.getUpperBound() != null)
> ? selectSym(tree, location, capture(site.getUpperBound()), env, resultInfo)
> : null;
> @@ -4032,11 +4037,13 @@
> return types.createErrorType(name, site.tsym, site).tsym;
> default:
> // The qualifier expression is of a primitive type -- only
> - // .class is allowed for these.
> + // .class and .default are allowed for these.
> if (name == names._class) {
> // In this case, we have already made sure in Select that
> // qualifier expression is a type.
> return syms.getClassField(site, types);
> + } else if (name == names._default) {
> + return new VarSymbol(STATIC, names._default, site, site.tsym);
> } else {
> log.error(pos, Errors.CantDeref(site));
> return syms.errSymbol;
> diff -r 7fca6c5e0d99 src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java
> --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java Fri Nov 22 15:19:11 2019 +0100
> +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java Sun Jan 19 13:49:09 2020 +0100
> @@ -2401,8 +2401,16 @@
> result = items.makeStackItem(pt);
> return;
> } else if (tree.name == names._default) {
> - code.emitop2(defaultvalue, checkDimension(tree.pos(), tree.type), PoolWriter::putClass);
> - result = items.makeStackItem(tree.type);
> + if (tree.type.asElement().isValue()) {
> + code.emitop2(defaultvalue, checkDimension(tree.pos(), tree.type), PoolWriter::putClass);
> + result = items.makeStackItem(tree.type);
> + } else if (tree.type.isReference()) {
> + code.emitop0(aconst_null);
> + result = items.makeStackItem(tree.type);
> + } else {
> + code.emitop0(zero(Code.typecode(tree.type)));
> + result = items.makeStackItem(tree.type);
> + }
> return;
> }
>
> diff -r 7fca6c5e0d99 src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java
> --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java Fri Nov 22 15:19:11 2019 +0100
> +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java Sun Jan 19 13:49:09 2020 +0100
> @@ -477,6 +477,22 @@
> }
> }
>
> + /** If next input token matches one of the two given tokens, skip it, otherwise report
> + * an error.
> + *
> + * @return The actual token kind.
> + */
> + public TokenKind accept2(TokenKind tk1, TokenKind tk2) {
> + TokenKind returnValue = token.kind;
> + if (token.kind == tk1 || token.kind == tk2) {
> + nextToken();
> + } else {
> + setErrorEndPos(token.pos);
> + reportSyntaxError(S.prevToken().endPos, Errors.Expected2(tk1, tk2));
> + }
> + return returnValue;
> + }
> +
> /** Report an illegal start of expression/type error at given position.
> */
> JCExpression illegal(int pos) {
> @@ -1327,7 +1343,7 @@
> case DEFAULT:
> if (typeArgs != null) return illegal();
> selectExprMode();
> - t = F.at(pos).Select(t, names._default);
> + t = to(F.at(pos).Select(t, names._default));
> nextToken();
> break loop;
> case CLASS:
> @@ -2221,7 +2237,7 @@
> return t;
> }
>
> - /** BracketsSuffixExpr = "." CLASS
> + /** BracketsSuffixExpr = "." (CLASS | DEFAULT)
> * BracketsSuffixType =
> */
> JCExpression bracketsSuffix(JCExpression t) {
> @@ -2229,7 +2245,7 @@
> selectExprMode();
> int pos = token.pos;
> nextToken();
> - accept(CLASS);
> + TokenKind selector = accept2(CLASS, DEFAULT);
> if (token.pos == endPosTable.errorEndPos) {
> // error recovery
> Name name;
> @@ -2247,7 +2263,7 @@
> // taking care to handle some interior dimension(s) being annotated.
> if ((tag == TYPEARRAY && TreeInfo.containsTypeAnnotation(t)) || tag == ANNOTATED_TYPE)
> syntaxError(token.pos, Errors.NoAnnotationsOnDotClass);
> - t = toP(F.at(pos).Select(t, names._class));
> + t = toP(F.at(pos).Select(t, selector == CLASS ? names._class : names._default));
> }
> } else if ((mode & TYPE) != 0) {
> if (token.kind != COLCOL) {
> diff -r 7fca6c5e0d99 src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties
> --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties Fri Nov 22 15:19:11 2019 +0100
> +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties Sun Jan 19 13:49:09 2020 +0100
> @@ -3545,9 +3545,6 @@
> compiler.err.value.does.not.support=\
> Inline types do not support {0}
>
> -compiler.err.make.default.with.nonvalue=\
> - Default value creation requires an inline type
> -
> compiler.err.value.may.not.extend=\
> Inline type may not extend another inline type or class
>
> diff -r 7fca6c5e0d99 test/jdk/java/util/Locale/LocaleProvidersRun.java
> --- a/test/jdk/java/util/Locale/LocaleProvidersRun.java Fri Nov 22 15:19:11 2019 +0100
> +++ b/test/jdk/java/util/Locale/LocaleProvidersRun.java Sun Jan 19 13:49:09 2020 +0100
> @@ -172,6 +172,8 @@
> .addToolArg("-cp")
> .addToolArg(Utils.TEST_CLASS_PATH)
> .addToolArg("-Djava.locale.providers=" + prefList)
> + .addToolArg("-Duser.language=en")
> + .addToolArg("-Duser.country=US")
> .addToolArg("--add-exports=java.base/sun.util.locale.provider=ALL-UNNAMED")
> .addToolArg("LocaleProviders")
> .addToolArg(methodName)
> diff -r 7fca6c5e0d99 test/langtools/tools/javac/diags/examples.not-yet.txt
> --- a/test/langtools/tools/javac/diags/examples.not-yet.txt Fri Nov 22 15:19:11 2019 +0100
> +++ b/test/langtools/tools/javac/diags/examples.not-yet.txt Sun Jan 19 13:49:09 2020 +0100
> @@ -203,7 +203,6 @@
> # Value types
> compiler.err.cyclic.value.type.membership
> compiler.err.value.does.not.support
> -compiler.err.make.default.with.nonvalue
> compiler.err.value.may.not.extend
> compiler.warn.potential.null.pollution
> compiler.err.empty.value.not.yet
> diff -r 7fca6c5e0d99 test/langtools/tools/javac/valhalla/lworld-values/CheckBadSelector.java
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/test/langtools/tools/javac/valhalla/lworld-values/CheckBadSelector.java Sun Jan 19 13:49:09 2020 +0100
> @@ -0,0 +1,14 @@
> +/*
> + * @test /nodynamiccopyright/
> + * @summary Check that syntax constraints still exist
> + *
> + * @compile/fail/ref=CheckBadSelector.out -XDrawDiagnostics CheckBadSelector.java
> + */
> +inline final class Point {
> +
> + void badSelector() {
> + Class<?> c = int.class;
> + int i = int.default;
> + int x = int.whatever;
> + }
> +}
> diff -r 7fca6c5e0d99 test/langtools/tools/javac/valhalla/lworld-values/CheckBadSelector.out
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/test/langtools/tools/javac/valhalla/lworld-values/CheckBadSelector.out Sun Jan 19 13:49:09 2020 +0100
> @@ -0,0 +1,2 @@
> +CheckBadSelector.java:12:21: compiler.err.expected2: class, default
> +1 error
> diff -r 7fca6c5e0d99 test/langtools/tools/javac/valhalla/lworld-values/CheckMakeDefault.java
> --- a/test/langtools/tools/javac/valhalla/lworld-values/CheckMakeDefault.java Fri Nov 22 15:19:11 2019 +0100
> +++ b/test/langtools/tools/javac/valhalla/lworld-values/CheckMakeDefault.java Sun Jan 19 13:49:09 2020 +0100
> @@ -23,11 +23,22 @@
> final int x;
> final int y;
>
> + final int nonbool = boolean.default;
> + final boolean nonbyte = byte.default;
> + final boolean nonchar = char.default;
> + final boolean nonint = int.default;
> + final boolean nonshort = short.default;
> + final boolean nonlong = long.default;
> + final boolean nonfloat = float.default;
> + final boolean nondouble = double.default;
> + final int nonString = String.default;
> + final int nonbyteArray = byte[].default;
> +
> Point() {}
> Point (int x, int y) {}
>
> - Point badFactory(int x, int y) {
> - return Point.default;
> + Point goodFactory(int x, int y) {
> + return Point.default; // Allowed in the new 'State of Valhalla'
> }
>
> static Point make(int x, int y) {
> diff -r 7fca6c5e0d99 test/langtools/tools/javac/valhalla/lworld-values/CheckMakeDefault.out
> --- a/test/langtools/tools/javac/valhalla/lworld-values/CheckMakeDefault.out Fri Nov 22 15:19:11 2019 +0100
> +++ b/test/langtools/tools/javac/valhalla/lworld-values/CheckMakeDefault.out Sun Jan 19 13:49:09 2020 +0100
> @@ -1,5 +1,13 @@
> CheckMakeDefault.java:9:12: compiler.err.illegal.combination.of.modifiers: interface, inline
> CheckMakeDefault.java:10:21: compiler.err.illegal.combination.of.modifiers: abstract, inline
> -CheckMakeDefault.java:13:26: compiler.err.make.default.with.nonvalue
> -CheckMakeDefault.java:35:25: compiler.err.make.default.with.nonvalue
> -4 errors
> +CheckMakeDefault.java:26:32: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: boolean, int)
> +CheckMakeDefault.java:27:33: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: byte, boolean)
> +CheckMakeDefault.java:28:33: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: char, boolean)
> +CheckMakeDefault.java:29:31: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: int, boolean)
> +CheckMakeDefault.java:30:35: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: short, boolean)
> +CheckMakeDefault.java:31:33: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: long, boolean)
> +CheckMakeDefault.java:32:35: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: float, boolean)
> +CheckMakeDefault.java:33:37: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: double, boolean)
> +CheckMakeDefault.java:34:33: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: java.lang.String, int)
> +CheckMakeDefault.java:35:36: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: byte[], int)
> +12 errors
> \ No newline at end of file
> diff -r 7fca6c5e0d99 test/langtools/tools/javac/valhalla/lworld-values/CheckValueFactoryWithReference.java
> --- a/test/langtools/tools/javac/valhalla/lworld-values/CheckValueFactoryWithReference.java Fri Nov 22 15:19:11 2019 +0100
> +++ /dev/null Thu Jan 01 00:00:00 1970 +0000
> @@ -1,12 +0,0 @@
> -/*
> - * @test /nodynamiccopyright/
> - * @summary Do not allow mismatched instantiation syntax between value & reference types.
> - *
> - * @compile/fail/ref=CheckValueFactoryWithReference.out -XDrawDiagnostics CheckValueFactoryWithReference.java
> - */
> -
> -final class CheckValueFactoryWithReference {
> - final Object o = Object.default;
> - inline final class Point { int x = 10; }
> - Point p = new Point();
> -}
> diff -r 7fca6c5e0d99 test/langtools/tools/javac/valhalla/lworld-values/CheckValueFactoryWithReference.out
> --- a/test/langtools/tools/javac/valhalla/lworld-values/CheckValueFactoryWithReference.out Fri Nov 22 15:19:11 2019 +0100
> +++ /dev/null Thu Jan 01 00:00:00 1970 +0000
> @@ -1,2 +0,0 @@
> -CheckValueFactoryWithReference.java:9:28: compiler.err.make.default.with.nonvalue
> -1 error
> diff -r 7fca6c5e0d99 test/langtools/tools/javac/valhalla/lworld-values/DefaultNonInlines.java
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/test/langtools/tools/javac/valhalla/lworld-values/DefaultNonInlines.java Sun Jan 19 13:49:09 2020 +0100
> @@ -0,0 +1,88 @@
> +/*
> + * Copyright (c) 2018, 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 Check default values for non-inline types
> + * @bug 8237067
> + * @summary [lworld] Provide linguistic support to denote default values.
> + * @run main/othervm DefaultNonInlines
> + */
> +
> +public class DefaultNonInlines {
> +
> + static inline class Val {
> + public int v = 42;
> + }
> +
> + static <T> void checkDefaultT(Class<T> clazz) throws Exception {
> + while (T.default != null)
> + throw new Exception("Generic object should default to null");
> + }
> +
> + public static void main(String[] args) throws Exception {
> + // Default value is set by inline class constructor
> + while (Val.default.v != int.default)
> + throw new Exception("inline object fields should default to defaults");
> +
> + while ((new Val()).v != 42)
> + throw new Exception("inline object fields should default to whatever constructor says");
> +
> + // Simple reference default is just null
> + while (String.default != null)
> + throw new Exception("reference object should default to null");
> +
> + // Reference default checked in method above
> + checkDefaultT(String.class);
> +
> + // Array type - differnt syntactically
> + while (int[].default != null)
> + throw new Exception("arrays should default to null");
> +
> + // Array type - differnt syntactically
> + while (boolean.default != false)
> + throw new Exception("boolean should default to false");
> +
> + while (char.default != '\0')
> + throw new Exception("char should default to '\0'");
> +
> + while (int.default != 0)
> + throw new Exception("int should default to 0");
> +
> + while (byte.default != 0)
> + throw new Exception("byte should default to 0");
> +
> + while (short.default != 0)
> + throw new Exception("short should default to 0");
> +
> + while (long.default != 0L)
> + throw new Exception("long should default to 0L");
> +
> + while (float.default != 0.0F)
> + throw new Exception("float should default to 0.0F");
> +
> + while (double.default != 0.0D)
> + throw new Exception("double should default to 0.0D");
> + }
> +}
>
More information about the valhalla-dev
mailing list