Keep Lua class field lifetime semantics aligned with Jass

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaTranslator.java

@@ -207,8 +207,9 @@ public LuaVariable initFor(ImClass a) {
      * per canonical IM field, indexed by that id; class descriptors remain static tables and are
      * reached through {@link #objectClass}. Allocation therefore creates no per-instance table.
      *
-     * <p>Destroy clears every field slot before putting the id on the free stack. As in the Jass
-     * backend, a stale reference aliases a later object after that id is recycled; before reuse its
+     * <p>Destroy only removes the live-object descriptor before putting the id on the free stack.
+     * Field storage intentionally retains its value, matching the Jass backend's array-backed
+     * fields. A stale reference aliases a later object after that id is recycled; before reuse its
      * descriptor is absent, so virtual dispatch fails and {@code instanceof} is false. Capturing
      * closures use the same representation and, like Jass closures, retain their id until destroyed.
      */
@@ -1031,12 +1032,6 @@ private void translateClass(ImClass c) {
         LuaFunction cleanup = luaClassCleanup.getFor(c);
         LuaVariable object = LuaAst.LuaVariable("object", LuaAst.LuaNoExpr());
         cleanup.getParams().add(object);
-        for (ImVar field : collectFieldsForAllocation(c)) {
-            cleanup.getBody().add(LuaAst.LuaAssignment(
-                LuaAst.LuaExprArrayAccess(LuaAst.LuaExprVarAccess(fieldStorage(field)),
-                    LuaAst.LuaExprlist(LuaAst.LuaExprVarAccess(object))),
-                LuaAst.LuaExprNull()));
-        }
         luaModel.add(cleanup);
         deferMainInit(LuaAst.LuaAssignment(
             LuaAst.LuaExprFieldAccess(LuaAst.LuaExprVarAccess(classVar), "__wurst_dealloc"),

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

@@ -60,6 +60,7 @@ private enum Phase { LIGHT, HEAVY }
     private final HashSet<String> trveWrapperFuncs = new HashSet<>();
     private final HashMap<String, HashSet<FunctionCall>> wrapperCalls = new HashMap<>();
     private final Map<ClassDef, Map<GlobalVarDef, Integer>> classVarInitOrderCache = new HashMap<>();
+    private final Map<GlobalVarDef, Boolean> guaranteedClassFieldInitCache = new IdentityHashMap<>();
 
     /**
      * When true, the build targets a legacy patch (pre-1.24) whose Blizzard-provided
@@ -83,6 +84,7 @@ public void validate(Collection<CompilationUnit> toCheck) {
             visitedFunctions = 0;
             heavyFunctions.clear();
             heavyBlocks.clear();
+            guaranteedClassFieldInitCache.clear();
 
             lightValidation(toCheck);
 
@@ -1747,9 +1749,323 @@ private void checkUninitializedVars(FunctionLike f) {
             && !f.getSource().getFile().endsWith("war3map.j")) {
             new DataflowAnomalyAnalysis(Utils.isJassCode(f)).execute(f);
         }
+        checkPotentiallyUninitializedClassFields(f);
         checkJassImplicitNullLocalsReadWithoutExplicitWrite(f);
     }
 
+    /**
+     * Instance fields without an initializer are reset to the language default when an object is
+     * allocated, but that value is often accidental. Warn when a method reads such a field and no
+     * constructor is known to assign it on every construction path. This deliberately stays a
+     * cheap, local check: it does not attempt interprocedural or path-sensitive reasoning.
+     */
+    private void checkPotentiallyUninitializedClassFields(FunctionLike function) {
+        if (function instanceof OnDestroyDef) {
+            return;
+        }
+
+        Set<GlobalVarDef> writtenFields = Collections.newSetFromMap(new IdentityHashMap<>());
+        Set<GlobalVarDef> warned = Collections.newSetFromMap(new IdentityHashMap<>());
+        FunctionCall delegatedConstructorCall = function instanceof ConstructorDef
+            ? getFirstThisConstructorCall((ConstructorDef) function) : null;
+        function.accept(new Element.DefaultVisitor() {
+            private void checkField(NameRef access) {
+                NameDef nameDef = access.attrNameDef();
+                if (!(nameDef instanceof GlobalVarDef field) || !field.attrIsDynamicClassMember()) {
+                    return;
+                }
+                if (isWriteTarget(access)) {
+                    return;
+                }
+                if (!(field.getInitialExpr() instanceof NoExpr)
+                    || (isCurrentInstanceAccess(access)
+                        && writtenFields.contains(field))
+                    || (!(function instanceof ConstructorDef) && hasGuaranteedConstructorAssignment(field))
+                    || (delegatedConstructorCall != null && !access.isSubtreeOf(delegatedConstructorCall)
+                        && hasGuaranteedConstructorAssignment(field))
+                    || !warned.add(field)) {
+                    return;
+                }
+                access.addWarning("Field '" + field.getName()
+                    + "' has no explicit initializer and is not definitely assigned by every constructor;"
+                    + " this access may observe its default value."
+                    + " Initialize it explicitly in every construction path.");
+            }
+
+            @Override
+            public void visit(ExprVarAccess access) {
+                super.visit(access);
+                checkField(access);
+            }
+
+            @Override
+            public void visit(ExprVarArrayAccess access) {
+                super.visit(access);
+                checkField(access);
+            }
+
+            @Override
+            public void visit(ExprMemberVarDot access) {
+                super.visit(access);
+                checkField(access);
+            }
+
+            @Override
+            public void visit(ExprMemberVarDotDot access) {
+                super.visit(access);
+                checkField(access);
+            }
+
+            @Override
+            public void visit(ExprMemberVarQuestionDot access) {
+                super.visit(access);
+                checkField(access);
+            }
+
+            @Override
+            public void visit(ExprMemberArrayVarDot access) {
+                super.visit(access);
+                checkField(access);
+            }
+
+            @Override
+            public void visit(ExprMemberArrayVarDotDot access) {
+                super.visit(access);
+                checkField(access);
+            }
+
+            @Override
+            public void visit(StmtSet assignment) {
+                super.visit(assignment);
+                if (!(assignment.getUpdatedExpr() instanceof NameRef access)
+                    || isInNestedClosure(access)
+                    || !isCurrentInstanceAccess(access)
+                    || !isWriteTarget(access)) {
+                    return;
+                }
+                NameDef nameDef = access.attrNameDef();
+                if (nameDef instanceof GlobalVarDef field && field.attrIsDynamicClassMember()
+                    && isWholeFieldAccess(access)) {
+                    writtenFields.add(field);
+                }
+            }
+
+        });
+    }
+
+    private Set<GlobalVarDef> collectWrittenDynamicFields(Element root) {
+        Set<GlobalVarDef> result = Collections.newSetFromMap(new IdentityHashMap<>());
+        root.accept(new Element.DefaultVisitor() {
+            private void collect(NameRef access) {
+                if (access.attrNearestExprClosure() != null
+                    || !isWriteTarget(access)
+                    || !isCurrentInstanceAccess(access)) {
+                    return;
+                }
+                NameDef nameDef = access.attrNameDef();
+                if (nameDef instanceof GlobalVarDef field && field.attrIsDynamicClassMember()
+                    && isWholeFieldAccess(access)) {
+                    result.add(field);
+                }
+            }
+
+            @Override
+            public void visit(ExprVarAccess access) {
+                super.visit(access);
+                collect(access);
+            }
+
+            @Override
+            public void visit(ExprVarArrayAccess access) {
+                super.visit(access);
+                collect(access);
+            }
+
+            @Override
+            public void visit(ExprClosure closure) {
+                // A closure runs later (and may never run), so writes in its body do not
+                // initialize the object during construction.
+            }
+
+            @Override
+            public void visit(ExprMemberVarDot access) {
+                super.visit(access);
+                collect(access);
+            }
+
+            @Override
+            public void visit(ExprMemberVarDotDot access) {
+                super.visit(access);
+                collect(access);
+            }
+
+            @Override
+            public void visit(ExprMemberVarQuestionDot access) {
+                super.visit(access);
+                collect(access);
+            }
+
+            @Override
+            public void visit(ExprMemberArrayVarDot access) {
+                super.visit(access);
+                collect(access);
+            }
+
+            @Override
+            public void visit(ExprMemberArrayVarDotDot access) {
+                super.visit(access);
+                collect(access);
+            }
+        });
+        return result;
+    }
+
+    private boolean hasGuaranteedConstructorAssignment(GlobalVarDef field) {
+        Boolean cached = guaranteedClassFieldInitCache.get(field);
+        if (cached != null) {
+            return cached;
+        }
+        List<ConstructorDef> constructors = constructorsFor(field);
+        if (constructors.isEmpty()) {
+            guaranteedClassFieldInitCache.put(field, false);
+            return false;
+        }
+        for (ConstructorDef constructor : constructors) {
+            if (!constructorAssignsField(constructor, field, Collections.newSetFromMap(new IdentityHashMap<>()))) {
+                guaranteedClassFieldInitCache.put(field, false);
+                return false;
+            }
+        }
+        guaranteedClassFieldInitCache.put(field, true);
+        return true;
+    }
+
+    private boolean isCurrentInstanceAccess(NameRef access) {
+        return access.attrImplicitParameter() instanceof ExprThis;
+    }
+
+    private boolean isInNestedClosure(NameRef access) {
+        return access.attrNearestExprClosure() != null;
+    }
+
+    private boolean isWholeFieldAccess(NameRef access) {
+        return !(access instanceof AstElementWithIndexes);
+    }
+
+    private boolean constructorAssignsField(ConstructorDef constructor, GlobalVarDef field,
+                                            Set<ConstructorDef> visiting) {
+        if (!visiting.add(constructor)) {
+            return false;
+        }
+        if (collectWrittenDynamicFields(constructor).contains(field)) {
+            return true;
+        }
+        FunctionCall thisCall = getFirstThisConstructorCall(constructor);
+        if (thisCall == null) {
+            return false;
+        }
+        ConstructorDef target = OverloadingResolver.resolveThisCall(constructorsFor(constructor), thisCall);
+        return target != null && target != constructor && constructorAssignsField(target, field, visiting);
+    }
+
+    private List<ConstructorDef> constructorsFor(GlobalVarDef field) {
+        Element current = field;
+        while (current != null) {
+            if (current instanceof ModuleInstanciation module) {
+                return module.getConstructors();
+            }
+            if (current instanceof ClassOrModule owner) {
+                return owner.getConstructors();
+            }
+            current = current.getParent();
+        }
+        return Collections.emptyList();
+    }
+
+    private List<ConstructorDef> constructorsFor(ConstructorDef constructor) {
+        Element current = constructor;
+        while (current != null) {
+            if (current instanceof ModuleInstanciation module) {
+                return module.getConstructors();
+            }
+            if (current instanceof ClassOrModule owner) {
+                return owner.getConstructors();
+            }
+            current = current.getParent();
+        }
+        return Collections.emptyList();
+    }
+
+    private void checkClassFieldInitializerReads(GlobalVarDef field) {
+        if (!field.attrIsDynamicClassMember() || !(field.getInitialExpr() instanceof Expr initializer)) {
+            return;
+        }
+        Set<GlobalVarDef> warned = Collections.newSetFromMap(new IdentityHashMap<>());
+        initializer.accept(new Element.DefaultVisitor() {
+            private void checkField(NameRef access) {
+                NameDef nameDef = access.attrNameDef();
+                if (!(nameDef instanceof GlobalVarDef referenced)
+                    || !referenced.attrIsDynamicClassMember()
+                    || !(referenced.getInitialExpr() instanceof NoExpr)
+                    || (!isCurrentInstanceAccess(access) && hasGuaranteedConstructorAssignment(referenced))
+                    || !warned.add(referenced)) {
+                    return;
+                }
+                access.addWarning("Field '" + referenced.getName()
+                    + "' is read from a field initializer without an explicit initializer;"
+                    + " this access may observe its default value."
+                    + " Initialize it explicitly before using it.");
+            }
+
+            @Override
+            public void visit(ExprClosure closure) {
+                // A closure runs later (and may never run), so its body is not field initialization.
+            }
+
+            @Override
+            public void visit(ExprVarAccess access) {
+                super.visit(access);
+                checkField(access);
+            }
+
+            @Override
+            public void visit(ExprVarArrayAccess access) {
+                super.visit(access);
+                checkField(access);
+            }
+
+            @Override
+            public void visit(ExprMemberVarDot access) {
+                super.visit(access);
+                checkField(access);
+            }
+
+            @Override
+            public void visit(ExprMemberVarDotDot access) {
+                super.visit(access);
+                checkField(access);
+            }
+
+            @Override
+            public void visit(ExprMemberVarQuestionDot access) {
+                super.visit(access);
+                checkField(access);
+            }
+
+            @Override
+            public void visit(ExprMemberArrayVarDot access) {
+                super.visit(access);
+                checkField(access);
+            }
+
+            @Override
+            public void visit(ExprMemberArrayVarDotDot access) {
+                super.visit(access);
+                checkField(access);
+            }
+        });
+    }
+
     /**
      * JASS compatibility shim: we currently synthesize "= null" for uninitialized non-primitive
      * locals to avoid invalid emitted JASS. Still report likely user bugs early when such a local
@@ -1836,11 +2152,15 @@ public void visit(ExprVarAccess varAccess) {
     }
 
     private boolean isWriteTarget(ExprVarAccess varAccess) {
-        if (!(varAccess.getParent() instanceof StmtSet)) {
+        return isWriteTarget((Element) varAccess);
+    }
+
+    private boolean isWriteTarget(Element access) {
+        if (!(access.getParent() instanceof StmtSet)) {
             return false;
         }
-        StmtSet set = (StmtSet) varAccess.getParent();
-        return set.getUpdatedExpr() == varAccess;
+        StmtSet set = (StmtSet) access.getParent();
+        return set.getUpdatedExpr() == access;
     }
 
     private @Nullable StmtSet nearestEnclosingStmtSet(Element e) {
@@ -3574,7 +3894,9 @@ private void checkVarDef(VarDef v) {
         }
 
         if (v instanceof GlobalVarDef) {
-            checkClassMemberInitializerOrder((GlobalVarDef) v);
+            GlobalVarDef field = (GlobalVarDef) v;
+            checkClassMemberInitializerOrder(field);
+            checkClassFieldInitializerReads(field);
         }
 
     }