Skip to content

Commit ec9a8cc

Browse files
stephan-herrmannstephan-herrmann
authored
[23] Flow analysis fails to recognize initialization in an instance initializer block (eclipse-jdt#3749)
+ arguments to explicit constructor call are prologue - if they are relevant don't skip prologue analysis + fine tune creation and usage of flow prologueInfo Fixes eclipse-jdt#3748
1 parent 2eb9a9b commit ec9a8cc

File tree

4 files changed

+144
-26
lines changed

4 files changed

+144
-26
lines changed

‎org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/ConstructorDeclaration.java

+30-22
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*******************************************************************************
2-
* Copyright (c) 2000, 2024 IBM Corporation and others.
2+
* Copyright (c) 2000, 2025 IBM Corporation and others.
33
*
44
* This program and the accompanying materials
55
* are made available under the terms of the Eclipse Public License 2.0
@@ -8,6 +8,10 @@
88
*
99
* SPDX-License-Identifier: EPL-2.0
1010
*
11+
* This is an implementation of an early-draft specification developed under the Java
12+
* Community Process (JCP) and is made available for testing and evaluation purposes
13+
* only. The code is not compatible with any specification of the JCP.
14+
*
1115
* Contributors:
1216
* IBM Corporation - initial API and implementation
1317
* Stephan Herrmann - Contributions for
@@ -47,6 +51,7 @@
4751
import org.eclipse.jdt.internal.compiler.flow.FlowContext;
4852
import org.eclipse.jdt.internal.compiler.flow.FlowInfo;
4953
import org.eclipse.jdt.internal.compiler.flow.InitializationFlowContext;
54+
import org.eclipse.jdt.internal.compiler.flow.UnconditionalFlowInfo;
5055
import org.eclipse.jdt.internal.compiler.impl.CompilerOptions;
5156
import org.eclipse.jdt.internal.compiler.impl.JavaFeature;
5257
import org.eclipse.jdt.internal.compiler.lookup.*;
@@ -71,6 +76,13 @@ public ConstructorDeclaration(CompilationResult compilationResult){
7176
}
7277

7378
enum AnalysisMode { ALL, PROLOGUE, REST }
79+
80+
FlowInfo getPrologueInfo() {
81+
if (this.prologueInfo != null)
82+
return this.prologueInfo;
83+
return new UnconditionalFlowInfo();
84+
}
85+
7486
/**
7587
* The flowInfo corresponds to non-static field initialization infos. It may be unreachable (155423), but still the explicit constructor call must be
7688
* analyzed as reachable, since it will be generated in the end.
@@ -89,17 +101,18 @@ public void analyseCode(ClassScope classScope, InitializationFlowContext initial
89101

90102
try {
91103
ExplicitConstructorCall lateConstructorCall = getLateConstructorCall();
92-
if (mode == AnalysisMode.PROLOGUE) {
93-
if (lateConstructorCall == null)
94-
return;
104+
if (mode == AnalysisMode.PROLOGUE
105+
&& lateConstructorCall == null
106+
&& (this.constructorCall == null || !this.constructorCall.hasArgumentNeedingAnalysis())) {
107+
return; // no relevant prologue present
95108
}
96109

97110
int nonStaticFieldInfoReachMode = flowInfo.reachMode();
98111
ExceptionHandlingFlowContext constructorContext;
99112
if (mode == AnalysisMode.REST) {
100113
// retrieve from first iteration (PROLOGUE):
101114
constructorContext = this.prologueContext;
102-
flowInfo = this.prologueInfo;
115+
flowInfo = this.prologueInfo.addInitializationsFrom(flowInfo);
103116
// skip the part already done during PROLOGUE analysis ...
104117
} else {
105118
flowInfo.setReachMode(initialReachMode);
@@ -196,6 +209,9 @@ public void analyseCode(ClassScope classScope, InitializationFlowContext initial
196209

197210
// propagate to constructor call
198211
if (this.constructorCall != null) {
212+
flowInfo = this.constructorCall.analyseCode(this.scope, constructorContext, flowInfo);
213+
if (mode == AnalysisMode.PROLOGUE && this.constructorCall.hasArgumentNeedingAnalysis())
214+
this.prologueInfo = flowInfo.copy();
199215
// if calling 'this(...)', then flag all non-static fields as definitely
200216
// set since they are supposed to be set inside other local constructor
201217
if (this.constructorCall.accessMode == ExplicitConstructorCall.This) {
@@ -206,7 +222,6 @@ public void analyseCode(ClassScope classScope, InitializationFlowContext initial
206222
}
207223
}
208224
}
209-
flowInfo = this.constructorCall.analyseCode(this.scope, constructorContext, flowInfo);
210225
}
211226

212227
// reuse the reachMode from non static field info
@@ -219,22 +234,10 @@ public void analyseCode(ClassScope classScope, InitializationFlowContext initial
219234
boolean enableSyntacticNullAnalysisForFields = compilerOptions.enableSyntacticNullAnalysisForFields;
220235
int complaintLevel = (nonStaticFieldInfoReachMode & FlowInfo.UNREACHABLE) == 0 ? Statement.NOT_COMPLAINED : Statement.COMPLAINED_FAKE_REACHABLE;
221236
for (Statement stat : this.statements) {
222-
switch (mode) {
223-
case PROLOGUE -> {
224-
if (stat == lateConstructorCall) {
225-
this.prologueInfo = flowInfo; // keep for second iteration, also signals the need for REST analysis
226-
return; // we're done for this time
227-
}
228-
}
229-
case REST -> {
230-
if (lateConstructorCall != null) {
231-
if (stat == lateConstructorCall) // if true this is where we start analysing
232-
lateConstructorCall = null; // no more checking for subsequent statements
233-
else
234-
continue; // skip statements already processed during PROLOGUE analysis
235-
}
236-
}
237-
default -> { /* nothing special */ }
237+
if (mode == AnalysisMode.REST && lateConstructorCall != null) {
238+
if (stat == lateConstructorCall) // if true this is where we start analysing
239+
lateConstructorCall = null; // no more checking for subsequent statements
240+
continue; // skip statements already processed during PROLOGUE analysis
238241
}
239242
if ((complaintLevel = stat.complainIfUnreachable(flowInfo, this.scope, complaintLevel, true)) < Statement.COMPLAINED_UNREACHABLE) {
240243
flowInfo = stat.analyseCode(this.scope, constructorContext, flowInfo);
@@ -245,6 +248,11 @@ public void analyseCode(ClassScope classScope, InitializationFlowContext initial
245248
if (compilerOptions.analyseResourceLeaks) {
246249
FakedTrackingVariable.cleanUpUnassigned(this.scope, stat, flowInfo, false);
247250
}
251+
if (mode == AnalysisMode.PROLOGUE && stat == lateConstructorCall) {
252+
// lateConstructor implies no this.constructorCall (which is handled above)
253+
this.prologueInfo = flowInfo; // keep for second iteration, also signals the need for REST analysis
254+
return; // we're done for this time
255+
}
248256
}
249257
}
250258
// check for missing returning path

