Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge remove-combined-closures into main #255

Merged
merged 1 commit into from
Jun 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 2 additions & 65 deletions ulox/ulox.core.tests/ClassTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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]
Expand Down
20 changes: 0 additions & 20 deletions ulox/ulox.core.tests/uloxs/Tests/MixinsMethods.ulox
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,6 @@ class MixedSingleMeth
var bar = 2;
}

class MixedCombined
{
mixin MixMe;

Modify(obj){obj.text = obj.text + cname;}
}

class StringContainer
{
var text = "";
Expand All @@ -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);
}
}
27 changes: 0 additions & 27 deletions ulox/ulox.core/Package/Runtime/Engine/VM.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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)
{
Expand Down
1 change: 0 additions & 1 deletion ulox/ulox.core/Package/Runtime/Enums/ValueType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ public enum ValueType : byte
Chunk,
NativeFunction,
Closure,
CombinedClosures,
Upvalue,
UserType,
Instance,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion ulox/ulox.core/Package/Runtime/Library/Classes/VMClass.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
Expand Down
28 changes: 2 additions & 26 deletions ulox/ulox.core/Package/Runtime/Types/UserTypeInternal.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
10 changes: 0 additions & 10 deletions ulox/ulox.core/Package/Runtime/Types/Value.cs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ public override string ToString()
return typenameClass.ToString();
return $"<object {val.asObject}>";

case ValueType.CombinedClosures:
default:
throw new System.NotImplementedException();
}
Expand All @@ -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:
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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:
Expand Down
Loading