eclipse-jdt / eclipse-jdt/eclipse.jdt.core

[Flexible constructors] Field initialization analysis should not be influenced by constructor locals initialization status in prologues.

Open
#5,405 0 comments 0 reactions 1 assignee Claimed by @srikanth-sankaran View on GitHub
Dominant language
Java
Stars
237
Forks
195
Avg merge
1d 12h
Merged PRs (30d)
47

Description

Found by code inspection.

I have been starting at this code on master for some time. I am convinced there is a defect here, but I don't yet have a tangible problem to report because the defect is perhaps masked somehow.

org.eclipse.jdt.internal.compiler.ast.TypeDeclaration.internalAnalyseCode(FlowContext, FlowInfo) reads:

```
...
// collect field initializations happening in constructor prologues
FlowInfo prologueInfo = null;
boolean allConstructorsHavePrologue = true;
for (AbstractMethodDeclaration method : this.methods) {
if (method instanceof ConstructorDeclaration constructor) {
FlowInfo ctorInfo = flowInfo.unconditionalFieldLessCopy();
constructor.analyseCode(this.scope, initializerContext, ctorInfo, ctorInfo.reachMode(), PROLOGUE_ANALYSIS);
ctorInfo = constructor.getPrologueInfo();
if (ctorInfo == null) {
allConstructorsHavePrologue = false;
} else if (ctorInfo.hasInits()) {
if (prologueInfo == null)
prologueInfo = ctorInfo.copy();
else
prologueInfo = prologueInfo.mergeDefiniteInitsWith(ctorInfo.unconditionalInits()); // will only evaluate field inits below
}
}
}
...
```
The prologue gathered from the constructors is still carrying along initialization status for *locals* declared inside the constructor and these are erroneously exposed to influence the field declarations.

Or in other words, it is missing the equivalent of this block of code from analyzeFieldInitializations() in https://github.com/eclipse-jdt/eclipse.jdt.core/pull/5383.

```
// field initialization analysis should not see constructor locals.
LocalVariableBinding[] locals = this.scope.locals;
if (locals != null) {
int numLocals = this.scope.localIndex;
for (int i = 0; i < numLocals; i++)
flowInfo.resetAssignmentInfo(locals[i]);
}
```

As a result, initialization status of constructor locals from prologues leaks into the `DualFlowInfo`. I can verify by stepping through the code while compiling:

```
public final class X {
final int x; // 1
final int y; // 2
final int z; // 4

{
final int zzz;
System.out.println(zzz);
x = 10;
z = 20;
}

X() {
int xx = 123; // 8
super();
y = 89;
}

X(int a) {
this.y = 10;
super();
}
}
```

that such a leak and overlay does happen between constructor parameter `a` and local `xx` and the initializer local `zzz` but perhaps due to a different bug somewhere else this problem gets masked and no ill effects are observed.

It is certainly an error that constructor locals should carry over to field analysis - this needs investigation.

https://github.com/eclipse-jdt/eclipse.jdt.core/pull/5383 - already addresses this while https://github.com/eclipse-jdt/eclipse.jdt.core/pull/5382 does not.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.