Skip to content

Commit

Permalink
Fix double evaluation when Operator Overloading is enabled (#945)
Browse files Browse the repository at this point in the history 
* Fix double evaluation when Operator Overloading is enabled.
* Small behaviour fixes to JintUnaryExpression
* Added test for the new feature. Fixed failing ones and reimplemented a bug that needs more work to actually fix correctly
  • Loading branch information
@n1xx1
n1xx1 authored Aug 3, 2021
1 parent 27d3646 commit 20dcf69
Show file tree
Hide file tree
Showing 3 changed files with 109 additions and 79 deletions.
26 changes: 25 additions & 1 deletion Jint.Tests/Runtime/OperatorOverloadingTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -315,5 +315,29 @@ public void OperatorOverloading_ShouldWorkOnDerivedClasses()
");
}

[Fact]
public void OperatorOverloading_ShouldEvaluateOnlyOnce()
{
RunTest(@"
var c;
var resolve = v => { c++; return v; };
c = 0;
var n1 = resolve(1) + 2;
equal(n1, 3);
equal(c, 1);
c = 0;
var n2 = resolve(2) + resolve(3);
equal(n2, 5);
equal(c, 2);
c = 0;
var n3 = -resolve(1);
equal(n3, -1);
equal(c, 1);
");
}

}
}
}
83 changes: 42 additions & 41 deletions Jint/Runtime/Interpreter/Expressions/JintBinaryExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ private JintBinaryExpression(Engine engine, BinaryExpression expression) : base(
_right = Build(_engine, expression.Right);
}

protected bool TryOperatorOverloading(string clrName, out object result)
protected bool TryOperatorOverloading(JsValue left, JsValue right, string clrName, out object result)
{
return TryOperatorOverloading(_engine, _left.GetValue(), _right.GetValue(), clrName, out result);
return TryOperatorOverloading(_engine, left, right, clrName, out result);
}

