icsharpcode / NRefactory

NRefactory - Refactoring Your C# Code
684 stars 262 forks source link

'logical and' operator is resolved as 'bitwise and' when user defined 'bitwise and' exists #501

Closed danelkhen closed 9 years ago

danelkhen commented 9 years ago

Hi, It seems like as soon as a class is overloading the '&' operator (and true and false opeartors as required), the conditional && operator is resolved as - OperatorType.AndAlso (correct), but UserDefinedOperatorMethod to op_BitwiseAnd.

Reproduction code:

public class MyTest
{
    public static void Main(string[] args)
    {
        MyBool a = new MyBool();
        MyBool b = new MyBool();
        var c = a && b;  //when uncommented, UserDefinedOperatorMethod resolves to op_BitwiseAnd() 
    }
}

public class MyBool
{
    bool value;

    public static implicit operator bool (MyBool x)
    {
        return new JsBoolean();
    }

    //comment from here to see the correct behavior
    public static MyBool operator &(MyBool x, MyBool y)
    {
        return new MyBool();
    }
    public static bool operator true(MyBool x)
    {
        return (x.value == true);
    }

    public static bool operator false(MyBool x)
    {
        return (x.value == false);
    }
    //comment until here to see the correct behavior
}
danelkhen commented 9 years ago

Same happens with 'logical or' and 'bitwise or'

dgrunwald commented 9 years ago

C# spec:

The operation x && y is evaluated as T.false(x) ? x : T.&(x, y), where T.false(x) is an invocation of the operator false declared in T , and T.&(x, y) is an invocation of the selected operator & .

NRefactory currently represents this as an OperatorResolveResult with OperatorType=AndAlso and UserDefinedOperatorMethod = the selected operator &. Which kind of ResolveResult would you expect?

danelkhen commented 9 years ago

Intersting, I didn't know that actually. Well if this is by design and the situation is easily detectable I guess the current behavior is as good as any. Another option is to resolve / expand this result according to the spec, which means: x && y => Trinary(False(x), BitwiseAnd(x, y))

But again, this is more of an expanded resolve result, so the current behavior is fine too.