Fix return value for table.remove() and table.insert()

README.html

@@ -974,6 +974,7 @@ Files are no longer hosted at LuaForge.
 <li>collectgarbage() now behaves same as collectgarbage("collect") (fixes issue #41).</li>
 <li>Allow access to Java inner classes using lua field syntax (fixes issue #40).</li>
 <li>List keyeq() and keyindex() methods as abstract on LuaTable.Entry (issue #37).</li>
+<li>Fix return value for table.remove() and table.insert() (issue #39)</li>
 
 </ul></td></tr>
 </table></td></tr></table>

src/core/org/luaj/vm2/lib/TableLib.java

@@ -90,20 +90,25 @@ public class TableLib extends TwoArgFunction {
 		}
 	}
 
-	// "insert" (table, [pos,] value) -> prev-ele
-	static class insert extends TableLibFunction {
-		public LuaValue call(LuaValue list) {
+	// "insert" (table, [pos,] value)
+	static class insert extends VarArgFunction {
+		public Varargs invoke(Varargs args) {
+			switch (args.narg()) {
+			case 0: case 1: {
 				return argerror(2, "value expected");
 			}
-		public LuaValue call(LuaValue table, LuaValue value) {
-			table.checktable().insert(table.length()+1,value);
+			case 2: {
+				LuaTable table = args.arg1().checktable();
+				table.insert(table.length()+1,args.arg(2));
 				return NONE;
 			}
-		public LuaValue call(LuaValue table, LuaValue pos, LuaValue value) {
-			table.checktable().insert(pos.checkint(),value);
+			default: {
+				args.arg1().checktable().insert(args.checkint(2),args.arg(3));
 				return NONE;
 			}
 			}
+		}
+	}
 	
 	// "pack" (...) -> table
 	static class pack extends VarArgFunction {
@@ -115,23 +120,22 @@ public class TableLib extends TwoArgFunction {
 	}
 
 	// "remove" (table [, pos]) -> removed-ele
-	static class remove extends TableLibFunction {
-		public LuaValue call(LuaValue list) {
-			return list.checktable().remove(0);
-		}
-		public LuaValue call(LuaValue list, LuaValue pos) {
-			return list.checktable().remove(pos.checkint());
+	static class remove extends VarArgFunction {
+		public Varargs invoke(Varargs args) {
+			return args.arg1().checktable().remove(args.optint(2, 0));
 		}
 	}
 
 	// "sort" (table [, comp])
-	static class sort extends TwoArgFunction {
-		public LuaValue call(LuaValue table, LuaValue compare) {
-			table.checktable().sort(compare.isnil()? NIL: compare.checkfunction());
+	static class sort extends VarArgFunction {
+		public Varargs invoke(Varargs args) {
+			args.arg1().checktable().sort(
+					args.arg(2).isnil()? NIL: args.arg(2).checkfunction());
 			return NONE;
 		}
 	}
 
+	
 	// "unpack", // (list [,i [,j]]) -> result1, ...
 	static class unpack extends VarArgFunction {
 		public Varargs invoke(Varargs args) {

test/junit/org/luaj/vm2/FragmentsTest.java

@@ -594,5 +594,11 @@ public class FragmentsTest extends TestSuite {
 		public void testBalancedMatchOnEmptyString() {
 			runFragment(LuaValue.NIL, "return (\"\"):match(\"%b''\")\n");
 		}
+		public void testReturnValueForTableRemove() {
+			runFragment(LuaValue.NONE, "return table.remove({ })");
+		}
+		public void testTypeOfTableRemoveReturnValue() {
+			runFragment(LuaValue.valueOf("nil"), "local k = table.remove({ }) return type(k)");
+		}
 	}
 }