internal static bool TryOperatorOverloading(Engine engine, JsValue leftValue, JsValue rightValue, string clrName, out object result)
Expand Down Expand Up @@ -267,14 +267,14 @@ public LessBinaryExpression(Engine engine, BinaryExpression expression) : base(e

protected override object EvaluateInternal()
{
var left = _left.GetValue();
var right = _right.GetValue();

if (_engine.Options._IsOperatorOverloadingAllowed
&& TryOperatorOverloading("op_LessThan", out var opResult))
&& TryOperatorOverloading(left, right, "op_LessThan", out var opResult))
{
return opResult;
}

var left = _left.GetValue();
var right = _right.GetValue();
var value = Compare(left, right);

return value._type == InternalTypes.Undefined
Expand All @@ -291,14 +291,15 @@ public GreaterBinaryExpression(Engine engine, BinaryExpression expression) : bas

protected override object EvaluateInternal()
{
var left = _left.GetValue();
var right = _right.GetValue();

if (_engine.Options._IsOperatorOverloadingAllowed
&& TryOperatorOverloading("op_GreaterThan", out var opResult))
&& TryOperatorOverloading(left, right, "op_GreaterThan", out var opResult))
{
return opResult;
}

var left = _left.GetValue();
var right = _right.GetValue();
var value = Compare(right, left, false);

return value._type == InternalTypes.Undefined
Expand All @@ -315,15 +316,15 @@ public PlusBinaryExpression(Engine engine, BinaryExpression expression) : base(e

protected override object EvaluateInternal()
{
var left = _left.GetValue();
var right = _right.GetValue();

if (_engine.Options._IsOperatorOverloadingAllowed
&& TryOperatorOverloading("op_Addition", out var opResult))
&& TryOperatorOverloading(left, right, "op_Addition", out var opResult))
{
return opResult;
}

var left = _left.GetValue();
var right = _right.GetValue();

if (AreIntegerOperands(left, right))
{
return JsNumber.Create(left.AsInteger() + right.AsInteger());
Expand All @@ -344,15 +345,15 @@ public MinusBinaryExpression(Engine engine, BinaryExpression expression) : base(

protected override object EvaluateInternal()
{
var left = _left.GetValue();
var right = _right.GetValue();

if (_engine.Options._IsOperatorOverloadingAllowed
&& TryOperatorOverloading("op_Subtraction", out var opResult))
&& TryOperatorOverloading(left, right, "op_Subtraction", out var opResult))
{
return opResult;
}

var left = _left.GetValue();
var right = _right.GetValue();

return AreIntegerOperands(left, right)
? JsNumber.Create(left.AsInteger() - right.AsInteger())
: JsNumber.Create(TypeConverter.ToNumber(left) - TypeConverter.ToNumber(right));
Expand All @@ -367,15 +368,15 @@ public TimesBinaryExpression(Engine engine, BinaryExpression expression) : base(

protected override object EvaluateInternal()
{
var left = _left.GetValue();
var right = _right.GetValue();

if (_engine.Options._IsOperatorOverloadingAllowed
&& TryOperatorOverloading("op_Multiply", out var opResult))
&& TryOperatorOverloading(left, right, "op_Multiply", out var opResult))
{
return opResult;
}

var left = _left.GetValue();
var right = _right.GetValue();

if (AreIntegerOperands(left, right))
{
return JsNumber.Create((long) left.AsInteger() * right.AsInteger());
Expand All @@ -398,15 +399,15 @@ public DivideBinaryExpression(Engine engine, BinaryExpression expression) : base

protected override object EvaluateInternal()
{
var left = _left.GetValue();
var right = _right.GetValue();

if (_engine.Options._IsOperatorOverloadingAllowed
&& TryOperatorOverloading("op_Division", out var opResult))
&& TryOperatorOverloading(left, right, "op_Division", out var opResult))
{
return opResult;
}

var left = _left.GetValue();
var right = _right.GetValue();

return Divide(left, right);
}
}
Expand All @@ -422,15 +423,15 @@ public EqualBinaryExpression(Engine engine, BinaryExpression expression, bool in

protected override object EvaluateInternal()
{
var left = _left.GetValue();
var right = _right.GetValue();

if (_engine.Options._IsOperatorOverloadingAllowed
&& TryOperatorOverloading(_invert ? "op_Inequality" : "op_Equality", out var opResult))
&& TryOperatorOverloading(left, right, _invert ? "op_Inequality" : "op_Equality", out var opResult))
{
return opResult;
}

var left = _left.GetValue();
var right = _right.GetValue();

return Equal(left, right) == !_invert
? JsBoolean.True
: JsBoolean.False;
Expand All @@ -448,15 +449,15 @@ public CompareBinaryExpression(Engine engine, BinaryExpression expression, bool

protected override object EvaluateInternal()
{
var leftValue = _left.GetValue();
var rightValue = _right.GetValue();

if (_engine.Options._IsOperatorOverloadingAllowed
&& TryOperatorOverloading(_leftFirst ? "op_GreaterThanOrEqual" : "op_LessThanOrEqual", out var opResult))
&& TryOperatorOverloading(leftValue, rightValue, _leftFirst ? "op_GreaterThanOrEqual" : "op_LessThanOrEqual", out var opResult))
{
return opResult;
}

var leftValue = _left.GetValue();
var rightValue = _right.GetValue();

var left = _leftFirst ? leftValue : rightValue;
var right = _leftFirst ? rightValue : leftValue;

Expand Down Expand Up @@ -525,15 +526,15 @@ public ModuloBinaryExpression(Engine engine, BinaryExpression expression) : base

protected override object EvaluateInternal()
{
var left = _left.GetValue();
var right = _right.GetValue();

if (_engine.Options._IsOperatorOverloadingAllowed
&& TryOperatorOverloading("op_Modulus", out var opResult))
&& TryOperatorOverloading(left, right, "op_Modulus", out var opResult))
{
return opResult;
}

var left = _left.GetValue();
var right = _right.GetValue();

if (AreIntegerOperands(left, right))
{
var leftInteger = left.AsInteger();
Expand Down Expand Up @@ -588,15 +589,15 @@ public BitwiseBinaryExpression(Engine engine, BinaryExpression expression) : bas

protected override object EvaluateInternal()
{
var left = _left.GetValue();
var right = _right.GetValue();

if (_engine.Options._IsOperatorOverloadingAllowed
&& TryOperatorOverloading(OperatorClrName, out var opResult))
&& TryOperatorOverloading(left, right, OperatorClrName, out var opResult))
{
return opResult;
}

var left = _left.GetValue();
var right = _right.GetValue();

if (AreIntegerOperands(left, right))
{
int leftValue = left.AsInteger();
Expand Down
79 changes: 42 additions & 37 deletions Jint/Runtime/Interpreter/Expressions/JintUnaryExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,50 +53,54 @@ public override JsValue GetValue()

protected override object EvaluateInternal()
{
if (_engine.Options._IsOperatorOverloadingAllowed)
{
string operatorClrName = null;
switch (_operator)
{
case UnaryOperator.Plus:
operatorClrName = "op_UnaryPlus";
break;
case UnaryOperator.Minus:
operatorClrName = "op_UnaryNegation";
break;
case UnaryOperator.BitwiseNot:
operatorClrName = "op_OnesComplement";
break;
case UnaryOperator.LogicalNot:
operatorClrName = "op_LogicalNot";
break;
default:
break;
}

if (operatorClrName != null &&
TryOperatorOverloading(_engine, _argument.GetValue(), operatorClrName, out var result))
{
return result;
}
}

switch (_operator)
{
case UnaryOperator.Plus:
var plusValue = _argument.GetValue();
return plusValue.IsInteger() && plusValue.AsInteger() != 0
? plusValue
: JsNumber.Create(TypeConverter.ToNumber(plusValue));
{
var v = _argument.GetValue();
if (_engine.Options._IsOperatorOverloadingAllowed &&
TryOperatorOverloading(_engine, v, "op_UnaryPlus", out var result))
{
return result;
}

return v.IsInteger() && v.AsInteger() != 0
? v
: JsNumber.Create(TypeConverter.ToNumber(v));
}
case UnaryOperator.Minus:
return EvaluateMinus(_argument.GetValue());
{
var v = _argument.GetValue();
if (_engine.Options._IsOperatorOverloadingAllowed &&
TryOperatorOverloading(_engine, v, "op_UnaryNegation", out var result))
{
return result;
}

return EvaluateMinus(v);
}
case UnaryOperator.BitwiseNot:
return JsNumber.Create(~TypeConverter.ToInt32(_argument.GetValue()));
{
var v = _argument.GetValue();
if (_engine.Options._IsOperatorOverloadingAllowed &&
TryOperatorOverloading(_engine, v, "op_OnesComplement", out var result))
{
return result;
}

return JsNumber.Create(~TypeConverter.ToInt32(v));
}
case UnaryOperator.LogicalNot:
return !TypeConverter.ToBoolean(_argument.GetValue()) ? JsBoolean.True : JsBoolean.False;
{
var v = _argument.GetValue();
if (_engine.Options._IsOperatorOverloadingAllowed &&
TryOperatorOverloading(_engine, v, "op_LogicalNot", out var result))
{
return result;
}

return !TypeConverter.ToBoolean(v) ? JsBoolean.True : JsBoolean.False;
}

case UnaryOperator.Delete:
var r = _argument.Evaluate() as Reference;
Expand Down Expand Up @@ -150,6 +154,7 @@ protected override object EvaluateInternal()
return Undefined.Instance;

case UnaryOperator.TypeOf:
{
var value = _argument.Evaluate();
r = value as Reference;
if (r != null)
Expand All @@ -161,8 +166,8 @@ protected override object EvaluateInternal()
}
}

// TODO: double evaluation problem
var v = _argument.GetValue();

if (v.IsUndefined())
{
return JsString.UndefinedString;
Expand All @@ -187,7 +192,7 @@ protected override object EvaluateInternal()
}

return JsString.ObjectString;

}
default:
ExceptionHelper.ThrowArgumentException();
return null;
Expand Down

0 comments on commit 20dcf69

Please sign in to comment.