Implement Operator Overloading #893
Conversation
| @@ -17,6 +30,85 @@ private JintBinaryExpression(Engine engine, BinaryExpression expression) : base( | |||
| _right = Build(engine, expression.Right); | |||
| } | |||
|
|
|||
| protected abstract object EvaluateBinaryExpression(); | |||
|
|
|||
| protected override object EvaluateInternal() | |||
There was a problem hiding this comment.
this part is a bit iffy for me, this is on performance hot path and now introduces another layer of indirection
There was a problem hiding this comment.
might require sealing, also minimize code size for non-default case, i.e. put if if (!_engine.Options._IsOperatorOverloadingAllowed) first and then call separate method for doing the operator overloading version?
There was a problem hiding this comment.
Removed the calls to virtual methods. Please check now.
|
Done all (most) binary and unary methods. I did not implement operator overloading for true and false operators. I guess that operator can be used in a context where Ecmascript spec asks to convert the value to boolean. For example |
|
Looking good, @sebastienros please take a look. |
Jint/Runtime/TypeConverter.cs
Outdated
| } | ||
| else | ||
| { | ||
| if (argType.GetMethods(BindingFlags.Public | BindingFlags.Static) |
There was a problem hiding this comment.
Is there a test for that and the scrore thing to understand what is happening?
There was a problem hiding this comment.
Basically this method finds the best match when there are multiple methods with the same name and number of arguments. Before my changes, this code was checking for a method that was perfectly matching to the passed arguments. If it could not find a perfect match, it would choose the first method it encountered (and this would cause an error in most cases).
My code gives each method a penalty based on how close each argument is to the parameter it is going to be casted to. Score is divided into different levels and assigned with a number in base 10. This is kind of a heuristic algorithm.
This is demonstrated in MethodAmbiguityTests.cs I just added.
To be honest, this became deeper than I initially thought. It deserves its own PR. I did not want to go overkill here. C# compiler can resolve the best method even better. But that is probably only possible with static analysis. Also when there are multiple matching methods, C# calls it an ambiguity and does not let you compile.
Also this can become a breaking change for people who use interop a lot and have methods with same names. Perfect match will still be same but when there are multiple non-matching methods, people may experience different results with this change. But in my opinion, before this change, the result would be undefined behavior.
Normally, Ecmascript does not allow operator overloading and the specs are pretty clear on how to handle operators. But since we are running on C#, we may as well use operators that are defined on the C# side. So I added an option for that. The option is off by default.
Binary operators will try to find a C# side overloaded operator first and use it. If it cannot find, will fall back to Ecmascript behavior. (I forgot to do it for unary so the PR is draft for now. Will do asap.)
Also, when this option is turned on, the default type converter will try to find implicit/explicit cast operator and use it as a last resort.