diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/EliminateGenerics.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/EliminateGenerics.java index 885fa6f47..6baf40263 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/EliminateGenerics.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/EliminateGenerics.java @@ -128,6 +128,41 @@ public void transformGenericNewOnly() { } eliminateRemainingGenericNewCalls(); assertNoReachableGenericNewMarkers(); + settleRemainingDispatches(); + } + + /** + * Neutralises the dispatches left in functions that were specialized. + *

+ * Such a function is dead: every reachable call to it was rewritten to its specialization, so a + * dispatch still sitting in the original can never run. This backend keeps generics rather than + * removing them wholesale, so those originals are still translated and the dispatch would reach + * a backend with no way to express it. + *

+ * Anything else is left alone. A dispatch may legitimately remain in a bounded generic that is + * merely declared and never called, and garbage removal deletes those later; deciding here + * would mean duplicating reachability. One which survives that far and still reaches the + * backend is reported there, where it is known to be both reachable and unresolvable. + */ + private void settleRemainingDispatches() { + List remaining = new ArrayList<>(); + prog.accept(new Element.DefaultVisitor() { + @Override + public void visit(ImTypeVarDispatch dispatch) { + super.visit(dispatch); + remaining.add(dispatch); + } + }); + for (ImTypeVarDispatch dispatch : remaining) { + ImFunction owner = dispatch.getNearestFunc(); + if (owner != null && specializedFunctions.containsRow(owner)) { + dispatch.replaceBy(defaultValueFor(dispatch.getTypeClassFunc().getReturnType())); + } + } + } + + private static ImExpr defaultValueFor(ImType type) { + return JassIm.ImNull(type.copy()); } @@ -271,7 +306,7 @@ public void visit(ImAlloc alloc) { // Constructing a class whose methods dispatch has to be specialised as well: // otherwise the constructor keeps a generic result type, and a method call on that // result never becomes concrete enough to resolve. - if (classNeedsSpecialization(alloc.getClazz().getClassDef(), visitedFunctions, visitedMethods)) { + if (classNeedsSpecialization(alloc.getClazz().getClassDef())) { found[0] = true; return; } @@ -320,44 +355,59 @@ && functionNeedsSpecialization(method.getImplementation(), visitedFunctions, vis /** * Whether constructing this class requires the concrete type argument, because one of its own * or inherited members dispatches on a type class bound. + *

+ * Deliberately a property of the class alone, not of the path that asked. An earlier version + * threaded the caller's visited set through here and memoised the answer, so a query made while + * one of the class's own functions was already being visited recorded a negative result that + * then stood for every later query. */ - private boolean classNeedsSpecialization(ImClass classDef, Set visitedFunctions, - Set visitedMethods) { - Boolean cached = classNeedsSpecialization.get(classDef); + private boolean classNeedsSpecialization(ImClass classDef) { + Boolean cached = classNeedsSpecializationCache.get(classDef); if (cached != null) { - // Already answered, or currently being answered: a class reached through its own - // members contributes nothing new to the decision. return cached; } - classNeedsSpecialization.put(classDef, false); - boolean result = false; + classNeedsSpecializationCache.put(classDef, false); + boolean result = classDispatchesOnBound(classDef, + Collections.newSetFromMap(new IdentityHashMap<>())); + classNeedsSpecializationCache.put(classDef, result); + return result; + } + + private boolean classDispatchesOnBound(ImClass classDef, Set visited) { + if (!visited.add(classDef)) { + return false; + } for (ImFunction f : classDef.getFunctions()) { - if (functionNeedsSpecialization(f, visitedFunctions, visitedMethods)) { - result = true; - break; + if (containsDispatch(f)) { + return true; } } - if (!result) { - for (ImMethod m : classDef.getMethods()) { - if (methodNeedsSpecialization(m, visitedFunctions, visitedMethods)) { - result = true; - break; - } + for (ImMethod m : classDef.getMethods()) { + if (m.getImplementation() != null && containsDispatch(m.getImplementation())) { + return true; } } - if (!result) { - for (ImClassType superType : classDef.getSuperClasses()) { - if (classNeedsSpecialization(superType.getClassDef(), visitedFunctions, visitedMethods)) { - result = true; - break; - } + for (ImClassType superType : classDef.getSuperClasses()) { + if (classDispatchesOnBound(superType.getClassDef(), visited)) { + return true; } } - classNeedsSpecialization.put(classDef, result); - return result; + return false; + } + + /** Whether this function body dispatches on a bound, without following calls out of it. */ + private static boolean containsDispatch(ImFunction f) { + boolean[] found = {false}; + f.accept(new Element.DefaultVisitor() { + @Override + public void visit(ImTypeVarDispatch dispatch) { + found[0] = true; + } + }); + return found[0]; } - private final Map classNeedsSpecialization = new IdentityHashMap<>(); + private final Map classNeedsSpecializationCache = new IdentityHashMap<>(); private void assertNoReachableGenericNewMarkers() { prog.accept(new Element.DefaultVisitor() { diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/ExprTranslation.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/ExprTranslation.java index 035e311d2..8b93faeee 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/ExprTranslation.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/ExprTranslation.java @@ -1,6 +1,7 @@ package de.peeeq.wurstscript.translation.lua.translation; import de.peeeq.wurstscript.WurstOperator; +import de.peeeq.wurstscript.attributes.CompileError; import de.peeeq.wurstscript.jassIm.*; import de.peeeq.wurstscript.luaAst.*; import de.peeeq.wurstscript.translation.imtranslation.ImTranslator; @@ -461,7 +462,12 @@ public static LuaExpr translate(ImCompiletimeExpr imCompiletimeExpr, LuaTranslat } public static LuaExpr translate(ImTypeVarDispatch imTypeVarDispatch, LuaTranslator tr) { - throw new Error("not implemented"); + // Reaching the backend means specialization never supplied a concrete type for this + // dispatch and the code is reachable, since unreachable functions have been removed by now. + throw new CompileError(imTypeVarDispatch.attrTrace().attrSource(), + "Type class dispatch of " + imTypeVarDispatch.getTypeClassFunc().getName() + + " could not be resolved for the Lua target: the concrete type is not available" + + " where it is used."); } public static LuaExpr translate(ImCast imCast, LuaTranslator tr) { diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/types/WurstTypeTypeParam.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/types/WurstTypeTypeParam.java index 927668c38..2e6cfc385 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/types/WurstTypeTypeParam.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/types/WurstTypeTypeParam.java @@ -13,6 +13,7 @@ import io.vavr.control.Option; import org.eclipse.jdt.annotation.Nullable; +import java.util.ArrayList; import java.util.List; import java.util.stream.Stream; @@ -98,13 +99,45 @@ public void addMemberMethods(Element node, String name, List result) { if (!staticRef) { return; } + // Bounds are ordered and an earlier one wins, but only over the same signature: two bounds + // may require the very same operation, and offering both would make every call ambiguous. + // Differently shaped overloads are not in competition, so later bounds still contribute + // them and overload resolution picks between them as usual. + List supplied = new ArrayList<>(); for (InterfaceDef bound : TypeClassConstraints.boundInterfaces(def)) { for (FuncDef method : bound.getMethods()) { - if (method.getName().equals(name)) { - result.add(requirementLink(node, bound, method)); + if (!method.getName().equals(name)) { + continue; } + FuncLink candidate = requirementLink(node, bound, method); + if (!alreadySupplied(supplied, candidate, node)) { + supplied.add(candidate); + } + } + } + result.addAll(supplied); + } + + /** True when an earlier bound already supplied a requirement of the same shape. */ + private static boolean alreadySupplied(List supplied, FuncLink candidate, Element node) { + for (FuncLink existing : supplied) { + List a = existing.getParameterTypes(); + List b = candidate.getParameterTypes(); + if (a.size() != b.size()) { + continue; + } + boolean same = true; + for (int i = 0; i < a.size(); i++) { + if (!a.get(i).equalsType(b.get(i), node)) { + same = false; + break; + } + } + if (same) { + return true; } } + return false; } @Override diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/TypeClassTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/TypeClassTests.java index 18aadaafc..c074a6c5d 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/TypeClassTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/TypeClassTests.java @@ -847,6 +847,124 @@ public void sameSimpleNameThroughRegistryFallback() { ); } + /** + * Two bounds may require the same operation. Bounds are ordered and the earlier one wins, + * rather than every call to the shared operation becoming ambiguous. + */ + @Test + public void duplicateRequirementAcrossBounds() { + testAssertOkLines(true, + "package test", + "native testSuccess()", + "interface First", + " function show(T x) returns string", + "interface Second", + " function other(T x) returns int", + " function show(T x) returns string", + "implements First", + " function show(int x) returns string", + " return \"first\"", + "implements Second", + " function other(int x) returns int", + " return 1", + " function show(int x) returns string", + " return \"second\"", + "function render(Q x) returns string", + " return Q.show(x)", + "init", + " if render(1) == \"first\"", + " testSuccess()" + ); + } + + /** + * Bounds only shadow each other when they require the same shape. A later bound still supplies + * a differently shaped overload, which overload resolution then chooses between. + */ + @Test + public void overloadFromLaterBoundStaysAvailable() { + testAssertOkLines(true, + "package test", + "native testSuccess()", + "interface First", + " function show(T x) returns string", + "interface Second", + " function show(T x) returns string", + " function show(int scale, T x) returns string", + "implements First", + " function show(int x) returns string", + " return \"first\"", + "implements Second", + " function show(int x) returns string", + " return \"second\"", + " function show(int scale, int x) returns string", + " return \"scaled\"", + "function render(Q x) returns string", + " return Q.show(x) + Q.show(2, x)", + "init", + " if render(1) == \"firstscaled\"", + " testSuccess()" + ); + } + + /** + * A bounded generic which is only declared, never called, stays valid: it is unreachable, so + * nothing has to supply a concrete type for it. + */ + @Test + public void unusedBoundedGenericFunctionLua() { + test().testLua(true).executeProg().lines( + "package test", + "native testSuccess()", + "interface Show", + " function show(T x) returns string", + "implements Show", + " function show(int x) returns string", + " return \"i\"", + "function unused(Q x) returns string", + " return Q.show(x)", + "init", + " testSuccess()" + ); + } + + /** The same for a bounded generic class which is never constructed. */ + @Test + public void unusedBoundedGenericClassLua() { + test().testLua(true).executeProg().lines( + "package test", + "native testSuccess()", + "interface Show", + " function show(T x) returns string", + "implements Show", + " function show(int x) returns string", + " return \"i\"", + "class Unused", + " function render(Q x) returns string", + " return Q.show(x)", + "init", + " testSuccess()" + ); + } + + /** Declared and unused on Jass too, which is where it already worked. */ + @Test + public void unusedBoundedGenericFunction() { + testAssertOkLines(true, + "package test", + "native testSuccess()", + "interface Show", + " function show(T x) returns string", + "implements Show", + " function show(int x) returns string", + " return \"i\"", + "function unused(Q x) returns string", + " return Q.show(x)", + "init", + " testSuccess()" + ); + } + /** A type parameter is not a value, so it may only appear as the receiver of a requirement. */ @Test public void typeParameterIsNotAValue() {