Skip to content

8359370: [lworld] allow instance fields of identity classes to be readable in the prologue phase #1490

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 22 commits into
base: lworld
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
fc30c88
8359370: [lworld] allow instance fields of identity classes to be rea…
vicente-romero-oracle Jun 12, 2025
17bf7b5
initial approach
vicente-romero-oracle Jun 17, 2025
78f8d69
experimental development
vicente-romero-oracle Jun 17, 2025
a5a6a35
cleanup
vicente-romero-oracle Jun 20, 2025
23ec9e4
additional cleanup
vicente-romero-oracle Jun 20, 2025
4ca62d8
additional cleanups
vicente-romero-oracle Jun 21, 2025
f74dc3b
adding some documentation
vicente-romero-oracle Jun 21, 2025
d0742b1
more cleanups
vicente-romero-oracle Jun 21, 2025
e29bb78
more refactorings
vicente-romero-oracle Jun 21, 2025
bc2e3d2
refacts
vicente-romero-oracle Jun 23, 2025
3f207f0
refact
vicente-romero-oracle Jun 23, 2025
680ead7
more simplifications
vicente-romero-oracle Jun 24, 2025
1a9b977
more simplifications
vicente-romero-oracle Jun 24, 2025
211f0dd
Merge branch 'JDK-8359370' of https://github.com/vicente-romero-oracl…
vicente-romero-oracle Jul 2, 2025
83b9255
updating tests
vicente-romero-oracle Jul 2, 2025
95fb2a8
adding comments, etc
vicente-romero-oracle Jul 3, 2025
9a172ea
do not generate proxies for mutable fields
vicente-romero-oracle Jul 11, 2025
f3f2913
updating golden file
vicente-romero-oracle Jul 15, 2025
475dc71
adding @enablePreview to some tests
vicente-romero-oracle Jul 15, 2025
4f0959c
addressing review comments
vicente-romero-oracle Jul 16, 2025
1531d1b
fixing bugs related to local classes, removing coupling in Flow
vicente-romero-oracle Jul 17, 2025
8323d15
removing unused field in Flow
vicente-romero-oracle Jul 17, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 28 additions & 7 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ public class Attr extends JCTree.Visitor {
final ArgumentAttr argumentAttr;
final MatchBindingsComputer matchBindingsComputer;
final AttrRecover attrRecover;
final LocalProxyVarsGen localProxyVarsGen;

public static Attr instance(Context context) {
Attr instance = context.get(attrKey);
Expand Down Expand Up @@ -163,6 +164,7 @@ protected Attr(Context context) {
argumentAttr = ArgumentAttr.instance(context);
matchBindingsComputer = MatchBindingsComputer.instance(context);
attrRecover = AttrRecover.instance(context);
localProxyVarsGen = LocalProxyVarsGen.instance(context);

Options options = Options.instance(context);

Expand Down Expand Up @@ -317,20 +319,14 @@ void checkAssignable(DiagnosticPosition pos, VarSymbol v, JCTree base, Env<AttrC
return;
}

// Check instance field assignments that appear in constructor prologues
// Check instance field assignments that appear in constructor prologues, like: `this.field = value;`
if (rs.isEarlyReference(env, base, v)) {

// Field may not be inherited from a superclass
if (v.owner != env.enclClass.sym) {
log.error(pos, Errors.CantRefBeforeCtorCalled(v));
return;
}

// Field may not have an initializer
if ((v.flags() & HASINIT) != 0) {
log.error(pos, Errors.CantAssignInitializedBeforeCtorCalled(v));
return;
}
}
}

Expand Down Expand Up @@ -4391,7 +4387,31 @@ public void visitIdent(JCIdent tree) {
}

result = checkId(tree, env1.enclClass.sym.type, sym, env, resultInfo);
checkIfAllowedInPrologue(tree);
}
// where
void checkIfAllowedInPrologue(JCTree tree) {
Assert.check(tree.hasTag(IDENT) || tree.hasTag(SELECT));
Symbol sym = TreeInfo.symbolFor(tree);
if (env.info.ctorPrologue && allowValueClasses) {
JCFieldAccess enclosingSelect = rs.new FindEnclosingSelect().scan(tree, env.tree);
if (enclosingSelect == null) { // this tree is standalone, not part of a more complex name
if (localProxyVarsGen.hasAST(env.enclMethod, tree)) {
if (sym.owner != env.enclClass.sym ||
TreeInfo.isExplicitThisOrSuperReference(types, (ClassType)env.enclClass.type, tree)) {
/* in this case we are seeing something like `super.field` or accessing a field of a
* super class while in the prologue of a subclass, at Resolve javac just didn't have enough
* information to determine this
*/
localProxyVarsGen.removeASTReadInPrologue(env.enclMethod, tree);
log.error(tree, Errors.CantRefBeforeCtorCalled(sym));
} else if (!sym.isFinal() && !sym.isStrict()) {
localProxyVarsGen.removeASTReadInPrologue(env.enclMethod, tree);
}
}
}
}
}

public void visitSelect(JCFieldAccess tree) {
// Determine the expected kind of the qualifier expression.
Expand Down Expand Up @@ -4527,6 +4547,7 @@ public void visitSelect(JCFieldAccess tree) {

env.info.selectSuper = selectSuperPrev;
result = checkId(tree, site, sym, env, resultInfo);
checkIfAllowedInPrologue(tree);
}
//where
/** Determine symbol referenced by a Select expression,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public enum CompileState {
TRANSPATTERNS(8),
LOWER(9),
UNLAMBDA(10),
STRICT_FIELDS_PROXIES(11),
FIELDS_PROXIES(11),
GENERATE(12);

CompileState(int value) {
Expand Down
63 changes: 40 additions & 23 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java
Original file line number Diff line number Diff line change
Expand Up @@ -487,15 +487,21 @@ protected void scanSyntheticBreak(TreeMaker make, JCTree swtch) {

// Do something with static or non-static field initializers and initialization blocks.
protected void forEachInitializer(JCClassDecl classDef, boolean isStatic, Consumer<? super JCTree> handler) {
forEachInitializer(classDef, isStatic, false, handler);
forEachInitializer(classDef, isStatic, InitializerDisc.PROCESS_ALL, handler);
}

enum InitializerDisc {
PROCESS_ALL,
EARLY_ONLY,
LATE_ONLY
}

/* Do something with static or non-static field initializers and initialization blocks.
* the `earlyOnly` argument will determine if we will deal or not with early variable instance
* the `discriminator` argument will determine if we will deal or not with early variable instance
* initializers we want to process only those before a super() invocation and ignore them after
* it.
*/
protected void forEachInitializer(JCClassDecl classDef, boolean isStatic, boolean earlyOnly,
protected void forEachInitializer(JCClassDecl classDef, boolean isStatic, InitializerDisc discriminator,
Consumer<? super JCTree> handler) {
if (classDef == initScanClass) // avoid infinite loops
return;
Expand All @@ -515,15 +521,18 @@ protected void forEachInitializer(JCClassDecl classDef, boolean isStatic, boolea
*/
boolean isDefStatic = ((TreeInfo.flags(def) | (TreeInfo.symbolFor(def) == null ? 0 : TreeInfo.symbolFor(def).flags_field)) & STATIC) != 0;
if (!def.hasTag(METHODDEF) && (isDefStatic == isStatic)) {
if (def instanceof JCVariableDecl varDecl) {
boolean isEarly = varDecl.init != null &&
varDecl.sym.isStrict() &&
!varDecl.sym.isStatic();
if (isEarly == earlyOnly) {
if (discriminator == InitializerDisc.PROCESS_ALL) {
handler.accept(def);
} else {
if (def instanceof JCVariableDecl varDecl) {
boolean isEarly = varDecl.init != null &&
!varDecl.sym.isStatic();
if (isEarly && discriminator == InitializerDisc.EARLY_ONLY) {
handler.accept(def);
}
} else if (discriminator == InitializerDisc.LATE_ONLY) {
handler.accept(def);
}
} else if (!earlyOnly) {
handler.accept(def);
}
}
}
Expand Down Expand Up @@ -2221,13 +2230,17 @@ protected boolean trackable(VarSymbol sym) {
return
sym.pos >= startPos &&
((sym.owner.kind == MTH || sym.owner.kind == VAR ||
isFinalOrStrictUninitializedField(sym)));
isTrackableField(sym)));
}

boolean isFinalOrStrictUninitializedField(VarSymbol sym) {
/* we want to track fields that are:
* - final regardless of "staticness"
* - non-final instance fields that lack an initializer
*/
boolean isTrackableField(VarSymbol sym) {
return sym.owner.kind == TYP &&
(((sym.flags() & (FINAL | HASINIT | PARAMETER)) == FINAL ||
(sym.flags() & (STRICT | HASINIT | PARAMETER)) == STRICT) &&
(sym.flags() & (STRICT | HASINIT | PARAMETER)) == STRICT) &&
classDef.sym.isEnclosedBy((ClassSymbol)sym.owner));
}

Expand Down Expand Up @@ -2560,15 +2573,18 @@ public void visitMethodDef(JCMethodDecl tree) {
scan(tree.body);

if (isConstructor) {
boolean isSynthesized = (tree.sym.flags() &
GENERATEDCONSTR) != 0;
boolean isAGeneratedConstructor = (tree.sym.flags() & GENERATEDCONSTR) != 0;
for (int i = firstadr; i < nextadr; i++) {
JCVariableDecl vardecl = vardecls[i];
VarSymbol var = vardecl.sym;
if (var.owner == classDef.sym && !var.isStatic()) {
// ignore non-final instance fields
if (allowValueClasses && var.owner.kind == TYP && !var.isFinal()) {
continue;
}
// choose the diagnostic position based on whether
// the ctor is default(synthesized) or not
if (isSynthesized && !isCompactOrGeneratedRecordConstructor) {
// the ctor is default(generated) or not
if (isAGeneratedConstructor && !isCompactOrGeneratedRecordConstructor) {
checkInit(TreeInfo.diagnosticPositionFor(var, vardecl),
var, Errors.VarNotInitializedInDefaultConstructor(var));
} else if (isCompactOrGeneratedRecordConstructor) {
Expand Down Expand Up @@ -3086,7 +3102,7 @@ public void visitApply(JCMethodInvocation tree) {
Name name = TreeInfo.name(tree.meth);
// let's process early initializers
if (name == names._super) {
forEachInitializer(classDef, false, true, def -> {
forEachInitializer(classDef, false, InitializerDisc.EARLY_ONLY, def -> {
scan(def);
clearPendingExits(false);
});
Expand All @@ -3108,7 +3124,7 @@ public void visitApply(JCMethodInvocation tree) {
checkInit(TreeInfo.diagEndPos(tree), var, Errors.StrictFieldNotHaveBeenInitializedBeforeSuper(var));
}
}
forEachInitializer(classDef, false, def -> {
forEachInitializer(classDef, false, InitializerDisc.LATE_ONLY, def -> {
scan(def);
clearPendingExits(false);
});
Expand All @@ -3118,7 +3134,7 @@ public void visitApply(JCMethodInvocation tree) {
else if (name == names._this) {
for (int address = firstadr; address < nextadr; address++) {
VarSymbol sym = vardecls[address].sym;
if (isFinalOrStrictUninitializedField(sym) && !sym.isStatic())
if (isTrackableField(sym) && !sym.isStatic())
letInit(tree.pos(), sym);
}
}
Expand Down Expand Up @@ -3199,9 +3215,10 @@ public void visitAssign(JCAssign tree) {
// assigned before reading their value
public void visitSelect(JCFieldAccess tree) {
super.visitSelect(tree);
if (TreeInfo.isThisQualifier(tree.selected) &&
tree.sym.kind == VAR) {
checkInit(tree.pos(), (VarSymbol)tree.sym);
if (TreeInfo.isThisQualifier(tree.selected) && tree.sym.kind == VAR) {
if (trackable((VarSymbol)tree.sym)) {
checkInit(tree.pos(), (VarSymbol) tree.sym);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,10 @@
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;

import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Symbol.ClassSymbol;
Expand Down Expand Up @@ -88,8 +90,8 @@ public static LocalProxyVarsGen instance(Context context) {
private TreeMaker make;
private final UnsetFieldsInfo unsetFieldsInfo;
private ClassSymbol currentClass = null;
private java.util.List<JCVariableDecl> strictInstanceFields;
private Map<JCMethodDecl, Set<Symbol>> strictFieldsReadInPrologue = new HashMap<>();
private java.util.List<JCVariableDecl> instanceFields;
private Map<JCMethodDecl, Set<JCTree>> ASTsReferencedInPrologue = new HashMap<>();

private final boolean noLocalProxyVars;

Expand All @@ -105,10 +107,27 @@ protected LocalProxyVarsGen(Context context) {
noLocalProxyVars = options.isSet("noLocalProxyVars");
}

public void addStrictFieldReadInPrologue(JCMethodDecl constructor, Symbol sym) {
Set<Symbol> fieldSet = strictFieldsReadInPrologue.getOrDefault(constructor, new HashSet<>());
fieldSet.add(sym);
strictFieldsReadInPrologue.put(constructor, fieldSet);
public void addASTReadInPrologue(JCMethodDecl constructor, JCTree tree) {
// better to have order for this one
Set<JCTree> treeSet = ASTsReferencedInPrologue.getOrDefault(constructor, new LinkedHashSet<>());
treeSet.add(tree);
ASTsReferencedInPrologue.put(constructor, treeSet);
}

public boolean removeASTReadInPrologue(JCMethodDecl constructor, JCTree tree) {
Set<JCTree> treeSet = ASTsReferencedInPrologue.get(constructor);
if (treeSet != null) {
return treeSet.remove(tree);
}
return false;
}

public boolean hasAST(JCMethodDecl constructor, JCTree tree) {
Set<JCTree> treeSet = ASTsReferencedInPrologue.get(constructor);
if (treeSet != null) {
return treeSet.contains(tree);
}
return false;
}

public JCTree translateTopLevelClass(JCTree cdef, TreeMaker make) {
Expand All @@ -128,32 +147,33 @@ public JCTree translateTopLevelClass(JCTree cdef, TreeMaker make) {
@Override
public void visitClassDef(JCClassDecl tree) {
ClassSymbol prevCurrentClass = currentClass;
java.util.List<JCVariableDecl> prevStrictInstanceFields = strictInstanceFields;
java.util.List<JCVariableDecl> instanceFieldsPrev = instanceFields;
try {
currentClass = tree.sym;
strictInstanceFields = tree.defs.stream()
instanceFields = tree.defs.stream()
.filter(t -> t.hasTag(VARDEF))
.map(t -> (JCVariableDecl)t)
.filter(vd -> vd.sym.isStrict() && !vd.sym.isStatic())
.filter(vd -> !vd.sym.isStatic())
.collect(List.collector());
super.visitClassDef(tree);
} finally {
currentClass = prevCurrentClass;
strictInstanceFields = prevStrictInstanceFields;
instanceFields = instanceFieldsPrev;
}
}

public void visitMethodDef(JCMethodDecl tree) {
if (strictFieldsReadInPrologue.get(tree) != null) {
Set<Symbol> fieldSet = strictFieldsReadInPrologue.get(tree);
if (ASTsReferencedInPrologue.get(tree) != null) {
Set<JCTree> ASTSet = ASTsReferencedInPrologue.get(tree);
java.util.List<JCVariableDecl> strictFieldsRead = new ArrayList<>();
for (JCVariableDecl sfield : strictInstanceFields) {
if (fieldSet.contains(sfield.sym)) {
Set<Symbol> symbols = ASTSet.stream().map(t -> TreeInfo.symbolFor(t)).collect(Collectors.toSet());
for (JCVariableDecl sfield : instanceFields) {
if (symbols.contains(sfield.sym)) {
strictFieldsRead.add(sfield);
}
}
addLocalProxiesFor(tree, strictFieldsRead);
strictFieldsReadInPrologue.remove(tree);
ASTsReferencedInPrologue.remove(tree);
}
super.visitMethodDef(tree);
}
Expand Down
Loading