From 9fe8e823f3a65a10bfa241cbfad32737bbf24913 Mon Sep 17 00:00:00 2001 From: "Archie L. Cobbs" Date: Wed, 1 Feb 2023 16:28:41 -0600 Subject: [PATCH 01/24] 8194743: Permit additional statements before this/super in constructors --- .../com/sun/tools/javac/comp/Attr.java | 67 +-- .../com/sun/tools/javac/comp/AttrContext.java | 5 + .../com/sun/tools/javac/comp/Check.java | 122 ++++- .../com/sun/tools/javac/comp/Flow.java | 154 +++--- .../com/sun/tools/javac/comp/Lower.java | 43 +- .../com/sun/tools/javac/comp/Resolve.java | 64 ++- .../classes/com/sun/tools/javac/jvm/Gen.java | 38 +- .../tools/javac/resources/compiler.properties | 22 +- .../com/sun/tools/javac/tree/TreeInfo.java | 138 ++++-- .../jdk/jshell/ExpressionToTypeInfo.java | 5 +- .../tools/javac/SuperInit/SuperInitFails.java | 147 ++++++ .../tools/javac/SuperInit/SuperInitFails.out | 23 + .../tools/javac/SuperInit/SuperInitGood.java | 451 ++++++++++++++++++ ...eFirst.java => CallOnlyInConstructor.java} | 9 +- .../diags/examples/CallsNotAllowedHere.java | 32 ++ ...rstInvocationMustBeAnotherConstructor.java | 2 +- .../examples/RedundantSuperclassInit.java | 31 ++ .../examples/ReturnBeforeSuperclassInit.java | 32 ++ .../javac/records/RecordCompilationTests.java | 6 +- 19 files changed, 1151 insertions(+), 240 deletions(-) create mode 100644 test/langtools/tools/javac/SuperInit/SuperInitFails.java create mode 100644 test/langtools/tools/javac/SuperInit/SuperInitFails.out create mode 100644 test/langtools/tools/javac/SuperInit/SuperInitGood.java rename test/langtools/tools/javac/diags/examples/{CallMustBeFirst.java => CallOnlyInConstructor.java} (84%) create mode 100644 test/langtools/tools/javac/diags/examples/CallsNotAllowedHere.java create mode 100644 test/langtools/tools/javac/diags/examples/RedundantSuperclassInit.java create mode 100644 test/langtools/tools/javac/diags/examples/ReturnBeforeSuperclassInit.java diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java index aa480afa3ba4b..95beb14d0ba55 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java @@ -938,6 +938,8 @@ public void visitClassDef(JCClassDecl tree) { Optional localCacheContext = Optional.ofNullable(env.info.attributionMode.isSpeculative ? argumentAttr.withLocalCacheContext() : null); + boolean ctorProloguePrev = env.info.ctorPrologue; + env.info.ctorPrologue = false; try { // Local and anonymous classes have not been entered yet, so we need to // do it now. @@ -973,6 +975,7 @@ public void visitClassDef(JCClassDecl tree) { } } finally { localCacheContext.ifPresent(LocalCacheContext::leave); + env.info.ctorPrologue = ctorProloguePrev; } } @@ -982,6 +985,8 @@ public void visitMethodDef(JCMethodDecl tree) { Lint lint = env.info.lint.augment(m); Lint prevLint = chk.setLint(lint); + boolean ctorProloguePrev = env.info.ctorPrologue; + env.info.ctorPrologue = false; MethodSymbol prevMethod = chk.setMethod(m); try { deferredLintHandler.flush(tree.pos()); @@ -1045,6 +1050,9 @@ public void visitMethodDef(JCMethodDecl tree) { chk.validate(tree.recvparam, newEnv); } + // Is this method a constructor? + boolean isConstructor = tree.name == names.init; + if (env.enclClass.sym.isRecord() && tree.sym.owner.kind == TYP) { // lets find if this method is an accessor Optional recordComponent = env.enclClass.sym.getRecordComponents().stream() @@ -1072,14 +1080,11 @@ public void visitMethodDef(JCMethodDecl tree) { } } - if (tree.name == names.init) { + if (isConstructor) { // if this a constructor other than the canonical one if ((tree.sym.flags_field & RECORD) == 0) { - JCMethodInvocation app = TreeInfo.firstConstructorCall(tree); - if (app == null || - TreeInfo.name(app.meth) != names._this || - !checkFirstConstructorStat(app, tree, false)) { - log.error(tree, Errors.FirstStatementMustBeCallToAnotherConstructor(env.enclClass.sym)); + if (!TreeInfo.hasConstructorCall(tree, names._this)) { + log.error(tree, Errors.NonCanonicalConstructorInvokeAnotherConstructor(env.enclClass.sym)); } } else { // but if it is the canonical: @@ -1105,11 +1110,7 @@ public void visitMethodDef(JCMethodDecl tree) { ); } - JCMethodInvocation app = TreeInfo.firstConstructorCall(tree); - if (app != null && - (TreeInfo.name(app.meth) == names._this || - TreeInfo.name(app.meth) == names._super) && - checkFirstConstructorStat(app, tree, false)) { + if (TreeInfo.hasAnyConstructorCall(tree)) { log.error(tree, Errors.InvalidCanonicalConstructorInRecord( Fragments.Canonical, env.enclClass.sym.name, Fragments.CanonicalMustNotContainExplicitConstructorInvocation)); @@ -1187,16 +1188,14 @@ public void visitMethodDef(JCMethodDecl tree) { // Add an implicit super() call unless an explicit call to // super(...) or this(...) is given // or we are compiling class java.lang.Object. - if (tree.name == names.init && owner.type != syms.objectType) { - JCBlock body = tree.body; - if (body.stats.isEmpty() || - TreeInfo.getConstructorInvocationName(body.stats, names) == names.empty) { - JCStatement supCall = make.at(body.pos).Exec(make.Apply(List.nil(), + if (isConstructor && owner.type != syms.objectType) { + if (!TreeInfo.hasAnyConstructorCall(tree)) { + JCStatement supCall = make.at(tree.body.pos).Exec(make.Apply(List.nil(), make.Ident(names._super), make.Idents(List.nil()))); - body.stats = body.stats.prepend(supCall); + tree.body.stats = tree.body.stats.prepend(supCall); } else if ((env.enclClass.sym.flags() & ENUM) != 0 && (tree.mods.flags & GENERATEDCONSTR) == 0 && - TreeInfo.isSuperCall(body.stats.head)) { + TreeInfo.hasConstructorCall(tree, names._super)) { // enum constructors are not allowed to call super // directly, so make sure there aren't any super calls // in enum constructors, except in the compiler @@ -1226,6 +1225,9 @@ public void visitMethodDef(JCMethodDecl tree) { annotate.queueScanTreeAndTypeAnnotate(tree.body, localEnv, m, null); annotate.flush(); + // Start of constructor prologue + localEnv.info.ctorPrologue = isConstructor; + // Attribute method body. attribStat(tree.body, localEnv); } @@ -1235,6 +1237,7 @@ public void visitMethodDef(JCMethodDecl tree) { } finally { chk.setLint(prevLint); chk.setMethod(prevMethod); + env.info.ctorPrologue = ctorProloguePrev; } } @@ -2492,9 +2495,8 @@ public void visitApply(JCMethodInvocation tree) { ListBuffer argtypesBuf = new ListBuffer<>(); if (isConstructorCall) { - // We are seeing a ...this(...) or ...super(...) call. - // Check that this is the first statement in a constructor. - checkFirstConstructorStat(tree, env.enclMethod, true); + // We are seeing a this()/super() call. End of constructor prologue. + env.info.ctorPrologue = false; // Record the fact // that this is a constructor call (using isSelfCall). @@ -2635,26 +2637,6 @@ Type adjustMethodReturnType(Symbol msym, Type qualifierType, Name methodName, Li } } - /** Check that given application node appears as first statement - * in a constructor call. - * @param tree The application node - * @param enclMethod The enclosing method of the application. - * @param error Should an error be issued? - */ - boolean checkFirstConstructorStat(JCMethodInvocation tree, JCMethodDecl enclMethod, boolean error) { - if (enclMethod != null && enclMethod.name == names.init) { - JCBlock body = enclMethod.body; - if (body.stats.head.hasTag(EXEC) && - ((JCExpressionStatement) body.stats.head).expr == tree) - return true; - } - if (error) { - log.error(tree.pos(), - Errors.CallMustBeFirstStmtInCtor(TreeInfo.name(tree.meth))); - } - return false; - } - /** Obtain a method type with given argument types. */ Type newMethodTemplate(Type restype, List argtypes, List typeargtypes) { @@ -5605,6 +5587,9 @@ private void attribClassBody(Env env, ClassSymbol c) { } } + // Check for proper placement of super()/init() calls. + chk.checkSuperInitCalls(tree); + // Check for cycles among non-initial constructors. chk.checkCyclicConstructors(tree); diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/AttrContext.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/AttrContext.java index 2a96b11316c01..fdc9a575ce32a 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/AttrContext.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/AttrContext.java @@ -57,6 +57,10 @@ public class AttrContext { */ boolean constructorArgs = false; + /** Are we in the 'prologue' part of a constructor, prior to an explicit this()/super()? + */ + boolean ctorPrologue = false; + /** Are we evaluating the selector of a `super' or type name? */ boolean selectSuper = false; @@ -138,6 +142,7 @@ AttrContext dup(WriteableScope scope) { info.staticLevel = staticLevel; info.isSelfCall = isSelfCall; info.constructorArgs = constructorArgs; + info.ctorPrologue = ctorPrologue; info.selectSuper = selectSuper; info.pendingResolutionPhase = pendingResolutionPhase; info.lint = lint; diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java index 893ba5699e661..d7230608ac2f0 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java @@ -3715,10 +3715,13 @@ void checkCyclicConstructors(JCClassDecl tree) { // enter each constructor this-call into the map for (List l = tree.defs; l.nonEmpty(); l = l.tail) { - JCMethodInvocation app = TreeInfo.firstConstructorCall(l.head); - if (app == null) continue; - JCMethodDecl meth = (JCMethodDecl) l.head; - if (TreeInfo.name(app.meth) == names._this) { + if (!l.head.hasTag(METHODDEF)) + continue; + JCMethodDecl meth = (JCMethodDecl)l.head; + if (meth.name != names.init) + continue; + JCMethodInvocation app = TreeInfo.findConstructorCall(meth); + if (app != null && TreeInfo.name(app.meth) == names._this) { callMap.put(meth.sym, TreeInfo.symbol(app.meth)); } else { meth.sym.flags_field |= ACYCLIC; @@ -3751,6 +3754,117 @@ private void checkCyclicConstructor(JCClassDecl tree, Symbol ctor, } } +/* ************************************************************************* + * Verify the proper placement of super()/this() calls. + * + * - super()/this() may only appear in constructors + * - There must be at most one super()/this() call per constructor + * - The super()/this() call, if any, must be a top-level statement in the + * constructor, i.e., not nested inside any other statement or block + * - There must be no return statements prior to the super()/this() call + **************************************************************************/ + + void checkSuperInitCalls(JCClassDecl tree) { + new SuperThisChecker().check(tree); + } + + private class SuperThisChecker extends TreeScanner { + + // Match this scan stack: 1=JCMethodDecl, 2=JCExpressionStatement, 3=JCMethodInvocation + private static final int MATCH_SCAN_DEPTH = 3; + + private boolean constructor; // this this method a constructor? + private JCReturn earlyReturn; // first return prior to the super()/init(), if any + private Name initCall; // whichever of "super" or "init" we've seen already + private int scanDepth; // current scan recursion depth in method body + + public void check(JCClassDecl classDef) { + scan(classDef.defs); + } + + @Override + public void visitMethodDef(JCMethodDecl tree) { + Assert.check(!constructor); + Assert.check(earlyReturn == null); + Assert.check(initCall == null); + Assert.check(scanDepth == 1); + + // Initialize state for this method + constructor = tree.name == names.init; + try { + + // Scan method body + if (tree.body != null) + scan(tree.body.stats); + + // Verify no 'return' seen prior to an explicit super()/this() call + if (constructor && earlyReturn != null && initCall != null) + log.error(earlyReturn.pos(), Errors.ReturnBeforeSuperclassInitialized(initCall)); + } finally { + constructor = false; + earlyReturn = null; + initCall = null; + } + } + + @Override + public void scan(JCTree tree) { + scanDepth++; + try { + super.scan(tree); + } finally { + scanDepth--; + } + } + + @Override + public void visitApply(JCMethodInvocation apply) { + do { + + // Is this a super() or this() call? + Name methodName = TreeInfo.name(apply.meth); + if (methodName != names._super && methodName != names._this) + break; + + // super()/this() calls must only appear in a constructor + if (!constructor) { + log.error(apply.pos(), Errors.CallMustOnlyAppearInCtor(methodName)); + break; + } + + // super()/this() calls must be a top level statement + if (scanDepth != MATCH_SCAN_DEPTH) { + log.error(apply.pos(), Errors.CallsNotAllowedHere(methodName)); + break; + } + + // super()/this() calls must not appear more than once + if (initCall != null) { + log.error(apply.pos(), Errors.RedundantSuperclassInit(methodName, initCall)); + break; + } + + // We found a legitimate super()/this() call; remember it + initCall = methodName; + } while (false); + + // Proceed + super.visitApply(apply); + } + + @Override + public void visitReturn(JCReturn tree) { + if (constructor && initCall == null && earlyReturn == null) + earlyReturn = tree; // we have seen a return but not (yet) a super()/this() + super.visitReturn(tree); + } + + @Override + public void visitClassDef(JCClassDecl tree) { + // don't descend any further + } + } + /* ************************************************************************* * Miscellaneous **************************************************************************/ diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java index 0c151058b18e2..7893933a95567 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java @@ -32,6 +32,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Set; +import java.util.function.Consumer; import java.util.stream.Collectors; import java.util.stream.StreamSupport; @@ -377,6 +378,13 @@ private JumpKind(Tag treeTag) { */ ListBuffer pendingExits; + /** A class whose initializers we are scanning. Because initializer + * scans can be triggered out of sequence when visiting certain nodes + * (e.g., super()), we protect against infinite loops that could be + * triggered by incorrect code (e.g., super() inside initializer). + */ + JCClassDecl initScanClass; + /** A pending exit. These are the statements return, break, and * continue. In addition, exception-throwing expressions or * statements are put here when not known to be caught. This @@ -462,6 +470,23 @@ protected void scanSyntheticBreak(TreeMaker make, JCTree swtch) { scan(brk); } } + + // Do something with all static or non-static field initializers and initialization blocks. + // Note: This method also sends nested class definitions to the handler. + protected void forEachInitializer(JCClassDecl classDef, boolean statik, Consumer handler) { + if (classDef == initScanClass) // avoid infinite loops + return; + JCClassDecl initScanClassPrev = initScanClass; + initScanClass = classDef; + try { + classDef.defs.stream() + .filter(def -> !def.hasTag(METHODDEF)) + .filter(def -> ((TreeInfo.flags(def) & STATIC) != 0) == statik) + .forEach(handler); + } finally { + initScanClass = initScanClassPrev; + } + } } /** @@ -527,22 +552,16 @@ public void visitClassDef(JCClassDecl tree) { try { // process all the static initializers - for (List l = tree.defs; l.nonEmpty(); l = l.tail) { - if (!l.head.hasTag(METHODDEF) && - (TreeInfo.flags(l.head) & STATIC) != 0) { - scanDef(l.head); - clearPendingExits(false); - } - } + forEachInitializer(tree, true, def -> { + scanDef(def); + clearPendingExits(false); + }); // process all the instance initializers - for (List l = tree.defs; l.nonEmpty(); l = l.tail) { - if (!l.head.hasTag(METHODDEF) && - (TreeInfo.flags(l.head) & STATIC) == 0) { - scanDef(l.head); - clearPendingExits(false); - } - } + forEachInitializer(tree, false, def -> { + scanDef(def); + clearPendingExits(false); + }); // process all the methods for (List l = tree.defs; l.nonEmpty(); l = l.tail) { @@ -1208,40 +1227,10 @@ public void visitClassDef(JCClassDecl tree) { try { // process all the static initializers - for (List l = tree.defs; l.nonEmpty(); l = l.tail) { - if (!l.head.hasTag(METHODDEF) && - (TreeInfo.flags(l.head) & STATIC) != 0) { - scan(l.head); - errorUncaught(); - } - } - - // add intersection of all thrown clauses of initial constructors - // to set of caught exceptions, unless class is anonymous. - if (!anonymousClass) { - boolean firstConstructor = true; - for (List l = tree.defs; l.nonEmpty(); l = l.tail) { - if (TreeInfo.isInitialConstructor(l.head)) { - List mthrown = - ((JCMethodDecl) l.head).sym.type.getThrownTypes(); - if (firstConstructor) { - caught = mthrown; - firstConstructor = false; - } else { - caught = chk.intersect(mthrown, caught); - } - } - } - } - - // process all the instance initializers - for (List l = tree.defs; l.nonEmpty(); l = l.tail) { - if (!l.head.hasTag(METHODDEF) && - (TreeInfo.flags(l.head) & STATIC) == 0) { - scan(l.head); - errorUncaught(); - } - } + forEachInitializer(tree, true, def -> { + scan(def); + errorUncaught(); + }); // in an anonymous class, add the set of thrown exceptions to // the throws clause of the synthetic constructor and propagate @@ -1296,7 +1285,7 @@ public void visitMethodDef(JCMethodDecl tree) { JCVariableDecl def = l.head; scan(def); } - if (TreeInfo.isInitialConstructor(tree)) + if (TreeInfo.hasConstructorCall(tree, names._super)) caught = chk.union(caught, mthrown); else if ((tree.sym.flags() & (BLOCK | STATIC)) != BLOCK) caught = mthrown; @@ -1592,8 +1581,18 @@ public void visitThrow(JCThrow tree) { public void visitApply(JCMethodInvocation tree) { scan(tree.meth); scan(tree.args); + + // Mark as thrown the exceptions thrown by the method being invoked for (List l = tree.meth.type.getThrownTypes(); l.nonEmpty(); l = l.tail) markThrown(tree, l.head); + + // After super(), scan initializers to uncover any exceptions they throw + if (TreeInfo.name(tree.meth) == names._super) { + forEachInitializer(classDef, false, def -> { + scan(def); + errorUncaught(); + }); + } } public void visitNewClass(JCNewClass tree) { @@ -1967,11 +1966,11 @@ public AssignAnalyzer() { uninitsWhenFalse = new Bits(true); } - private boolean isInitialConstructor = false; + private boolean isConstructor; @Override protected void markDead() { - if (!isInitialConstructor) { + if (!isConstructor) { inits.inclRange(returnadr, nextadr); } else { for (int address = returnadr; address < nextadr; address++) { @@ -2222,13 +2221,10 @@ public void visitClassDef(JCClassDecl tree) { } // process all the static initializers - for (List l = tree.defs; l.nonEmpty(); l = l.tail) { - if (!l.head.hasTag(METHODDEF) && - (TreeInfo.flags(l.head) & STATIC) != 0) { - scan(l.head); - clearPendingExits(false); - } - } + forEachInitializer(tree, true, def -> { + scan(def); + clearPendingExits(false); + }); // define all the instance fields for (List l = tree.defs; l.nonEmpty(); l = l.tail) { @@ -2243,15 +2239,6 @@ public void visitClassDef(JCClassDecl tree) { } } - // process all the instance initializers - for (List l = tree.defs; l.nonEmpty(); l = l.tail) { - if (!l.head.hasTag(METHODDEF) && - (TreeInfo.flags(l.head) & STATIC) == 0) { - scan(l.head); - clearPendingExits(false); - } - } - // process all the methods for (List l = tree.defs; l.nonEmpty(); l = l.tail) { if (l.head.hasTag(METHODDEF)) { @@ -2290,13 +2277,16 @@ public void visitMethodDef(JCMethodDecl tree) { int returnadrPrev = returnadr; Assert.check(pendingExits.isEmpty()); - boolean lastInitialConstructor = isInitialConstructor; + boolean isConstructorPrev = isConstructor; try { - isInitialConstructor = TreeInfo.isInitialConstructor(tree); + isConstructor = TreeInfo.isConstructor(tree); - if (!isInitialConstructor) { + // We only track field initialization inside constructors + if (!isConstructor) { firstadr = nextadr; } + + // Mark all method parameters as DA for (List l = tree.params; l.nonEmpty(); l = l.tail) { JCVariableDecl def = l.head; scan(def); @@ -2312,7 +2302,7 @@ public void visitMethodDef(JCMethodDecl tree) { boolean isCompactOrGeneratedRecordConstructor = (tree.sym.flags() & Flags.COMPACT_RECORD_CONSTRUCTOR) != 0 || (tree.sym.flags() & (GENERATEDCONSTR | RECORD)) == (GENERATEDCONSTR | RECORD); - if (isInitialConstructor) { + if (isConstructor) { boolean isSynthesized = (tree.sym.flags() & GENERATEDCONSTR) != 0; for (int i = firstadr; i < nextadr; i++) { @@ -2355,7 +2345,7 @@ public void visitMethodDef(JCMethodDecl tree) { nextadr = nextadrPrev; firstadr = firstadrPrev; returnadr = returnadrPrev; - isInitialConstructor = lastInitialConstructor; + isConstructor = isConstructorPrev; } } finally { lint = lintPrev; @@ -2371,7 +2361,7 @@ private void clearPendingExits(boolean inMethod) { Assert.check((inMethod && exit.tree.hasTag(RETURN)) || log.hasErrorOn(exit.tree.pos()), exit.tree); - if (inMethod && isInitialConstructor) { + if (inMethod && isConstructor) { Assert.check(exit instanceof AssignPendingExit); inits.assign(((AssignPendingExit) exit).exit_inits); for (int i = firstadr; i < nextadr; i++) { @@ -2838,6 +2828,24 @@ public void visitThrow(JCThrow tree) { public void visitApply(JCMethodInvocation tree) { scanExpr(tree.meth); scanExprs(tree.args); + + // If super(): at this point all initialization blocks will execute + Name name = TreeInfo.name(tree.meth); + if (isConstructor && name == names._super) { + forEachInitializer(classDef, false, def -> { + scan(def); + clearPendingExits(false); + }); + } + + // If this(): at this point all final uninitialized fields will get initialized + if (isConstructor && name == names._this) { + for (int address = firstadr; address < nextadr; address++) { + VarSymbol sym = vardecls[address].sym; + if (isFinalUninitializedField(sym) && !sym.isStatic()) + letInit(tree.pos(), sym); + } + } } public void visitNewClass(JCNewClass tree) { diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java index 17493b86ac5d9..0ecf2cda14272 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java @@ -2256,19 +2256,20 @@ public void visitClassDef(JCClassDecl tree) { tree.defs = tree.defs.prepend(l.head); enterSynthetic(tree.pos(), l.head.sym, currentClass.members()); } - // If this$n was accessed, add the field definition and - // update initial constructors to initialize it + // If this$n was accessed, add the field definition and prepend + // initializer code to any super() invocation to initialize it if (currentClass.hasOuterInstance() && shouldEmitOuterThis(currentClass)) { tree.defs = tree.defs.prepend(otdef); enterSynthetic(tree.pos(), otdef.sym, currentClass.members()); - for (JCTree def : tree.defs) { - if (TreeInfo.isInitialConstructor(def)) { - JCMethodDecl mdef = (JCMethodDecl) def; - mdef.body.stats = mdef.body.stats.prepend( - initOuterThis(mdef.body.pos, mdef.params.head.sym)); - } - } + tree.defs.stream() + .filter(TreeInfo::isConstructor) + .map(JCMethodDecl.class::cast) + .filter(mdef -> TreeInfo.hasConstructorCall(mdef, names._super)) + .forEach(mdef -> { + List initializer = List.of(initOuterThis(mdef.body.pos, mdef.params.head.sym)); + TreeInfo.mapSuperCalls(mdef.body, supercall -> make.Block(0, initializer.append(supercall))); + }); } proxies = prevProxies; @@ -2731,19 +2732,18 @@ private void visitMethodDefInternal(JCMethodDecl tree) { tree.params = tree.params.prepend(otdef); } - // If this is an initial constructor, i.e., it does not start with - // this(...), insert initializers for this$n and proxies - // before (pre-1.4, after) the call to superclass constructor. - JCStatement selfCall = translate(tree.body.stats.head); + // Determine whether this constructor has a super() invocation + boolean invokesSuper = TreeInfo.hasConstructorCall(tree, names._super); - List added = List.nil(); + // Create initializers for this$n and proxies + ListBuffer added = new ListBuffer<>(); if (fvs.nonEmpty()) { List addedargtypes = List.nil(); for (List l = fvs; l.nonEmpty(); l = l.tail) { m.capturedLocals = m.capturedLocals.prepend((VarSymbol) (proxies.get(l.head))); - if (TreeInfo.isInitialConstructor(tree)) { + if (invokesSuper) { added = added.prepend( initField(tree.body.pos, proxies.get(l.head), prevProxies.get(l.head))); } @@ -2757,13 +2757,18 @@ private void visitMethodDefInternal(JCMethodDecl tree) { syms.methodClass); } + // Recursively translate existing local statements + tree.body.stats = translate(tree.body.stats); + + // Prepend initializers in front of super() call + if (added.nonEmpty()) { + List initializers = added.toList(); + TreeInfo.mapSuperCalls(tree.body, supercall -> make.Block(0, initializers.append(supercall))); + } + // pop local variables from proxy stack proxies = prevProxies; - // recursively translate following local statements and - // combine with this- or super-call - List stats = translate(tree.body.stats.tail); - tree.body.stats = stats.prepend(selfCall).prependList(added); outerThisStack = prevOuterThisStack; } else { Map prevLambdaTranslationMap = diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java index 9b4653a039a7f..7bc86ae833b38 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java @@ -1504,13 +1504,15 @@ Symbol findVar(Env env, Name name) { sym = findField(env1, env1.enclClass.sym.type, name, env1.enclClass.sym); } if (sym.exists()) { - if (staticOnly && - sym.kind == VAR && + if (sym.kind == VAR && sym.owner.kind == TYP && - (sym.flags() & STATIC) == 0) - return new StaticError(sym); - else - return sym; + (sym.flags() & STATIC) == 0) { + if (staticOnly) + return new StaticError(sym); + if (env1.info.ctorPrologue && (sym.flags_field & SYNTHETIC) == 0) + return new RefBeforeCtorCalledError(sym); + } + return sym; } else { bestSoFar = bestOf(bestSoFar, sym); } @@ -2007,11 +2009,15 @@ Symbol findFun(Env env, Name name, env1, env1.enclClass.sym.type, name, argtypes, typeargtypes, allowBoxing, useVarargs); if (sym.exists()) { - if (staticOnly && - sym.kind == MTH && - sym.owner.kind == TYP && - (sym.flags() & STATIC) == 0) return new StaticError(sym); - else return sym; + if (sym.kind == MTH && + sym.owner.kind == TYP && + (sym.flags() & STATIC) == 0) { + if (staticOnly) + return new StaticError(sym); + if (env1.info.ctorPrologue && env1 == env) + return new RefBeforeCtorCalledError(sym); + } + return sym; } else { bestSoFar = bestOf(bestSoFar, sym); } @@ -3757,7 +3763,10 @@ Symbol resolveSelf(DiagnosticPosition pos, if (env1.enclClass.sym == c) { Symbol sym = env1.info.scope.findFirst(name); if (sym != null) { - if (staticOnly) sym = new StaticError(sym); + if (staticOnly) + sym = new StaticError(sym); + else if (env1.info.ctorPrologue) + sym = new RefBeforeCtorCalledError(sym); return accessBase(sym, pos, env.enclClass.sym.type, name, true); } @@ -3771,6 +3780,8 @@ Symbol resolveSelf(DiagnosticPosition pos, //this might be a default super call if one of the superinterfaces is 'c' for (Type t : pruneInterfaces(env.enclClass.type)) { if (t.tsym == c) { + if (env.info.ctorPrologue) + log.error(pos, Errors.CantRefBeforeCtorCalled(name)); env.info.defaultSuperCallSite = t; return new VarSymbol(0, names._super, types.asSuper(env.enclClass.type, c), env.enclClass.sym); @@ -3872,8 +3883,8 @@ Type resolveImplicitThis(DiagnosticPosition pos, Env env, Type t, b Type thisType = (t.tsym.owner.kind.matches(KindSelector.VAL_MTH) ? resolveSelf(pos, env, t.getEnclosingType().tsym, names._this) : resolveSelfContaining(pos, env, t.tsym, isSuperCall)).type; - if (env.info.isSelfCall && thisType.tsym == env.enclClass.sym) { - log.error(pos, Errors.CantRefBeforeCtorCalled("this")); + if ((env.info.isSelfCall || env.info.ctorPrologue) && thisType.tsym == env.enclClass.sym) { + log.error(pos, Errors.CantRefBeforeCtorCalled(names._this)); } return thisType; } @@ -4588,6 +4599,31 @@ JCDiagnostic getDiagnostic(JCDiagnostic.DiagnosticType dkind, } } + /** + * Specialization of {@link StaticError} for accesses in a constructor prologue. + */ + class RefBeforeCtorCalledError extends InvalidSymbolError { + + RefBeforeCtorCalledError(Symbol sym) { + super(STATICERR, sym, "prologue error"); + } + + @Override + JCDiagnostic getDiagnostic(JCDiagnostic.DiagnosticType dkind, + DiagnosticPosition pos, + Symbol location, + Type site, + Name name, + List argtypes, + List typeargtypes) { + Symbol errSym = ((sym.kind == TYP && sym.type.hasTag(CLASS)) + ? types.erasure(sym.type).tsym + : sym); + return diags.create(dkind, log.currentSource(), pos, + "cant.ref.before.ctor.called", errSym); + } + } + /** * InvalidSymbolError error class indicating that a pair of symbols * (either methods, constructors or operands) are ambiguous diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java index 6f58c8b4f818c..7a788ed275f8e 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java @@ -540,44 +540,18 @@ private void checkStringConstant(DiagnosticPosition pos, Object constValue) { nerrs++; } - /** Insert instance initializer code into initial constructor. + /** Insert instance initializer code into constructors prior to the super() call. * @param md The tree potentially representing a * constructor's definition. * @param initCode The list of instance initializer statements. * @param initTAs Type annotations from the initializer expression. */ void normalizeMethod(JCMethodDecl md, List initCode, List initTAs) { - if (md.name == names.init && TreeInfo.isInitialConstructor(md)) { - // We are seeing a constructor that does not call another - // constructor of the same class. - List stats = md.body.stats; - ListBuffer newstats = new ListBuffer<>(); - - if (stats.nonEmpty()) { - // Copy initializers of synthetic variables generated in - // the translation of inner classes. - while (TreeInfo.isSyntheticInit(stats.head)) { - newstats.append(stats.head); - stats = stats.tail; - } - // Copy superclass constructor call - newstats.append(stats.head); - stats = stats.tail; - // Copy remaining synthetic initializers. - while (stats.nonEmpty() && - TreeInfo.isSyntheticInit(stats.head)) { - newstats.append(stats.head); - stats = stats.tail; - } - // Now insert the initializer code. - newstats.appendList(initCode); - // And copy all remaining statements. - while (stats.nonEmpty()) { - newstats.append(stats.head); - stats = stats.tail; - } - } - md.body.stats = newstats.toList(); + if (md.name == names.init && TreeInfo.hasConstructorCall(md, names._super)) { + // We are seeing a constructor that has a super() call. + // Find the super() invocation and append the given initializer code. + TreeInfo.mapSuperCalls(md.body, supercall -> make.Block(0, initCode.prepend(supercall))); + if (md.body.endpos == Position.NOPOS) md.body.endpos = TreeInfo.endPos(md.body.stats.last()); diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties index 478a0a162c2a3..fa9f3c69f6be1 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties @@ -230,8 +230,20 @@ compiler.err.switch.expression.no.result.expressions=\ switch expression does not have any result expressions # 0: name -compiler.err.call.must.be.first.stmt.in.ctor=\ - call to {0} must be first statement in constructor +compiler.err.call.must.only.appear.in.ctor=\ + calls to {0}() may only appear within constructors + +# 0: name, 1: name +compiler.err.redundant.superclass.init=\ + redundant call to {0}() after {1}() has already been invoked + +# 0: name +compiler.err.calls.not.allowed.here=\ + calls to {0}() not allowed here + +# 0: name +compiler.err.return.before.superclass.initialized=\ + ''return'' not allowed prior to {0}() # 0: symbol kind, 1: name, 2: symbol kind, 3: type, 4: message segment compiler.err.cant.apply.symbol.noargs=\ @@ -376,7 +388,7 @@ compiler.err.annotation.decl.not.allowed.here=\ compiler.err.cant.inherit.from.final=\ cannot inherit from final {0} -# 0: symbol or string +# 0: symbol or name compiler.err.cant.ref.before.ctor.called=\ cannot reference {0} before supertype constructor has been called @@ -3786,8 +3798,8 @@ compiler.err.invalid.supertype.record=\ classes cannot directly extend {0} # 0: symbol -compiler.err.first.statement.must.be.call.to.another.constructor=\ - constructor is not canonical, so its first statement must invoke another constructor of class {0} +compiler.err.non.canonical.constructor.invoke.another.constructor=\ + constructor is not canonical, so it must invoke another constructor of class {0} compiler.err.instance.initializer.not.allowed.in.records=\ instance initializers not allowed in records diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeInfo.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeInfo.java index a9d406c9584bb..d67f23ec8ec48 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeInfo.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeInfo.java @@ -50,6 +50,7 @@ import javax.lang.model.element.ElementKind; import javax.tools.JavaFileObject; +import java.util.function.Function; import java.util.function.Predicate; import java.util.function.ToIntFunction; @@ -113,25 +114,6 @@ public static boolean hasConstructors(List trees) { return false; } - /** Is there a constructor invocation in the given list of trees? - */ - public static Name getConstructorInvocationName(List trees, Names names) { - for (JCTree tree : trees) { - if (tree.hasTag(EXEC)) { - JCExpressionStatement stat = (JCExpressionStatement)tree; - if (stat.expr.hasTag(APPLY)) { - JCMethodInvocation apply = (JCMethodInvocation)stat.expr; - Name methName = TreeInfo.name(apply.meth); - if (methName == names._this || - methName == names._super) { - return methName; - } - } - } - } - return names.empty; - } - public static boolean isMultiCatch(JCCatch catchClause) { return catchClause.param.vartype.hasTag(TYPEUNION); } @@ -238,32 +220,104 @@ public static List recordFieldTypes(JCClassDecl tree) { .collect(List.collector()); } - /** Is this a constructor whose first (non-synthetic) statement is not - * of the form this(...)? - */ - public static boolean isInitialConstructor(JCTree tree) { - JCMethodInvocation app = firstConstructorCall(tree); - if (app == null) return false; - Name meth = name(app.meth); - return meth == null || meth != meth.table.names._this; + /** Is the given method a constructor containing a super() or this() call? + */ + public static boolean hasAnyConstructorCall(JCMethodDecl tree) { + return hasConstructorCall(tree, null); + } + + /** Is the given method a constructor containing a super() and/or this() call? + * The "target" is either names._this, names._super, or null for either/both. + */ + public static boolean hasConstructorCall(JCMethodDecl tree, Name target) { + JCMethodInvocation app = findConstructorCall(tree); + return app != null && (target == null || target == name(app.meth)); } - /** Return the first call in a constructor definition. */ - public static JCMethodInvocation firstConstructorCall(JCTree tree) { - if (!tree.hasTag(METHODDEF)) return null; - JCMethodDecl md = (JCMethodDecl) tree; + /** Find the first super() or init() call in the given constructor. + */ + public static JCMethodInvocation findConstructorCall(JCMethodDecl md) { Names names = md.name.table.names; - if (md.name != names.init) return null; - if (md.body == null) return null; - List stats = md.body.stats; - // Synthetic initializations can appear before the super call. - while (stats.nonEmpty() && isSyntheticInit(stats.head)) - stats = stats.tail; - if (stats.isEmpty()) return null; - if (!stats.head.hasTag(EXEC)) return null; - JCExpressionStatement exec = (JCExpressionStatement) stats.head; - if (!exec.expr.hasTag(APPLY)) return null; - return (JCMethodInvocation)exec.expr; + if (md.name != names.init || md.body == null) + return null; + return new ConstructorCallFinder(names).find(md).head; + } + + /** Finds all calls to this() and/or super() in a given constructor. + * We can't assume they will be "top level" statements, because + * some synthetic calls to super() are added inside { } blocks. + * So we must recurse through the method's entire syntax tree. + */ + private static class ConstructorCallFinder extends TreeScanner { + + final ListBuffer calls = new ListBuffer<>(); + final Names names; + + ConstructorCallFinder(Names names) { + this.names = names; + } + + List find(JCMethodDecl meth) { + scan(meth); + return calls.toList(); + } + + @Override + public void visitApply(JCMethodInvocation invoke) { + Name name = TreeInfo.name(invoke.meth); + if ((name == names._this || name == names._super)) + calls.append(invoke); + super.visitApply(invoke); + } + + @Override + public void visitClassDef(JCClassDecl tree) { + // don't descend any further + } + + @Override + public void visitLambda(JCLambda tree) { + // don't descend any further + } + } + + /** Finds super() invocations and translates them using the given mapping. + */ + public static void mapSuperCalls(JCBlock block, Function mapper) { + block.stats = block.stats.map(new TreeInfo.SuperCallTranslator(mapper)::translate); + } + + /** Finds all super() invocations and translates them somehow. + */ + private static class SuperCallTranslator extends TreeTranslator { + + final Function translator; + + /** Constructor. + * + * @param translator translates super() invocations, returning replacement statement or null for no change + */ + SuperCallTranslator(Function translator) { + this.translator = translator; + } + + // Because it returns void, anywhere super() can legally appear must be a location where a JCStatement + // could also appear, so it's OK that we are replacing a JCExpressionStatement with a JCStatement here. + @Override + public void visitExec(JCExpressionStatement stat) { + if (!TreeInfo.isSuperCall(stat) || (result = this.translator.apply(stat)) == null) + super.visitExec(stat); + } + + @Override + public void visitClassDef(JCClassDecl tree) { + // don't descend any further + } + + @Override + public void visitLambda(JCLambda tree) { + // don't descend any further + } } /** Return true if a tree represents a diamond new expr. */ diff --git a/src/jdk.jshell/share/classes/jdk/jshell/ExpressionToTypeInfo.java b/src/jdk.jshell/share/classes/jdk/jshell/ExpressionToTypeInfo.java index 414d58b589d94..e9eaccea32149 100644 --- a/src/jdk.jshell/share/classes/jdk/jshell/ExpressionToTypeInfo.java +++ b/src/jdk.jshell/share/classes/jdk/jshell/ExpressionToTypeInfo.java @@ -56,6 +56,7 @@ import com.sun.tools.javac.code.Type; import com.sun.tools.javac.code.Types; import com.sun.tools.javac.tree.JCTree.JCClassDecl; +import com.sun.tools.javac.tree.JCTree.JCMethodDecl; import com.sun.tools.javac.tree.TreeInfo; import com.sun.tools.javac.util.List; import com.sun.tools.javac.util.ListBuffer; @@ -428,7 +429,9 @@ private ExpressionInfo treeToInfo(TreePath tp) { MethodInvocationTree superCall = clazz.getMembers() .stream() - .map(TreeInfo::firstConstructorCall) + .filter(JCMethodDecl.class::isInstance) + .map(JCMethodDecl.class::cast) + .map(TreeInfo::findConstructorCall) .findAny() .get(); TreePath superCallPath diff --git a/test/langtools/tools/javac/SuperInit/SuperInitFails.java b/test/langtools/tools/javac/SuperInit/SuperInitFails.java new file mode 100644 index 0000000000000..b133db710d697 --- /dev/null +++ b/test/langtools/tools/javac/SuperInit/SuperInitFails.java @@ -0,0 +1,147 @@ +/* + * @test /nodynamiccopyright/ + * @bug 8194743 + * @summary Permit additional statements before this/super in constructors + * + * @compile/fail/ref=SuperInitFails.out -XDrawDiagnostics SuperInitFails.java + */ +import java.util.concurrent.atomic.AtomicReference; +public class SuperInitFails extends AtomicReference implements Iterable { + + private int x; + +/// GOOD EXAMPLES + + public SuperInitFails() { // this should be OK + } + + public SuperInitFails(Object x) { + this.x = x.hashCode(); // this should be OK + } + + public SuperInitFails(byte x) { + super(); // this should be OK + } + + public SuperInitFails(char x) { + this((int)x); // this should be OK + } + +/// FAIL EXAMPLES + + { + this(1); // this should FAIL + } + + { + super(); // this should FAIL + } + + void normalMethod1() { + super(); // this should FAIL + } + + void normalMethod2() { + this(); // this should FAIL + } + + void normalMethod3() { + Runnable r = () -> super(); // this should FAIL + } + + void normalMethod4() { + Runnable r = () -> this(); // this should FAIL + } + + public SuperInitFails(short x) { + hashCode(); // this should FAIL + super(); + } + + public SuperInitFails(float x) { + this.hashCode(); // this should FAIL + super(); + } + + public SuperInitFails(int x) { + super.hashCode(); // this should FAIL + super(); + } + + public SuperInitFails(long x) { + SuperInitFails.this.hashCode(); // this should FAIL + super(); + } + + public SuperInitFails(double x) { + SuperInitFails.super.hashCode(); // this should FAIL + super(); + } + + public SuperInitFails(byte[] x) { + { + super(); // this should FAIL + } + } + + public SuperInitFails(char[] x) { + if (x.length == 0) + return; // this should FAIL + super(); + } + + public SuperInitFails(short[] x) { + this.x = x.length; // this should FAIL + super(); + } + + public SuperInitFails(float[] x) { + System.identityHashCode(this); // this should FAIL + super(); + } + + public SuperInitFails(int[] x) { + this(this); // this should FAIL + } + + public SuperInitFails(long[] x) { + this(Object.this); // this should FAIL + } + + public SuperInitFails(double[] x) { + Iterable.super.spliterator(); // this should FAIL + super(); + } + + public SuperInitFails(byte[][] x) { + super(new Object() { + { + super(); // this should FAIL + } + }); + } + + public SuperInitFails(char[][] x) { + new Inner1(); // this should FAIL + super(); + } + + class Inner1 { + } + + record Record1(int value) { + Record1(float x) { // this should FAIL + } + } + + record Record2(int value) { + Record2(float x) { // this should FAIL + super(); + } + } + + @Override + public java.util.Iterator iterator() { + return null; + } +} diff --git a/test/langtools/tools/javac/SuperInit/SuperInitFails.out b/test/langtools/tools/javac/SuperInit/SuperInitFails.out new file mode 100644 index 0000000000000..f3fb8867e13fc --- /dev/null +++ b/test/langtools/tools/javac/SuperInit/SuperInitFails.out @@ -0,0 +1,23 @@ +SuperInitFails.java:57:9: compiler.err.cant.ref.before.ctor.called: hashCode() +SuperInitFails.java:62:9: compiler.err.cant.ref.before.ctor.called: this +SuperInitFails.java:67:9: compiler.err.cant.ref.before.ctor.called: super +SuperInitFails.java:72:23: compiler.err.cant.ref.before.ctor.called: this +SuperInitFails.java:77:23: compiler.err.cant.ref.before.ctor.called: super +SuperInitFails.java:94:9: compiler.err.cant.ref.before.ctor.called: this +SuperInitFails.java:99:33: compiler.err.cant.ref.before.ctor.called: this +SuperInitFails.java:104:14: compiler.err.cant.ref.before.ctor.called: this +SuperInitFails.java:108:20: compiler.err.not.encl.class: java.lang.Object +SuperInitFails.java:112:17: compiler.err.cant.ref.before.ctor.called: super +SuperInitFails.java:119:22: compiler.err.call.must.only.appear.in.ctor: super +SuperInitFails.java:125:9: compiler.err.cant.ref.before.ctor.called: this +SuperInitFails.java:133:9: compiler.err.non.canonical.constructor.invoke.another.constructor: SuperInitFails.Record1 +SuperInitFails.java:138:9: compiler.err.non.canonical.constructor.invoke.another.constructor: SuperInitFails.Record2 +SuperInitFails.java:33:13: compiler.err.call.must.only.appear.in.ctor: this +SuperInitFails.java:37:14: compiler.err.call.must.only.appear.in.ctor: super +SuperInitFails.java:41:14: compiler.err.call.must.only.appear.in.ctor: super +SuperInitFails.java:45:13: compiler.err.call.must.only.appear.in.ctor: this +SuperInitFails.java:49:33: compiler.err.call.must.only.appear.in.ctor: super +SuperInitFails.java:53:32: compiler.err.call.must.only.appear.in.ctor: this +SuperInitFails.java:83:18: compiler.err.calls.not.allowed.here: super +SuperInitFails.java:89:13: compiler.err.return.before.superclass.initialized: super +22 errors diff --git a/test/langtools/tools/javac/SuperInit/SuperInitGood.java b/test/langtools/tools/javac/SuperInit/SuperInitGood.java new file mode 100644 index 0000000000000..8f9379562b003 --- /dev/null +++ b/test/langtools/tools/javac/SuperInit/SuperInitGood.java @@ -0,0 +1,451 @@ +/* + * Copyright (c) 2022, 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 8194743 + * @summary Test valid placements of super()/this() in constructors + */ + +import java.util.concurrent.atomic.AtomicReference; + +public class SuperInitGood { + + SuperInitGood(Object obj) { + } + + SuperInitGood(int x) { + } + + // Default constructor provided by compiler + static class Test0 { + } + + // No explicit calls to this()/super() + static class Test1 { + Test1() { + } + Test1(int a) { + this.hashCode(); + } + } + + // Explicit calls to this()/super() + static class Test2 { + static int i; + Test2() { + this(0); + } + Test2(int i) { + Test2.i = i; + super(); + } + Test2(T obj) { + this(java.util.Objects.hashCode(obj)); + } + public T get() { + return null; + } + } + + // Explicit this()/super() with stuff in front + static class Test3 { + int x; + final int y; + final int z; + + Test3() { + new Object().hashCode(); + new Object().hashCode(); + super(); + this.x = new Object().hashCode(); + this.y = new Object().hashCode() % 17; + this.z = this.x + this.y; + } + } + + // Reference within constructor to outer class that's also my superclass + class Test5 extends SuperInitGood { + Test5(Object obj) { + if (obj == null) + throw new IllegalArgumentException(); + super(SuperInitGood.this); // NOT a 'this' reference + } + } + + // Initialization blocks + class Test6 { + final long startTime; + final int x; + { + this.x = 12; + } + Test6() { + long now = System.nanoTime(); + long then = now + 1000000L; + while (System.nanoTime() < then) { + try { + Thread.sleep(1); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + break; + } + } + super(); + this.startTime = now; + } + } + + // Mix up inner classes, proxies, and super() calls + // Copied mostly from UnverifiableInitForNestedLocalClassTest.java + public static void test7(final String arg) { + final String inlined = " inlined "; + class LocalClass { + String m() { + return "LocalClass " + arg + inlined; + } + + class SubClass extends LocalClass { + @Override + String m() { + return "SubClass " + arg + inlined; + } + } + + class SubSubClass extends SubClass { + @Override + String m() { + return "SubSubClass " + arg + inlined; + } + } + + class AnotherLocal { + class AnotherSub extends LocalClass { + AnotherSub() { + } + AnotherSub(int x) { + this((char)x); + } + AnotherSub(char y) { + super(); + } + @Override + String m() { + return "AnotherSub " + arg + inlined; + } + } + } + } + } + + // Anonymous inner class + public static void test8() { + new Test2(null) { + @Override + public Byte get() { + return (byte)-1; + } + }; + } + + // Qualified super() invocation + public static class Test9 extends Test5 { + + public Test9(SuperInitGood implicit, Object obj) { + obj.hashCode(); + implicit.super(obj); + } + } + + // Copied from WhichImplicitThis6 + public static class Test10 { + private int i; + public Test10(int i) {} + public class Sub extends Test10 { + public Sub() { + super(i); // i is not inherited, so it is the enclosing i + } + } + } + + // Two constructors where only one invokes super() + public static class Test11 { + public Test11() { + } + public Test11(int x) { + super(); + } + } + + // Nested version of the previous test + public static class Test12 { + Test12() { + class Sub { + public Sub() { + } + public Sub(int j) { + super(); + } + } + } + } + + // Nested super()'s requiring initialization code appended + public static class Test13 extends SuperInitGood { + final int x = new Object().hashCode(); + Test13() { + super(new Object() { + public void foo() { + class Bar { + final int y = new Object().hashCode(); + Bar() { + super(); + } + Bar(int ignored) { + } + } + } + }); + } + } + + // Initializer in initializer block + public static class Test14 { + final int x; // initialized in constructor + final int y; // initialized in initialization block + final int z = 13; // initialized with intializer value + public Test14() { + this(0); + } + public Test14(boolean z) { + this.x = z ? 1 : 0; + } + public Test14(int x) { + super(); + this.x = x; + } + { + this.y = -1; + } + } + + // Qualified super() invocation with superclass instance + public static class Test15 { + + final String name; + + public Test15(String name) { + this.name = name; + } + + public class Test15b extends Test15 { + + public Test15b(String name) { + super(name); + } + + public String getName() { + return Test15.this.name; + } + } + } + + public static class Test15c extends Test15.Test15b { + public Test15c(Test15 a, String name) { + a.super(name); + } + } + + // Mixing up outer instances, proxies, and initializers + public static class Test16 { + + final String x = String.valueOf(new Object().hashCode()); + + public void run() { + + final String y = String.valueOf(new Object().hashCode()); + + class Sub { + + final String z; + + Sub(String z, int ignored) { + this(z, (float)ignored); + } + + Sub(String z, float ignored) { + this.z = z; + } + + Sub(String z, byte ignored) { + super(); + this.z = z; + } + + Sub(String z, char ignored) { + this(z, (int)ignored); + } + + String x() { + return x; + } + + String y() { + return y; + } + + String z() { + return z; + } + } + + final String z = String.valueOf(new Object().hashCode()); + + final Sub[] subs = new Sub[] { + new Sub(z, 1), + new Sub(z, -1), + new Sub(z, (float)0), + new Sub(z, (byte)0), + new Sub(z, (char)0) + }; + + for (int i = 0; i < subs.length; i++) { + //System.err.println("i = " + i); + final Sub sub = subs[i]; + final String subx = sub.x(); + final String suby = sub.y(); + final String subz = sub.z(); + if (!x.equals(subx)) + throw new RuntimeException("x=" + x + " but sub[" + i + "].x()=" + subx); + if (!y.equals(suby)) + throw new RuntimeException("y=" + y + " but sub[" + i + "].y()=" + suby); + if (!z.equals(subz)) + throw new RuntimeException("z=" + z + " but sub[" + i + "].z()=" + subz); + } + } + } + + // Records + public class Test17 { + + record Rectangle(float length, float width) { } + + record StringHolder(String string) { + StringHolder { + java.util.Objects.requireNonNull(string); + } + } + + record ValueHolder(int value) { + ValueHolder(float x) { + if (Float.isNaN(x)) + throw new IllegalArgumentException(); + this((int)x); + } + } + } + + // Exceptions thrown by initializer block + public static class Test18 extends AtomicReference { + + { + if ((this.get().hashCode() % 3) == 0) + throw new MyException(); + } + + public Test18(Object obj) throws MyException { + super(obj); + } + + public Test18(boolean fail) throws MyException { + Object obj; + for (obj = new Object(); true; obj = new Object()) { + if (((obj.hashCode() % 3) == 0) != fail) + continue; + break; + } + this(obj); + } + + public static class MyException extends Exception { + } + } + + // super()/this() within outer try block but inside inner class + public static class Test19 { + public Test19(int x) { + try { + new Test1(x) { + @Override + public int hashCode() { + return x ^ super.hashCode(); + } + }; + } catch (StackOverflowError e) { + // ignore + } + } + } + + public static void main(String[] args) { + new Test0(); + new Test1(); + new Test1(7); + new Test2(); + new Test2<>(args); + new Test3(); + new SuperInitGood(3).new Test5(3); + new SuperInitGood(3).new Test6(); + SuperInitGood.test7("foo"); + SuperInitGood.test8(); + new Test9(new SuperInitGood(5), "abc"); + new Test10(7); + new Test11(9); + new Test12(); + new Test13(); + Test14 t14 = new Test14(); + assert t14.x == 0 && t14.y == -1 && t14.z == 13; + t14 = new Test14(7); + assert t14.x == 7 && t14.y == -1 && t14.z == 13; + new Test15c(new Test15("foo"), "bar"); + new Test16().run(); + new Test17.StringHolder("foo"); + try { + new Test17.StringHolder(null); + throw new Error(); + } catch (NullPointerException e) { + // expected + } + try { + new Test18(true); + assert false : "expected exception"; + } catch (Test18.MyException e) { + // expected + } + try { + new Test18(false); + } catch (Test18.MyException e) { + assert false : "unexpected exception: " + e; + } + new Test19(123); + } +} diff --git a/test/langtools/tools/javac/diags/examples/CallMustBeFirst.java b/test/langtools/tools/javac/diags/examples/CallOnlyInConstructor.java similarity index 84% rename from test/langtools/tools/javac/diags/examples/CallMustBeFirst.java rename to test/langtools/tools/javac/diags/examples/CallOnlyInConstructor.java index 3f0987062450c..f1018be46cebe 100644 --- a/test/langtools/tools/javac/diags/examples/CallMustBeFirst.java +++ b/test/langtools/tools/javac/diags/examples/CallOnlyInConstructor.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2023, 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,11 +21,10 @@ * questions. */ -// key: compiler.err.call.must.be.first.stmt.in.ctor +// key: compiler.err.call.must.only.appear.in.ctor -class CallMustBeFirst { - CallMustBeFirst() { - int i = 0; +class CallOnlyInConstructor { + void foo() { super(); } } diff --git a/test/langtools/tools/javac/diags/examples/CallsNotAllowedHere.java b/test/langtools/tools/javac/diags/examples/CallsNotAllowedHere.java new file mode 100644 index 0000000000000..bd608030ba37a --- /dev/null +++ b/test/langtools/tools/javac/diags/examples/CallsNotAllowedHere.java @@ -0,0 +1,32 @@ +/* + * Copyright (c) 2023, 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. + */ + +// key: compiler.err.calls.not.allowed.here + +class CallsNotAllowedHere { + public CallsNotAllowedHere() { + { + super(); + } + } +} diff --git a/test/langtools/tools/javac/diags/examples/FirstInvocationMustBeAnotherConstructor.java b/test/langtools/tools/javac/diags/examples/FirstInvocationMustBeAnotherConstructor.java index e2478244cc90c..da7b69ba16cd1 100644 --- a/test/langtools/tools/javac/diags/examples/FirstInvocationMustBeAnotherConstructor.java +++ b/test/langtools/tools/javac/diags/examples/FirstInvocationMustBeAnotherConstructor.java @@ -21,7 +21,7 @@ * questions. */ -// key: compiler.err.first.statement.must.be.call.to.another.constructor +// key: compiler.err.non.canonical.constructor.invoke.another.constructor record R(int x) { public R(int x, int y) { this.x = x; } diff --git a/test/langtools/tools/javac/diags/examples/RedundantSuperclassInit.java b/test/langtools/tools/javac/diags/examples/RedundantSuperclassInit.java new file mode 100644 index 0000000000000..82c5faf1d0443 --- /dev/null +++ b/test/langtools/tools/javac/diags/examples/RedundantSuperclassInit.java @@ -0,0 +1,31 @@ +/* + * Copyright (c) 2023, 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. + */ + +// key: compiler.err.redundant.superclass.init + +class RedundantSuperclassInit { + RedundantSuperclassInit() { + super(); + super(); + } +} diff --git a/test/langtools/tools/javac/diags/examples/ReturnBeforeSuperclassInit.java b/test/langtools/tools/javac/diags/examples/ReturnBeforeSuperclassInit.java new file mode 100644 index 0000000000000..033a843062560 --- /dev/null +++ b/test/langtools/tools/javac/diags/examples/ReturnBeforeSuperclassInit.java @@ -0,0 +1,32 @@ +/* + * Copyright (c) 2023, 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. + */ + +// key: compiler.err.return.before.superclass.initialized + +class ReturnBeforeSuperclassInit { + ReturnBeforeSuperclassInit(boolean maybe) { + if (maybe) + return; + super(); + } +} diff --git a/test/langtools/tools/javac/records/RecordCompilationTests.java b/test/langtools/tools/javac/records/RecordCompilationTests.java index 656246620a11f..b8f5628d5a7ed 100644 --- a/test/langtools/tools/javac/records/RecordCompilationTests.java +++ b/test/langtools/tools/javac/records/RecordCompilationTests.java @@ -409,11 +409,11 @@ record R(String v) { assertFail("compiler.err.invalid.canonical.constructor.in.record", "record R(int x, int y) { public R(int y, int x) { this.x = this.y = 0; }}"); - // first invocation should be one to the canonical - assertFail("compiler.err.first.statement.must.be.call.to.another.constructor", + // constructor is not canonical, so it must only invoke another constructor + assertFail("compiler.err.non.canonical.constructor.invoke.another.constructor", "record R(int x, int y) { public R(int y, int x, int z) { this.x = this.y = 0; } }"); - assertFail("compiler.err.first.statement.must.be.call.to.another.constructor", + assertFail("compiler.err.non.canonical.constructor.invoke.another.constructor", "record R(int x, int y) { public R(int y, int x, int z) { super(); this.x = this.y = 0; } }"); assertOK("record R(int x, int y) { " + From 61c0eacb808512db05abfea8ce954c9e0e8f7b0a Mon Sep 17 00:00:00 2001 From: "Archie L. Cobbs" Date: Mon, 20 Mar 2023 14:12:45 -0500 Subject: [PATCH 02/24] Use for() loop instead of stream for efficiency. --- .../classes/com/sun/tools/javac/comp/Lower.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java index 47c8db513e34d..55199725547fd 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java @@ -2263,14 +2263,15 @@ public void visitClassDef(JCClassDecl tree) { tree.defs = tree.defs.prepend(otdef); enterSynthetic(tree.pos(), otdef.sym, currentClass.members()); - tree.defs.stream() - .filter(TreeInfo::isConstructor) - .map(JCMethodDecl.class::cast) - .filter(mdef -> TreeInfo.hasConstructorCall(mdef, names._super)) - .forEach(mdef -> { - List initializer = List.of(initOuterThis(mdef.body.pos, mdef.params.head.sym)); - TreeInfo.mapSuperCalls(mdef.body, supercall -> make.Block(0, initializer.append(supercall))); - }); + for (JCTree def : tree.defs) { + if (TreeInfo.isConstructor(def)) { + JCMethodDecl mdef = (JCMethodDecl)def; + if (TreeInfo.hasConstructorCall(mdef, names._super)) { + List initializer = List.of(initOuterThis(mdef.body.pos, mdef.params.head.sym)); + TreeInfo.mapSuperCalls(mdef.body, supercall -> make.Block(0, initializer.append(supercall))); + } + } + } } proxies = prevProxies; From ceb41f8c481d2123a2a9943fae6a3874327ce6e0 Mon Sep 17 00:00:00 2001 From: "Archie L. Cobbs" Date: Mon, 24 Apr 2023 20:57:45 -0500 Subject: [PATCH 03/24] Fix Javadoc comment. --- .../share/classes/com/sun/tools/javac/comp/Resolve.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java index 640b197ed0255..5718c4f2757fe 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java @@ -4610,7 +4610,8 @@ JCDiagnostic getDiagnostic(JCDiagnostic.DiagnosticType dkind, } /** - * Specialization of {@link StaticError} for accesses in a constructor prologue. + * Specialization of {@link InvalidSymbolError} for illegal + * early accesses within a constructor prologue. */ class RefBeforeCtorCalledError extends InvalidSymbolError { From 7c874ebb81f1f97510acc9bde63f203d9f0cf9de Mon Sep 17 00:00:00 2001 From: "Archie L. Cobbs" Date: Tue, 25 Apr 2023 11:06:24 -0500 Subject: [PATCH 04/24] Fix typo in comment. --- .../share/classes/com/sun/tools/javac/comp/Check.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java index 50f23d34aa3d6..bfefeb54636e8 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java @@ -3943,7 +3943,7 @@ private class SuperThisChecker extends TreeScanner { // Match this scan stack: 1=JCMethodDecl, 2=JCExpressionStatement, 3=JCMethodInvocation private static final int MATCH_SCAN_DEPTH = 3; - private boolean constructor; // this this method a constructor? + private boolean constructor; // is this method a constructor? private JCReturn earlyReturn; // first return prior to the super()/init(), if any private Name initCall; // whichever of "super" or "init" we've seen already private int scanDepth; // current scan recursion depth in method body From 0eac8b681ed9a95a8ab3affd8f5200dcc3fb26bb Mon Sep 17 00:00:00 2001 From: "Archie L. Cobbs" Date: Tue, 25 Apr 2023 11:07:24 -0500 Subject: [PATCH 05/24] Use for() loop instead of stream for efficiency. --- .../share/classes/com/sun/tools/javac/comp/Flow.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java index 2fd34c1e56504..90c585717e24c 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java @@ -481,10 +481,11 @@ protected void forEachInitializer(JCClassDecl classDef, boolean statik, Consumer JCClassDecl initScanClassPrev = initScanClass; initScanClass = classDef; try { - classDef.defs.stream() - .filter(def -> !def.hasTag(METHODDEF)) - .filter(def -> ((TreeInfo.flags(def) & STATIC) != 0) == statik) - .forEach(handler); + for (List defs = classDef.defs; defs.nonEmpty(); defs = defs.tail) { + JCTree def = defs.head; + if (!def.hasTag(METHODDEF) && ((TreeInfo.flags(def) & STATIC) != 0) == statik) + handler.accept(def); + } } finally { initScanClass = initScanClassPrev; } From 0b8a8c7508944c10ab9eb79284e966a4f6155f34 Mon Sep 17 00:00:00 2001 From: "Archie L. Cobbs" Date: Tue, 25 Apr 2023 21:05:44 -0500 Subject: [PATCH 06/24] Fix typo in comment. --- .../share/classes/com/sun/tools/javac/comp/Attr.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java index bc12475eae667..2b93e122de4dd 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java @@ -5588,7 +5588,7 @@ private void attribClassBody(Env env, ClassSymbol c) { } } - // Check for proper placement of super()/init() calls. + // Check for proper placement of super()/this() calls. chk.checkSuperInitCalls(tree); // Check for cycles among non-initial constructors. From 0e638da2daa59fb1ab60622e216d2a1dcd3937ee Mon Sep 17 00:00:00 2001 From: "Archie L. Cobbs" Date: Tue, 25 Apr 2023 21:07:06 -0500 Subject: [PATCH 07/24] Small refactoring to avoid redundant test. --- .../com/sun/tools/javac/comp/Flow.java | 32 +++++++++++-------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java index 90c585717e24c..4583a3a53a568 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java @@ -2840,21 +2840,25 @@ public void visitApply(JCMethodInvocation tree) { scanExpr(tree.meth); scanExprs(tree.args); - // If super(): at this point all initialization blocks will execute - Name name = TreeInfo.name(tree.meth); - if (isConstructor && name == names._super) { - forEachInitializer(classDef, false, def -> { - scan(def); - clearPendingExits(false); - }); - } + // Handle superclass constructor invocations + if (isConstructor) { - // If this(): at this point all final uninitialized fields will get initialized - if (isConstructor && name == names._this) { - for (int address = firstadr; address < nextadr; address++) { - VarSymbol sym = vardecls[address].sym; - if (isFinalUninitializedField(sym) && !sym.isStatic()) - letInit(tree.pos(), sym); + // If super(): at this point all initialization blocks will execute + Name name = TreeInfo.name(tree.meth); + if (name == names._super) { + forEachInitializer(classDef, false, def -> { + scan(def); + clearPendingExits(false); + }); + } + + // If this(): at this point all final uninitialized fields will get initialized + else if (name == names._this) { + for (int address = firstadr; address < nextadr; address++) { + VarSymbol sym = vardecls[address].sym; + if (isFinalUninitializedField(sym) && !sym.isStatic()) + letInit(tree.pos(), sym); + } } } } From 3c9322b61ee8cd6d045f14c48238a6dce64aa98a Mon Sep 17 00:00:00 2001 From: "Archie L. Cobbs" Date: Tue, 16 May 2023 16:30:39 -0500 Subject: [PATCH 08/24] Make "statements before super()" support a preview feature. Thanks to Jim Laskey for help with preview logic. --- .../com/sun/tools/javac/code/Preview.java | 1 + .../com/sun/tools/javac/code/Source.java | 1 + .../com/sun/tools/javac/comp/Check.java | 15 +++++++-- .../tools/javac/resources/compiler.properties | 3 ++ .../tools/javac/SuperInit/SuperInitFails.java | 2 +- .../tools/javac/SuperInit/SuperInitFails.out | 2 ++ .../tools/javac/SuperInit/SuperInitGood.java | 1 + .../examples/ReturnBeforeSuperclassInit.java | 3 ++ .../diags/examples/StatementsBeforeSuper.java | 33 +++++++++++++++++++ 9 files changed, 58 insertions(+), 3 deletions(-) create mode 100644 test/langtools/tools/javac/diags/examples/StatementsBeforeSuper.java diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Preview.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Preview.java index fc023219b33d8..8aa8aca924d9c 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Preview.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Preview.java @@ -213,6 +213,7 @@ public boolean isPreview(Feature feature) { case PATTERN_SWITCH -> true; case UNCONDITIONAL_PATTERN_IN_INSTANCEOF -> true; case RECORD_PATTERNS -> true; + case SUPER_INIT -> true; //Note: this is a backdoor which allows to optionally treat all features as 'preview' (for testing). //When real preview features will be added, this method can be implemented to return 'true' diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Source.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Source.java index a8d99141ac608..8730fa95fb5c8 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Source.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Source.java @@ -236,6 +236,7 @@ public enum Feature { REDUNDANT_STRICTFP(JDK17), UNCONDITIONAL_PATTERN_IN_INSTANCEOF(JDK19, Fragments.FeatureUnconditionalPatternsInInstanceof, DiagKind.PLURAL), RECORD_PATTERNS(JDK19, Fragments.FeatureDeconstructionPatterns, DiagKind.PLURAL), + SUPER_INIT(JDK21, Fragments.FeatureSuperInit, DiagKind.NORMAL), WARN_ON_ILLEGAL_UTF8(MIN, JDK21), ; diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java index bfefeb54636e8..a0f75f703922f 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java @@ -3944,6 +3944,7 @@ private class SuperThisChecker extends TreeScanner { private static final int MATCH_SCAN_DEPTH = 3; private boolean constructor; // is this method a constructor? + private boolean firstStatement; // at the first statement in method? private JCReturn earlyReturn; // first return prior to the super()/init(), if any private Name initCall; // whichever of "super" or "init" we've seen already private int scanDepth; // current scan recursion depth in method body @@ -3964,13 +3965,19 @@ public void visitMethodDef(JCMethodDecl tree) { try { // Scan method body - if (tree.body != null) - scan(tree.body.stats); + if (tree.body != null) { + firstStatement = true; + for (List l = tree.body.stats; l.nonEmpty(); l = l.tail) { + scan(l.head); + firstStatement = false; + } + } // Verify no 'return' seen prior to an explicit super()/this() call if (constructor && earlyReturn != null && initCall != null) log.error(earlyReturn.pos(), Errors.ReturnBeforeSuperclassInitialized(initCall)); } finally { + firstStatement = false; constructor = false; earlyReturn = null; initCall = null; @@ -4014,6 +4021,10 @@ public void visitApply(JCMethodInvocation apply) { break; } + // If super()/this() isn't first, require "statements before super()" feature + if (!firstStatement) + preview.checkSourceLevel(apply.pos(), Feature.SUPER_INIT); + // We found a legitimate super()/this() call; remember it initCall = methodName; } while (false); diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties index aa2bf6e6413fc..5193266ae66fe 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties @@ -3142,6 +3142,9 @@ compiler.misc.feature.pattern.switch=\ compiler.misc.feature.unconditional.patterns.in.instanceof=\ unconditional patterns in instanceof +compiler.misc.feature.super.init=\ + statements before super() + compiler.warn.underscore.as.identifier=\ as of release 9, ''_'' is a keyword, and may not be used as an identifier diff --git a/test/langtools/tools/javac/SuperInit/SuperInitFails.java b/test/langtools/tools/javac/SuperInit/SuperInitFails.java index b133db710d697..dfa50cdc98575 100644 --- a/test/langtools/tools/javac/SuperInit/SuperInitFails.java +++ b/test/langtools/tools/javac/SuperInit/SuperInitFails.java @@ -3,7 +3,7 @@ * @bug 8194743 * @summary Permit additional statements before this/super in constructors * - * @compile/fail/ref=SuperInitFails.out -XDrawDiagnostics SuperInitFails.java + * @compile/fail/ref=SuperInitFails.out -XDrawDiagnostics --enable-preview -source ${jdk.version} SuperInitFails.java */ import java.util.concurrent.atomic.AtomicReference; public class SuperInitFails extends AtomicReference implements Iterable { diff --git a/test/langtools/tools/javac/SuperInit/SuperInitFails.out b/test/langtools/tools/javac/SuperInit/SuperInitFails.out index f3fb8867e13fc..123fc3a027d96 100644 --- a/test/langtools/tools/javac/SuperInit/SuperInitFails.out +++ b/test/langtools/tools/javac/SuperInit/SuperInitFails.out @@ -20,4 +20,6 @@ SuperInitFails.java:49:33: compiler.err.call.must.only.appear.in.ctor: super SuperInitFails.java:53:32: compiler.err.call.must.only.appear.in.ctor: this SuperInitFails.java:83:18: compiler.err.calls.not.allowed.here: super SuperInitFails.java:89:13: compiler.err.return.before.superclass.initialized: super +- compiler.note.preview.filename: SuperInitFails.java, DEFAULT +- compiler.note.preview.recompile 22 errors diff --git a/test/langtools/tools/javac/SuperInit/SuperInitGood.java b/test/langtools/tools/javac/SuperInit/SuperInitGood.java index 8f9379562b003..8b00c518b353a 100644 --- a/test/langtools/tools/javac/SuperInit/SuperInitGood.java +++ b/test/langtools/tools/javac/SuperInit/SuperInitGood.java @@ -24,6 +24,7 @@ * @test * @bug 8194743 * @summary Test valid placements of super()/this() in constructors + * @compile --enable-preview -source ${jdk.version} SuperInitGood.java */ import java.util.concurrent.atomic.AtomicReference; diff --git a/test/langtools/tools/javac/diags/examples/ReturnBeforeSuperclassInit.java b/test/langtools/tools/javac/diags/examples/ReturnBeforeSuperclassInit.java index 033a843062560..dc883b59b8f69 100644 --- a/test/langtools/tools/javac/diags/examples/ReturnBeforeSuperclassInit.java +++ b/test/langtools/tools/javac/diags/examples/ReturnBeforeSuperclassInit.java @@ -22,6 +22,9 @@ */ // key: compiler.err.return.before.superclass.initialized +// key: compiler.note.preview.filename +// key: compiler.note.preview.recompile +// options: --enable-preview -source ${jdk.version} class ReturnBeforeSuperclassInit { ReturnBeforeSuperclassInit(boolean maybe) { diff --git a/test/langtools/tools/javac/diags/examples/StatementsBeforeSuper.java b/test/langtools/tools/javac/diags/examples/StatementsBeforeSuper.java new file mode 100644 index 0000000000000..a24edee5802df --- /dev/null +++ b/test/langtools/tools/javac/diags/examples/StatementsBeforeSuper.java @@ -0,0 +1,33 @@ +/* + * Copyright (c) 2023, 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. + */ + + // key: compiler.misc.feature.super.init + // key: compiler.warn.preview.feature.use + // options: --enable-preview -source ${jdk.version} -Xlint:preview + +class StatementsBeforeSuper { + StatementsBeforeSuper() { + System.out.println(); + super(); + } +} From 73bb327083672bd80e6cd7feab42a85233aadcd6 Mon Sep 17 00:00:00 2001 From: "Archie L. Cobbs" Date: Tue, 16 May 2023 21:26:42 -0500 Subject: [PATCH 09/24] Use @enablePreview in tests in preference to explicit command line flags. --- test/langtools/tools/javac/SuperInit/SuperInitFails.java | 4 ++-- test/langtools/tools/javac/SuperInit/SuperInitGood.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/langtools/tools/javac/SuperInit/SuperInitFails.java b/test/langtools/tools/javac/SuperInit/SuperInitFails.java index dfa50cdc98575..bdf0e1fe5f727 100644 --- a/test/langtools/tools/javac/SuperInit/SuperInitFails.java +++ b/test/langtools/tools/javac/SuperInit/SuperInitFails.java @@ -2,8 +2,8 @@ * @test /nodynamiccopyright/ * @bug 8194743 * @summary Permit additional statements before this/super in constructors - * - * @compile/fail/ref=SuperInitFails.out -XDrawDiagnostics --enable-preview -source ${jdk.version} SuperInitFails.java + * @compile/fail/ref=SuperInitFails.out -XDrawDiagnostics SuperInitFails.java + * @enablePreview */ import java.util.concurrent.atomic.AtomicReference; public class SuperInitFails extends AtomicReference implements Iterable { diff --git a/test/langtools/tools/javac/SuperInit/SuperInitGood.java b/test/langtools/tools/javac/SuperInit/SuperInitGood.java index 8b00c518b353a..f82ee6188f1fe 100644 --- a/test/langtools/tools/javac/SuperInit/SuperInitGood.java +++ b/test/langtools/tools/javac/SuperInit/SuperInitGood.java @@ -24,7 +24,7 @@ * @test * @bug 8194743 * @summary Test valid placements of super()/this() in constructors - * @compile --enable-preview -source ${jdk.version} SuperInitGood.java + * @enablePreview */ import java.util.concurrent.atomic.AtomicReference; From eb1eb5a068e822cd11b15dfebeda737b31ce6575 Mon Sep 17 00:00:00 2001 From: "Archie L. Cobbs" Date: Tue, 23 May 2023 15:08:50 -0500 Subject: [PATCH 10/24] Add unit tests with local class decl's prior to super(). --- .../tools/javac/SuperInit/SuperInitFails.java | 10 ++++++++++ .../tools/javac/SuperInit/SuperInitFails.out | 3 ++- .../tools/javac/SuperInit/SuperInitGood.java | 14 ++++++++++++++ 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/test/langtools/tools/javac/SuperInit/SuperInitFails.java b/test/langtools/tools/javac/SuperInit/SuperInitFails.java index bdf0e1fe5f727..7f5fb27cc47a9 100644 --- a/test/langtools/tools/javac/SuperInit/SuperInitFails.java +++ b/test/langtools/tools/javac/SuperInit/SuperInitFails.java @@ -144,4 +144,14 @@ record Record2(int value) { public java.util.Iterator iterator() { return null; } + + public SuperInitFails(short[][] x) { + class Foo { + Foo() { + SuperInitFails.this.hashCode(); + } + }; + new Foo(); // this should FAIL + super(); + } } diff --git a/test/langtools/tools/javac/SuperInit/SuperInitFails.out b/test/langtools/tools/javac/SuperInit/SuperInitFails.out index 123fc3a027d96..bd0b8314eec9c 100644 --- a/test/langtools/tools/javac/SuperInit/SuperInitFails.out +++ b/test/langtools/tools/javac/SuperInit/SuperInitFails.out @@ -12,6 +12,7 @@ SuperInitFails.java:119:22: compiler.err.call.must.only.appear.in.ctor: super SuperInitFails.java:125:9: compiler.err.cant.ref.before.ctor.called: this SuperInitFails.java:133:9: compiler.err.non.canonical.constructor.invoke.another.constructor: SuperInitFails.Record1 SuperInitFails.java:138:9: compiler.err.non.canonical.constructor.invoke.another.constructor: SuperInitFails.Record2 +SuperInitFails.java:154:9: compiler.err.cant.ref.before.ctor.called: this SuperInitFails.java:33:13: compiler.err.call.must.only.appear.in.ctor: this SuperInitFails.java:37:14: compiler.err.call.must.only.appear.in.ctor: super SuperInitFails.java:41:14: compiler.err.call.must.only.appear.in.ctor: super @@ -22,4 +23,4 @@ SuperInitFails.java:83:18: compiler.err.calls.not.allowed.here: super SuperInitFails.java:89:13: compiler.err.return.before.superclass.initialized: super - compiler.note.preview.filename: SuperInitFails.java, DEFAULT - compiler.note.preview.recompile -22 errors +23 errors diff --git a/test/langtools/tools/javac/SuperInit/SuperInitGood.java b/test/langtools/tools/javac/SuperInit/SuperInitGood.java index f82ee6188f1fe..4409cfdb44b7b 100644 --- a/test/langtools/tools/javac/SuperInit/SuperInitGood.java +++ b/test/langtools/tools/javac/SuperInit/SuperInitGood.java @@ -407,6 +407,19 @@ public int hashCode() { } } + // local class declared before super(), but not used until after super() + public static class Test20 { + public Test20() { + class Foo { + Foo() { + Test20.this.hashCode(); + } + } + super(); + new Foo(); + } + } + public static void main(String[] args) { new Test0(); new Test1(); @@ -448,5 +461,6 @@ public static void main(String[] args) { assert false : "unexpected exception: " + e; } new Test19(123); + new Test20(); } } From 9a2cb457c66f480413bed835e2d327f4947110e7 Mon Sep 17 00:00:00 2001 From: "Archie L. Cobbs" Date: Tue, 23 May 2023 15:16:36 -0500 Subject: [PATCH 11/24] Update unit test after merged-in commit eaa80ad08. --- test/langtools/tools/javac/diags/examples.not-yet.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/test/langtools/tools/javac/diags/examples.not-yet.txt b/test/langtools/tools/javac/diags/examples.not-yet.txt index 5ea7fdc1d6062..6e78ca644bf0f 100644 --- a/test/langtools/tools/javac/diags/examples.not-yet.txt +++ b/test/langtools/tools/javac/diags/examples.not-yet.txt @@ -141,7 +141,6 @@ compiler.warn.invalid.path # this warning is genera compiler.err.invalid.path # this error is generated only in Windows systems compiler.note.multiple.elements # needs user code compiler.err.preview.feature.disabled.classfile # preview feature support: needs compilation against classfile -compiler.warn.preview.feature.use # preview feature support: not generated currently compiler.warn.preview.feature.use.classfile # preview feature support: needs compilation against classfile compiler.note.preview.plural.additional # preview feature support: diag test causes intermittent failures (see JDK-8201498) compiler.misc.bad.intersection.target.for.functional.expr # currently not generated, should be removed? From 543a596a5cf7d092eaf9068e23b8b720e71769f2 Mon Sep 17 00:00:00 2001 From: "Archie L. Cobbs" Date: Tue, 23 May 2023 15:20:50 -0500 Subject: [PATCH 12/24] Rename unit test to be consistent with other feature exampless. --- ...entsBeforeSuper.java => FeatureStatementsBeforeSuper.java} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename test/langtools/tools/javac/diags/examples/{StatementsBeforeSuper.java => FeatureStatementsBeforeSuper.java} (94%) diff --git a/test/langtools/tools/javac/diags/examples/StatementsBeforeSuper.java b/test/langtools/tools/javac/diags/examples/FeatureStatementsBeforeSuper.java similarity index 94% rename from test/langtools/tools/javac/diags/examples/StatementsBeforeSuper.java rename to test/langtools/tools/javac/diags/examples/FeatureStatementsBeforeSuper.java index a24edee5802df..f0e44836850b4 100644 --- a/test/langtools/tools/javac/diags/examples/StatementsBeforeSuper.java +++ b/test/langtools/tools/javac/diags/examples/FeatureStatementsBeforeSuper.java @@ -25,8 +25,8 @@ // key: compiler.warn.preview.feature.use // options: --enable-preview -source ${jdk.version} -Xlint:preview -class StatementsBeforeSuper { - StatementsBeforeSuper() { +class FeatureStatementsBeforeSuper { + FeatureStatementsBeforeSuper() { System.out.println(); super(); } From 276e449d3d4f96fab8d3794f7879204b06627b64 Mon Sep 17 00:00:00 2001 From: "Archie L. Cobbs" Date: Fri, 26 May 2023 16:24:21 -0500 Subject: [PATCH 13/24] Fix mistake in previous merge commit 80ba6be4. --- .../share/classes/com/sun/tools/javac/code/Source.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Source.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Source.java index fc7efdce7a8f8..8ef442ff73ff1 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Source.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Source.java @@ -235,8 +235,8 @@ public enum Feature { CASE_NULL(JDK21, Fragments.FeatureCaseNull, DiagKind.NORMAL), PATTERN_SWITCH(JDK21, Fragments.FeaturePatternSwitch, DiagKind.PLURAL), REDUNDANT_STRICTFP(JDK17), - UNCONDITIONAL_PATTERN_IN_INSTANCEOF(JDK19, Fragments.FeatureUnconditionalPatternsInInstanceof, DiagKind.PLURAL), - RECORD_PATTERNS(JDK19, Fragments.FeatureDeconstructionPatterns, DiagKind.PLURAL), + UNCONDITIONAL_PATTERN_IN_INSTANCEOF(JDK21, Fragments.FeatureUnconditionalPatternsInInstanceof, DiagKind.PLURAL), + RECORD_PATTERNS(JDK21, Fragments.FeatureDeconstructionPatterns, DiagKind.PLURAL), STRING_TEMPLATES(JDK21, Fragments.FeatureStringTemplates, DiagKind.PLURAL), SUPER_INIT(JDK21, Fragments.FeatureSuperInit, DiagKind.NORMAL), WARN_ON_ILLEGAL_UTF8(MIN, JDK21), From c6cf80fc62138be5d156418608d3b56b446a3ac0 Mon Sep 17 00:00:00 2001 From: "Archie L. Cobbs" Date: Fri, 7 Jul 2023 14:02:40 -0500 Subject: [PATCH 14/24] Use TreeInfo.isConstructor() for detecting constructors. --- .../share/classes/com/sun/tools/javac/comp/Attr.java | 2 +- .../share/classes/com/sun/tools/javac/comp/Check.java | 6 ++---- .../share/classes/com/sun/tools/javac/jvm/Gen.java | 2 +- .../share/classes/com/sun/tools/javac/tree/TreeInfo.java | 5 ++--- 4 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java index 3a8253ef816c4..4b6c035ad1bb9 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java @@ -1050,7 +1050,7 @@ public void visitMethodDef(JCMethodDecl tree) { } // Is this method a constructor? - boolean isConstructor = tree.name == names.init; + boolean isConstructor = TreeInfo.isConstructor(tree); if (env.enclClass.sym.isRecord() && tree.sym.owner.kind == TYP) { // lets find if this method is an accessor diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java index dbe9d17790c8a..cedece2b34db9 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java @@ -3919,11 +3919,9 @@ void checkCyclicConstructors(JCClassDecl tree) { // enter each constructor this-call into the map for (List l = tree.defs; l.nonEmpty(); l = l.tail) { - if (!l.head.hasTag(METHODDEF)) + if (!TreeInfo.isConstructor(l.head)) continue; JCMethodDecl meth = (JCMethodDecl)l.head; - if (meth.name != names.init) - continue; JCMethodInvocation app = TreeInfo.findConstructorCall(meth); if (app != null && TreeInfo.name(app.meth) == names._this) { callMap.put(meth.sym, TreeInfo.symbol(app.meth)); @@ -3995,7 +3993,7 @@ public void visitMethodDef(JCMethodDecl tree) { Assert.check(scanDepth == 1); // Initialize state for this method - constructor = tree.name == names.init; + constructor = TreeInfo.isConstructor(tree); try { // Scan method body diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java index fd1058c9106a8..896990d20a5fa 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java @@ -548,7 +548,7 @@ private void checkStringConstant(DiagnosticPosition pos, Object constValue) { * @param initTAs Type annotations from the initializer expression. */ void normalizeMethod(JCMethodDecl md, List initCode, List initTAs) { - if (md.name == names.init && TreeInfo.hasConstructorCall(md, names._super)) { + if (TreeInfo.isConstructor(md) && TreeInfo.hasConstructorCall(md, names._super)) { // We are seeing a constructor that has a super() call. // Find the super() invocation and append the given initializer code. TreeInfo.mapSuperCalls(md.body, supercall -> make.Block(0, initCode.prepend(supercall))); diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeInfo.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeInfo.java index 9757fa2110109..d2e87f9a343a3 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeInfo.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeInfo.java @@ -237,10 +237,9 @@ public static boolean hasConstructorCall(JCMethodDecl tree, Name target) { /** Find the first super() or init() call in the given constructor. */ public static JCMethodInvocation findConstructorCall(JCMethodDecl md) { - Names names = md.name.table.names; - if (md.name != names.init || md.body == null) + if (!TreeInfo.isConstructor(md) || md.body == null) return null; - return new ConstructorCallFinder(names).find(md).head; + return new ConstructorCallFinder(md.name.table.names).find(md).head; } /** Finds all calls to this() and/or super() in a given constructor. From b0b13b2ad22f1d3a8a16e2ba368d925c7c050e46 Mon Sep 17 00:00:00 2001 From: "Archie L. Cobbs" Date: Fri, 7 Jul 2023 16:15:02 -0500 Subject: [PATCH 15/24] Add unit test verifying super() can't appear inside a lambda. --- test/langtools/tools/javac/SuperInit/SuperInitFails.java | 6 ++++++ test/langtools/tools/javac/SuperInit/SuperInitFails.out | 3 ++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/test/langtools/tools/javac/SuperInit/SuperInitFails.java b/test/langtools/tools/javac/SuperInit/SuperInitFails.java index 7f5fb27cc47a9..039564d729b2f 100644 --- a/test/langtools/tools/javac/SuperInit/SuperInitFails.java +++ b/test/langtools/tools/javac/SuperInit/SuperInitFails.java @@ -154,4 +154,10 @@ class Foo { new Foo(); // this should FAIL super(); } + + public SuperInitFails(float[][] x) { + Runnable r = () -> { + super(); // this should FAIL + }; + } } diff --git a/test/langtools/tools/javac/SuperInit/SuperInitFails.out b/test/langtools/tools/javac/SuperInit/SuperInitFails.out index bd0b8314eec9c..dc4ba76d2aa91 100644 --- a/test/langtools/tools/javac/SuperInit/SuperInitFails.out +++ b/test/langtools/tools/javac/SuperInit/SuperInitFails.out @@ -21,6 +21,7 @@ SuperInitFails.java:49:33: compiler.err.call.must.only.appear.in.ctor: super SuperInitFails.java:53:32: compiler.err.call.must.only.appear.in.ctor: this SuperInitFails.java:83:18: compiler.err.calls.not.allowed.here: super SuperInitFails.java:89:13: compiler.err.return.before.superclass.initialized: super +SuperInitFails.java:160:18: compiler.err.calls.not.allowed.here: super - compiler.note.preview.filename: SuperInitFails.java, DEFAULT - compiler.note.preview.recompile -23 errors +24 errors From 599d761b3099e6eeaaa07e5aefdeecf32808346a Mon Sep 17 00:00:00 2001 From: "Archie L. Cobbs" Date: Fri, 7 Jul 2023 16:25:13 -0500 Subject: [PATCH 16/24] Create and cache a single instance of the oft-used SuperThisChecker. --- .../classes/com/sun/tools/javac/comp/Check.java | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java index cedece2b34db9..9232d4fb93af3 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java @@ -128,6 +128,9 @@ public class Check { // Attr as it visits new method declarations. private MethodSymbol method; + // Cached instance of SuperThisChecker + private final SuperThisChecker superThisChecker; + public static Check instance(Context context) { Check instance = context.get(checkKey); if (instance == null) @@ -152,6 +155,7 @@ protected Check(Context context) { Options options = Options.instance(context); lint = Lint.instance(context); fileManager = context.get(JavaFileManager.class); + superThisChecker = new SuperThisChecker(); source = Source.instance(context); target = Target.instance(context); @@ -3967,7 +3971,7 @@ private void checkCyclicConstructor(JCClassDecl tree, Symbol ctor, **************************************************************************/ void checkSuperInitCalls(JCClassDecl tree) { - new SuperThisChecker().check(tree); + superThisChecker.check(tree); } private class SuperThisChecker extends TreeScanner { @@ -3981,7 +3985,16 @@ private class SuperThisChecker extends TreeScanner { private Name initCall; // whichever of "super" or "init" we've seen already private int scanDepth; // current scan recursion depth in method body + public void resetState() { + constructor = false; + firstStatement = false; + earlyReturn = null; + initCall = null; + scanDepth = 0; + } + public void check(JCClassDecl classDef) { + resetState(); scan(classDef.defs); } From 5cced39d49cbf9447dac6444820471188166333c Mon Sep 17 00:00:00 2001 From: "Archie L. Cobbs" Date: Wed, 6 Sep 2023 19:22:58 -0500 Subject: [PATCH 17/24] Eliminate "isSelfCall" flag, which is now made obsolete by "ctorPrologue". --- .../com/sun/tools/javac/comp/Attr.java | 40 ++++--------------- .../com/sun/tools/javac/comp/AttrContext.java | 5 --- .../com/sun/tools/javac/comp/Enter.java | 2 - .../com/sun/tools/javac/comp/Lower.java | 6 +-- .../com/sun/tools/javac/comp/Resolve.java | 2 +- .../com/sun/tools/javac/comp/TypeEnter.java | 1 - .../com/sun/tools/javac/tree/TreeInfo.java | 12 ------ .../AnonymousInSuperCallNegTest.out | 2 +- .../tools/javac/SuperInit/SuperInitFails.java | 8 ++++ .../tools/javac/SuperInit/SuperInitFails.out | 4 +- .../tools/javac/SuperInit/SuperInitGood.java | 14 +++++++ 11 files changed, 36 insertions(+), 60 deletions(-) diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java index b25e47dbcaa8c..4ad05e5f51d57 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java @@ -961,12 +961,10 @@ public void visitClassDef(JCClassDecl tree) { // make sure class has been completed: c.complete(); - // If this class appears as an anonymous class - // in a superclass constructor call - // disable implicit outer instance from being passed. + // If this class appears as an anonymous class in a constructor + // prologue, disable implicit outer instance from being passed. // (This would be an illegal access to "this before super"). - if (env.info.isSelfCall && - env.tree.hasTag(NEWCLASS)) { + if (ctorProloguePrev && env.tree.hasTag(NEWCLASS)) { c.flags_field |= NOOUTERTHIS; } attribClass(tree.pos(), c); @@ -2510,12 +2508,6 @@ public void visitApply(JCMethodInvocation tree) { ListBuffer argtypesBuf = new ListBuffer<>(); if (isConstructorCall) { - // We are seeing a this()/super() call. End of constructor prologue. - env.info.ctorPrologue = false; - - // Record the fact - // that this is a constructor call (using isSelfCall). - localEnv.info.isSelfCall = true; // Attribute arguments, yielding list of argument types. localEnv.info.constructorArgs = true; @@ -2524,6 +2516,9 @@ public void visitApply(JCMethodInvocation tree) { argtypes = argtypesBuf.toList(); typeargtypes = attribTypes(tree.typeargs, localEnv); + // Done with this()/super() parameters. End of constructor prologue. + env.info.ctorPrologue = false; + // Variable `site' points to the class in which the called // constructor is defined. Type site = env.enclClass.sym.type; @@ -4324,16 +4319,6 @@ public void visitIdent(JCIdent tree) { checkAssignable(tree.pos(), v, null, env); } - // In a constructor body, - // if symbol is a field or instance method, check that it is - // not accessed before the supertype constructor is called. - if (symEnv.info.isSelfCall && - sym.kind.matches(KindSelector.VAL_MTH) && - sym.owner.kind == TYP && - (sym.flags() & STATIC) == 0) { - chk.earlyRefError(tree.pos(), sym.kind == VAR ? - sym : thisSym(tree.pos(), env)); - } Env env1 = env; if (sym.kind != ERR && sym.kind != TYP && sym.owner != null && sym.owner != env1.enclClass.sym) { @@ -4445,18 +4430,7 @@ public void visitSelect(JCFieldAccess tree) { } if (isType(sitesym)) { - if (sym.name == names._this || sym.name == names._super) { - // If `C' is the currently compiled class, check that - // `C.this' does not appear in an explicit call to a constructor - // also make sure that `super` is not used in constructor invocations - if (env.info.isSelfCall && - ((sym.name == names._this && - site.tsym == env.enclClass.sym) || - sym.name == names._super && env.info.constructorArgs && - (sitesym.isInterface() || site.tsym == env.enclClass.sym))) { - chk.earlyRefError(tree.pos(), sym); - } - } else { + if (sym.name != names._this && sym.name != names._super) { // Check if type-qualified fields or methods are static (JLS) if ((sym.flags() & STATIC) == 0 && sym.name != names._super && diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/AttrContext.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/AttrContext.java index fdc9a575ce32a..d108c493ce3c8 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/AttrContext.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/AttrContext.java @@ -49,10 +49,6 @@ public class AttrContext { */ int staticLevel = 0; - /** Is this an environment for a this(...) or super(...) call? - */ - boolean isSelfCall = false; - /** are we analyzing the arguments for a constructor invocation? */ boolean constructorArgs = false; @@ -140,7 +136,6 @@ AttrContext dup(WriteableScope scope) { AttrContext info = new AttrContext(); info.scope = scope; info.staticLevel = staticLevel; - info.isSelfCall = isSelfCall; info.constructorArgs = constructorArgs; info.ctorPrologue = ctorPrologue; info.selectSuper = selectSuper; diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Enter.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Enter.java index a50788ed4b803..a76e7f6a09e84 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Enter.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Enter.java @@ -201,7 +201,6 @@ public Env classEnv(JCClassDecl tree, Env env) { env.dup(tree, env.info.dup(WriteableScope.create(tree.sym))); localEnv.enclClass = tree; localEnv.outer = env; - localEnv.info.isSelfCall = false; localEnv.info.lint = null; // leave this to be filled in by Attr, // when annotations have been processed localEnv.info.isAnonymousDiamond = TreeInfo.isDiamond(env.tree); @@ -254,7 +253,6 @@ public Env moduleEnv(JCModuleDecl tree, Env env) { env.dup(tree, env.info.dup(WriteableScope.create(tree.sym))); localEnv.enclClass = predefClassDef; localEnv.outer = env; - localEnv.info.isSelfCall = false; localEnv.info.lint = null; // leave this to be filled in by Attr, // when annotations have been processed return localEnv; diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java index ab888898326f8..20cfb85f91731 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java @@ -1886,8 +1886,7 @@ JCExpression makeOwnerThisN(DiagnosticPosition pos, Symbol sym, boolean preciseM Symbol c = sym.owner; List ots = outerThisStack; if (ots.isEmpty()) { - log.error(pos, Errors.NoEnclInstanceOfTypeInScope(c)); - Assert.error(); + chk.earlyRefError(pos, c); return makeNull(); } VarSymbol ot = ots.head; @@ -1898,8 +1897,7 @@ JCExpression makeOwnerThisN(DiagnosticPosition pos, Symbol sym, boolean preciseM do { ots = ots.tail; if (ots.isEmpty()) { - log.error(pos, Errors.NoEnclInstanceOfTypeInScope(c)); - Assert.error(); + chk.earlyRefError(pos, c); return tree; } ot = ots.head; diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java index 76ebe26b19dcc..3026e986233ec 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java @@ -3881,7 +3881,7 @@ Type resolveImplicitThis(DiagnosticPosition pos, Env env, Type t, b Type thisType = (t.tsym.owner.kind.matches(KindSelector.VAL_MTH) ? resolveSelf(pos, env, t.getEnclosingType().tsym, names._this) : resolveSelfContaining(pos, env, t.tsym, isSuperCall)).type; - if ((env.info.isSelfCall || env.info.ctorPrologue) && thisType.tsym == env.enclClass.sym) { + if (env.info.ctorPrologue && thisType.tsym == env.enclClass.sym) { log.error(pos, Errors.CantRefBeforeCtorCalled(names._this)); } return thisType; diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TypeEnter.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TypeEnter.java index 313ee5c16d144..ef74ccc9877ca 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TypeEnter.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TypeEnter.java @@ -570,7 +570,6 @@ protected Env baseEnv(JCClassDecl tree, Env env) { Env localEnv = outer.dup(tree, outer.info.dup(baseScope)); localEnv.baseClause = true; localEnv.outer = outer; - localEnv.info.isSelfCall = false; return localEnv; } diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeInfo.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeInfo.java index d2e87f9a343a3..5339cf03eb832 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeInfo.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeInfo.java @@ -152,18 +152,6 @@ public static Name calledMethodName(JCTree tree) { return null; } - /** Is this a call to this or super? - */ - public static boolean isSelfCall(JCTree tree) { - Name name = calledMethodName(tree); - if (name != null) { - Names names = name.table.names; - return name==names._this || name==names._super; - } else { - return false; - } - } - /** Is this tree a 'this' identifier? */ public static boolean isThisQualifier(JCTree tree) { diff --git a/test/langtools/tools/javac/AnonymousClass/AnonymousInSuperCallNegTest.out b/test/langtools/tools/javac/AnonymousClass/AnonymousInSuperCallNegTest.out index c04b45bce1e7d..87c1f41cd23b8 100644 --- a/test/langtools/tools/javac/AnonymousClass/AnonymousInSuperCallNegTest.out +++ b/test/langtools/tools/javac/AnonymousClass/AnonymousInSuperCallNegTest.out @@ -1,2 +1,2 @@ -AnonymousInSuperCallNegTest.java:23:49: compiler.err.cant.ref.before.ctor.called: x +AnonymousInSuperCallNegTest.java:23:49: compiler.err.cant.ref.before.ctor.called: AnonymousInSuperCallNegTest.JavacBug 1 error diff --git a/test/langtools/tools/javac/SuperInit/SuperInitFails.java b/test/langtools/tools/javac/SuperInit/SuperInitFails.java index 039564d729b2f..cd0fc3fd903b0 100644 --- a/test/langtools/tools/javac/SuperInit/SuperInitFails.java +++ b/test/langtools/tools/javac/SuperInit/SuperInitFails.java @@ -160,4 +160,12 @@ public SuperInitFails(float[][] x) { super(); // this should FAIL }; } + + public SuperInitFails(int[][] z) { + super((Runnable)() -> x); // this should FAIL + } + + public SuperInitFails(long[][] z) { + super(new Inner1()); // this should FAIL + } } diff --git a/test/langtools/tools/javac/SuperInit/SuperInitFails.out b/test/langtools/tools/javac/SuperInit/SuperInitFails.out index dc4ba76d2aa91..88d50f638d93a 100644 --- a/test/langtools/tools/javac/SuperInit/SuperInitFails.out +++ b/test/langtools/tools/javac/SuperInit/SuperInitFails.out @@ -13,6 +13,8 @@ SuperInitFails.java:125:9: compiler.err.cant.ref.before.ctor.called: this SuperInitFails.java:133:9: compiler.err.non.canonical.constructor.invoke.another.constructor: SuperInitFails.Record1 SuperInitFails.java:138:9: compiler.err.non.canonical.constructor.invoke.another.constructor: SuperInitFails.Record2 SuperInitFails.java:154:9: compiler.err.cant.ref.before.ctor.called: this +SuperInitFails.java:165:31: compiler.err.cant.ref.before.ctor.called: x +SuperInitFails.java:169:15: compiler.err.cant.ref.before.ctor.called: this SuperInitFails.java:33:13: compiler.err.call.must.only.appear.in.ctor: this SuperInitFails.java:37:14: compiler.err.call.must.only.appear.in.ctor: super SuperInitFails.java:41:14: compiler.err.call.must.only.appear.in.ctor: super @@ -24,4 +26,4 @@ SuperInitFails.java:89:13: compiler.err.return.before.superclass.initialized: su SuperInitFails.java:160:18: compiler.err.calls.not.allowed.here: super - compiler.note.preview.filename: SuperInitFails.java, DEFAULT - compiler.note.preview.recompile -24 errors +26 errors diff --git a/test/langtools/tools/javac/SuperInit/SuperInitGood.java b/test/langtools/tools/javac/SuperInit/SuperInitGood.java index 4409cfdb44b7b..cbcbee8fef0bb 100644 --- a/test/langtools/tools/javac/SuperInit/SuperInitGood.java +++ b/test/langtools/tools/javac/SuperInit/SuperInitGood.java @@ -420,6 +420,19 @@ class Foo { } } + // local class inside super() parameter list + public static class Test21 extends AtomicReference { + private int x; + public Test21() { + super(switch ("foo".hashCode()) { + default -> { + class Nested {{ System.out.println(x); }} // class is NOT instantiated - OK + yield "bar"; + } + }); + } + } + public static void main(String[] args) { new Test0(); new Test1(); @@ -462,5 +475,6 @@ public static void main(String[] args) { } new Test19(123); new Test20(); + new Test21(); } } From a5510b79adbe870fdf09821dbac623421c9c734b Mon Sep 17 00:00:00 2001 From: "Archie L. Cobbs" Date: Sat, 16 Sep 2023 16:42:39 -0500 Subject: [PATCH 18/24] Change the error message to match the applicable spec rule. --- .../share/classes/com/sun/tools/javac/comp/Check.java | 9 --------- .../share/classes/com/sun/tools/javac/comp/Lower.java | 6 +++--- .../javac/AnonymousClass/AnonymousInSuperCallNegTest.out | 2 +- 3 files changed, 4 insertions(+), 13 deletions(-) diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java index 675aff31f0466..f9cb01a87257b 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java @@ -354,15 +354,6 @@ Type typeTagError(DiagnosticPosition pos, JCDiagnostic required, Object found) { return types.createErrorType(found instanceof Type type ? type : syms.errType); } - /** Report an error that symbol cannot be referenced before super - * has been called. - * @param pos Position to be used for error reporting. - * @param sym The referenced symbol. - */ - void earlyRefError(DiagnosticPosition pos, Symbol sym) { - log.error(pos, Errors.CantRefBeforeCtorCalled(sym)); - } - /** Report duplicate declaration error. */ void duplicateError(DiagnosticPosition pos, Symbol sym) { diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java index 20cfb85f91731..3e5c5a957100b 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java @@ -1847,7 +1847,7 @@ JCExpression makeOuterThis(DiagnosticPosition pos, TypeSymbol c) { ot = ots.head; } while (ot.owner != otc); if (otc.owner.kind != PCK && !otc.hasOuterInstance()) { - chk.earlyRefError(pos, c); + log.error(pos, Errors.NoEnclInstanceOfTypeInScope(c)); Assert.error(); // should have been caught in Attr return makeNull(); } @@ -1886,7 +1886,7 @@ JCExpression makeOwnerThisN(DiagnosticPosition pos, Symbol sym, boolean preciseM Symbol c = sym.owner; List ots = outerThisStack; if (ots.isEmpty()) { - chk.earlyRefError(pos, c); + log.error(pos, Errors.NoEnclInstanceOfTypeInScope(c)); return makeNull(); } VarSymbol ot = ots.head; @@ -1897,7 +1897,7 @@ JCExpression makeOwnerThisN(DiagnosticPosition pos, Symbol sym, boolean preciseM do { ots = ots.tail; if (ots.isEmpty()) { - chk.earlyRefError(pos, c); + log.error(pos, Errors.NoEnclInstanceOfTypeInScope(c)); return tree; } ot = ots.head; diff --git a/test/langtools/tools/javac/AnonymousClass/AnonymousInSuperCallNegTest.out b/test/langtools/tools/javac/AnonymousClass/AnonymousInSuperCallNegTest.out index 87c1f41cd23b8..140521689a095 100644 --- a/test/langtools/tools/javac/AnonymousClass/AnonymousInSuperCallNegTest.out +++ b/test/langtools/tools/javac/AnonymousClass/AnonymousInSuperCallNegTest.out @@ -1,2 +1,2 @@ -AnonymousInSuperCallNegTest.java:23:49: compiler.err.cant.ref.before.ctor.called: AnonymousInSuperCallNegTest.JavacBug +AnonymousInSuperCallNegTest.java:23:49: compiler.err.no.encl.instance.of.type.in.scope: AnonymousInSuperCallNegTest.JavacBug 1 error From 599cf0e52f91cda048f691c0f022b79ccd63b499 Mon Sep 17 00:00:00 2001 From: "Archie L. Cobbs" Date: Fri, 22 Sep 2023 16:51:25 -0500 Subject: [PATCH 19/24] Remove obsolete flag "constructorArgs". --- .../share/classes/com/sun/tools/javac/comp/Attr.java | 2 -- .../share/classes/com/sun/tools/javac/comp/AttrContext.java | 5 ----- 2 files changed, 7 deletions(-) diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java index 3743814b07cc1..2623c040fb52e 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java @@ -2521,9 +2521,7 @@ public void visitApply(JCMethodInvocation tree) { if (isConstructorCall) { // Attribute arguments, yielding list of argument types. - localEnv.info.constructorArgs = true; KindSelector kind = attribArgs(KindSelector.MTH, tree.args, localEnv, argtypesBuf); - localEnv.info.constructorArgs = false; argtypes = argtypesBuf.toList(); typeargtypes = attribTypes(tree.typeargs, localEnv); diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/AttrContext.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/AttrContext.java index d108c493ce3c8..b8afce009a591 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/AttrContext.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/AttrContext.java @@ -49,10 +49,6 @@ public class AttrContext { */ int staticLevel = 0; - /** are we analyzing the arguments for a constructor invocation? - */ - boolean constructorArgs = false; - /** Are we in the 'prologue' part of a constructor, prior to an explicit this()/super()? */ boolean ctorPrologue = false; @@ -136,7 +132,6 @@ AttrContext dup(WriteableScope scope) { AttrContext info = new AttrContext(); info.scope = scope; info.staticLevel = staticLevel; - info.constructorArgs = constructorArgs; info.ctorPrologue = ctorPrologue; info.selectSuper = selectSuper; info.pendingResolutionPhase = pendingResolutionPhase; From 150d794c1e6a694144ec10b8d735ce9e3eef0d5e Mon Sep 17 00:00:00 2001 From: "Archie L. Cobbs" Date: Mon, 25 Sep 2023 08:26:58 -0500 Subject: [PATCH 20/24] Change variable name "statik" -> "isStatic" per review recommendation. --- .../share/classes/com/sun/tools/javac/comp/Flow.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java index db4a939d87555..06e3b56536525 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java @@ -482,7 +482,7 @@ protected void scanSyntheticBreak(TreeMaker make, JCTree swtch) { // Do something with all static or non-static field initializers and initialization blocks. // Note: This method also sends nested class definitions to the handler. - protected void forEachInitializer(JCClassDecl classDef, boolean statik, Consumer handler) { + protected void forEachInitializer(JCClassDecl classDef, boolean isStatic, Consumer handler) { if (classDef == initScanClass) // avoid infinite loops return; JCClassDecl initScanClassPrev = initScanClass; @@ -490,7 +490,7 @@ protected void forEachInitializer(JCClassDecl classDef, boolean statik, Consumer try { for (List defs = classDef.defs; defs.nonEmpty(); defs = defs.tail) { JCTree def = defs.head; - if (!def.hasTag(METHODDEF) && ((TreeInfo.flags(def) & STATIC) != 0) == statik) + if (!def.hasTag(METHODDEF) && ((TreeInfo.flags(def) & STATIC) != 0) == isStatic) handler.accept(def); } } finally { From 3e75e19e23083efb219617269fed96502c0fa247 Mon Sep 17 00:00:00 2001 From: "Archie L. Cobbs" Date: Mon, 25 Sep 2023 10:47:04 -0500 Subject: [PATCH 21/24] Have RefBeforeCtorCalledError extend StaticError and customize debug strings. --- .../classes/com/sun/tools/javac/comp/Resolve.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java index 3026e986233ec..4c7c177163f03 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java @@ -4587,7 +4587,11 @@ JCDiagnostic inaccessiblePackageReason(Env env, PackageSymbol sym) class StaticError extends InvalidSymbolError { StaticError(Symbol sym) { - super(STATICERR, sym, "static error"); + this(sym, "static error"); + } + + StaticError(Symbol sym, String debugName) { + super(STATICERR, sym, debugName); } @Override @@ -4610,10 +4614,10 @@ JCDiagnostic getDiagnostic(JCDiagnostic.DiagnosticType dkind, * Specialization of {@link InvalidSymbolError} for illegal * early accesses within a constructor prologue. */ - class RefBeforeCtorCalledError extends InvalidSymbolError { + class RefBeforeCtorCalledError extends StaticError { RefBeforeCtorCalledError(Symbol sym) { - super(STATICERR, sym, "prologue error"); + super(sym, "prologue error"); } @Override @@ -4745,7 +4749,7 @@ class BadMethodReferenceError extends StaticError { boolean unboundLookup; public BadMethodReferenceError(Symbol sym, boolean unboundLookup) { - super(sym); + super(sym, "bad method ref error"); this.unboundLookup = unboundLookup; } From 355edfb6b7839b390bc1a87651b9bc69ee2dd77c Mon Sep 17 00:00:00 2001 From: "Archie L. Cobbs" Date: Mon, 25 Sep 2023 14:27:04 -0500 Subject: [PATCH 22/24] Address review comments relating to error messages. --- .../share/classes/com/sun/tools/javac/comp/Check.java | 2 +- .../classes/com/sun/tools/javac/resources/compiler.properties | 4 ++-- test/langtools/tools/javac/SuperInit/SuperInitFails.out | 4 ++-- .../tools/javac/diags/examples/CallsNotAllowedHere.java | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java index f9cb01a87257b..6f08aa0db6d7e 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java @@ -4047,7 +4047,7 @@ public void visitApply(JCMethodInvocation apply) { // super()/this() calls must be a top level statement if (scanDepth != MATCH_SCAN_DEPTH) { - log.error(apply.pos(), Errors.CallsNotAllowedHere(methodName)); + log.error(apply.pos(), Errors.CtorCallsNotAllowedHere(methodName)); break; } diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties index 7447af788f63b..acaa2c140ed21 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties @@ -242,12 +242,12 @@ compiler.err.redundant.superclass.init=\ redundant call to {0}() after {1}() has already been invoked # 0: name -compiler.err.calls.not.allowed.here=\ +compiler.err.ctor.calls.not.allowed.here=\ calls to {0}() not allowed here # 0: name compiler.err.return.before.superclass.initialized=\ - ''return'' not allowed prior to {0}() + ''return'' not allowed before {0}() # 0: symbol kind, 1: name, 2: symbol kind, 3: type, 4: message segment compiler.err.cant.apply.symbol.noargs=\ diff --git a/test/langtools/tools/javac/SuperInit/SuperInitFails.out b/test/langtools/tools/javac/SuperInit/SuperInitFails.out index 88d50f638d93a..4fc71ec3f083c 100644 --- a/test/langtools/tools/javac/SuperInit/SuperInitFails.out +++ b/test/langtools/tools/javac/SuperInit/SuperInitFails.out @@ -21,9 +21,9 @@ SuperInitFails.java:41:14: compiler.err.call.must.only.appear.in.ctor: super SuperInitFails.java:45:13: compiler.err.call.must.only.appear.in.ctor: this SuperInitFails.java:49:33: compiler.err.call.must.only.appear.in.ctor: super SuperInitFails.java:53:32: compiler.err.call.must.only.appear.in.ctor: this -SuperInitFails.java:83:18: compiler.err.calls.not.allowed.here: super +SuperInitFails.java:83:18: compiler.err.ctor.calls.not.allowed.here: super SuperInitFails.java:89:13: compiler.err.return.before.superclass.initialized: super -SuperInitFails.java:160:18: compiler.err.calls.not.allowed.here: super +SuperInitFails.java:160:18: compiler.err.ctor.calls.not.allowed.here: super - compiler.note.preview.filename: SuperInitFails.java, DEFAULT - compiler.note.preview.recompile 26 errors diff --git a/test/langtools/tools/javac/diags/examples/CallsNotAllowedHere.java b/test/langtools/tools/javac/diags/examples/CallsNotAllowedHere.java index bd608030ba37a..2f9239f09c024 100644 --- a/test/langtools/tools/javac/diags/examples/CallsNotAllowedHere.java +++ b/test/langtools/tools/javac/diags/examples/CallsNotAllowedHere.java @@ -21,7 +21,7 @@ * questions. */ -// key: compiler.err.calls.not.allowed.here +// key: compiler.err.ctor.calls.not.allowed.here class CallsNotAllowedHere { public CallsNotAllowedHere() { From 2356ac6f93a0b27687bc81ae26d856cadaee068c Mon Sep 17 00:00:00 2001 From: "Archie L. Cobbs" Date: Mon, 25 Sep 2023 14:29:03 -0500 Subject: [PATCH 23/24] After further review, revert 599d761 to avoid mutable state lying around. --- .../classes/com/sun/tools/javac/comp/Check.java | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java index 6f08aa0db6d7e..676e1fd38b247 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java @@ -128,9 +128,6 @@ public class Check { // Attr as it visits new method declarations. private MethodSymbol method; - // Cached instance of SuperThisChecker - private final SuperThisChecker superThisChecker; - public static Check instance(Context context) { Check instance = context.get(checkKey); if (instance == null) @@ -155,7 +152,6 @@ protected Check(Context context) { Options options = Options.instance(context); lint = Lint.instance(context); fileManager = context.get(JavaFileManager.class); - superThisChecker = new SuperThisChecker(); source = Source.instance(context); target = Target.instance(context); @@ -3962,7 +3958,7 @@ private void checkCyclicConstructor(JCClassDecl tree, Symbol ctor, **************************************************************************/ void checkSuperInitCalls(JCClassDecl tree) { - superThisChecker.check(tree); + new SuperThisChecker().check(tree); } private class SuperThisChecker extends TreeScanner { @@ -3976,16 +3972,7 @@ private class SuperThisChecker extends TreeScanner { private Name initCall; // whichever of "super" or "init" we've seen already private int scanDepth; // current scan recursion depth in method body - public void resetState() { - constructor = false; - firstStatement = false; - earlyReturn = null; - initCall = null; - scanDepth = 0; - } - public void check(JCClassDecl classDef) { - resetState(); scan(classDef.defs); } From 738c10a57de0f31cde21542a011684cfde42a1e2 Mon Sep 17 00:00:00 2001 From: "Archie L. Cobbs" Date: Thu, 28 Sep 2023 11:11:18 -0500 Subject: [PATCH 24/24] Reword new errors to use the JLS term for "explicit constructor invocation". --- .../com/sun/tools/javac/comp/Check.java | 8 ++++---- .../tools/javac/resources/compiler.properties | 12 ++++------- .../tools/javac/SuperInit/SuperInitFails.out | 20 +++++++++---------- 3 files changed, 18 insertions(+), 22 deletions(-) diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java index 676e1fd38b247..4f5af816fde1c 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java @@ -3998,7 +3998,7 @@ public void visitMethodDef(JCMethodDecl tree) { // Verify no 'return' seen prior to an explicit super()/this() call if (constructor && earlyReturn != null && initCall != null) - log.error(earlyReturn.pos(), Errors.ReturnBeforeSuperclassInitialized(initCall)); + log.error(earlyReturn.pos(), Errors.ReturnBeforeSuperclassInitialized); } finally { firstStatement = false; constructor = false; @@ -4028,19 +4028,19 @@ public void visitApply(JCMethodInvocation apply) { // super()/this() calls must only appear in a constructor if (!constructor) { - log.error(apply.pos(), Errors.CallMustOnlyAppearInCtor(methodName)); + log.error(apply.pos(), Errors.CallMustOnlyAppearInCtor); break; } // super()/this() calls must be a top level statement if (scanDepth != MATCH_SCAN_DEPTH) { - log.error(apply.pos(), Errors.CtorCallsNotAllowedHere(methodName)); + log.error(apply.pos(), Errors.CtorCallsNotAllowedHere); break; } // super()/this() calls must not appear more than once if (initCall != null) { - log.error(apply.pos(), Errors.RedundantSuperclassInit(methodName, initCall)); + log.error(apply.pos(), Errors.RedundantSuperclassInit); break; } diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties index acaa2c140ed21..47603f60b28bc 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties @@ -233,21 +233,17 @@ compiler.err.switch.expression.empty=\ compiler.err.switch.expression.no.result.expressions=\ switch expression does not have any result expressions -# 0: name compiler.err.call.must.only.appear.in.ctor=\ - calls to {0}() may only appear within constructors + explicit constructor invocation may only appear within a constructor body -# 0: name, 1: name compiler.err.redundant.superclass.init=\ - redundant call to {0}() after {1}() has already been invoked + redundant explicit constructor invocation -# 0: name compiler.err.ctor.calls.not.allowed.here=\ - calls to {0}() not allowed here + explicit constructor invocation not allowed here -# 0: name compiler.err.return.before.superclass.initialized=\ - ''return'' not allowed before {0}() + ''return'' not allowed before explicit constructor invocation # 0: symbol kind, 1: name, 2: symbol kind, 3: type, 4: message segment compiler.err.cant.apply.symbol.noargs=\ diff --git a/test/langtools/tools/javac/SuperInit/SuperInitFails.out b/test/langtools/tools/javac/SuperInit/SuperInitFails.out index 4fc71ec3f083c..60bbd83dcc034 100644 --- a/test/langtools/tools/javac/SuperInit/SuperInitFails.out +++ b/test/langtools/tools/javac/SuperInit/SuperInitFails.out @@ -8,22 +8,22 @@ SuperInitFails.java:99:33: compiler.err.cant.ref.before.ctor.called: this SuperInitFails.java:104:14: compiler.err.cant.ref.before.ctor.called: this SuperInitFails.java:108:20: compiler.err.not.encl.class: java.lang.Object SuperInitFails.java:112:17: compiler.err.cant.ref.before.ctor.called: super -SuperInitFails.java:119:22: compiler.err.call.must.only.appear.in.ctor: super +SuperInitFails.java:119:22: compiler.err.call.must.only.appear.in.ctor SuperInitFails.java:125:9: compiler.err.cant.ref.before.ctor.called: this SuperInitFails.java:133:9: compiler.err.non.canonical.constructor.invoke.another.constructor: SuperInitFails.Record1 SuperInitFails.java:138:9: compiler.err.non.canonical.constructor.invoke.another.constructor: SuperInitFails.Record2 SuperInitFails.java:154:9: compiler.err.cant.ref.before.ctor.called: this SuperInitFails.java:165:31: compiler.err.cant.ref.before.ctor.called: x SuperInitFails.java:169:15: compiler.err.cant.ref.before.ctor.called: this -SuperInitFails.java:33:13: compiler.err.call.must.only.appear.in.ctor: this -SuperInitFails.java:37:14: compiler.err.call.must.only.appear.in.ctor: super -SuperInitFails.java:41:14: compiler.err.call.must.only.appear.in.ctor: super -SuperInitFails.java:45:13: compiler.err.call.must.only.appear.in.ctor: this -SuperInitFails.java:49:33: compiler.err.call.must.only.appear.in.ctor: super -SuperInitFails.java:53:32: compiler.err.call.must.only.appear.in.ctor: this -SuperInitFails.java:83:18: compiler.err.ctor.calls.not.allowed.here: super -SuperInitFails.java:89:13: compiler.err.return.before.superclass.initialized: super -SuperInitFails.java:160:18: compiler.err.ctor.calls.not.allowed.here: super +SuperInitFails.java:33:13: compiler.err.call.must.only.appear.in.ctor +SuperInitFails.java:37:14: compiler.err.call.must.only.appear.in.ctor +SuperInitFails.java:41:14: compiler.err.call.must.only.appear.in.ctor +SuperInitFails.java:45:13: compiler.err.call.must.only.appear.in.ctor +SuperInitFails.java:49:33: compiler.err.call.must.only.appear.in.ctor +SuperInitFails.java:53:32: compiler.err.call.must.only.appear.in.ctor +SuperInitFails.java:83:18: compiler.err.ctor.calls.not.allowed.here +SuperInitFails.java:89:13: compiler.err.return.before.superclass.initialized +SuperInitFails.java:160:18: compiler.err.ctor.calls.not.allowed.here - compiler.note.preview.filename: SuperInitFails.java, DEFAULT - compiler.note.preview.recompile 26 errors