‎org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/ExplicitConstructorCall.java

+20-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*******************************************************************************
2-
* Copyright (c) 2000, 2024 IBM Corporation and others.
2+
* Copyright (c) 2000, 2025 IBM Corporation and others.
33
*
44
* This program and the accompanying materials
55
* are made available under the terms of the Eclipse Public License 2.0
@@ -8,6 +8,10 @@
88
*
99
* SPDX-License-Identifier: EPL-2.0
1010
*
11+
* This is an implementation of an early-draft specification developed under the Java
12+
* Community Process (JCP) and is made available for testing and evaluation purposes
13+
* only. The code is not compatible with any specification of the JCP.
14+
*
1115
* Contributors:
1216
* IBM Corporation - initial API and implementation
1317
* Stephan Herrmann - Contributions for
@@ -42,6 +46,7 @@
4246
import org.eclipse.jdt.internal.compiler.codegen.Opcodes;
4347
import org.eclipse.jdt.internal.compiler.flow.FlowContext;
4448
import org.eclipse.jdt.internal.compiler.flow.FlowInfo;
49+
import org.eclipse.jdt.internal.compiler.impl.Constant;
4550
import org.eclipse.jdt.internal.compiler.impl.JavaFeature;
4651
import org.eclipse.jdt.internal.compiler.lookup.*;
4752

@@ -124,6 +129,20 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl
124129
}
125130
}
126131

