8237067: Implementation of <anyType>.default - (lworld branch)

Jesper Steen Møller jesper at selskabet.org
Mon Feb 3 22:16:04 UTC 2020


Hi again

I’ve fixed the issues with the patch:

> On 3 Feb 2020, at 12.01, Srikanth <srikanth.adayapalam at oracle.com> wrote:
> 
> (1) The second version of the patch is missing the changes that were present in the first for compiler.properties and examples.not-yet.txt. These are not crucial to the functionality you are adding, but were still a good idea to include from a housekeeping/hygiene point of view.
> 

Missing ‘hg’ foo there. Fixed.


> (2) CheckBadSelector.java: the copyright has mutated to the "other" version. We have two versions of copyright header - (a) one for negative tests that would trigger a compiler diagnostic and (b) a fuller one for tests that are meant to be run.
> 
> The former would look like:
> 
> /*
> * @test /nodynamiccopyright/
> * @summary Check that syntax constraints still exist
> *
> * @compile/fail/ref=CheckBadSelector.out -XDrawDiagnostics CheckBadSelector.java
> */
> 
> While the latter is the usual full copyright.
> 
> I believe mixing up the styles would create issues in some testing environments.
> 
> See https://openjdk.java.net/groups/compiler/tests.html <https://openjdk.java.net/groups/compiler/tests.html> 10. Golden file tests <> 
> Also https://openjdk.java.net/jtreg/faq.html <https://openjdk.java.net/jtreg/faq.html> (search for nodynamic)
> 

Fixed.

> (3) Same copyright related comment for CheckFeatureGate.java
> 

Fixed.

> (4) On the feature gating changes to the parser: In general I find that it will result in fewer and cleaner errors if (a) the new constructs are tolerated in the parser but rejected in the attributor based on language level. or (b) if we check for feature level using statement
> like: checkSourceLevel(Feature.INLINE_TYPES); and continue to parse in 14 mode. (i.e parse T.default even in 13, BUT *after* having complained against it) As it is, many of the diagnostics in CheckFeatureGate.out don't add value.
> 

Fixed as you suggested. Now I see that you actually suggested as much in the first review.  I’ve split the test to test both gates (both for the use of “inline” and for “default”).

-Jesper

Patch here:

diff -r e0e612ef5bc9 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	Mon Feb 03 19:18:20 2020 +0530
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java	Mon Feb 03 23:11:25 2020 +0100
@@ -165,6 +165,7 @@
         allowLambda = Feature.LAMBDA.allowedInSource(source);
         allowDefaultMethods = Feature.DEFAULT_METHODS.allowedInSource(source);
         allowStaticInterfaceMethods = Feature.STATIC_INTERFACE_METHODS.allowedInSource(source);
+        allowInlineTypes = Feature.INLINE_TYPES.allowedInSource(source);
         allowReifiableTypesInInstanceof =
                 Feature.REIFIABLE_TYPES_INSTANCEOF.allowedInSource(source) &&
                 (!preview.isPreview(Feature.REIFIABLE_TYPES_INSTANCEOF) || preview.isEnabled());
@@ -199,6 +200,10 @@
      */
     boolean allowDefaultMethods;
 
+    /** Switch: allow inline types?
+     */
+    boolean allowInlineTypes;
+
     /** Switch: static interface methods enabled?
      */
     boolean allowStaticInterfaceMethods;
@@ -4019,6 +4024,10 @@
         if (tree.name == names._this || tree.name == names._super ||
                 tree.name == names._class || tree.name == names._default)
         {
+            if (tree.name == names._default && !allowInlineTypes) {
+                log.error(DiagnosticFlag.SOURCE_LEVEL, tree.pos(),
+                        Feature.INLINE_TYPES.error(sourceName));
+            }
             skind = KindSelector.TYP;
         } else {
             if (pkind().contains(KindSelector.PCK))
@@ -4040,10 +4049,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;
+                }
             }
         }
 
@@ -4201,12 +4215,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);
@@ -4216,6 +4225,10 @@
             case WILDCARD:
                 throw new AssertionError(tree);
             case TYPEVAR:
+                if (name == names._default) {
+                    // Be sure to return the default value before examining bounds
+                    return new VarSymbol(STATIC, names._default, site, site.tsym);
+                }
                 // Normally, site.getUpperBound() shouldn't be null.
                 // It should only happen during memberEnter/attribBase
                 // when determining the super type which *must* beac
