Skip to content

Commit 84a8570

Browse files
FrottyFrotty
committed
Complete constructor and closure field flow
1 parent 1230deb commit 84a8570

2 files changed

Lines changed: 188 additions & 7 deletions

File tree

‎de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/validation/WurstValidator.java‎

Lines changed: 102 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ private enum Phase { LIGHT, HEAVY }
6161
private final HashMap<String, HashSet<FunctionCall>> wrapperCalls = new HashMap<>();
6262
private final Map<ClassDef, Map<GlobalVarDef, Integer>> classVarInitOrderCache = new HashMap<>();
6363
private final Map<GlobalVarDef, Boolean> guaranteedClassFieldInitCache = new IdentityHashMap<>();
64+
private final Map<GlobalVarDef, List<GlobalVarDef>> moduleFieldCopiesCache = new IdentityHashMap<>();
6465

6566
/**
6667
* When true, the build targets a legacy patch (pre-1.24) whose Blizzard-provided
@@ -85,6 +86,7 @@ public void validate(Collection<CompilationUnit> toCheck) {
8586
heavyFunctions.clear();
8687
heavyBlocks.clear();
8788
guaranteedClassFieldInitCache.clear();
89+
moduleFieldCopiesCache.clear();
8890

8991
lightValidation(toCheck);
9092

@@ -1764,7 +1766,8 @@ private void checkPotentiallyUninitializedClassFields(FunctionLike function) {
17641766
return;
17651767
}
17661768

1767-
Set<GlobalVarDef> writtenFields = Collections.newSetFromMap(new IdentityHashMap<>());
1769+
Deque<Set<GlobalVarDef>> writtenFieldScopes = new ArrayDeque<>();
1770+
writtenFieldScopes.push(Collections.newSetFromMap(new IdentityHashMap<>()));
17681771
Set<GlobalVarDef> warned = Collections.newSetFromMap(new IdentityHashMap<>());
17691772
FunctionCall delegatedConstructorCall = function instanceof ConstructorDef
17701773
? getFirstThisConstructorCall((ConstructorDef) function) : null;
@@ -1779,9 +1782,14 @@ private void checkField(NameRef access) {
17791782
}
17801783
if (!(field.getInitialExpr() instanceof NoExpr)
17811784
|| (isCurrentInstanceAccess(access)
1782-
&& writtenFields.contains(field))
1785+
&& writtenFieldScopes.peek().contains(field))
17831786
|| ((!isCurrentInstanceAccess(access) || !(function instanceof ConstructorDef))
17841787
&& hasGuaranteedConstructorAssignment(field))
1788+
|| (isCurrentInstanceAccess(access)
1789+
&& function instanceof ConstructorDef
1790+
&& delegatedConstructorCall == null
1791+
&& !access.isSubtreeOf(((ConstructorDef) function).getSuperConstructorCall())
1792+
&& initializedBySuperConstructor((ConstructorDef) function, field))
17851793
|| (delegatedConstructorCall != null && !access.isSubtreeOf(delegatedConstructorCall)
17861794
&& hasGuaranteedConstructorAssignment(field))
17871795
|| !warned.add(field)) {
@@ -1835,19 +1843,27 @@ public void visit(ExprMemberArrayVarDotDot access) {
18351843
checkField(access);
18361844
}
18371845

1846+
@Override
1847+
public void visit(ExprClosure closure) {
1848+
Set<GlobalVarDef> closureScope = Collections.newSetFromMap(new IdentityHashMap<>());
1849+
closureScope.addAll(writtenFieldScopes.peek());
1850+
writtenFieldScopes.push(closureScope);
1851+
super.visit(closure);
1852+
writtenFieldScopes.pop();
1853+
}
1854+
18381855
@Override
18391856
public void visit(StmtSet assignment) {
18401857
super.visit(assignment);
18411858
if (!(assignment.getUpdatedExpr() instanceof NameRef access)
1842-
|| isInNestedClosure(access)
18431859
|| !isCurrentInstanceAccess(access)
18441860
|| !isWriteTarget(access)) {
18451861
return;
18461862
}
18471863
NameDef nameDef = access.attrNameDef();
18481864
if (nameDef instanceof GlobalVarDef field && field.attrIsDynamicClassMember()
18491865
&& isWholeFieldAccess(access)) {
1850-
writtenFields.add(field);
1866+
writtenFieldScopes.peek().add(field);
18511867
}
18521868
}
18531869

@@ -1927,17 +1943,27 @@ private boolean hasGuaranteedConstructorAssignment(GlobalVarDef field) {
19271943
return cached;
19281944
}
19291945
List<ConstructorDef> constructors = constructorsFor(field);
1946+
if (allConstructorsAssign(constructors, field)
1947+
|| moduleFieldCopies(field).stream().anyMatch(copy ->
1948+
allConstructorsAssign(constructorsFor(copy), copy)
1949+
|| allConstructorsAssign(enclosingClassConstructors(copy), copy))
1950+
|| allConstructorsAssign(enclosingClassConstructors(field), field)) {
1951+
guaranteedClassFieldInitCache.put(field, true);
1952+
return true;
1953+
}
1954+
guaranteedClassFieldInitCache.put(field, false);
1955+
return false;
1956+
}
1957+
1958+
private boolean allConstructorsAssign(List<ConstructorDef> constructors, GlobalVarDef field) {
19301959
if (constructors.isEmpty()) {
1931-
guaranteedClassFieldInitCache.put(field, false);
19321960
return false;
19331961
}
19341962
for (ConstructorDef constructor : constructors) {
19351963
if (!constructorAssignsField(constructor, field, Collections.newSetFromMap(new IdentityHashMap<>()))) {
1936-
guaranteedClassFieldInitCache.put(field, false);
19371964
return false;
19381965
}
19391966
}
1940-
guaranteedClassFieldInitCache.put(field, true);
19411967
return true;
19421968
}
19431969

@@ -1997,6 +2023,73 @@ private List<ConstructorDef> constructorsFor(ConstructorDef constructor) {
19972023
return Collections.emptyList();
19982024
}
19992025

2026+
private List<ConstructorDef> enclosingClassConstructors(GlobalVarDef field) {
2027+
Element current = field;
2028+
while (current != null) {
2029+
if (current instanceof ClassDef classDef) {
2030+
return classDef.getConstructors();
2031+
}
2032+
current = current.getParent();
2033+
}
2034+
return Collections.emptyList();
2035+
}
2036+
2037+
private List<GlobalVarDef> moduleFieldCopies(GlobalVarDef field) {
2038+
List<GlobalVarDef> cached = moduleFieldCopiesCache.get(field);
2039+
if (cached != null) {
2040+
return cached;
2041+
}
2042+
ClassOrModule owner = field.attrNearestClassOrModule();
2043+
if (!(owner instanceof ModuleDef module)) {
2044+
cached = Collections.emptyList();
2045+
moduleFieldCopiesCache.put(field, cached);
2046+
return cached;
2047+
}
2048+
int fieldIndex = module.getVars().indexOf(field);
2049+
if (fieldIndex < 0) {
2050+
cached = Collections.emptyList();
2051+
moduleFieldCopiesCache.put(field, cached);
2052+
return cached;
2053+
}
2054+
List<GlobalVarDef> copies = new ArrayList<>();
2055+
prog.accept(new Element.DefaultVisitor() {
2056+
@Override
2057+
public void visit(ModuleInstanciation instantiation) {
2058+
if (instantiation.attrModuleOrigin() == module
2059+
&& fieldIndex < instantiation.getVars().size()) {
2060+
copies.add(instantiation.getVars().get(fieldIndex));
2061+
}
2062+
super.visit(instantiation);
2063+
}
2064+
});
2065+
cached = List.copyOf(copies);
2066+
moduleFieldCopiesCache.put(field, cached);
2067+
return cached;
2068+
}
2069+
2070+
private boolean initializedBySuperConstructor(ConstructorDef constructor, GlobalVarDef field) {
2071+
ConstructorDef superConstructor = constructor.attrSuperConstructor();
2072+
return superConstructor != null
2073+
&& constructorAssignsField(superConstructor, field,
2074+
Collections.newSetFromMap(new IdentityHashMap<>()));
2075+
}
2076+
2077+
private boolean initializedBySuperclass(GlobalVarDef initializedField, GlobalVarDef referencedField) {
2078+
ClassDef child = initializedField.attrNearestClassDef();
2079+
ClassDef declaringClass = referencedField.attrNearestClassDef();
2080+
if (child == null || declaringClass == null || child == declaringClass) {
2081+
return false;
2082+
}
2083+
WurstTypeClass superType = child.attrTypC().extendedClass();
2084+
while (superType != null) {
2085+
if (superType.getClassDef() == declaringClass) {
2086+
return hasGuaranteedConstructorAssignment(referencedField);
2087+
}
2088+
superType = superType.extendedClass();
2089+
}
2090+
return false;
2091+
}
2092+
20002093
private void checkClassFieldInitializerReads(GlobalVarDef field) {
20012094
if (!field.attrIsDynamicClassMember() || !(field.getInitialExpr() instanceof Expr initializer)) {
20022095
return;
@@ -2009,6 +2102,8 @@ private void checkField(NameRef access) {
20092102
|| !referenced.attrIsDynamicClassMember()
20102103
|| !(referenced.getInitialExpr() instanceof NoExpr)
20112104
|| (!isCurrentInstanceAccess(access) && hasGuaranteedConstructorAssignment(referenced))
2105+
|| (isCurrentInstanceAccess(access)
2106+
&& initializedBySuperclass(field, referenced))
20122107
|| !warned.add(referenced)) {
20132108
return;
20142109
}

‎de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/ClassesTests.java‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,92 @@ public void doesNotWarnForClosureReadAfterPriorAssignment() {
336336
result.getGui().getWarningList().toString());
337337
}
338338

339+
@Test
340+
public void doesNotWarnForClosureReadAfterClosureAssignment() {
341+
CompilationResult result = test()
342+
.setStopOnFirstError(false)
343+
.executeProg(false)
344+
.lines(
345+
"package Test",
346+
"interface Reader",
347+
" function read() returns int",
348+
"function consume(Reader reader)",
349+
"class Counter",
350+
" int value",
351+
" construct()",
352+
" consume() ->",
353+
" value = 1",
354+
" return value"
355+
);
356+
357+
assertFalse(result.getGui().getWarningList().stream()
358+
.anyMatch(w -> w.getMessage().contains("no explicit initializer and is not definitely assigned")),
359+
result.getGui().getWarningList().toString());
360+
}
361+
362+
@Test
363+
public void doesNotWarnForInheritedFieldAfterSuperclassConstructor() {
364+
CompilationResult result = test()
365+
.setStopOnFirstError(false)
366+
.executeProg(false)
367+
.lines(
368+
"package Test",
369+
"class Base",
370+
" int value",
371+
" construct()",
372+
" value = 1",
373+
"class Child extends Base",
374+
" construct()",
375+
" int copy = value"
376+
);
377+
378+
assertFalse(result.getGui().getWarningList().stream()
379+
.anyMatch(w -> w.getMessage().contains("no explicit initializer and is not definitely assigned")),
380+
result.getGui().getWarningList().toString());
381+
}
382+
383+
@Test
384+
public void doesNotWarnForInheritedFieldInitializerAfterSuperclassConstructor() {
385+
CompilationResult result = test()
386+
.setStopOnFirstError(false)
387+
.executeProg(false)
388+
.lines(
389+
"package Test",
390+
"class Base",
391+
" int value",
392+
" construct()",
393+
" value = 1",
394+
"class Child extends Base",
395+
" int copy = value"
396+
);
397+
398+
assertFalse(result.getGui().getWarningList().stream()
399+
.anyMatch(w -> w.getMessage().contains("read from a field initializer")),
400+
result.getGui().getWarningList().toString());
401+
}
402+
403+
@Test
404+
public void doesNotWarnWhenClassConstructorAssignsModuleField() {
405+
CompilationResult result = test()
406+
.setStopOnFirstError(false)
407+
.executeProg(false)
408+
.lines(
409+
"package Test",
410+
"module Values",
411+
" int value",
412+
" function get() returns int",
413+
" return value",
414+
"class Counter",
415+
" use Values",
416+
" construct()",
417+
" value = 1"
418+
);
419+
420+
assertFalse(result.getGui().getWarningList().stream()
421+
.anyMatch(w -> w.getMessage().contains("no explicit initializer and is not definitely assigned")),
422+
result.getGui().getWarningList().toString());
423+
}
424+
339425
@Test
340426
public void classes1() throws IOException {
341427
testAssertOkFile(new File(TEST_DIR + "Classes_1.wurst"), true);

0 commit comments

Comments
 (0)