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 5d51e55e0..e0f2ccf17 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 @@ -95,11 +95,8 @@ public CobaltLuaMachine(MachineEnvironment environment, InputStream bios) throws private void addAPI(LuaState state, LuaTable globals, ILuaAPI api) throws LuaError { // Add the methods of an API to the global table - var table = wrapLuaObject(api); - if (table == null) { - LOG.warn("API {} does not provide any methods", api); - table = new LuaTable(); - } + var table = new LuaTable(); + if (!makeLuaObject(api, table)) LOG.warn("API {} does not provide any methods", api); var names = api.getNames(); for (var name : names) globals.rawset(name, table); @@ -163,13 +160,16 @@ public void close() { timeout.removeListener(timeoutListener); } - @Nullable - private LuaTable wrapLuaObject(Object object) { - var table = new LuaTable(); - var found = luaMethods.forEachMethod(object, (target, name, method, info) -> + /** + * Populate a table with methods from an object. + * + * @param object The object to draw methods from. + * @param table The table to fill. + * @return Whether any methods were found. + */ + private boolean makeLuaObject(Object object, LuaTable table) { + return luaMethods.forEachMethod(object, (target, name, method, info) -> table.rawset(name, new ResultInterpreterFunction(this, method, target, context, name))); - - return found ? table : null; } private LuaValue toValue(@Nullable Object object, @Nullable IdentityHashMap values) throws LuaError { @@ -184,47 +184,35 @@ private LuaValue toValue(@Nullable Object object, @Nullable 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()); + var function = new ResultInterpreterFunction(this, FUNCTION_METHOD, object, context, object.toString()); + values.put(object, function); + return function; } if (object instanceof IDynamicLuaObject) { - LuaValue wrapped = wrapLuaObject(object); - if (wrapped == null) wrapped = new LuaTable(); - return wrapped; + var table = new LuaTable(); + makeLuaObject(object, table); + values.put(object, table); + return table; } + // The following objects may be recursive. In these instances, we need to be careful to store the value *before* + // recursing, to avoid stack overflows. + if (object instanceof Map map) { + // Don't share singleton values, and instead convert them to a new table. + if (LuaUtil.isSingletonMap(map)) return new LuaTable(); + var table = new LuaTable(); + values.put(object, table); + for (var pair : map.entrySet()) { var key = toValue(pair.getKey(), values); var value = toValue(pair.getValue(), values); @@ -234,7 +222,12 @@ private LuaValue toValue(@Nullable Object object, @Nullable IdentityHashMap objects) { + // Don't share singleton values, and instead convert them to a new table. + if (LuaUtil.isSingletonCollection(objects)) return new LuaTable(); + var table = new LuaTable(objects.size(), 0); + values.put(object, table); + var i = 0; for (var child : objects) table.rawset(++i, toValue(child, values)); return table; @@ -242,11 +235,20 @@ private LuaValue toValue(@Nullable Object object, @Nullable IdentityHashMap rest) { * @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; + public static boolean isSingletonCollection(Collection value) { + return value == EMPTY_LIST || value == EMPTY_SET; + } + + /** + * Determine whether a value is a singleton map, such as one created with {@link Map#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 map. + */ + public static boolean isSingletonMap(Map value) { + return value == EMPTY_MAP; } } 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 95a50b369..9c5980adf 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 @@ -202,6 +202,14 @@ describe("The os library", function() expect(xs[1]):eq(xs[2]) end) + it("handles recursive tables", function() + local tbl = {} + tbl[1] = tbl + + local xs = roundtrip(tbl) + expect(xs):eq(xs[1]) + 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 = {}