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

Custom binary operators not supported #21

Closed
modernist opened this issue May 8, 2014 · 5 comments
Closed

Custom binary operators not supported #21

modernist opened this issue May 8, 2014 · 5 comments
Labels
Milestone

Comments

@modernist
Copy link

A ParserException occurs when attempting to evaluate an expression that uses custom binary operators on objects. Add the following code to OperatorsTest.cs to reproduce.

[TestMethod]
public void Can_use_overloaded_binary_operators()
{
    var target = new Interpreter();

    var x = new TypeWithOverloadedBinaryOperators(3);
    target.SetVariable("x", x);

    string y = "5";
    Assert.IsFalse(x == y);
    Assert.IsFalse(target.Eval<bool>("x == y", new Parameter("y", y)));

    y = "3";
    Assert.IsTrue(x == y);
    Assert.IsTrue(target.Eval<bool>("x == y", new Parameter("y", y)));

    Assert.IsFalse(target.Eval<bool>("x == \"4\""));
    Assert.IsTrue(target.Eval<bool>("x == \"3\""));
}

struct TypeWithOverloadedBinaryOperators
{
    private int _value;

    public TypeWithOverloadedBinaryOperators(int value)
    {
        _value = value;
    }

    public static bool operator ==(TypeWithOverloadedBinaryOperators instance, string value)
    {
        return instance._value.ToString().Equals(value);
    }

    public static bool operator !=(TypeWithOverloadedBinaryOperators instance, string value)
    {
        return !instance._value.ToString().Equals(value);
    }

    public override bool Equals(object obj)
    {
        if (obj == null)
            return false;
        if (obj is TypeWithOverloadedBinaryOperators)
        {
            return this._value.Equals(((TypeWithOverloadedBinaryOperators)obj)._value);
        }
        return base.Equals(obj);
    }

    public override int GetHashCode()
    {
        return _value.GetHashCode();
    }
}

causes the following:

Operator '==' incompatible with operand types 'TypeWithOverloadedBinaryOperators' and 'String' (at index 2).
       at DynamicExpresso.ExpressionParser.CheckAndPromoteOperands(Type signatures, String opName, Expression& left, Expression& right, Int32 errorPos)
@davideicardi davideicardi added this to the 0.11.3 milestone May 8, 2014
@davideicardi
Copy link
Member

Yes, you are right. I confirm the bug. I will try to fix it in the following days.

@modernist
Copy link
Author

I just realized the issue is more generic. Overloadable operator methods have special names when retrieved via Reflection, such as op_Addition for +, etc. That's why ExressionParser.FindMethods fails to find these methods. So, it's necessary to support the overloadable operators as mentioned here:
http://msdn.microsoft.com/en-us/library/vstudio/ms229032(v=vs.100).aspx

@davideicardi
Copy link
Member

I have just fixed the bug. It was caused by a code that try to convert all operator parameters to a specific primitive type. For example if you write "1 == 0.5" internally the parser convert the parameters so that they use the same type (in this case Int32). Of course this code have no sense for custom operators. So I simply removed the exception if the above code cannot understand the base type.

I'm not sure if my explanation is clear ... anyway now the test passed without problem!

Thank you for bug report, write me if you find other related or not related problems.

@modernist
Copy link
Author

Hello,

Thanks for your prompt response. However, I'm afraid the issue still occurs when using overloaded operators on classes. For example, if I change TypeWithOverloadedBinaryOperators to a class, and even if I reference the type that contains the overloaded operators, I get a ParseExeption.

Consider the following minimal changes in the corresponding unit test:

[TestMethod]
public void Can_use_overloaded_operators()
{
    var target = new Interpreter();
    target.Reference(typeof(TypeWithOverloadedBinaryOperators)); //this has no effect

    var x = new TypeWithOverloadedBinaryOperators(3);
    target.SetVariable("x", x);

    string y = "5";
    Assert.IsFalse(x == y); //this works and calls the overloaded operator
    Assert.IsFalse(target.Eval<bool>("x == y", new Parameter("y", y))); //this fails

    //the rest is same as before
}

class TypeWithOverloadedBinaryOperators //converted from struct to class
{
    //same as before
}

The exception details are:

DynamicExpresso.ParseException was unhandled by user code
  HResult=-2146233088
  Message=Operator '==' incompatible with operand types 'TypeWithOverloadedBinaryOperators' and 'String' (at index 2).
  Source=DynamicExpresso.Core
  Position=2
  StackTrace:
       at DynamicExpresso.ExpressionParser.ParseComparison()
       at DynamicExpresso.ExpressionParser.ParseLogicalAnd()
       at DynamicExpresso.ExpressionParser.ParseLogicalOr()
       at DynamicExpresso.ExpressionParser.ParseConditional()
       at DynamicExpresso.ExpressionParser.ParseExpressionSegment()
       at DynamicExpresso.ExpressionParser.ParseExpressionSegment(Type returnType)
       at DynamicExpresso.ExpressionParser.Parse()
       at DynamicExpresso.Interpreter.ParseExpression(String expressionText, Type expressionType, ParameterExpression[] parameters)
       at DynamicExpresso.Interpreter.Parse(String expressionText, Type expressionType, Parameter[] parameters)
       at DynamicExpresso.Interpreter.Eval(String expressionText, Type expressionType, Parameter[] parameters)
       at DynamicExpresso.Interpreter.Eval[T](String expressionText, Parameter[] parameters)
       at DynamicExpresso.UnitTest.OperatorsTest.Can_use_overloaded_operators()

Perhaps a different approach is needed, e.g. like the one used for invoking extension methods?

@davideicardi
Copy link
Member

Ok, now I hope it is fixed. There was some other similar checks for reference type only. I'm not 100% sure to have covered all scenarios because I basically just removed some check.

Anyway I have just released version 0.11.4.0 . Write me for any other problem.

samuelcadieux pushed a commit to samuelcadieux/DynamicExpresso that referenced this issue May 4, 2015
samuelcadieux pushed a commit to samuelcadieux/DynamicExpresso that referenced this issue May 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants