Merge pull request #255 from stevehalliwell/remove-combined-closures

Merge remove-combined-closures into main

ulox/ulox.core.tests/ClassTests.cs

@@ -1296,70 +1296,7 @@ class Foo
         }
 
         [Test]
-        public void Mixin_WhenMultipleCombined_ShouldHaveAll()
-        {
-            testEngine.Run(@"
-class MixMe
-{
-    Speak(){print(cname);}
-}
-
-class MixMe2
-{
-    Speak(){print(cname);}
-}
-
-class Foo 
-{
-    mixin 
-        MixMe,
-        MixMe2;
-
-    Speaketh(){print(cname);}
-}
-
-var foo = Foo();
-foo.Speaketh();");
-
-            Assert.AreEqual("Foo", testEngine.InterpreterResult);
-        }
-
-        [Test]
-        public void Mixin_WhenCombinedAndNamesClash_ShouldHaveAllPrint()
-        {
-            testEngine.Run(@"
-
-class MixMe
-{
-    Speak(){print(cname);}
-}
-
-class MixMe2
-{
-    Speak(){print(cname);}
-}
-
-
-class MixMe3
-{
-    Speak(){print(cname);}
-}
-
-class Foo 
-{
-    mixin MixMe, MixMe2, MixMe3;
-
-    Speak(){print(cname);}
-}
-
-var foo = Foo();
-foo.Speak();");
-
-            Assert.AreEqual("MixMeMixMe2MixMe3Foo", testEngine.InterpreterResult);
-        }
-
-        [Test]
-        public void Mixin_WhenInstanceMethodsCombinedAndNamesClash_ShouldHaveAllPrint()
+        public void Mixin_WhenInstanceMethodsCombinedAndNamesClash_ShouldError()
         {
             testEngine.Run(@"
 class MixMe
@@ -1392,7 +1329,7 @@ class Foo
 var foo = Foo();
 foo.Speak();");
 
-            Assert.AreEqual("MixMeMixMe2MixMe3Foo", testEngine.InterpreterResult);
+            StringAssert.StartsWith("Cannot AddMethod on <Class Foo>, already contains method '<closure", testEngine.InterpreterResult);
         }
 
         [Test]

ulox/ulox.core.tests/uloxs/Tests/MixinsMethods.ulox

@@ -10,13 +10,6 @@ class MixedSingleMeth
     var bar = 2;
 }
 
-class MixedCombined
-{
-    mixin MixMe;
-
-    Modify(obj){obj.text = obj.text + cname;}
-}
-
 class StringContainer
 {
     var text = "";
@@ -34,17 +27,4 @@ testset MixinsMethods
 
         Assert.AreEqual(expected, result);
     }
-
-    test CombinedMethodsUsed
-    {
-        var expected = "MixMeMixedCombined";
-        var result;
-        var obj = StringContainer();
-        var underTest = MixedCombined();
-
-        underTest.Modify(obj);
-        result = obj.text;
-
-        Assert.AreEqual(expected, result);
-    }
 }

ulox/ulox.core/Package/Runtime/Engine/VM.cs

@@ -976,30 +976,12 @@ public void PushCallFrameFromValue(Value callee, byte argCount)
                 CallMethod(callee.val.asBoundMethod, argCount);
                 break;
 
-            case ValueType.CombinedClosures:
-                CallCombinedClosures(callee, argCount);
-                break;
-
             default:
                 ThrowRuntimeException($"Invalid Call, value type {callee.type} is not handled");
                 break;
             }
         }
 
-        [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        private void CallCombinedClosures(Value callee, byte argCount)
-        {
-            var combinedClosures = callee.val.asCombined;
-            var stackCopyStartIndex = _valueStack.Count - argCount - 1;
-            for (int i = 0; i < combinedClosures.Count; i++)
-            {
-                DuplicateStackValuesNew(stackCopyStartIndex, argCount);
-
-                var closure = combinedClosures[i];
-                Call(closure, argCount);
-            }
-        }
-
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         private void Call(ClosureInternal closureInternal, byte argCount)
         {
@@ -1032,15 +1014,6 @@ private void PushFrameCallNative(CallFrame.NativeCallDelegate nativeCallDel, byt
             });
         }
 
