Clear metatable cache when updating values on existing nodes

Fixes #90

Author: SquidDev

src/main/java/org/squiddev/cobalt/LuaTable.java

@@ -623,6 +623,17 @@ private static LuaValue value(Object[] values, int slot, boolean weak) {
 		return strengthened;
 	}
 
+	/**
+	 * Set the value of a node. This is the inverse of {@link #value(int)}.
+	 *
+	 * @param slot  The node slot.
+	 * @param value The new value.
+	 */
+	private void setNodeValue(int slot, LuaValue value) {
+		values[slot] = weakValues ? weaken(value) : value;
+		metatableFlags = 0;
+	}
+
 	/**
 	 * Insert a new key into a hash table.
 	 * <p>
@@ -793,7 +804,7 @@ private boolean trySet(int key, LuaValue value, LuaValue keyValue) {
 			if (hasNewIndex()) return false;
 		} else {
 			if (value(node) == NIL && hasNewIndex()) return false;
-			values[node] = weakValues ? weaken(value) : value;
+			setNodeValue(node, value);
 			return true;
 		}
 
@@ -818,7 +829,7 @@ boolean trySet(LuaValue key, LuaValue value) throws LuaError {
 			if (hasNewIndex()) return false;
 		} else {
 			if (value(node) == NIL && hasNewIndex()) return false;
-			values[node] = weakValues ? weaken(value) : value;
+			setNodeValue(node, value);
 			return true;
 		}
 
@@ -846,7 +857,7 @@ private void rawset(int key, LuaValue value, LuaValue valueOf) {
 
 			// newKey will have handled this otherwise
 			if (node != -1) {
-				values[node] = weakValues ? weaken(value) : value;
+				setNodeValue(node, value);
 				return;
 			}
 		} while (true);
@@ -870,9 +881,7 @@ public void rawsetImpl(LuaValue key, LuaValue value) {
 
 			// newKey will have handled this otherwise
 			if (node != -1) {
-				// if (value.isNil() && !weakKeys) node.key = weaken((LuaValue) node.key);
-				values[node] = weakValues ? weaken(value) : value;
-				metatableFlags = 0;
+				setNodeValue(node, value);
 				return;
 			}
 		} while (true);

src/test/java/org/squiddev/cobalt/table/TableOperations.java

@@ -1,12 +1,10 @@
 package org.squiddev.cobalt.table;
 
-import org.squiddev.cobalt.LuaError;
-import org.squiddev.cobalt.LuaTable;
-import org.squiddev.cobalt.LuaValue;
-import org.squiddev.cobalt.Varargs;
+import org.squiddev.cobalt.*;
 
 import java.lang.reflect.Array;
 import java.lang.reflect.Field;
+import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
@@ -20,9 +18,11 @@ public final class TableOperations {
 	private static final Field nodes;
 	private static final Field array;
 	private static final Field lastFree;
+	private static final Method trySet;
 
 	static {
 		Field nodesField, arrayField, lastFreeField;
+		Method trySetMethod;
 		try {
 			nodesField = LuaTable.class.getDeclaredField("keys");
 			nodesField.setAccessible(true);
@@ -32,12 +32,16 @@ public final class TableOperations {
 
 			lastFreeField = LuaTable.class.getDeclaredField("lastFree");
 			lastFreeField.setAccessible(true);
+
+			trySetMethod = LuaTable.class.getDeclaredMethod("trySet", LuaValue.class, LuaValue.class);
+			trySetMethod.setAccessible(true);
 		} catch (ReflectiveOperationException e) {
 			throw new RuntimeException(e);
 		}
 		nodes = nodesField;
 		array = arrayField;
 		lastFree = lastFreeField;
+		trySet = trySetMethod;
 	}
 
 	private TableOperations() {
@@ -105,4 +109,22 @@ public static int getLastFree(LuaTable table) {
 			throw new RuntimeException(e);
 		}
 	}
+
+	/**
+	 * Set a value on a table, with the same behaviour as {@link OperationHelper#setTable(LuaState, LuaValue, LuaValue, LuaValue)}.
+	 *
+	 * @param table The table to update.
+	 * @param key   The key to set.
+	 * @param value The value to set.
+	 */
+	public static void setValue(LuaTable table, LuaValue key, LuaValue value) throws LuaError {
+		boolean success;
+		try {
+			success = (boolean) trySet.invoke(table, key, value);
+		} catch (ReflectiveOperationException e) {
+			throw new RuntimeException(e);
+		}
+
+		if (!success) table.rawset(key, value);
+	}
 }

src/test/java/org/squiddev/cobalt/table/TableTest.java

@@ -26,6 +26,7 @@
 
 import org.junit.jupiter.api.Test;
 import org.squiddev.cobalt.*;
+import org.squiddev.cobalt.function.LibFunction;
 
 import java.util.ArrayList;
 
@@ -277,4 +278,25 @@ public void testPresizeRehashes() throws LuaError {
 		assertEquals(Constants.NIL, t.next(valueOf(7)));
 	}
 
+	@Test
+	public void testCachedMetamethod() throws LuaError {
+		var t = new LuaTable();
+		var f = LibFunction.create(s -> Constants.NIL);
+
+		// The field is absent initially.
+		assertEquals(Constants.NIL, t.rawget(CachedMetamethod.INDEX));
+
+		// After setting the field, it is now present.
+		TableOperations.setValue(t, CachedMetamethod.INDEX.getKey(), f);
+		assertEquals(f, t.rawget(CachedMetamethod.INDEX));
+
+		// If we clear it, the field is no longer there.
+		TableOperations.setValue(t, CachedMetamethod.INDEX.getKey(), Constants.NIL);
+		assertEquals(Constants.NIL, t.rawget(CachedMetamethod.INDEX));
+
+		// HOWEVER, the node *is* still present. So setting it again will not create a new node. Ensure that we still
+		// reset the cache.
+		TableOperations.setValue(t, CachedMetamethod.INDEX.getKey(), f);
+		assertEquals(f, t.rawget(CachedMetamethod.INDEX));
+	}
 }