@@ -4240,11 +4253,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 is 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 e0e612ef5bc9 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	Mon Feb 03 19:18:20 2020 +0530
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java	Mon Feb 03 23:11:25 2020 +0100
@@ -2405,7 +2405,13 @@
             result = items.makeStackItem(pt);
             return;
         } else if (tree.name == names._default) {
-            code.emitop2(defaultvalue, checkDimension(tree.pos(), tree.type), PoolWriter::putClass);
+            if (tree.type.asElement().isValue()) {
+                code.emitop2(defaultvalue, checkDimension(tree.pos(), tree.type), PoolWriter::putClass);
+            } else if (tree.type.isReference()) {
+                code.emitop0(aconst_null);
+            } else {
+                code.emitop0(zero(Code.typecode(tree.type)));
+            }
             result = items.makeStackItem(tree.type);
             return;
         }
diff -r e0e612ef5bc9 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	Mon Feb 03 19:18:20 2020 +0530
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java	Mon Feb 03 23:11:25 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) {
@@ -1326,7 +1342,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:
@@ -2228,7 +2244,7 @@
             selectExprMode();
             int pos = token.pos;
             nextToken();
-            accept(CLASS);
+            TokenKind selector = accept2(CLASS, DEFAULT);
             if (token.pos == endPosTable.errorEndPos) {
                 // error recovery
                 Name name;
@@ -2246,7 +2262,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 e0e612ef5bc9 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	Mon Feb 03 19:18:20 2020 +0530
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties	Mon Feb 03 23:11:25 2020 +0100
@@ -3565,9 +3565,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 e0e612ef5bc9 test/langtools/tools/javac/diags/examples.not-yet.txt
--- a/test/langtools/tools/javac/diags/examples.not-yet.txt	Mon Feb 03 19:18:20 2020 +0530
+++ b/test/langtools/tools/javac/diags/examples.not-yet.txt	Mon Feb 03 23:11:25 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 e0e612ef5bc9 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	Mon Feb 03 23:11:25 2020 +0100
@@ -0,0 +1,15 @@
+/*
+ * @test /nodynamiccopyright/
+ * @bug 8237067
+ * @summary [lworld] Check good and bad selectors on a type name
+ * @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 e0e612ef5bc9 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	Mon Feb 03 23:11:25 2020 +0100
@@ -0,0 +1,2 @@
+CheckBadSelector.java:13:21: compiler.err.expected2: class, default
+1 error
\ No newline at end of file
diff -r e0e612ef5bc9 test/langtools/tools/javac/valhalla/lworld-values/CheckFeatureGate1.java
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/langtools/tools/javac/valhalla/lworld-values/CheckFeatureGate1.java	Mon Feb 03 23:11:25 2020 +0100
@@ -0,0 +1,13 @@
+/*
+ * @test /nodynamiccopyright/
+ * @bug 8237067
+ * @summary Check that feature gated constructs are not allowed in previous versions.
+ * @compile/fail/ref=CheckFeatureGate1.out --release=13 -XDrawDiagnostics CheckFeatureGate1.java
+ */
+
+public class CheckFeatureGate1 {
+
+    static inline class Val {
+        public int v = 42;
+    }
+}
diff -r e0e612ef5bc9 test/langtools/tools/javac/valhalla/lworld-values/CheckFeatureGate1.out
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/langtools/tools/javac/valhalla/lworld-values/CheckFeatureGate1.out	Mon Feb 03 23:11:25 2020 +0100
@@ -0,0 +1,2 @@
+CheckFeatureGate1.java:10:12: compiler.err.feature.not.supported.in.source: (compiler.misc.feature.inline.type), 13, 14
+1 error
\ No newline at end of file
diff -r e0e612ef5bc9 test/langtools/tools/javac/valhalla/lworld-values/CheckFeatureGate2.java
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/langtools/tools/javac/valhalla/lworld-values/CheckFeatureGate2.java	Mon Feb 03 23:11:25 2020 +0100
@@ -0,0 +1,21 @@
+/*
+ * @test /nodynamiccopyright/
+ * @bug 8237067
+ * @summary Check that .default is not allowed in previous versions.
+ * @compile/fail/ref=CheckFeatureGate2.out --release=13 -XDrawDiagnostics CheckFeatureGate2.java
+ */
+
+public class CheckFeatureGate2 {
+
+    static <T> void checkDefaultT(Class<T> clazz) throws Exception {
+        while (T.default != null)
+            throw new AssertionError("Generic object should default to null");
+    }
+
+    public static void main(String[] args) throws Exception {
+        int a = int.default;
+        String s = String.default;
+        int[] ia = int[].default;
+        checkDefaultT(Object.class);
+    }
+}
diff -r e0e612ef5bc9 test/langtools/tools/javac/valhalla/lworld-values/CheckFeatureGate2.out
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/langtools/tools/javac/valhalla/lworld-values/CheckFeatureGate2.out	Mon Feb 03 23:11:25 2020 +0100
@@ -0,0 +1,2 @@
+CheckFeatureGate2.java:11:17: compiler.err.feature.not.supported.in.source: (compiler.misc.feature.inline.type), 13, 14
+1 error
\ No newline at end of file
diff -r e0e612ef5bc9 test/langtools/tools/javac/valhalla/lworld-values/CheckMakeDefault.java
--- a/test/langtools/tools/javac/valhalla/lworld-values/CheckMakeDefault.java	Mon Feb 03 19:18:20 2020 +0530
+++ b/test/langtools/tools/javac/valhalla/lworld-values/CheckMakeDefault.java	Mon Feb 03 23:11:25 2020 +0100
@@ -10,26 +10,33 @@
     inline abstract class A { int x = 10; } // Error
     static final class Sinner {
         static Sinner make() {
-            return Sinner.default; // NO: Sinner is not a value class.
+            return Sinner.default;
         }
     }
 
     inline static final class SinnerValue {
         static SinnerValue make() {
-            return SinnerValue.default; // OK.
+            return SinnerValue.default;
         } int x = 10;
     }
 
     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;
-    }
-
     static Point make(int x, int y) {
        Point p = Point.default;
        String s = String.default;
diff -r e0e612ef5bc9 test/langtools/tools/javac/valhalla/lworld-values/CheckMakeDefault.out
--- a/test/langtools/tools/javac/valhalla/lworld-values/CheckMakeDefault.out	Mon Feb 03 19:18:20 2020 +0530
+++ b/test/langtools/tools/javac/valhalla/lworld-values/CheckMakeDefault.out	Mon Feb 03 23:11:25 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 e0e612ef5bc9 test/langtools/tools/javac/valhalla/lworld-values/CheckValueFactoryWithReference.java
--- a/test/langtools/tools/javac/valhalla/lworld-values/CheckValueFactoryWithReference.java	Mon Feb 03 19:18:20 2020 +0530
+++ /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 e0e612ef5bc9 test/langtools/tools/javac/valhalla/lworld-values/CheckValueFactoryWithReference.out
--- a/test/langtools/tools/javac/valhalla/lworld-values/CheckValueFactoryWithReference.out	Mon Feb 03 19:18:20 2020 +0530
+++ /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 e0e612ef5bc9 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	Mon Feb 03 23:11:25 2020 +0100
@@ -0,0 +1,92 @@
+/*
+ * Copyright (c) 2020, 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 eve
+ *
+ * n 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 -Dtest.compiler.opts=-release=13 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 AssertionError("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 AssertionError("inline object fields should default to defaults");
+
+        while ((new Val()).v != 42)
+            throw new AssertionError("inline object fields should default to whatever constructor says");
+
+        // Simple reference default is just null
+        while (String.default != null)
+            throw new AssertionError("reference object should default to null");
+
+        // Reference default checked in method above
+        checkDefaultT(String.class);
+
+        // Array type - different syntactically
+        while (int[].default != null)
+            throw new AssertionError("arrays should default to null");
+
+        while (boolean.default != false)
+            throw new AssertionError("boolean should default to false");
+
+        while (char.default != '\0')
+            throw new AssertionError("char should default to '\0'");
+
+        while (int.default != 0)
+            throw new AssertionError("int should default to 0");
+
+        while (byte.default != 0)
+            throw new AssertionError("byte should default to 0");
+
+        while (short.default != 0)
+            throw new AssertionError("short should default to 0");
+
+        while (long.default != 0L)
+            throw new AssertionError("long should default to 0L");
+
+        while (float.default != 0.0F)
+            throw new AssertionError("float should default to 0.0F");
+
+        while (double.default != 0.0D)
+            throw new AssertionError("double should default to 0.0D");
+
+        // Note: The while loops above implicitly test that the SomeType.default does not
+        // return a constant expression.
+    }
+}




More information about the valhalla-dev mailing list