From fec78c3d4a73f53b6aac5ac9b9a82a4793fd052b Mon Sep 17 00:00:00 2001 From: Stephan Herrmann Date: Sun, 14 Jul 2024 20:36:13 +0200 Subject: [PATCH] Feature work: + detect early construction context affecting supers + implement 2-phase search for uninitialized types: 1. outer scopes 2. the same including superclass navigation + clarify this vs super reference issues - no detail checks: super is never legal in early construction context + clarify that methods may be searched in outers, too Polish: + at 23 always enterEarlyConstructionContext() + exception for records was not relevant + minor code cleanup & comment updates --- .../compiler/ast/AllocationExpression.java | 1 + .../compiler/ast/ConstructorDeclaration.java | 27 +++++------ .../compiler/ast/ExplicitConstructorCall.java | 4 +- .../internal/compiler/ast/FieldReference.java | 2 +- .../compiler/ast/InstanceOfExpression.java | 2 +- .../internal/compiler/ast/MessageSend.java | 2 +- .../jdt/internal/compiler/ast/Statement.java | 4 -- .../internal/compiler/ast/SuperReference.java | 4 +- .../internal/compiler/impl/JavaFeature.java | 27 +++++++++-- .../internal/compiler/lookup/ClassScope.java | 2 + .../jdt/internal/compiler/lookup/Scope.java | 48 +++++++++++++------ .../regression/SuperAfterStatementsTest.java | 29 +++++++++++ 12 files changed, 107 insertions(+), 45 deletions(-) diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/AllocationExpression.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/AllocationExpression.java index c5f0206ffe7..235d55f32a4 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/AllocationExpression.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/AllocationExpression.java @@ -551,6 +551,7 @@ protected void checkEarlyConstructionContext(BlockScope scope) { if (uninitialized != null) scope.problemReporter().allocationInEarlyConstructionContext(this, this.resolvedType, uninitialized); } + // if JEP 482 is not enabled, problems will be detected when looking for enclosing instance(s) } /** diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/ConstructorDeclaration.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/ConstructorDeclaration.java index 9a070ef4ec0..1d5a55e12a2 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/ConstructorDeclaration.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/ConstructorDeclaration.java @@ -156,6 +156,10 @@ public void analyseCode(ClassScope classScope, InitializationFlowContext initial // nullity, owning and mark as assigned analyseArguments(classScope.environment(), flowInfo, initializerFlowContext, this.arguments, this.binding); + if (JavaFeature.FLEXIBLE_CONSTRUCTOR_BODIES.matchesCompliance(this.scope.compilerOptions())) { + this.scope.enterEarlyConstructionContext(); + } + // propagate to constructor call if (this.constructorCall != null) { // if calling 'this(...)', then flag all non-static fields as definitely @@ -168,17 +172,12 @@ public void analyseCode(ClassScope classScope, InitializationFlowContext initial } } } - if (getLateConstructorCall() == null) - flowInfo = this.constructorCall.analyseCode(this.scope, constructorContext, flowInfo); + flowInfo = this.constructorCall.analyseCode(this.scope, constructorContext, flowInfo); } // reuse the reachMode from non static field info flowInfo.setReachMode(nonStaticFieldInfoReachMode); - if (getLateConstructorCall() != null) { // TODO also for ctor arguments? - this.scope.enterEarlyConstructionContext(); - } - // propagate to statements if (this.statements != null) { CompilerOptions compilerOptions = this.scope.compilerOptions(); @@ -438,6 +437,11 @@ private void internalGenerateCode(ClassScope classScope, ClassFile classFile) { generateSyntheticFieldInitializationsIfNecessary(this.scope, codeStream, declaringClass); codeStream.recordPositionsFrom(0, this.bodyStart > 0 ? this.bodyStart : this.sourceStart); } + + if (JavaFeature.FLEXIBLE_CONSTRUCTOR_BODIES.matchesCompliance(this.scope.compilerOptions())) { + this.scope.enterEarlyConstructionContext(); + } + // generate constructor call if (this.constructorCall != null) { this.constructorCall.generateCode(this.scope, codeStream); @@ -459,9 +463,6 @@ private void internalGenerateCode(ClassScope classScope, ClassFile classFile) { } // generate statements if (this.statements != null) { - if (getLateConstructorCall() != null) { - this.scope.enterEarlyConstructionContext(); - } for (Statement statement : this.statements) { statement.generateCode(this.scope, codeStream); if (!this.compilationResult.hasErrors() && (codeStream.stackDepth != 0 || codeStream.operandStack.size() != 0)) { @@ -682,11 +683,9 @@ public void resolveStatements() { } if (lateConstructorCall != null) { this.constructorCall = null; // not used with JEP 482, conversely, constructorCall!=null signals no JEP 482 context - if (!sourceType.isRecord()) { // explicit constructor call is never legal for records - this.scope.problemReporter().validateJavaFeatureSupport(JavaFeature.FLEXIBLE_CONSTRUCTOR_BODIES, - lateConstructorCall.sourceStart, - lateConstructorCall.sourceEnd); - } + this.scope.problemReporter().validateJavaFeatureSupport(JavaFeature.FLEXIBLE_CONSTRUCTOR_BODIES, + lateConstructorCall.sourceStart, + lateConstructorCall.sourceEnd); } else { this.constructorCall.resolve(this.scope); } diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/ExplicitConstructorCall.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/ExplicitConstructorCall.java index 1291329c38c..a88e2936b68 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/ExplicitConstructorCall.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/ExplicitConstructorCall.java @@ -320,7 +320,6 @@ public void resolve(BlockScope scope) { if (!checkAndFlagExplicitConstructorCallInCanonicalConstructor(methodDeclaration, scope)) return; } - boolean isFirstStatement = true; boolean hasError = false; ConstructorDeclaration constructorDeclaration = (ConstructorDeclaration) methodDeclaration; if (methodDeclaration == null || !methodDeclaration.isConstructor()) { @@ -331,7 +330,6 @@ public void resolve(BlockScope scope) { constructorCall = constructorDeclaration.getLateConstructorCall(); } if (constructorCall != null && constructorCall != this) { - isFirstStatement = false; hasError = true; } } @@ -361,7 +359,7 @@ public void resolve(BlockScope scope) { } return; } - methodScope.isConstructorCall = isFirstStatement; // JEP 482 uses other mechanisms + methodScope.isConstructorCall = true; ReferenceBinding receiverType = scope.enclosingReceiverType(); boolean rcvHasError = false; if (this.accessMode != ExplicitConstructorCall.This) { diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/FieldReference.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/FieldReference.java index 68323dff413..ada4a8b2f33 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/FieldReference.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/FieldReference.java @@ -682,7 +682,7 @@ public TypeBinding resolveType(BlockScope scope) { } // the case receiverType.isArrayType and token = 'length' is handled by the scope API FieldBinding fieldBinding = this.binding = scope.getField(this.actualReceiverType, this.token, this); - if (this.receiver instanceof ThisReference) { + if (this.receiver instanceof ThisReference ref && ref.isThis()) { checkFieldAccessInEarlyConstructionContext(scope, fieldBinding.name, fieldBinding, this.actualReceiverType); } if (!fieldBinding.isValidBinding()) { diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/InstanceOfExpression.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/InstanceOfExpression.java index e3c155632e2..1535564a187 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/InstanceOfExpression.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/InstanceOfExpression.java @@ -289,7 +289,7 @@ private void storeExpressionValue(CodeStream codeStream) { LocalVariableBinding local = this.expression.localVariableBinding(); local = local != null ? local : this.secretExpressionValue; if (local != null) - codeStream.store(local, false); // FIXME, see https://github.com/eclipse-jdt/eclipse.jdt.core/pull/2499#pullrequestreview-2161555334 + codeStream.store(local, false); } @Override diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/MessageSend.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/MessageSend.java index 3f99ad5bb71..f646fc307e7 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/MessageSend.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/MessageSend.java @@ -1020,7 +1020,7 @@ public TypeBinding resolveType(BlockScope scope) { this.bits |= NeedReceiverGenericCast; } } - if (this.actualReceiverType != null && scope.isInsideEarlyConstructionContext(this.actualReceiverType, false) && + if (this.actualReceiverType != null && scope.isInsideEarlyConstructionContext(this.actualReceiverType, true) && (this.receiver instanceof ThisReference thisReference && thisReference.isImplicitThis())) { scope.problemReporter().messageSendInEarlyConstructionContext(this); } diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/Statement.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/Statement.java index 87d24eb4435..00ee5f50186 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/Statement.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/Statement.java @@ -8,10 +8,6 @@ * * SPDX-License-Identifier: EPL-2.0 * - * This is an implementation of an early-draft specification developed under the Java - * Community Process (JCP) and is made available for testing and evaluation purposes - * only. The code is not compatible with any specification of the JCP. - * * Contributors: * IBM Corporation - initial API and implementation * Stephan Herrmann - Contributions for diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/SuperReference.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/SuperReference.java index 510729f4cb9..4d90aa1abc0 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/SuperReference.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/SuperReference.java @@ -71,8 +71,10 @@ public StringBuilder printExpression(int indent, StringBuilder output){ public TypeBinding resolveType(BlockScope scope) { this.constant = Constant.NotAConstant; - if (scope.isInsideEarlyConstructionContext(null, false)) + if (scope.isInsideEarlyConstructionContext(null, false)) { + // always error, no need to check any details: scope.problemReporter().errorExpressionInEarlyConstructionContext(this); + } ReferenceBinding enclosingReceiverType = scope.enclosingReceiverType(); if (!checkAccess(scope, enclosingReceiverType)) return null; diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/impl/JavaFeature.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/impl/JavaFeature.java index 2f2e8ad4e94..8ebda21e5f7 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/impl/JavaFeature.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/impl/JavaFeature.java @@ -79,11 +79,28 @@ public enum JavaFeature { new char[][] {}, true), /** - * JEP 482. The primary enabling check is in ConstructorDeclaration.getLateConstructor(). - * If that method returns non-null, then ClassScope.enterEarlyConstructionContext() will follow, - * which enables further analysis and actions. - * In particular, when scope.isInsideEarlyConstructionContext() answers true, - * then we can be sure that availability of this feature has been checked / reported. + * JEP 482. Some locations only check if compliance is sufficient to use this feature, + * to leave checking for actual enablement for later. This is done so we can report + * that a preview feature could potentially kick in even when it is disabled. + *
+ *
Initial check in ConstructorDeclaration.resolveStatements(); + *
At 23 we always call enterEarlyConstructionContext() to enable many downstream analyses.
+ * Check actual support only when a "late constructor" has been found.
+ * Similar for analyseCode() and generateCode(). + *
Differentiate error messages based on enablement: + *
    + *
  • AllocationExpression.checkEarlyConstructionContext() + *
  • ExplicitConstructorCall.resolve(BlockScope) + *
  • ThisReference.checkAccess(BlockScope, ReferenceBinding) + *
+ *
Main checks during resolve: Reference.checkFieldAccessInEarlyConstructionContext() + *
applies all strategy variants from above + *
Individual exceptions from old rules + *
  • MethodScope.findField()
  • Scope.getBinding(char[], int, InvocationSite, boolean)
+ *
Main code gen change in TypeDeclaration.manageEnclosingInstanceAccessIfNecessary() + *
Only if feature is actually supported, we will generate special synthetid args & fields
+ * Uses some feature-specific help from BlockScope.getEmulationPath() + *
*/ FLEXIBLE_CONSTRUCTOR_BODIES(ClassFileConstants.JDK23, Messages.bind(Messages.flexible_constructor_bodies), diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ClassScope.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ClassScope.java index c4d1660af18..ffbad73ad4b 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ClassScope.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ClassScope.java @@ -82,6 +82,8 @@ public class ClassScope extends Scope { * into {@code scopesInEarlyConstruction}, for use during generateCode(), which doesn't have the * context of the lambda declaration. *

+ *

All this is always active at compliance 23, see {@link JavaFeature#FLEXIBLE_CONSTRUCTOR_BODIES} + * for details on where enablement is actually checked.

*/ public boolean insideEarlyConstructionContext = false; diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/Scope.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/Scope.java index fb712686a41..6dd903f4f96 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/Scope.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/Scope.java @@ -5697,29 +5697,47 @@ public void leaveEarlyConstructionContext() { public boolean isInsideEarlyConstructionContext(TypeBinding targetClass, boolean considerEnclosings) { return getMatchingUninitializedType(targetClass, considerEnclosings) != null; } - + private enum MatchPhase { WITHOUT_SUPERS, WITH_SUPERS } + private static MatchPhase[] SinglePass = new MatchPhase[] { MatchPhase.WITHOUT_SUPERS }; public TypeBinding getMatchingUninitializedType(TypeBinding targetClass, boolean considerEnclosings) { + MatchPhase[] phases = MatchPhase.values(); if (targetClass == null) { targetClass = enclosingReceiverType(); + phases = SinglePass; } else if (!(targetClass instanceof ReferenceBinding)) { return null; } - ClassScope currentEnclosing = classScope(); - while (currentEnclosing != null) { - SourceTypeBinding enclosingType = currentEnclosing.referenceContext.binding; - TypeBinding currentTarget = targetClass; - while (currentTarget != null) { - if (TypeBinding.equalsEquals(enclosingType, currentTarget.actualType())) { - if (currentEnclosing.insideEarlyConstructionContext) - return enclosingType; - } - if (!considerEnclosings - || (currentTarget instanceof ReferenceBinding currentRefBind && !currentRefBind.hasEnclosingInstanceContext())) { - break; + // First iteration ignores superclasses, to prefer finding the target in outers, rather than supers. + // Note on performance: while deeply nested loops look painful, poor-man's measurements showed good results. + for (Enum phase : phases) { + // 1. Scope in->out + ClassScope currentEnclosing = classScope(); + while (currentEnclosing != null) { + SourceTypeBinding enclosingType = currentEnclosing.referenceContext.binding; + // 2. targetClass to supers + TypeBinding currentTarget = targetClass; + while (currentTarget != null) { + // 3. enclosing type to supers (in phase 2 only) + TypeBinding tmpEnclosing = enclosingType; + while (tmpEnclosing != null) { // this loop is not effective during PHASE.WITHOUT_SUPERS + if (TypeBinding.equalsEquals(tmpEnclosing, currentTarget.actualType())) { + if (currentEnclosing.insideEarlyConstructionContext) + return enclosingType; + return null; + } + if (phase == MatchPhase.WITH_SUPERS) + tmpEnclosing = tmpEnclosing.superclass(); + else + break; + } + if (!considerEnclosings + || (currentTarget instanceof ReferenceBinding currentRefBind && !currentRefBind.hasEnclosingInstanceContext())) { + break; + } + currentTarget = currentTarget.enclosingType(); } - currentTarget = currentTarget.enclosingType(); + currentEnclosing = currentEnclosing.parent.classScope(); } - currentEnclosing = currentEnclosing.parent.classScope(); } return null; } diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SuperAfterStatementsTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SuperAfterStatementsTest.java index 1fd2624f92d..ad98d66a0d5 100644 --- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SuperAfterStatementsTest.java +++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SuperAfterStatementsTest.java @@ -595,6 +595,35 @@ class Inner {} "You are using a preview language feature that may or may not be supported in a future release\n" + "----------\n"); } + public void test011_inherited() { + runNegativeTest(new String[] { + "X.java", + """ + class Super { + class Inner {} + } + class Outer extends Super { + Outer() { + new Inner(); // Error - 'this' is enclosing instance + super(); + } + } + """ + }, + """ + ---------- + 1. ERROR in X.java (at line 6) + new Inner(); // Error - 'this' is enclosing instance + ^^^^^^^^^^^ + Cannot instantiate class Super.Inner in an early construction context of class Outer + ---------- + 2. WARNING in X.java (at line 7) + super(); + ^^^^^^^^ + You are using a preview language feature that may or may not be supported in a future release + ---------- + """); + } public void test011_nested() { runConformTest(new String[] { "Outer.java",