132+
public boolean hasArgumentNeedingAnalysis() {
133+
if (this.arguments != null) {
134+
for (Expression arg : this.arguments) {
135+
if (arg.constant != Constant.NotAConstant)
136+
continue;
137+
if (arg instanceof SingleNameReference ref
138+
&& ref.binding != null && ref.binding.isParameter())
139+
continue;
140+
return true;
141+
}
142+
}
143+
return false;
144+
}
145+
127146
/**
128147
* Constructor call code generation
129148
*

‎org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/TypeDeclaration.java

+11-3
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import org.eclipse.jdt.internal.compiler.ASTVisitor;
4343
import org.eclipse.jdt.internal.compiler.ClassFile;
4444
import org.eclipse.jdt.internal.compiler.CompilationResult;
45+
import org.eclipse.jdt.internal.compiler.ast.ConstructorDeclaration.AnalysisMode;
4546
import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants;
4647
import org.eclipse.jdt.internal.compiler.codegen.CodeStream;
4748
import org.eclipse.jdt.internal.compiler.flow.FlowContext;
@@ -900,8 +901,9 @@ private void internalAnalyseCode(FlowContext flowContext, FlowInfo flowInfo) {
900901
AbstractMethodDeclaration method = this.methods[i];
901902
if (method.isConstructor()) {
902903
FlowInfo ctorInfo = flowInfo.copy();
903-
((ConstructorDeclaration) method).analyseCode(this.scope, initializerContext, ctorInfo,
904-
ctorInfo.reachMode(), ConstructorDeclaration.AnalysisMode.PROLOGUE);
904+
ConstructorDeclaration constructor = (ConstructorDeclaration) method;
905+
constructor.analyseCode(this.scope, initializerContext, ctorInfo, ctorInfo.reachMode(), AnalysisMode.PROLOGUE);
906+
ctorInfo = constructor.getPrologueInfo();
905907
if (prologueInfo == null)
906908
prologueInfo = ctorInfo.copy();
907909
else
@@ -911,8 +913,14 @@ private void internalAnalyseCode(FlowContext flowContext, FlowInfo flowInfo) {
911913
if (prologueInfo != null) {
912914
// field initializers should see inits from ctor prologues:
913915
for (FieldBinding field : this.binding.fields()) {
914-
if (prologueInfo.isDefinitelyAssigned(field))
916+
if (prologueInfo.isDefinitelyAssigned(field)) {
915917
nonStaticFieldInfo.markAsDefinitelyAssigned(field);
918+
} else if (prologueInfo.isPotentiallyAssigned(field)) {
919+
// mimic missing method markAsPotentiallyAssigned(field):
920+
UnconditionalFlowInfo assigned = FlowInfo.initial(this.maxFieldCount);
921+
assigned.markAsDefinitelyAssigned(field);
922+
nonStaticFieldInfo.addPotentialInitializationsFrom(assigned);
923+
}
916924
}
917925
}
918926
}

‎org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SuperAfterStatementsTest.java

+83
Original file line numberDiff line numberDiff line change
@@ -3137,4 +3137,87 @@ public static void main(String... args) {
31373137
},
31383138
"1");
31393139
}
3140+
3141+
public void testGH3748a() {
3142+
runNegativeTest(new String[] {
3143+
"X.java",
3144+
"""
3145+
public class X {
3146+
final int fin1, fin2;
3147+
{
3148+
fin1 = 0;
3149+
fin2 = 1;
3150+
}
3151+
X() {
3152+
int abc = 0; // Commenting out this line brings out the error
3153+
this(fin1 = 10);
3154+
fin2 = 11;
3155+
}
3156+
X(int x) {}
3157+
}
3158+
"""
3159+
},
3160+
"""
3161+
----------
3162+
1. ERROR in X.java (at line 4)
3163+
fin1 = 0;
3164+
^^^^
3165+
The final field fin1 may already have been assigned
3166+
----------
3167+
2. WARNING in X.java (at line 9)
3168+
this(fin1 = 10);
3169+
^^^^^^^^^^^^^^^^
3170+
You are using a preview language feature that may or may not be supported in a future release
3171+
----------
3172+
3. WARNING in X.java (at line 9)
3173+
this(fin1 = 10);
3174+
^^^^
3175+
You are using a preview language feature that may or may not be supported in a future release
3176+
----------
3177+
4. ERROR in X.java (at line 10)
3178+
fin2 = 11;
3179+
^^^^
3180+
The final field fin2 may already have been assigned
3181+
----------
3182+
""");
3183+
}
3184+
3185+
public void testGH3748b() {
3186+
runNegativeTest(new String[] {
3187+
"X.java",
3188+
"""
3189+
public class X {
3190+
final int fin1;
3191+
final int fin2;
3192+
{
3193+
fin1 = 0;
3194+
fin2 = 1;
3195+
}
3196+
X() {
3197+
this(fin1 = 10);
3198+
fin2 = 11;
3199+
}
3200+
X(int x) {}
3201+
}
3202+
"""
3203+
},
3204+
"""
3205+
----------
3206+
1. ERROR in X.java (at line 5)
3207+
fin1 = 0;
3208+
^^^^
3209+
The final field fin1 may already have been assigned
3210+
----------
3211+
2. WARNING in X.java (at line 9)
3212+
this(fin1 = 10);
3213+
^^^^
3214+
You are using a preview language feature that may or may not be supported in a future release
3215+
----------
3216+
3. ERROR in X.java (at line 10)
3217+
fin2 = 11;
3218+
^^^^
3219+
The final field fin2 may already have been assigned
3220+
----------
3221+
""");
3222+
}
31403223
}

0 commit comments

Comments
 (0)