Skip to content

Commit 01240db

Browse files
[Java 25] Bogus error: The blank final field o may not have been initialized (#4435)
Improved solution: * Fix cases were additionally a prologue exists * Clarify boundaries between PROLOGUE this() and REST Fixes #4416 --------- Co-authored-by: Srikanth Sankaran <[email protected]>
1 parent 4b07f97 commit 01240db

File tree

2 files changed

+120
-27
lines changed

2 files changed

+120
-27
lines changed

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

Lines changed: 29 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,11 @@ public void analyseCode(ClassScope classScope, InitializationFlowContext initial
8989
public void analyseCode(ClassScope classScope, InitializationFlowContext initializerFlowContext, FlowInfo flowInfo, int initialReachMode, AnalysisMode mode) {
9090
// Effect of 'AnalysisMode mode':
9191
// ALL: analyse in one go as normal.
92-
// PROLOGUE: analyse only statements *before* the explicit constructor call (if any)
93-
// REST: analyse only starting with the explicit constructor call, if none present behaves like ALL
92+
// PROLOGUE: analyse only statements up-to the explicit constructor call (arguments of this call are technically prologue, too)
93+
// if no relevant prologue exists, this invocation does nothing, and due to prologueInfo==null the next invocation will use mode ALL
94+
// REST: analyse only statements after the explicit constructor call
9495
// FlowContext and FlowInfo produced during PROLOGUE will be held in fields prologueContext and prologueInfo for use during REST
96+
// prologueInfo is furthermore assumed to happen *before* any field initializers, see its use in TypeDeclaration.internalAnalyseCode()
9597
if (this.ignoreFurtherInvestigation)
9698
return;
9799

@@ -111,16 +113,6 @@ public void analyseCode(ClassScope classScope, InitializationFlowContext initial
111113
constructorContext = this.prologueContext;
112114
flowInfo = this.prologueInfo.addInitializationsFrom(flowInfo);
113115
// skip the part already done during PROLOGUE analysis ...
114-
if (this.constructorCall != null) {
115-
if (this.constructorCall.accessMode == ExplicitConstructorCall.This) {
116-
FieldBinding[] fields = this.binding.declaringClass.fields();
117-
for (FieldBinding field : fields) {
118-
if (!field.isStatic()) {
119-
flowInfo.markAsDefinitelyAssigned(field);
120-
}
121-
}
122-
}
123-
}
124116
} else {
125117
flowInfo.setReachMode(initialReachMode);
126118

@@ -223,20 +215,16 @@ public void analyseCode(ClassScope classScope, InitializationFlowContext initial
223215
// propagate to constructor call
224216
if (this.constructorCall != null) {
225217
flowInfo = this.constructorCall.analyseCode(this.scope, constructorContext, flowInfo);
226-
if (mode == AnalysisMode.PROLOGUE && hasArgumentNeedingAnalysis)
227-
this.prologueInfo = flowInfo.copy();
228-
// if calling 'this(...)', then flag all non-static fields as definitely
229-
// set since they are supposed to be set inside other local constructor
230-
if (this.constructorCall.accessMode == ExplicitConstructorCall.This) {
231-
FieldBinding[] fields = this.binding.declaringClass.fields();
232-
for (FieldBinding field : fields) {
233-
if (!field.isStatic()) {
234-
flowInfo.markAsDefinitelyAssigned(field);
235-
}
236-
}
218+
if (mode == AnalysisMode.PROLOGUE) {
219+
if (hasArgumentNeedingAnalysis)
220+
this.prologueInfo = flowInfo.copy();
221+
return;
237222
}
238223
}
239224
}
225+
if (this.constructorCall != null && mode != AnalysisMode.PROLOGUE) {
226+
markFieldsAsInitializedAfterThisCall(this.constructorCall, flowInfo);
227+
}
240228
// reuse the reachMode from non static field info
241229
flowInfo.setReachMode(nonStaticFieldInfoReachMode);
242230

@@ -247,8 +235,10 @@ public void analyseCode(ClassScope classScope, InitializationFlowContext initial
247235
int complaintLevel = (nonStaticFieldInfoReachMode & FlowInfo.UNREACHABLE) == 0 ? Statement.NOT_COMPLAINED : Statement.COMPLAINED_FAKE_REACHABLE;
248236
for (Statement stat : this.statements) {
249237
if (mode == AnalysisMode.REST && lateConstructorCall != null) {
250-
if (stat == lateConstructorCall) // if true this is where we start analysing
238+
if (stat == lateConstructorCall) { // if true this is where we start analysing
239+
markFieldsAsInitializedAfterThisCall(lateConstructorCall, flowInfo);
251240
lateConstructorCall = null; // no more checking for subsequent statements
241+
}
252242
continue; // skip statements already processed during PROLOGUE analysis
253243
}
254244
if ((complaintLevel = stat.complainIfUnreachable(flowInfo, this.scope, complaintLevel, true)) < Statement.COMPLAINED_UNREACHABLE) {
@@ -265,9 +255,8 @@ public void analyseCode(ClassScope classScope, InitializationFlowContext initial
265255
}
266256
}
267257
if (mode == AnalysisMode.PROLOGUE) {
268-
if (this.prologueInfo == null) // don't overwrite info stored in the context of this.constructorCall
269-
this.prologueInfo = flowInfo; // keep for second iteration, also signals the need for REST analysis
270-
return; // we're done for this time
258+
this.prologueInfo = flowInfo; // keep for second iteration, also signals the need for REST analysis
259+
return; // we're done for this time
271260
}
272261
}
273262
// check for missing returning path
@@ -305,6 +294,19 @@ public void analyseCode(ClassScope classScope, InitializationFlowContext initial
305294
}
306295
}
307296

297+
private void markFieldsAsInitializedAfterThisCall(ExplicitConstructorCall call, FlowInfo flowInfo) {
298+
if (call.accessMode == ExplicitConstructorCall.This) {
299+
// if calling 'this(...)', then flag all non-static fields as definitely
300+
// set since they are supposed to be set inside other local constructor
301+
FieldBinding[] fields = this.binding.declaringClass.fields();
302+
for (FieldBinding field : fields) {
303+
if (!field.isStatic()) {
304+
flowInfo.markAsDefinitelyAssigned(field);
305+
}
306+
}
307+
}
308+
}
309+
308310
@Override
309311
public AbstractVariableDeclaration[] arguments(boolean includedElided) {
310312
return includedElided && this.isCompactConstructor() ? this.protoArguments : super.arguments(includedElided);

org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/InitializationTests.java

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,29 @@ public WrongNotInitialized() {
513513
"""
514514
});
515515
}
516+
public void testIssue4416_withPrologue() {
517+
if (this.complianceLevel < ClassFileConstants.JDK25)
518+
return; // uses flexible constructor bodies
519+
runConformTest(new String[] {
520+
"WrongNotInitialized.java",
521+
"""
522+
public class WrongNotInitialized {
523+
private final Object o;
524+
525+
public WrongNotInitialized(final Object o) {
526+
super();
527+
this.o = o;
528+
}
529+
530+
public WrongNotInitialized() {
531+
System.out.println();
532+
this(new Object());
533+
System.out.println(o.toString());
534+
}
535+
}
536+
"""
537+
});
538+
}
516539
// https://github.com/eclipse-jdt/eclipse.jdt.core/issues/4416
517540
// Bogus error: The blank final field o may not have been initialized
518541
public void testIssue4416b() {
@@ -537,6 +560,31 @@ public class Test {
537560
"""
538561
});
539562
}
563+
public void testIssue4416b_withPrologue() {
564+
if (this.complianceLevel < ClassFileConstants.JDK25)
565+
return; // uses flexible constructor bodies
566+
runConformTest(new String[] {
567+
"Test.java",
568+
"""
569+
public class Test {
570+
571+
final int x;
572+
573+
Test(int x) {
574+
this.x = x;
575+
}
576+
577+
Test(Test a) {
578+
System.out.println();
579+
this(a.x);
580+
//this(1);
581+
582+
System.out.println(x);
583+
}
584+
}
585+
"""
586+
});
587+
}
540588
// https://github.com/eclipse-jdt/eclipse.jdt.core/issues/4416
541589
// Bogus error: The blank final field o may not have been initialized
542590
public void testIssue4416c() {
@@ -556,6 +604,24 @@ public TestRecord(int b)
556604
"""
557605
});
558606
}
607+
public void testIssue4416c_withPrologue() {
608+
if (this.complianceLevel < ClassFileConstants.JDK25)
609+
return; // uses flexible constructor bodies
610+
runConformTest(new String[] {
611+
"TestRecord.java",
612+
"""
613+
public record TestRecord(String a)
614+
{
615+
public TestRecord(int b)
616+
{
617+
System.out.println();
618+
this(String.valueOf(b));
619+
System.out.println(a.length());
620+
}
621+
}
622+
"""
623+
});
624+
}
559625
// https://github.com/eclipse-jdt/eclipse.jdt.core/issues/4416
560626
// Bogus error: The blank final field o may not have been initialized
561627
public void testIssue4416d() {
@@ -578,6 +644,31 @@ public class Test {
578644
"""
579645
});
580646
}
647+
//https://github.com/eclipse-jdt/eclipse.jdt.core/issues/4416
648+
//Bogus error: The blank final field o may not have been initialized
649+
public void testIssue4416d_withPrologue() {
650+
if (this.complianceLevel < ClassFileConstants.JDK25)
651+
return; // uses flexible constructor bodies
652+
runConformTest(new String[] {
653+
"Test.java",
654+
"""
655+
public class Test {
656+
657+
private final Object r;
658+
659+
Test() {
660+
System.out.println();
661+
this(Integer.valueOf(1));
662+
r.hashCode();
663+
}
664+
665+
Test(Object r) {
666+
this.r = r;
667+
}
668+
}
669+
"""
670+
});
671+
}
581672
public static Class testClass() {
582673
return InitializationTests.class;
583674
}

0 commit comments

Comments
 (0)