8242478: compiler implementation for records (Second Preview)

Reviewed-by: mcimadamore, jlahoda, darcy

src/java.base/share/classes/java/io/ObjectStreamClass.java

@@ -1585,7 +1585,7 @@ public class ObjectStreamClass implements Serializable {
                                           .map(RecordComponent::getType)
                                           .toArray(Class<?>[]::new);
             try {
-                Constructor<?> ctr = cls.getConstructor(paramTypes);
+                Constructor<?> ctr = cls.getDeclaredConstructor(paramTypes);
                 ctr.setAccessible(true);
                 return MethodHandles.lookup().unreflectConstructor(ctr);
             } catch (IllegalAccessException | NoSuchMethodException e) {

src/jdk.compiler/share/classes/com/sun/tools/javac/code/Flags.java

@@ -371,7 +371,7 @@ public class Flags {
     public static final int
         AccessFlags           = PUBLIC | PROTECTED | PRIVATE,
         LocalClassFlags       = FINAL | ABSTRACT | STRICTFP | ENUM | SYNTHETIC,
-        LocalRecordFlags      = LocalClassFlags | STATIC,
+        StaticLocalFlags      = LocalClassFlags | STATIC | INTERFACE | ANNOTATION,
         MemberClassFlags      = LocalClassFlags | INTERFACE | AccessFlags,
         MemberRecordFlags     = MemberClassFlags | STATIC,
         ClassFlags            = LocalClassFlags | INTERFACE | PUBLIC | ANNOTATION,

src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java

@@ -1491,7 +1491,10 @@ public abstract class Symbol extends AnnoConstruct implements PoolConstant, Elem
 
         public RecordComponent getRecordComponent(JCVariableDecl var, boolean addIfMissing, List<JCAnnotation> annotations) {
             for (RecordComponent rc : recordComponents) {
-                if (rc.name == var.name) {
+                /* it could be that a record erroneously declares two record components with the same name, in that
+                 * case we need to use the position to disambiguate
+                 */
+                if (rc.name == var.name && var.pos == rc.pos) {
                     return rc;
                 }
             }
@@ -1753,7 +1756,13 @@ public abstract class Symbol extends AnnoConstruct implements PoolConstant, Elem
     public static class RecordComponent extends VarSymbol implements RecordComponentElement {
         public MethodSymbol accessor;
         public JCTree.JCMethodDecl accessorMeth;
+        /* the original annotations applied to the record component
+         */
         private final List<JCAnnotation> originalAnnos;
+        /* if the user happens to erroneously declare two components with the same name, we need a way to differentiate
+         * them, the code will fail anyway but we need to keep the information for better error recovery
+         */
+        private final int pos;
 
         /**
          * Construct a record component, given its flags, name, type and owner.
@@ -1761,10 +1770,15 @@ public abstract class Symbol extends AnnoConstruct implements PoolConstant, Elem
         public RecordComponent(JCVariableDecl fieldDecl, List<JCAnnotation> annotations) {
             super(PUBLIC, fieldDecl.sym.name, fieldDecl.sym.type, fieldDecl.sym.owner);
             this.originalAnnos = annotations;
+            this.pos = fieldDecl.pos;
         }
 
         public List<JCAnnotation> getOriginalAnnos() { return originalAnnos; }
 
+        public boolean isVarargs() {
+            return type.hasTag(TypeTag.ARRAY) && ((ArrayType)type).isVarargs();
+        }
+
         @Override @DefinedBy(Api.LANGUAGE_MODEL)
         @SuppressWarnings("preview")
         public ElementKind getKind() {

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java

@@ -274,7 +274,7 @@ public class Attr extends JCTree.Visitor {
         Symbol owner = env.info.scope.owner;
            // owner refers to the innermost variable, method or
            // initializer block declaration at this point.
-        return
+        boolean isAssignable =
             v.owner == owner
             ||
             ((owner.name == names.init ||    // i.e. we are in a constructor
@@ -284,6 +284,8 @@ public class Attr extends JCTree.Visitor {
              v.owner == owner.owner
              &&
              ((v.flags() & STATIC) != 0) == Resolve.isStatic(env));
+        boolean insideCompactConstructor = env.enclMethod != null && TreeInfo.isCompactConstructor(env.enclMethod);
+        return isAssignable & !insideCompactConstructor;
     }
 
     /** Check that variable can be assigned to.
@@ -1078,15 +1080,34 @@ public class Attr extends JCTree.Visitor {
                     } else {
                         // but if it is the canonical:
 
-                        // if user generated, then it shouldn't explicitly invoke any other constructor
+                        /* if user generated, then it shouldn't:
+                         *     - have an accessibility stricter than that of the record type
+                         *     - explicitly invoke any other constructor
+                         */
                         if ((tree.sym.flags_field & GENERATEDCONSTR) == 0) {
+                            if (Check.protection(m.flags()) > Check.protection(env.enclClass.sym.flags())) {
+                                log.error(tree,
+                                        (env.enclClass.sym.flags() & AccessFlags) == 0 ?
+                                            Errors.InvalidCanonicalConstructorInRecord(
+                                                Fragments.Canonical,
+                                                env.enclClass.sym.name,
+                                                Fragments.CanonicalMustNotHaveStrongerAccess("package")
+                                            ) :
+                                            Errors.InvalidCanonicalConstructorInRecord(
+                                                    Fragments.Canonical,
+                                                    env.enclClass.sym.name,
+                                                    Fragments.CanonicalMustNotHaveStrongerAccess(asFlagSet(env.enclClass.sym.flags() & AccessFlags))
+                                            )
+                                );
+                            }
+
                             JCMethodInvocation app = TreeInfo.firstConstructorCall(tree);
                             if (app != null &&
                                     (TreeInfo.name(app.meth) == names._this ||
                                             TreeInfo.name(app.meth) == names._super) &&
                                     checkFirstConstructorStat(app, tree, false)) {
                                 log.error(tree, Errors.InvalidCanonicalConstructorInRecord(
-                                        Fragments.Canonical, tree.sym.name,
+                                        Fragments.Canonical, env.enclClass.sym.name,
                                         Fragments.CanonicalMustNotContainExplicitConstructorInvocation));
                             }
                         }
@@ -1094,19 +1115,24 @@ public class Attr extends JCTree.Visitor {
                         // also we want to check that no type variables have been defined
                         if (!tree.typarams.isEmpty()) {
                             log.error(tree, Errors.InvalidCanonicalConstructorInRecord(
-                                    Fragments.Canonical, tree.sym.name, Fragments.CanonicalMustNotDeclareTypeVariables));
+                                    Fragments.Canonical, env.enclClass.sym.name, Fragments.CanonicalMustNotDeclareTypeVariables));
                         }
 
                         /* and now we need to check that the constructor's arguments are exactly the same as those of the
                          * record components
                          */
-                        List<Type> recordComponentTypes = TreeInfo.recordFields(env.enclClass).map(vd -> vd.sym.type);
+                        List<? extends RecordComponent> recordComponents = env.enclClass.sym.getRecordComponents();
+                        List<Type> recordFieldTypes = TreeInfo.recordFields(env.enclClass).map(vd -> vd.sym.type);
                         for (JCVariableDecl param: tree.params) {
-                            if (!types.isSameType(param.type, recordComponentTypes.head)) {
+                            boolean paramIsVarArgs = (param.sym.flags_field & VARARGS) != 0;
+                            if (!types.isSameType(param.type, recordFieldTypes.head) ||
+                                    (recordComponents.head.isVarargs() != paramIsVarArgs)) {
                                 log.error(param, Errors.InvalidCanonicalConstructorInRecord(
-                                        Fragments.Canonical, tree.sym.name, Fragments.TypeMustBeIdenticalToCorrespondingRecordComponentType));
+                                        Fragments.Canonical, env.enclClass.sym.name,
+                                        Fragments.TypeMustBeIdenticalToCorrespondingRecordComponentType));
                             }
-                            recordComponentTypes = recordComponentTypes.tail;
+                            recordComponents = recordComponents.tail;
+                            recordFieldTypes = recordFieldTypes.tail;
                         }
                     }
                 }
@@ -1180,11 +1206,6 @@ public class Attr extends JCTree.Visitor {
                             log.error(tree, Errors.InvalidCanonicalConstructorInRecord(
                                     Fragments.Canonical, env.enclClass.sym.name, Fragments.CanonicalWithNameMismatch));
                         }
-                        if (!tree.sym.isPublic()) {
-                            log.error(tree, Errors.InvalidCanonicalConstructorInRecord(
-                                    TreeInfo.isCompactConstructor(tree) ? Fragments.Compact : Fragments.Canonical,
-                                    env.enclClass.sym.name, Fragments.CanonicalConstructorMustBePublic));
-                        }
                         if (tree.sym.type.asMethodType().thrown != null && !tree.sym.type.asMethodType().thrown.isEmpty()) {
                             log.error(tree,
                                     Errors.InvalidCanonicalConstructorInRecord(

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java

@@ -1211,26 +1211,23 @@ public class Check {
             break;
         case TYP:
             if (sym.isLocal()) {
-                mask = (flags & RECORD) != 0 ? LocalRecordFlags : LocalClassFlags;
+                boolean implicitlyStatic = !sym.isAnonymous() &&
-                if ((flags & RECORD) != 0) {
+                        ((flags & RECORD) != 0 || (flags & ENUM) != 0 || (flags & INTERFACE) != 0);
-                    implicit = STATIC;
+                boolean staticOrImplicitlyStatic = (flags & STATIC) != 0 || implicitlyStatic;
+                mask = staticOrImplicitlyStatic && allowRecords ? StaticLocalFlags : LocalClassFlags;
+                implicit = implicitlyStatic ? STATIC : implicit;
+                if (staticOrImplicitlyStatic) {
                     if (sym.owner.kind == TYP) {
-                        log.error(pos, Errors.RecordDeclarationNotAllowedInInnerClasses);
+                        log.error(pos, Errors.StaticDeclarationNotAllowedInInnerClasses);
                     }
                 }
-                if ((sym.owner.flags_field & STATIC) == 0 &&
-                    (flags & ENUM) != 0) {
-                    log.error(pos, Errors.EnumsMustBeStatic);
-                }
             } else if (sym.owner.kind == TYP) {
                 mask = (flags & RECORD) != 0 ? MemberRecordFlags : MemberClassFlags;
                 if (sym.owner.owner.kind == PCK ||
                     (sym.owner.flags_field & STATIC) != 0)
                     mask |= STATIC;
-                else if ((flags & ENUM) != 0) {
+                else if ((flags & ENUM) != 0 || (flags & RECORD) != 0) {
-                    log.error(pos, Errors.EnumsMustBeStatic);
+                    log.error(pos, Errors.StaticDeclarationNotAllowedInInnerClasses);
-                } else if ((flags & RECORD) != 0) {
-                    log.error(pos, Errors.RecordDeclarationNotAllowedInInnerClasses);
                 }
                 // Nested interfaces and enums are always STATIC (Spec ???)
                 if ((flags & (INTERFACE | ENUM | RECORD)) != 0 ) implicit = STATIC;
@@ -1264,7 +1261,7 @@ public class Check {
             }
             else {
                 log.error(pos,
-                          Errors.ModNotAllowedHere(asFlagSet(illegal)));
+                        Errors.ModNotAllowedHere(asFlagSet(illegal)));
             }
         }
         else if ((sym.kind == TYP ||
@@ -2070,11 +2067,21 @@ public class Check {
      */
     void checkOverride(Env<AttrContext> env, JCMethodDecl tree, MethodSymbol m) {
         ClassSymbol origin = (ClassSymbol)m.owner;
-        if ((origin.flags() & ENUM) != 0 && names.finalize.equals(m.name))
+        if ((origin.flags() & ENUM) != 0 && names.finalize.equals(m.name)) {
             if (m.overrides(syms.enumFinalFinalize, origin, types, false)) {
                 log.error(tree.pos(), Errors.EnumNoFinalize);
                 return;
             }
+        }
+        if (allowRecords && origin.isRecord()) {
+            // let's find out if this is a user defined accessor in which case the @Override annotation is acceptable
+            Optional<? extends RecordComponent> recordComponent = origin.getRecordComponents().stream()
+                    .filter(rc -> rc.accessor == tree.sym && (rc.accessor.flags_field & GENERATED_MEMBER) == 0).findFirst();
+            if (recordComponent.isPresent()) {
+                return;
+            }
+        }
+
         for (Type t = origin.type; t.hasTag(CLASS);
              t = types.supertype(t)) {
             if (t != origin.type) {

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TypeEnter.java

@@ -57,7 +57,6 @@ import static com.sun.tools.javac.code.TypeTag.ERROR;
 import com.sun.tools.javac.resources.CompilerProperties.Fragments;
 
 import static com.sun.tools.javac.code.TypeTag.*;
-import static com.sun.tools.javac.code.TypeTag.BOT;
 import static com.sun.tools.javac.tree.JCTree.Tag.*;
 
 import com.sun.tools.javac.util.Dependencies.CompletionCause;
@@ -1025,7 +1024,6 @@ public class TypeEnter implements Completer {
             List<JCTree> defsToEnter = isRecord ?
                     tree.defs.diff(alreadyEntered) : tree.defs;
             memberEnter.memberEnter(defsToEnter, env);
-            List<JCTree> defsBeforeAddingNewMembers = tree.defs;
             if (isRecord) {
                 addRecordMembersIfNeeded(tree, env);
             }
@@ -1048,7 +1046,7 @@ public class TypeEnter implements Completer {
                         new TreeCopier<JCTree>(make.at(tree.pos)).copy(rec.getOriginalAnnos());
                 JCMethodDecl getter = make.at(tree.pos).
                         MethodDef(
-                                make.Modifiers(Flags.PUBLIC | Flags.GENERATED_MEMBER, originalAnnos),
+                                make.Modifiers(PUBLIC | Flags.GENERATED_MEMBER, originalAnnos),
                           tree.sym.name,
                           /* we need to special case for the case when the user declared the type as an ident
                            * if we don't do that then we can have issues if type annotations are applied to the
@@ -1123,7 +1121,7 @@ public class TypeEnter implements Completer {
         private void addRecordMembersIfNeeded(JCClassDecl tree, Env<AttrContext> env) {
             if (lookupMethod(tree.sym, names.toString, List.nil()) == null) {
                 JCMethodDecl toString = make.
-                    MethodDef(make.Modifiers(Flags.PUBLIC | Flags.RECORD | Flags.GENERATED_MEMBER),
+                    MethodDef(make.Modifiers(Flags.PUBLIC | Flags.RECORD | Flags.FINAL | Flags.GENERATED_MEMBER),
                               names.toString,
                               make.Type(syms.stringType),
                               List.nil(),
@@ -1223,9 +1221,6 @@ public class TypeEnter implements Completer {
                     (types.supertype(owner().type).tsym == syms.enumSym)) {
                     // constructors of true enums are private
                     flags = PRIVATE | GENERATEDCONSTR;
-                } else if ((owner().flags_field & RECORD) != 0) {
-                    // record constructors are public
-                    flags = PUBLIC | GENERATEDCONSTR;
                 } else {
                     flags = (owner().flags() & AccessFlags) | GENERATEDCONSTR;
                 }
@@ -1313,21 +1308,25 @@ public class TypeEnter implements Completer {
     }
 
     class RecordConstructorHelper extends BasicConstructorHelper {
-
+        boolean lastIsVarargs;
-        List<VarSymbol> recordFieldSymbols;
         List<JCVariableDecl> recordFieldDecls;
 
-        RecordConstructorHelper(TypeSymbol owner, List<JCVariableDecl> recordFieldDecls) {
+        RecordConstructorHelper(ClassSymbol owner, List<JCVariableDecl> recordFieldDecls) {
             super(owner);
             this.recordFieldDecls = recordFieldDecls;
-            this.recordFieldSymbols = recordFieldDecls.map(vd -> vd.sym);
+            this.lastIsVarargs = owner.getRecordComponents().stream().anyMatch(rc -> rc.isVarargs());
         }
 
         @Override
         public Type constructorType() {
             if (constructorType == null) {
-                List<Type> argtypes = recordFieldSymbols.map(v -> (v.flags_field & Flags.VARARGS) != 0 ? types.elemtype(v.type) : v.type);
+                ListBuffer<Type> argtypes = new ListBuffer<>();
-                constructorType = new MethodType(argtypes, syms.voidType, List.nil(), syms.methodClass);
+                JCVariableDecl lastField = recordFieldDecls.last();
+                for (JCVariableDecl field : recordFieldDecls) {
+                    argtypes.add(field == lastField && lastIsVarargs ? types.elemtype(field.sym.type) : field.sym.type);
+                }
+
+                constructorType = new MethodType(argtypes.toList(), syms.voidType, List.nil(), syms.methodClass);
             }
             return constructorType;
         }
@@ -1340,11 +1339,14 @@ public class TypeEnter implements Completer {
              */
             csym.flags_field |= Flags.COMPACT_RECORD_CONSTRUCTOR | GENERATEDCONSTR;
             ListBuffer<VarSymbol> params = new ListBuffer<>();
-            for (VarSymbol p : recordFieldSymbols) {
+            JCVariableDecl lastField = recordFieldDecls.last();
-                params.add(new VarSymbol(GENERATED_MEMBER | PARAMETER | RECORD | ((p.flags_field & Flags.VARARGS) != 0 ? Flags.VARARGS : 0), p.name, p.type, csym));
+            for (JCVariableDecl field : recordFieldDecls) {
+                params.add(new VarSymbol(
+                        GENERATED_MEMBER | PARAMETER | RECORD | (field == lastField && lastIsVarargs ? Flags.VARARGS : 0),
+                        field.name, field.sym.type, csym));
             }
             csym.params = params.toList();
-            csym.flags_field |= RECORD | PUBLIC;
+            csym.flags_field |= RECORD;
             return csym;
         }
 

src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java

@@ -2573,7 +2573,9 @@ public class JavacParser implements Parser {
             dc = token.comment(CommentStyle.JAVADOC);
             return List.of(classOrRecordOrInterfaceOrEnumDeclaration(modifiersOpt(), dc));
         case ENUM:
-            log.error(DiagnosticFlag.SYNTAX, token.pos, Errors.LocalEnum);
+            if (!allowRecords) {
+                log.error(DiagnosticFlag.SYNTAX, token.pos, Errors.LocalEnum);
+            }
             dc = token.comment(CommentStyle.JAVADOC);
             return List.of(classOrRecordOrInterfaceOrEnumDeclaration(modifiersOpt(), dc));
         case IDENTIFIER:
@@ -3895,7 +3897,10 @@ public class JavacParser implements Parser {
     }
 
     private EnumeratorEstimate estimateEnumeratorOrMember(Name enumName) {
-        if (token.kind == TokenKind.IDENTIFIER && token.name() != enumName) {
+        // if we are seeing a record declaration inside of an enum we want the same error message as expected for a
+        // let's say an interface declaration inside an enum
+        if (token.kind == TokenKind.IDENTIFIER && token.name() != enumName &&
+                (!allowRecords || !isRecordStart())) {
             Token next = S.token(1);
             switch (next.kind) {
                 case LPAREN: case LBRACE: case COMMA: case SEMI:
@@ -3904,6 +3909,11 @@ public class JavacParser implements Parser {
         }
         switch (token.kind) {
             case IDENTIFIER: case MONKEYS_AT: case LT:
+                if (token.kind == IDENTIFIER) {
+                    if (allowRecords && isRecordStart()) {
+                        return EnumeratorEstimate.MEMBER;
+                    }
+                }
                 return EnumeratorEstimate.UNKNOWN;
             default:
                 return EnumeratorEstimate.MEMBER;

src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties

@@ -824,9 +824,6 @@ compiler.err.modifier.not.allowed.here=\
 compiler.err.intf.not.allowed.here=\
     interface not allowed here
 
-compiler.err.enums.must.be.static=\
-    enum declarations allowed only in static contexts
-
 # 0: symbol, 1: symbol
 compiler.err.name.clash.same.erasure=\
     name clash: {0} and {1} have the same erasure
@@ -3483,9 +3480,6 @@ compiler.misc.canonical=\
 compiler.misc.compact=\
     compact
 
-compiler.misc.canonical.constructor.must.be.public=\
-    canonical constructor must be public
-
 # 0: fragment
 compiler.misc.throws.clause.not.allowed.for.canonical.constructor=\
     throws clause not allowed for {0} constructor
@@ -3500,11 +3494,15 @@ compiler.misc.canonical.must.not.declare.type.variables=\
     canonical constructor must not declare type variables
 
 compiler.misc.type.must.be.identical.to.corresponding.record.component.type=\
-    type must match that of the corresponding record component\
+    type and arity must match that of the corresponding record component\
 
 compiler.misc.canonical.must.not.contain.explicit.constructor.invocation=\
     canonical constructor must not contain explicit constructor invocation
 
+# 0: set of flag or string
+compiler.misc.canonical.must.not.have.stronger.access=\
+    attempting to assign stronger access privileges; was {0}
+
 # other
 compiler.err.record.cannot.declare.instance.fields=\
     field declaration must be static\n\
@@ -3520,8 +3518,8 @@ compiler.err.first.statement.must.be.call.to.another.constructor=\
 compiler.err.instance.initializer.not.allowed.in.records=\
     instance initializers not allowed in records
 
-compiler.err.record.declaration.not.allowed.in.inner.classes=\
+compiler.err.static.declaration.not.allowed.in.inner.classes=\
-    record declarations not allowed in inner classes
+    static declarations not allowed in inner classes
 
 compiler.err.record.header.expected=\
     record header expected

test/jdk/java/io/Serializable/records/ConstructorPermissionTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2019, 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
@@ -72,7 +72,6 @@ public class ConstructorPermissionTest {
                 try { new Socket("localhost", 8080); }
                 catch (IOException unexpected) { throw new AssertionError(unexpected); }
             }
-            this.x = x;
         }
     }
 
@@ -80,7 +79,6 @@ public class ConstructorPermissionTest {
         public R3 {
             if (firstDataSetCreated)
                 ProcessHandle.current();
-            this.args = args;
         }
     }
 

test/langtools/lib/combo/tools/javac/combo/CompilationTestCase.java

@@ -27,7 +27,9 @@ package tools.javac.combo;
 import java.io.File;
 import java.io.IOException;
 
+import java.util.Arrays;
 import java.util.function.Consumer;
+import java.util.stream.IntStream;
 
 import javax.tools.Diagnostic;
 
@@ -63,6 +65,26 @@ public class CompilationTestCase extends JavacTemplateTestBase {
         compileOptions = options.clone();
     }
 
+    protected void appendCompileOptions(String... additionalOptions) {
+        String[] moreOptions = additionalOptions.clone();
+        String[] newCompileOptions = Arrays.copyOf(compileOptions, compileOptions.length + additionalOptions.length);
+        IntStream.range(0, additionalOptions.length).forEach(i -> {
+            newCompileOptions[newCompileOptions.length - additionalOptions.length + i] = additionalOptions[i];
+        });
+        compileOptions = newCompileOptions;
+    }
+
+    protected void removeLastCompileOptions(int i) {
+        if (i < 0) {
+            throw new AssertionError("unexpected negative value " + i);
+        }
+        if (i >= compileOptions.length) {
+            compileOptions = new String[] {};
+        } else {
+            compileOptions = Arrays.copyOf(compileOptions, compileOptions.length - i);
+        }
+    }
+
     protected void setDefaultFilename(String name) {
         defaultFileName = name;
     }

test/langtools/lib/combo/tools/javac/combo/Diagnostics.java

@@ -76,7 +76,7 @@ public class Diagnostics implements javax.tools.DiagnosticListener<JavaFileObjec
     /** Do the diagnostics contain the specified warning key? */
     public boolean containsWarningKey(String key) {
         return diags.stream()
-                    .filter(d -> d.getKind() == Diagnostic.Kind.WARNING)
+                    .filter(d -> d.getKind() == Diagnostic.Kind.WARNING || d.getKind() == Diagnostic.Kind.MANDATORY_WARNING)
                     .anyMatch(d -> d.getCode().equals(key));
     }
 

test/langtools/lib/combo/tools/javac/combo/JavacTemplateTestBase.java

@@ -181,8 +181,9 @@ public abstract class JavacTemplateTestBase {
     protected void assertCompileSucceededWithWarning(String warning) {
         if (diags.errorsFound())
             fail("Expected successful compilation");
-        if (!diags.containsWarningKey(warning))
+        if (!diags.containsWarningKey(warning)) {
-            fail("Expected compilation warning " + warning);
+            fail(String.format("Expected compilation warning with %s, found %s", warning, diags.keys()));
+        }
     }
 
     /**

test/langtools/tools/javac/IllegalAnnotation.java

@@ -4,6 +4,7 @@
  * @summary javac crash when declare an annotation type illegally
  *
  * @compile/fail/ref=IllegalAnnotation.out -XDrawDiagnostics IllegalAnnotation.java
+ * @compile --enable-preview -source ${jdk.version} IllegalAnnotation.java
  */
 class IllegalAnnotation {
     {

test/langtools/tools/javac/IllegalAnnotation.out

@@ -1,2 +1,2 @@
-IllegalAnnotation.java:10:10: compiler.err.annotation.decl.not.allowed.here
+IllegalAnnotation.java:11:10: compiler.err.annotation.decl.not.allowed.here
 1 error

test/langtools/tools/javac/InterfaceInInner.out

@@ -1,2 +1,2 @@
-InterfaceInInner.java:12:13: compiler.err.intf.not.allowed.here
+InterfaceInInner.java:12:13: compiler.err.static.declaration.not.allowed.in.inner.classes
 1 error

test/langtools/tools/javac/LocalInterface.java

@@ -0,0 +1,13 @@
+/**
+ * @test  /nodynamiccopyright/
+ * @bug 8242478
+ * @summary test for local interfaces
+ * @compile/fail/ref=LocalInterface.out -XDrawDiagnostics LocalInterface.java
+ * @compile --enable-preview -source ${jdk.version} LocalInterface.java
+ */
+class LocalInterface {
+    void m() {
+        interface I {}
+    }
+}
+

test/langtools/tools/javac/LocalInterface.out

@@ -0,0 +1,2 @@
+LocalInterface.java:10:9: compiler.err.intf.not.allowed.here
+1 error

test/langtools/tools/javac/LocalRecord.java

@@ -0,0 +1,34 @@
+/*
+ * 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.
+ *
+ * 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 8242478
+ * @summary test local records
+ * @compile --enable-preview -source ${jdk.version} LocalRecord.java
+ */
+class LocalRecord {
+    void m() {
+        record R() {}
+    }
+}

test/langtools/tools/javac/diags/examples/CanonicalCantHaveStrongerAccessPrivileges.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ * 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
@@ -22,12 +22,13 @@
  */
 
 // key: compiler.err.invalid.canonical.constructor.in.record
-// key: compiler.misc.canonical.constructor.must.be.public
+// key: compiler.misc.canonical.must.not.have.stronger.access
 // key: compiler.note.preview.filename
 // key: compiler.note.preview.recompile
 // key: compiler.misc.canonical
 // options: --enable-preview -source ${jdk.version}
 
-record R(int i) {
+public record CanonicalCantHaveStrongerAccessPrivileges() {
-    R(int i) { this.i = i; }
+    private CanonicalCantHaveStrongerAccessPrivileges {}
 }
+

test/langtools/tools/javac/diags/examples/EnumsMustBeStatic.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2010, 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
@@ -21,7 +21,7 @@
  * questions.
  */
 
-// key: compiler.err.enums.must.be.static
+// key: compiler.err.static.declaration.not.allowed.in.inner.classes
 
 class EnumsMustBeStatic {
     class Nested {

test/langtools/tools/javac/diags/examples/RecordsNotAllowedInInnerClasses.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2019, 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
@@ -21,7 +21,7 @@
  * questions.
  */
 
-// key: compiler.err.record.declaration.not.allowed.in.inner.classes
+// key: compiler.err.static.declaration.not.allowed.in.inner.classes
 // key: compiler.note.preview.filename
 // key: compiler.note.preview.recompile
 // options: --enable-preview -source ${jdk.version}

test/langtools/tools/javac/enum/LocalEnum.java

@@ -4,6 +4,7 @@
  * @summary javac fails to reject local enums
  * @author gafter
  * @compile/fail/ref=LocalEnum.out -XDrawDiagnostics  LocalEnum.java
+ * @compile --enable-preview -source ${jdk.version}  LocalEnum.java
  */
 
 public class LocalEnum {

test/langtools/tools/javac/enum/LocalEnum.out

@@ -1,2 +1,2 @@
-LocalEnum.java:11:9: compiler.err.local.enum
+LocalEnum.java:12:9: compiler.err.local.enum
 1 error

test/langtools/tools/javac/enum/NestedEnum.out

@@ -1,2 +1,2 @@
-NestedEnum.java:12:9: compiler.err.enums.must.be.static
+NestedEnum.java:12:9: compiler.err.static.declaration.not.allowed.in.inner.classes
 1 error

test/langtools/tools/javac/enum/T5081785.out

@@ -1,5 +1,5 @@
-T5081785.java:29:9: compiler.err.enums.must.be.static
+T5081785.java:29:9: compiler.err.static.declaration.not.allowed.in.inner.classes
-T5081785.java:12:13: compiler.err.enums.must.be.static
+T5081785.java:12:13: compiler.err.static.declaration.not.allowed.in.inner.classes
-T5081785.java:19:27: compiler.err.enums.must.be.static
+T5081785.java:19:27: compiler.err.static.declaration.not.allowed.in.inner.classes
-T5081785.java:24:31: compiler.err.enums.must.be.static
+T5081785.java:24:31: compiler.err.static.declaration.not.allowed.in.inner.classes
 4 errors

test/langtools/tools/javac/processing/model/element/CheckingTypeAnnotationsOnRecords.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2019, 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
@@ -88,7 +88,6 @@ public class CheckingTypeAnnotationsOnRecords extends TestRunner {
     }
 
     public static void main(String... args) throws Exception {
-        System.out.println(System.getProperties());
         new CheckingTypeAnnotationsOnRecords().runTests();
     }
 

test/langtools/tools/javac/processing/model/element/TestRecordDesugar.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2010, 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
@@ -258,7 +258,7 @@ public class TestRecordDesugar extends JavacTestingAbstractProcessor {
                                    name = "modulus",
                                    type = TypeKind.DOUBLE),
 
-                      @ElementInfo(modifiers = {Modifier.PUBLIC},
+                      @ElementInfo(modifiers = {Modifier.PUBLIC, Modifier.FINAL},
                                    name = "toString",
                                    type = TypeKind.DECLARED,
                                    origin = Elements.Origin.EXPLICIT),
@@ -284,7 +284,7 @@ public class TestRecordDesugar extends JavacTestingAbstractProcessor {
                                    origin = Elements.Origin.EXPLICIT),
 
                       @ElementInfo(kind = ElementKind.CONSTRUCTOR,
-                                   modifiers = {Modifier.PUBLIC},
+                                   modifiers = {},
                                    name = "<init>",
                                    type = TypeKind.VOID,
                                    origin = Elements.Origin.MANDATED),
@@ -329,7 +329,7 @@ public class TestRecordDesugar extends JavacTestingAbstractProcessor {
                                    name = "modulus",
                                    type = TypeKind.DOUBLE),
 
-                      @ElementInfo(modifiers = {Modifier.PUBLIC},
+                      @ElementInfo(modifiers = {Modifier.PUBLIC, Modifier.FINAL},
                                    name = "toString",
                                    type = TypeKind.DECLARED,
                                    origin = Elements.Origin.EXPLICIT),

test/langtools/tools/javac/records/LocalStaticDeclarations.java

@@ -0,0 +1,229 @@
+/*
+ * 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.
+ *
+ * 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 8242293
+ * @summary allow for local interfaces and enums plus nested records, interfaces and enums
+ * @library /tools/javac/lib
+ * @modules jdk.compiler/com.sun.tools.javac.api
+ *          jdk.compiler/com.sun.tools.javac.file
+ *          jdk.compiler/com.sun.tools.javac.util
+ * @build combo.ComboTestHelper
+ * @compile --enable-preview -source ${jdk.version} LocalStaticDeclarations.java
+ * @run main/othervm --enable-preview LocalStaticDeclarations
+ */
+
+import javax.lang.model.element.Element;
+import javax.tools.Diagnostic;
+import javax.tools.JavaFileObject;
+
+import com.sun.tools.javac.util.Assert;
+
+import com.sun.tools.javac.api.ClientCodeWrapper;
+import com.sun.tools.javac.util.JCDiagnostic;
+import com.sun.tools.javac.util.List;
+import combo.ComboInstance;
+import combo.ComboParameter;
+import combo.ComboTask;
+import combo.ComboTask.Result;
+import combo.ComboTestHelper;
+
+public class LocalStaticDeclarations extends ComboInstance<LocalStaticDeclarations> {
+
+    static final String sourceTemplate =
+            """
+            import java.lang.annotation.*;
+            class Test {
+                int INSTANCE_FIELD = 0;
+                static int STATIC_FIELD = 0;
+                // instance initializer
+                { int LOCAL_VARIABLE = 0;
+                    #{CONTAINER}
+                }
+                Test() {
+                    #{CONTAINER}
+                }
+                void m() {
+                    int LOCAL_VARIABLE = 0;
+                    #{CONTAINER}
+                }
+                static void foo() {
+                    int LOCAL_VARIABLE = 0;
+                    #{CONTAINER}
+                }
+            }
+            """;
+
+    enum Container implements ComboParameter {
+        NO_CONTAINER("#{STATIC_LOCAL}"),
+        INTERFACE("interface CI { #{STATIC_LOCAL} }"),
+        ANNOTATION("@interface CA { #{STATIC_LOCAL} }"),
+        ANONYMOUS(
+                """
+                    new Object() {
+                        // instance initializer
+                        {
+                            #{STATIC_LOCAL}
+                        }
+
+                        void m() {
+                            #{STATIC_LOCAL}
+                        }
+                    };
+                """
+        ),
+        RECORD("record CR() { #{STATIC_LOCAL} }"),
+        CLASS("class CC { #{STATIC_LOCAL} }"),
+        ENUM("enum CE { #{STATIC_LOCAL} }"),
+        LAMBDA("Runnable run = () -> { #{STATIC_LOCAL} };");
+
+        String container;
+
+        Container(String container) {
+            this.container = container;
+        }
+
+        public String expand(String optParameter) {
+            return container;
+        }
+    }
+
+    enum StaticLocalDecl implements ComboParameter {
+        ENUM("enum E { E1; #{MEMBER} }"),
+        RECORD("record R() { #{MEMBER} }"),
+        ANNOTATION("@interface A { #{MEMBER} }"),
+        INTERFACE("interface I { #{MEMBER} }");
+
+        String localDecl;
+
+        StaticLocalDecl(String localDecl) {
+            this.localDecl = localDecl;
+        }
+
+        public String expand(String optParameter) {
+            return localDecl;
+        }
+    }
+
+    enum Member implements ComboParameter {
+        NONE(""),
+        METHOD("int foo() { return #{EXPR}; }"),
+        DEFAULT_METHOD("default int foo() { return #{EXPR}; }");
+
+        String member;
+
+        Member(String member) {
+            this.member = member;
+        }
+
+        public String expand(String optParameter) {
+            return member;
+        }
+    }
+
+    enum Expression implements ComboParameter {
+         LITERAL("1"),
+         STATIC_FIELD("STATIC_FIELD"),
+         LOCAL_VARIABLE("LOCAL_VARIABLE"),
+         INSTANCE_FIELD("INSTANCE_FIELD");
+
+         String expr;
+
+         Expression(String expr) {
+             this.expr = expr;
+         }
+
+        public String expand(String optParameter) {
+            return expr;
+        }
+    }
+
+    public static void main(String... args) throws Exception {
+        new combo.ComboTestHelper<LocalStaticDeclarations>()
+                .withFilter(LocalStaticDeclarations::notTriviallyIncorrect)
+                .withDimension("CONTAINER", (x, t) -> { x.container = t; }, Container.values())
+                .withDimension("STATIC_LOCAL", (x, t) -> { x.decl = t; }, StaticLocalDecl.values())
+                .withDimension("MEMBER", (x, t) -> { x.member = t; }, Member.values())
+                .withDimension("EXPR", (x, expr) -> x.expr = expr, Expression.values())
+                .run(LocalStaticDeclarations::new);
+    }
+
+    Container container;
+    StaticLocalDecl decl;
+    Member member;
+    Expression expr;
+
+    @Override
+    public void doWork() throws Throwable {
+        newCompilationTask()
+                .withOptions(new String[]{"--enable-preview", "-source", Integer.toString(Runtime.version().feature())})
+                .withSourceFromTemplate("Test", sourceTemplate)
+                .generate(this::check);
+    }
+
+    boolean notTriviallyIncorrect() {
+        return decl == StaticLocalDecl.INTERFACE && (member == Member.DEFAULT_METHOD || member == Member.NONE) ||
+               decl != StaticLocalDecl.INTERFACE && (member == Member.METHOD || member == Member.NONE) &&
+               ((decl != StaticLocalDecl.ANNOTATION) ||
+               (decl == StaticLocalDecl.ANNOTATION && member == Member.NONE));
+    }
+
+    void check(ComboTask.Result<Iterable<? extends JavaFileObject>> result) {
+        if (shouldFail()) {
+            Assert.check(result.hasErrors(), result.compilationInfo());
+            if (!expectedDiagFound(result)) {
+                fail("test failing with unexpected error message\n" + result.compilationInfo());
+            }
+        } else {
+            Assert.check(!result.hasErrors(), result.compilationInfo());
+        }
+    }
+
+    boolean shouldFail() {
+        return ((container != Container.NO_CONTAINER &&
+                container != Container.LAMBDA &&
+                container != Container.ANONYMOUS)) ||
+                (member != Member.NONE && !acceptableExpr());
+    }
+
+    boolean acceptableExpr() {
+        return (expr == Expression.LITERAL || expr == Expression.STATIC_FIELD);
+    }
+
+    boolean expectedDiagFound(ComboTask.Result<Iterable<? extends JavaFileObject>> result) {
+        if ((container == Container.NO_CONTAINER ||
+                container == Container.LAMBDA ||
+                container == Container.ANONYMOUS) &&
+                !acceptableExpr()) {
+            return result.containsKey("compiler.err.non-static.cant.be.ref");
+        } else if (container == Container.ENUM) {
+            if (decl == StaticLocalDecl.ANNOTATION) {
+                return result.containsKey("compiler.err.expected");
+            } else {
+                return result.containsKey("compiler.err.enum.constant.expected" );
+            }
+        }
+        return result.containsKey("compiler.err.static.declaration.not.allowed.in.inner.classes" );
+    }
+}

test/langtools/tools/javac/records/RecordCompilationTests.java

@@ -36,7 +36,8 @@
  *      jdk.jdeps/com.sun.tools.classfile
  * @build JavacTestingAbstractProcessor
  * @compile --enable-preview -source ${jdk.version} RecordCompilationTests.java
- * @run testng/othervm --enable-preview RecordCompilationTests
+ * @run testng/othervm -DuseAP=false --enable-preview RecordCompilationTests
+ * @run testng/othervm -DuseAP=true --enable-preview RecordCompilationTests
  */
 
 import java.io.File;
@@ -55,6 +56,7 @@ import java.util.stream.Stream;
 
 import com.sun.tools.javac.util.Assert;
 
+import javax.annotation.processing.AbstractProcessor;
 import javax.annotation.processing.RoundEnvironment;
 import javax.annotation.processing.SupportedAnnotationTypes;
 
@@ -70,13 +72,17 @@ import javax.lang.model.element.VariableElement;
 import javax.lang.model.type.ArrayType;
 import javax.lang.model.type.TypeMirror;
 
+import com.sun.tools.classfile.AccessFlags;
 import com.sun.tools.classfile.Annotation;
 import com.sun.tools.classfile.Attribute;
 import com.sun.tools.classfile.Attributes;
 import com.sun.tools.classfile.ClassFile;
+import com.sun.tools.classfile.Code_attribute;
 import com.sun.tools.classfile.ConstantPool;
+import com.sun.tools.classfile.ConstantPool.CONSTANT_Fieldref_info;
 import com.sun.tools.classfile.ConstantPool.CPInfo;
 import com.sun.tools.classfile.Field;
+import com.sun.tools.classfile.Instruction;
 import com.sun.tools.classfile.Method;
 import com.sun.tools.classfile.Record_attribute;
 import com.sun.tools.classfile.Record_attribute.ComponentInfo;
@@ -99,20 +105,51 @@ import tools.javac.combo.CompilationTestCase;
 import static java.lang.annotation.ElementType.*;
 import static org.testng.Assert.assertEquals;
 
+/** Records are the first feature which sports automatic injection of (declarative and type) annotations : from a
+ *  given record component to one or more record members, if applicable.
+ *  This implies that the record's implementation can be stressed with the presence of annotation processors. Which is
+ *  something the implementator could easily skip. For this reason this test is executed twice, once without the
+ *  presence of any annotation processor and one with a simple annotation processor (which does not annotation processing
+ *  at all) just to force at least a round of annotation processing.
+ *
+ *  Tests needing special compilation options need to store current options, set its customs options by invoking method
+ *  `setCompileOptions` and then reset the previous compilation options for other tests. To see an example of this check
+ *  method: testAnnos()
+ */
+
 @Test
 public class RecordCompilationTests extends CompilationTestCase {
-
     // @@@ When records become a permanent feature, we don't need these any more
-    private static String[] PREVIEW_OPTIONS = {"--enable-preview", "-source",
+    private static String[] PREVIEW_OPTIONS = {
-                                               Integer.toString(Runtime.version().feature())};
+            "--enable-preview",
+            "-source", Integer.toString(Runtime.version().feature())
+    };
+
+    private static String[] PREVIEW_OPTIONS_WITH_AP = {
+            "--enable-preview",
+            "-source", Integer.toString(Runtime.version().feature()),
+            "-processor", SimplestAP.class.getName()
+    };
 
     private static final List<String> BAD_COMPONENT_NAMES = List.of(
             "clone", "finalize", "getClass", "hashCode",
             "notify", "notifyAll", "toString", "wait");
 
-    {
+    /* simplest annotation processor just to force a round of annotation processing for all tests
+     */
+    @SupportedAnnotationTypes("*")
+    public static class SimplestAP extends AbstractProcessor {
+        @Override
+        public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment roundEnv) {
+            return true;
+        }
+    }
+
+    public RecordCompilationTests() {
+        boolean useAP = System.getProperty("useAP") == null ? false : System.getProperty("useAP").equals("true");
         setDefaultFilename("R.java");
-        setCompileOptions(PREVIEW_OPTIONS);
+        setCompileOptions(useAP ? PREVIEW_OPTIONS_WITH_AP : PREVIEW_OPTIONS);
+        System.out.println(useAP ? "running all tests using an annotation processor" : "running all tests without annotation processor");
     }
 
     public void testMalformedDeclarations() {
@@ -130,7 +167,8 @@ public class RecordCompilationTests extends CompilationTestCase {
         assertFail("compiler.err.already.defined", "record R(int x, int x) {}");
         for (String s : List.of("var", "record"))
             assertFail("compiler.err.restricted.type.not.allowed.here", "record R(# x) { }", s);
-        for (String s : List.of("public", "private", "volatile", "final"))
+        for (String s : List.of("public", "protected", "private", "static", "final", "transient", "volatile",
+                "abstract", "synchronized", "native", "strictfp")) // missing: sealed and non-sealed
             assertFail("compiler.err.record.cant.declare.field.modifiers", "record R(# String foo) { }", s);
         assertFail("compiler.err.varargs.must.be.last", "record R(int... x, int... y) {}");
         assertFail("compiler.err.instance.initializer.not.allowed.in.records", "record R(int i) { {} }");
@@ -210,7 +248,14 @@ public class RecordCompilationTests extends CompilationTestCase {
 
     public void testNoExtendRecord() {
         assertFail("compiler.err.invalid.supertype.record",
-                   "class R extends Record { public String toString() { return null; } public int hashCode() { return 0; } public boolean equals(Object o) { return false; } } }");
+                   """
+                   class R extends Record {
+                       public String toString() { return null; }
+                       public int hashCode() { return 0; }
+                       public boolean equals(Object o) { return false; }
+                   }
+                   """
+        );
     }
 
     public void testFieldDeclarations() {
@@ -246,6 +291,14 @@ public class RecordCompilationTests extends CompilationTestCase {
                 "    public int x() { return x; };" +
                 "}");
 
+        assertOK("public record R(int... x) {\n" +
+                "    public int[] x() { return x; };" +
+                "}");
+
+        assertOK("public record R(int x) {\n" +
+                "    public final int x() { return 0; };" +
+                "}");
+
         assertOK("public record R(int x) {\n" +
                 "    public final int x() { return 0; };" +
                 "}");
@@ -293,8 +346,7 @@ public class RecordCompilationTests extends CompilationTestCase {
         for (String goodCtor : List.of(
                 "public R(int x) { this(x, 0); }",
                 "public R(int x, int y) { this.x = x; this.y = y; }",
-                "public R { }",
+                "public R { }"))
-                "public R { this.x = 0; }"))
             assertOK("record R(int x, int y) { # }", goodCtor);
 
         assertOK("import java.util.*; record R(String x, String y) {  public R { Objects.requireNonNull(x); Objects.requireNonNull(y); } }");
@@ -308,12 +360,6 @@ public class RecordCompilationTests extends CompilationTestCase {
                                 "public R(int _x, int _y) { this.x = _x; this.y = _y; }"))
             assertFail("compiler.err.invalid.canonical.constructor.in.record", "record R(int x, int y) { # }", s);
 
-        // canonical ctor must be public
-        for (String s : List.of("", "protected", "private"))
-            assertFail("compiler.err.invalid.canonical.constructor.in.record", "record R(int x, int y) { # }",
-                       "# R(int x, int y) { this.x = x; this.y = y; }",
-                       s);
-
         // ctor args must match types
         assertFail("compiler.err.invalid.canonical.constructor.in.record",
                 "import java.util.*;\n" +
@@ -434,13 +480,7 @@ public class RecordCompilationTests extends CompilationTestCase {
         assertFail("compiler.err.already.defined", template);
     }
 
-    public void testLocalRecords() {
+    public void testStaticLocalTypes() {
-        assertOK("class R { \n" +
-                "    void m() { \n" +
-                "        record RR(int x) { };\n" +
-                "    }\n" +
-                "}");
-
         // local records can also be final
         assertOK("class R { \n" +
                 "    void m() { \n" +
@@ -488,49 +528,26 @@ public class RecordCompilationTests extends CompilationTestCase {
                 "        record RR(int x) { public int x() { return z; }};\n" +
                 "    }\n" +
                 "}");
-        // can be contained inside a lambda
-        assertOK("""
-                class Outer {
-                    Runnable run = () -> {
-                        record TestRecord(int i) {}
-                    };
-                }
-                """);
-
         // Can't self-shadow
         assertFail("compiler.err.already.defined",
-                   "class R { \n" +
+                """
-                   "    void m() { \n" +
+                class R {
-                   "        record R(int x) { };\n" +
+                    void m() {
-                   "    }\n" +
+                        record R(int x) { };
-                   "}");
+                    }
-    }
+                }
-
+                """
-    public void testCompactDADU() {
+        );
-        // trivial cases
+        // can't be explicitly static
-        assertOK("record R() { public R {} }");
+        assertFail("compiler.err.illegal.start.of.expr",
-        assertOK("record R(int x) { public R {} }");
+                """
-
+                class R {
-        // throwing an unchecked exception
+                    void m() {
-        assertOK("record R(int x) { public R { if (x < 0) { this.x = x; throw new RuntimeException(); }} }");
+                        static record RR(int x) { };
-
+                    }
-        assertOK("record R(int x) { public R { if (x < 0) { this.x = x; throw new RuntimeException(); }} }");
+                }
-
+                """
-        // x is not DA nor DU in the body of the constructor hence error
+        );
-        assertFail("compiler.err.var.might.not.have.been.initialized", "record R(int x) { # }",
-                "public R { if (x < 0) { this.x = -x; } }");
-
-        // if static fields are not DA then error
-        assertFail("compiler.err.var.might.not.have.been.initialized",
-                "record R() { # }", "static final String x;");
-
-        // ditto
-        assertFail("compiler.err.var.might.not.have.been.initialized",
-                "record R() { # }", "static final String x; public R {}");
-
-        // ditto
-        assertFail("compiler.err.var.might.not.have.been.initialized",
-                "record R(int i) { # }", "static final String x; public R {}");
     }
 
     public void testReturnInCanonical_Compact() {
@@ -561,13 +578,16 @@ public class RecordCompilationTests extends CompilationTestCase {
     }
 
     public void testRecordsInsideInner() {
-        assertFail("compiler.err.record.declaration.not.allowed.in.inner.classes",
+        assertFail("compiler.err.static.declaration.not.allowed.in.inner.classes",
-                "class Outer {\n" +
+                """
-                "    class Inner {\n" +
+                class Outer {
-                "        record R(int a) {}\n" +
+                    class Inner {
-                "    }\n" +
+                        record R(int a) {}
-                "}");
+                    }
-        assertFail("compiler.err.record.declaration.not.allowed.in.inner.classes",
+                }
+                """
+        );
+        assertFail("compiler.err.static.declaration.not.allowed.in.inner.classes",
                 """
                 class Outer {
                     public void test() {
@@ -577,7 +597,7 @@ public class RecordCompilationTests extends CompilationTestCase {
                     }
                 }
                 """);
-        assertFail("compiler.err.record.declaration.not.allowed.in.inner.classes",
+        assertFail("compiler.err.static.declaration.not.allowed.in.inner.classes",
                 """
                 class Outer {
                     Runnable run = new Runnable() {
@@ -586,7 +606,7 @@ public class RecordCompilationTests extends CompilationTestCase {
                     };
                 }
                 """);
-        assertFail("compiler.err.record.declaration.not.allowed.in.inner.classes",
+        assertFail("compiler.err.static.declaration.not.allowed.in.inner.classes",
                 """
                 class Outer {
                     void m() {
@@ -646,6 +666,47 @@ public class RecordCompilationTests extends CompilationTestCase {
         Assert.check(numberOfFieldRefs == 1);
     }
 
+    /*  check that fields are initialized in a canonical constructor in the same declaration order as the corresponding
+     *  record component
+     */
+    public void testCheckInitializationOrderInCompactConstructor() throws Exception {
+        int putField1 = -1;
+        int putField2 = -1;
+        File dir = assertOK(true, "record R(int i, String s) { R {} }");
+        for (final File fileEntry : dir.listFiles()) {
+            if (fileEntry.getName().equals("R.class")) {
+                ClassFile classFile = ClassFile.read(fileEntry);
+                for (Method method : classFile.methods) {
+                    if (method.getName(classFile.constant_pool).equals("<init>")) {
+                        Code_attribute code_attribute = (Code_attribute) method.attributes.get("Code");
+                        for (Instruction instruction : code_attribute.getInstructions()) {
+                            if (instruction.getMnemonic().equals("putfield")) {
+                                if (putField1 != -1 && putField2 != -1) {
+                                    throw new AssertionError("was expecting only two putfield instructions in this method");
+                                }
+                                if (putField1 == -1) {
+                                    putField1 = instruction.getShort(1);
+                                } else if (putField2 == -1) {
+                                    putField2 = instruction.getShort(1);
+                                }
+                            }
+                        }
+                        // now we need to check that we are assigning to `i` first and to `s` afterwards
+                        CONSTANT_Fieldref_info fieldref_info1 = (CONSTANT_Fieldref_info)classFile.constant_pool.get(putField1);
+                        if (!fieldref_info1.getNameAndTypeInfo().getName().equals("i")) {
+                            throw new AssertionError("was expecting variable name 'i'");
+                        }
+
+                        CONSTANT_Fieldref_info fieldref_info2 = (CONSTANT_Fieldref_info)classFile.constant_pool.get(putField2);
+                        if (!fieldref_info2.getNameAndTypeInfo().getName().equals("s")) {
+                            throw new AssertionError("was expecting variable name 's'");
+                        }
+                    }
+                }
+            }
+        }
+    }
+
     public void testAcceptRecordId() {
         String[] testOptions = {/* no options */};
         setCompileOptions(testOptions);
@@ -982,6 +1043,250 @@ public class RecordCompilationTests extends CompilationTestCase {
                 }
             }
         }
+    }
 
+    public void testMethodsInheritedFromRecordArePublicAndFinal() throws Exception {
+        int numberOfFieldRefs = 0;
+        File dir = assertOK(true, "record R() {}");
+        for (final File fileEntry : dir.listFiles()) {
+            if (fileEntry.getName().equals("R.class")) {
+                ClassFile classFile = ClassFile.read(fileEntry);
+                for (Method method : classFile.methods)
+                    switch (method.getName(classFile.constant_pool)) {
+                        case "toString", "equals", "hashCode" ->
+                            Assert.check(method.access_flags.is(AccessFlags.ACC_PUBLIC) && method.access_flags.is(AccessFlags.ACC_FINAL));
+                        default -> { /* do nothing */ }
+                    }
+            }
+        }
+    }
+
+    private static final List<String> ACCESSIBILITY = List.of(
+            "public", "protected", "", "private");
+
+    public void testCanonicalAccessibility() throws Exception {
+        // accessibility of canonical can't be stronger than that of the record type
+        for (String a1 : ACCESSIBILITY) {
+            for (String a2 : ACCESSIBILITY) {
+                if (protection(a2) > protection(a1)) {
+                    assertFail("compiler.err.invalid.canonical.constructor.in.record", "class R {# record RR() { # RR {} } }", a1, a2);
+                } else {
+                    assertOK("class R {# record RR() { # RR {} } }", a1, a2);
+                }
+            }
+        }
+
+        // now lets check that when compiler the compiler generates the canonical, it has the same accessibility
+        // as the record type
+        for (String a : ACCESSIBILITY) {
+            File dir = assertOK(true, "class R {# record RR() {} }", a);
+            for (final File fileEntry : dir.listFiles()) {
+                if (fileEntry.getName().equals("R$RR.class")) {
+                    ClassFile classFile = ClassFile.read(fileEntry);
+                    for (Method method : classFile.methods)
+                        if (method.getName(classFile.constant_pool).equals("<init>")) {
+                            Assert.check(method.access_flags.flags == accessFlag(a),
+                                    "was expecting access flag " + accessFlag(a) + " but found " + method.access_flags.flags);
+                        }
+                }
+            }
+        }
+    }
+
+    private int protection(String access) {
+        switch (access) {
+            case "private": return 3;
+            case "protected": return 1;
+            case "public": return 0;
+            case "": return 2;
+            default:
+                throw new AssertionError();
+        }
+    }
+
+    private int accessFlag(String access) {
+        switch (access) {
+            case "private": return AccessFlags.ACC_PRIVATE;
+            case "protected": return AccessFlags.ACC_PROTECTED;
+            case "public": return AccessFlags.ACC_PUBLIC;
+            case "": return 0;
+            default:
+                throw new AssertionError();
+        }
+    }
+
+    public void testSameArity() {
+        for (String source : List.of(
+                """
+                record R(int... args) {
+                    public R(int... args) {
+                        this.args = args;
+                    }
+                }
+                """,
+                """
+                record R(int[] args) {
+                    public R(int[] args) {
+                        this.args = args;
+                    }
+                }
+                """
+        )) {
+            assertOK(source);
+        }
+
+        for (String source : List.of(
+                """
+                record R(int... args) {
+                    public R(int[] args) {
+                        this.args = args;
+                    }
+                }
+                """,
+                """
+                record R(int... args) {
+                    public R(int[] args) {
+                        this.args = args;
+                    }
+                }
+                """,
+                """
+                record R(String... args) {
+                    public R(String[] args) {
+                        this.args = args;
+                    }
+                }
+                """,
+                """
+                record R(String... args) {
+                    public R(String[] args) {
+                        this.args = args;
+                    }
+                }
+                """
+        )) {
+            assertFail("compiler.err.invalid.canonical.constructor.in.record", source);
+        }
+    }
+
+    public void testSafeVararsAnno() {
+        assertFail("compiler.err.annotation.type.not.applicable",
+                """
+                @SafeVarargs
+                record R<T>(T... t) {}
+                """,
+                """
+                @SafeVarargs
+                record R<T>(T... t) {
+                    R(T... t) {
+                        this.t = t;
+                    }
+                }
+                """
+        );
+
+        assertOK(
+                """
+                record R<T>(T... t) {
+                    @SafeVarargs
+                    R(T... t) {
+                        this.t = t;
+                    }
+                }
+                """
+        );
+
+        appendCompileOptions("-Xlint:unchecked");
+        assertOKWithWarning("compiler.warn.unchecked.varargs.non.reifiable.type",
+                """
+                record R<T>(T... t) {
+                    R(T... t) {
+                        this.t = t;
+                    }
+                }
+                """
+        );
+        removeLastCompileOptions(1);
+
+        assertOK(
+                """
+                @SuppressWarnings("unchecked")
+                record R<T>(T... t) {
+                    R(T... t) {
+                        this.t = t;
+                    }
+                }
+                """
+        );
+
+        assertOK(
+                """
+                record R<T>(T... t) {
+                    @SuppressWarnings("unchecked")
+                    R(T... t) {
+                        this.t = t;
+                    }
+                }
+                """
+        );
+    }
+
+    public void testOverrideAtAccessor() {
+        assertOK(
+                """
+                record R(int i) {
+                    @Override
+                    public int i() { return i; }
+                }
+                """,
+                """
+                record R(int i, int j) {
+                    @Override
+                    public int i() { return i; }
+                    public int j() { return j; }
+                }
+                """,
+                """
+                interface I { int i(); }
+                record R(int i) implements I {
+                    @Override
+                    public int i() { return i; }
+                }
+                """,
+                """
+                interface I { int i(); }
+                record R(int i) implements I {
+                    public int i() { return i; }
+                }
+                """,
+                """
+                interface I { default int i() { return 0; } }
+                record R(int i) implements I {
+                    @Override
+                    public int i() { return i; }
+                }
+                """
+        );
+    }
+
+    public void testNoAssigmentInsideCompactRecord() {
+        assertFail("compiler.err.cant.assign.val.to.final.var",
+                """
+                record R(int i) {
+                    R {
+                        this.i = i;
+                    }
+                }
+                """
+        );
+        assertFail("compiler.err.cant.assign.val.to.final.var",
+                """
+                record R(int i) {
+                    R {
+                        (this).i = i;
+                    }
+                }
+                """
+        );
     }
 }

test/langtools/tools/javac/records/RecordMemberTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2019, 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
@@ -44,46 +44,39 @@ import static org.testng.Assert.*;
 
 @Test
 public class RecordMemberTests {
-    record R1(int i, int j) {}
+    public record R1(int i, int j) {}
 
-    record R2(int i, int j) {
+    public record R2(int i, int j) {
         public R2 {}
     }
 
-    record R3(int i, int j) {
+    public record R3(int i, int j) {
-        public R3 {
+        public R3(int i, int j) {
-            this.i = i;
-        }
-    }
-
-    record R4(int i, int j) {
-        public R4 {
             this.i = i;
             this.j = j;
         }
     }
 
-    record R5(int i, int j) {
+    public record R4(int i, int j) {
-        public R5 { this.i = this.j = 0; }
+        public R4(int i, int j) { this.i = this.j = 0; }
     }
 
     R1 r1 = new R1(1, 2);
     R2 r2 = new R2(1, 2);
     R3 r3 = new R3(1, 2);
     R4 r4 = new R4(1, 2);
-    R5 r5 = new R5(1, 2);
 
     public void testConstruction() {
-        for (int i : new int[] { r1.i, r2.i, r3.i, r4.i,
+        for (int i : new int[] { r1.i, r2.i, r3.i,
-                                 r1.i(), r2.i(), r3.i(), r4.i() })
+                                 r1.i(), r2.i(), r3.i() })
             assertEquals(i, 1);
 
-        for (int j : new int[] { r1.j, r2.j, r3.j, r4.j,
+        for (int j : new int[] { r1.j, r2.j, r3.j,
-                                 r1.j(), r2.j(), r3.j(), r4.j() })
+                                 r1.j(), r2.j(), r3.j() })
             assertEquals(j, 2);
 
-        assertEquals(r5.i, 0);
+        assertEquals(r4.i, 0);
-        assertEquals(r5.j, 0);
+        assertEquals(r4.j, 0);
     }
 
     public void testConstructorParameterNames() throws ReflectiveOperationException {

test/langtools/tools/javac/records/VarargsRecordsTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2019, 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
@@ -41,9 +41,9 @@ import static org.testng.Assert.*;
  */
 @Test
 public class VarargsRecordsTest {
-    record RI(int... xs) { }
+    public record RI(int... xs) { }
-    record RII(int x, int... xs) { }
+    public record RII(int x, int... xs) { }
-    record RX(int[] xs) { }
+    public record RX(int[] xs) { }
 
     RI r1 = new RI();
     RI r2 = new RI(1);