-        [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        private void DuplicateStackValuesNew(int startAt, int count)
-        {
-            for (int i = 0; i <= count; i++)
-            {
-                Push(_valueStack[startAt + i]);
-            }
-        }
-
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         private void DoGetPropertyOp(Chunk chunk, byte constantIndex, Value targetVal)
         {

ulox/ulox.core/Package/Runtime/Enums/ValueType.cs

@@ -9,7 +9,6 @@ public enum ValueType : byte
         Chunk,
         NativeFunction,
         Closure,
-        CombinedClosures,
         Upvalue,
         UserType,
         Instance,

ulox/ulox.core/Package/Runtime/Library/Classes/NativeListClass.cs

@@ -285,7 +285,6 @@ public int Compare(Value x, Value y)
                 case ValueType.Chunk:
                 case ValueType.NativeFunction:
                 case ValueType.Closure:
-                case ValueType.CombinedClosures:
                 case ValueType.Upvalue:
                 case ValueType.UserType:
                 case ValueType.Instance:

ulox/ulox.core/Package/Runtime/Library/Classes/VMClass.cs

@@ -11,8 +11,8 @@ public sealed class VMClass : UserTypeInternal
         public VMClass(Func<Vm> createVM) : base(new HashedString("VM"), UserType.Native)
         {
             CreateVM = createVM;
-            AddMethod(ClassTypeCompilette.InitMethodName, Value.New(InitInstance, 1, 0), null);
             this.AddMethodsToClass(
+                (ClassTypeCompilette.InitMethodName.String, Value.New(InitInstance, 1, 0)),
                 (nameof(AddGlobal), Value.New(AddGlobal, 1, 2)),
                 (nameof(GetGlobal), Value.New(GetGlobal, 1, 1)),
                 (nameof(Start), Value.New(Start, 1, 1)),

ulox/ulox.core/Package/Runtime/Types/UserTypeInternal.cs

@@ -107,34 +107,10 @@ public virtual InstanceInternal MakeInstance()
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public void AddMethod(HashedString key, Value method, Vm vm)
         {
-            // This is used internally by the vm only does not need to check for frozen
+            // This is used internally during type construction so does not need to check for frozen
             if (Methods.Get(key, out var existing))
             {
-                //combine
-                if (existing.type == ValueType.Closure)
-                {
-                    var existingArity = existing.val.asClosure.chunk.Arity;
-                    var newArity = method.val.asClosure.chunk.Arity;
-                    if (existingArity != newArity)
-                        vm.ThrowRuntimeException($"Cannot mixin method '{key}' as it has a different arity '{newArity}' to the existing method '{existingArity}'.");
-
-                    //make a combine
-                    var temp = Value.Combined();
-                    temp.val.asCombined.Add(method.val.asClosure);
-                    temp.val.asCombined.Add(existing.val.asClosure);
-                    existing = temp;
-                }
-                else
-                {
-                    var existingArity = existing.val.asCombined[0].chunk.Arity;
-                    var newArity = method.val.asClosure.chunk.Arity;
-                    if (existingArity != newArity)
-                        vm.ThrowRuntimeException($"Cannot mixin method '{key}' as it has a different arity '{newArity}' to the existing method '{existingArity}'.");
-
-                    existing.val.asCombined.Insert(0, method.val.asClosure);
-                }
-
-                method = existing;
+                vm.ThrowRuntimeException($"Cannot {nameof(AddMethod)} on {this}, already contains method '{existing}'.");
             }
 
             Methods.AddOrSet(key, method);

ulox/ulox.core/Package/Runtime/Types/Value.cs

@@ -88,7 +88,6 @@ public override string ToString()
                     return typenameClass.ToString();
                 return $"<object {val.asObject}>";
 
-            case ValueType.CombinedClosures:
             default:
                 throw new System.NotImplementedException();
             }
@@ -104,7 +103,6 @@ public static Value Copy(Value copyFrom)
                 var newInst = new InstanceInternal();
                 newInst.CopyFrom(inst);
                 return Value.New(newInst);
-            case ValueType.CombinedClosures:
             case ValueType.Null:
             case ValueType.Double:
             case ValueType.Bool:
@@ -187,10 +185,6 @@ public static Value Null()
         public static Value Object(object obj)
             => New(ValueType.Object, new ValueTypeDataUnion() { asObject = obj });
 
-        [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        public static Value Combined()
-            => New(ValueType.CombinedClosures, new ValueTypeDataUnion() { asObject = new List<ClosureInternal>() });
-
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public override bool Equals(object obj)
         {
@@ -238,7 +232,6 @@ public static bool Compare(ref Value lhs, ref Value rhs)
                     return lhs.val.asClosure == rhs.val.asClosure;
 
                 case ValueType.NativeFunction:
-                case ValueType.CombinedClosures:
                 case ValueType.Upvalue:
                 case ValueType.BoundMethod:
                 case ValueType.Chunk:
@@ -261,7 +254,6 @@ public override int GetHashCode()
 
             case ValueType.String:
                 return val.asString.Hash;
-            case ValueType.CombinedClosures:
             case ValueType.Null:
             case ValueType.Chunk:
             case ValueType.NativeFunction:
@@ -292,7 +284,6 @@ public Value GetClassType()
             case ValueType.Chunk:
             case ValueType.NativeFunction:
             case ValueType.Closure:
-            case ValueType.CombinedClosures:
             case ValueType.BoundMethod:
             case ValueType.Upvalue:
                 break;
@@ -321,7 +312,6 @@ internal static Value UpdateFrom(Value lhs, Value rhs, Vm vm)
             case ValueType.BoundMethod:
             case ValueType.UserType:
             case ValueType.Upvalue:
-            case ValueType.CombinedClosures:
             case ValueType.Closure:
             case ValueType.NativeFunction:
             case ValueType.Chunk: