diff --git a/projects/core-api/src/main/java/dan200/computercraft/api/lua/MethodResult.java b/projects/core-api/src/main/java/dan200/computercraft/api/lua/MethodResult.java index aec0f8615..8b834f54c 100644 --- a/projects/core-api/src/main/java/dan200/computercraft/api/lua/MethodResult.java +++ b/projects/core-api/src/main/java/dan200/computercraft/api/lua/MethodResult.java @@ -8,9 +8,7 @@ import dan200.computercraft.api.peripheral.IComputerAccess; import javax.annotation.Nullable; import java.nio.ByteBuffer; -import java.util.Collection; -import java.util.Map; -import java.util.Objects; +import java.util.*; /** * The result of invoking a Lua method. @@ -55,6 +53,12 @@ public final class MethodResult { *

* In order to provide a custom object with methods, one may return a {@link IDynamicLuaObject}, or an arbitrary * class with {@link LuaFunction} annotations. Anything else will be converted to {@code nil}. + *

+ * Shared objects in a {@link MethodResult} will preserve their sharing when converted to Lua values. For instance, + * {@code Map m = new HashMap(); return MethodResult.of(m, m); } will return two values {@code a}, {@code b} + * where {@code a == b}. The one exception to this is Java's singleton collections ({@link List#of()}, + * {@link Set#of()} and {@link Map#of()}), which are always converted to new table. This is not true for other + * singleton collections, such as those provided by {@link Collections} or Guava. * * @param value The value to return to the calling Lua function. * @return A method result which returns immediately with the given value. diff --git a/projects/core/src/main/java/dan200/computercraft/core/lua/CobaltLuaMachine.java b/projects/core/src/main/java/dan200/computercraft/core/lua/CobaltLuaMachine.java index 2b97f00fb..5d51e55e0 100644 --- a/projects/core/src/main/java/dan200/computercraft/core/lua/CobaltLuaMachine.java +++ b/projects/core/src/main/java/dan200/computercraft/core/lua/CobaltLuaMachine.java @@ -13,6 +13,7 @@ import dan200.computercraft.core.Logging; import dan200.computercraft.core.computer.TimeoutState; import dan200.computercraft.core.methods.LuaMethod; import dan200.computercraft.core.methods.MethodSupplier; +import dan200.computercraft.core.util.LuaUtil; import dan200.computercraft.core.util.Nullability; import dan200.computercraft.core.util.SanitisedError; import org.slf4j.Logger; @@ -183,10 +184,35 @@ public class CobaltLuaMachine implements ILuaMachine { return ValueFactory.valueOf(bytes); } + // Don't share singleton values, and instead convert them to a new table. + if (LuaUtil.isSingletonCollection(object)) return new LuaTable(); + if (values == null) values = new IdentityHashMap<>(1); var result = values.get(object); if (result != null) return result; + var wrapped = toValueWorker(object, values); + if (wrapped == null) { + LOG.warn(Logging.JAVA_ERROR, "Received unknown type '{}', returning nil.", object.getClass().getName()); + return Constants.NIL; + } + + values.put(object, wrapped); + return wrapped; + } + + /** + * Convert a complex Java object (such as a collection or Lua object) to a Lua value. + *

+ * This is a worker function for {@link #toValue(Object, IdentityHashMap)}, which handles the actual construction + * of values, without reading/writing from the value map. + * + * @param object The object to convert. + * @param values The map of Java to Lua values. + * @return The converted value, or {@code null} if it could not be converted. + * @throws LuaError If the value could not be converted. + */ + private @Nullable LuaValue toValueWorker(Object object, IdentityHashMap values) throws LuaError { if (object instanceof ILuaFunction) { return new ResultInterpreterFunction(this, FUNCTION_METHOD, object, context, object.toString()); } @@ -194,15 +220,12 @@ public class CobaltLuaMachine implements ILuaMachine { if (object instanceof IDynamicLuaObject) { LuaValue wrapped = wrapLuaObject(object); if (wrapped == null) wrapped = new LuaTable(); - values.put(object, wrapped); return wrapped; } if (object instanceof Map map) { var table = new LuaTable(); - values.put(object, table); - - for (Map.Entry pair : map.entrySet()) { + for (var pair : map.entrySet()) { var key = toValue(pair.getKey(), values); var value = toValue(pair.getValue(), values); if (!key.isNil() && !value.isNil()) table.rawset(key, value); @@ -212,27 +235,18 @@ public class CobaltLuaMachine implements ILuaMachine { if (object instanceof Collection objects) { var table = new LuaTable(objects.size(), 0); - values.put(object, table); var i = 0; - for (Object child : objects) table.rawset(++i, toValue(child, values)); + for (var child : objects) table.rawset(++i, toValue(child, values)); return table; } if (object instanceof Object[] objects) { var table = new LuaTable(objects.length, 0); - values.put(object, table); for (var i = 0; i < objects.length; i++) table.rawset(i + 1, toValue(objects[i], values)); return table; } - var wrapped = wrapLuaObject(object); - if (wrapped != null) { - values.put(object, wrapped); - return wrapped; - } - - LOG.warn(Logging.JAVA_ERROR, "Received unknown type '{}', returning nil.", object.getClass().getName()); - return Constants.NIL; + return wrapLuaObject(object); } Varargs toValues(@Nullable Object[] objects) throws LuaError { diff --git a/projects/core/src/main/java/dan200/computercraft/core/util/LuaUtil.java b/projects/core/src/main/java/dan200/computercraft/core/util/LuaUtil.java index 86205d319..0ba8dd1a0 100644 --- a/projects/core/src/main/java/dan200/computercraft/core/util/LuaUtil.java +++ b/projects/core/src/main/java/dan200/computercraft/core/util/LuaUtil.java @@ -4,9 +4,18 @@ package dan200.computercraft.core.util; +import dan200.computercraft.core.lua.ILuaMachine; + import java.util.Collection; +import java.util.List; +import java.util.Map; +import java.util.Set; public class LuaUtil { + private static final List EMPTY_LIST = List.of(); + private static final Set EMPTY_SET = Set.of(); + private static final Map EMPTY_MAP = Map.of(); + public static Object[] consArray(Object value, Collection rest) { if (rest.isEmpty()) return new Object[]{ value }; @@ -14,7 +23,20 @@ public class LuaUtil { var out = new Object[rest.size() + 1]; out[0] = value; var i = 1; - for (Object additionalType : rest) out[i++] = additionalType; + for (var additionalType : rest) out[i++] = additionalType; return out; } + + /** + * Determine whether a value is a singleton collection, such as one created with {@link List#of()}. + *

+ * These collections are treated specially by {@link ILuaMachine} implementations: we skip sharing for them, and + * create a new table each time. + * + * @param value The value to test. + * @return Whether this is a singleton collection. + */ + public static boolean isSingletonCollection(Object value) { + return value == EMPTY_LIST || value == EMPTY_SET || value == EMPTY_MAP; + } } diff --git a/projects/core/src/test/java/dan200/computercraft/core/computer/ComputerTest.java b/projects/core/src/test/java/dan200/computercraft/core/computer/ComputerTest.java index 98124fc89..674816828 100644 --- a/projects/core/src/test/java/dan200/computercraft/core/computer/ComputerTest.java +++ b/projects/core/src/test/java/dan200/computercraft/core/computer/ComputerTest.java @@ -5,11 +5,14 @@ package dan200.computercraft.core.computer; import com.google.common.io.CharStreams; +import dan200.computercraft.api.lua.ILuaAPI; +import dan200.computercraft.api.lua.LuaFunction; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import java.io.InputStreamReader; import java.nio.charset.StandardCharsets; +import java.util.List; import java.util.Objects; import static java.time.Duration.ofSeconds; @@ -30,6 +33,26 @@ public class ComputerTest { }); } + @Test + public void testDuplicateObjects() { + class CustomApi implements ILuaAPI { + @Override + public String[] getNames() { + return new String[]{ "custom" }; + } + + @LuaFunction + public final Object[] getObjects() { + return new Object[]{ List.of(), List.of() }; + } + } + + ComputerBootstrap.run(""" + local x, y = custom.getObjects() + assert(x ~= y) + """, i -> i.addApi(new CustomApi()), 50); + } + public static void main(String[] args) throws Exception { var stream = ComputerTest.class.getClassLoader().getResourceAsStream("benchmark.lua"); try (var reader = new InputStreamReader(Objects.requireNonNull(stream), StandardCharsets.UTF_8)) { diff --git a/projects/core/src/test/resources/test-rom/spec/apis/os_spec.lua b/projects/core/src/test/resources/test-rom/spec/apis/os_spec.lua index 562dcc7ff..95a50b369 100644 --- a/projects/core/src/test/resources/test-rom/spec/apis/os_spec.lua +++ b/projects/core/src/test/resources/test-rom/spec/apis/os_spec.lua @@ -188,4 +188,31 @@ describe("The os library", function() expect.error(os.loadAPI, nil):eq("bad argument #1 (string expected, got nil)") end) end) + + describe("os.queueEvent", function() + local function roundtrip(...) + local event_name = ("event_%08x"):format(math.random(1, 0x7FFFFFFF)) + os.queueEvent(event_name, ...) + return select(2, os.pullEvent(event_name)) + end + + it("preserves references in tables", function() + local tbl = {} + local xs = roundtrip({ tbl, tbl }) + expect(xs[1]):eq(xs[2]) + end) + + it("does not preserve references in separate args", function() + -- I'm not sure I like this behaviour, but it is what CC has always done. + local tbl = {} + local xs, ys = roundtrip(tbl, tbl) + expect(xs):ne(ys) + end) + + it("clones objects", function() + local tbl = {} + local xs = roundtrip(tbl) + expect(xs):ne(tbl) + end) + end) end) diff --git a/projects/core/src/test/resources/test-rom/spec/modules/cc/internal/syntax/lexer_spec.md b/projects/core/src/test/resources/test-rom/spec/modules/cc/internal/syntax/lexer_spec.md index 8cb2e646f..9d5af0531 100644 --- a/projects/core/src/test/resources/test-rom/spec/modules/cc/internal/syntax/lexer_spec.md +++ b/projects/core/src/test/resources/test-rom/spec/modules/cc/internal/syntax/lexer_spec.md @@ -19,7 +19,7 @@ We can lex some basic comments: ``` ```txt -1:1-1:37 COMMENT -- A basic singleline comment +1:1-1:29 COMMENT -- A basic singleline comment 2:1-2:27 COMMENT --[ Not a multiline comment 3:1-3:34 COMMENT --[= Also not a multiline comment! ```