dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
18.95k stars 4.02k forks source link

Overloaded operators need to be smarter #49095

Closed VBAndCs closed 3 years ago

VBAndCs commented 3 years ago

I have a project to auto-generate a class that can be used as a flag with many methods to support flag operations. The flag is exactly a wrapper around Integer, which is saved internally in a Value field. I added a widening and Narowing Ctype operators to go back and forth between my Flag classes and Integer:

Class MyFlag
    Public Shared ReadOnly X As New MyFlag("X", 1)
    Public Shared ReadOnly Y As New MyFlag("Y", 2)
    Public Shared ReadOnly Z As New MyFlag("Z", 4)

    Dim Value As Integer

    Private Sub New(value As Integer)
        Me.Value = value
    End Sub

    Private Sub New(name As String, value As Integer)
        _Name = name
        Me.Value = value
    End Sub

    Public Shared Widening Operator CType(value As Integer) As MyFlag
        Return New MyFlag(value)
    End Operator

    Public Shared Widening  Operator CType(flag As MyFlag) As Integer
        Return flag.Value
    End Operator

I supposed this will be enough, but VB refused to do any arithmetic or logical operation involving Flag and Integer! Dim x = MyFlag.X + 1

My first question: Why can't VB convert the Integer operand to the flag type, or convert the flag operand to Integer? Note that when I added the + operator, I could use it to add a Byte to the Flag. VB casts Byte to integer so it can use the operator. So, whey can't it cast the flag to Integer to use the Integer.+? Note that this is also not working: Dim x = 1 + MyFlag.X It has nothing to do with the order of the operands.

Secondly: Why can't I mark the Value field with some attribute say <UnderlyingType> pr mark the class itself < UnderlyingType(GetType(Integer), "Value">, so that VB use this filed in all operation (treating them all as immutable operators), or at least use the given CType operators to substitute Integer with the Type or vice versa?

And finally: Why do I had to override unnecessary operator? = doesn't need <> > doesn’t need < and if the class has = and <, it doesn’t also need <= nor >=! So, why we are wasting our lives writing all that?

In current language design, I had to add all this non-sense code:


    Public Shared Widening Operator CType(value As Integer) As MyFlag
        Return New MyFlag(value)
    End Operator

    Public Shared Narrowing Operator CType(flag As MyFlag) As Integer
        Return flag.Value
    End Operator

    Public Shared Operator +(flag As MyFlag, value As Integer) As Integer
        Return flag.Value + value
    End Operator

    Public Shared Operator -(flag As MyFlag, value As Integer) As Integer
        Return flag.Value - value
    End Operator

    Public Shared Operator *(flag As MyFlag, value As Integer) As Integer
        Return flag.Value * value
    End Operator

    Public Shared Operator /(flag As MyFlag, value As Integer) As Double
        Return flag.Value / value
    End Operator

    Public Shared Operator ^(flag As MyFlag, value As Integer) As Long
        Return flag.Value ^ value
    End Operator

    Public Shared Operator Or(flag As MyFlag, value As Integer) As MyFlag
        Return New MyFlag(flag.Value Or value)
    End Operator

    Public Shared Operator And(flag As MyFlag, value As Integer) As MyFlag
        Return New MyFlag(flag.Value And value)
    End Operator

    Public Shared Operator Xor(flag As MyFlag, value As Integer) As MyFlag
        Return New MyFlag(flag.Value Xor value)
    End Operator

    Public Shared Operator Not(flag As MyFlag) As MyFlag
        Return New MyFlag(Not flag.Value)
    End Operator

    Public Shared Operator IsTrue(flag As MyFlag) As Boolean
        Return flag.Value > 0
    End Operator

    Public Shared Operator IsFalse(flag As MyFlag) As Boolean
        Return flag.Value = 0
    End Operator

    Public Shared Operator =(flag As MyFlag, value As Integer) As Boolean
        Return flag.Value = value
    End Operator

    Public Shared Operator <>(flag As MyFlag, value As Integer) As Boolean
        Return flag.Value <> value
    End Operator

    Public Shared Operator >(flag As MyFlag, value As Integer) As Boolean
        Return flag.Value > value
    End Operator

    Public Shared Operator <(flag As MyFlag, value As Integer) As Boolean
        Return flag.Value < value
    End Operator

    Public Shared Operator >=(flag As MyFlag, value As Integer) As Boolean
        Return flag.Value >= value
    End Operator

    Public Shared Operator <=(flag As MyFlag, value As Integer) As Boolean
        Return flag.Value <= value
    End Operator

    Public Shared Operator +(flag1 As MyFlag, flag2 As MyFlag) As Integer
        Return flag1.Value + flag2.Value
    End Operator

    Public Shared Operator -(flag1 As MyFlag, flag2 As MyFlag) As Integer
        Return flag1.Value - flag2.Value
    End Operator

    Public Shared Operator *(flag1 As MyFlag, flag2 As MyFlag) As Integer
        Return flag1.Value * flag2.Value
    End Operator

    Public Shared Operator /(flag1 As MyFlag, flag2 As MyFlag) As Double
        Return flag1.Value / flag2.Value
    End Operator

    Public Shared Operator ^(flag1 As MyFlag, flag2 As MyFlag) As Long
        Return flag1.Value ^ flag2.Value
    End Operator

    Public Shared Operator Or(flag1 As MyFlag, flag2 As MyFlag) As MyFlag
        Return New MyFlag(flag1.Value Or flag2.Value)
    End Operator

    Public Shared Operator And(flag1 As MyFlag, flag2 As MyFlag) As MyFlag
        Return New MyFlag(flag1.Value And flag2.Value)
    End Operator

    Public Shared Operator Xor(flag1 As MyFlag, flag2 As MyFlag) As MyFlag
        Return New MyFlag(flag1.Value Xor flag2.Value)
    End Operator

    Public Shared Operator =(flag1 As MyFlag, flag2 As MyFlag) As Boolean
        Return flag1.Value = flag2.Value
    End Operator

    Public Shared Operator <>(flag1 As MyFlag, flag2 As MyFlag) As Boolean
        Return flag1.Value <> flag2.Value
    End Operator

    Public Shared Operator >(flag1 As MyFlag, flag2 As MyFlag) As Boolean
        Return flag1.Value > flag2.Value
    End Operator

    Public Shared Operator <(flag1 As MyFlag, flag2 As MyFlag) As Boolean
        Return flag1.Value < flag2.Value
    End Operator

    Public Shared Operator >=(flag1 As MyFlag, flag2 As MyFlag) As Boolean
        Return flag1.Value >= flag2.Value
    End Operator

    Public Shared Operator <=(flag1 As MyFlag, flag2 As MyFlag) As Boolean
        Return flag1.Value <= flag2.Value
    End Operator
CyrusNajmabadi commented 3 years ago

My first question: Why can't VB convert the Integer operand to the flag type, or convert the flag operand to Integer?

Because the language isn't spec'ed to do taht. If you want it to do that, please file a suggestion over at dotnet/vblang. This has been mentioned to you in the past. Please do not ask the compiler to deviate from the language spec.

CyrusNajmabadi commented 3 years ago

Secondly: Why can't I mark the Value field with some attribute say pr mark the class itself < UnderlyingType(GetType(Integer), "Value">, so that VB use this filed in all operation (treating them all as immutable operators), or at least use the given CType operators to substitute Integer with the Type or vice versa?

You are welcome to raise an issue/proposal on vblang asking for this. Roslyn is not the place for this.

CyrusNajmabadi commented 3 years ago

Why do I had to override unnecessary operator? = doesn't need <>

The language specification requires it. If you want that changed, you need to file a language change request.

In the meantime, the IDE can help you by generating tihs all on your behalf. Source generators will likely help here in the future as well.

VBAndCs commented 3 years ago

@CyrusNajmabadi Where is the VB specification that prevents widening an operand to Integer in the Integer.+ operator?

What I am descrying here is an ancient VB behavior that can't be prevented by any specifications what ever!

If fact, C# does it right:

    class Foo
    {
        int value = 0;

        public static implicit operator int(Foo f) => f.value;
    }
        static void Main(string[] args)
        {
            var f = new Foo();
            var x = f + 1;
        }

So, since when the strictly famous C# does conversions that VB prevents by specifications?

Class Foo
    Dim Value As Integer = 0

    Public Shared Widening Operator CType(f As Foo) As Integer
        Return f.Value
    End Operator
End Class

Sub main()
   Dim f As New Foo
   Dim x = f + 1  ' Error
End Sub

Definitely this is a bug in the operator resolution in Roslyn, as it for some reason fails to see the Integer.+ as a possible choice.

CyrusNajmabadi commented 3 years ago

@VBAndCs I'm not getting pulled into another discussion with you where you ignore what is said. I have told you how to properly proceed here. Language change requests go through vblang.

VBAndCs commented 3 years ago

from https://docs.microsoft.com/en-us/dotnet/visual-basic/reference/language-specification/expressions

Given an operator type and a set of operands, operator resolution determines which operator to use for the operands. When resolving operators, user-defined operators will be considered first, using the following steps: 1.First, all of the candidate operators are collected. The candidate operators are all of the user-defined operators of the particular operator type in the source type and all of the user-defined operators of the particular type in the target type. If the source type and destination type are related, common operators are only considered once. 2.Then, overload resolution is applied to the operators and operands to select the most specific operator. In the case of binary operators, this may result in a late-bound call.

Note that the first word in user-defined operators will be considered first implies that intrinsic operators will be conseded in the next phase. This may be not clear for the implementers, and may be the source of this bug. So, the Integer.+ opetrator must be in this list as there is an operand of type Integer, and above all the target type is Integer.

Now this is the second rule in resolutin rules:

Any operand whose type is not defined for the operator is converted using the following steps and the operator is resolved against the new types: ◦The operand is converted to the next widest type that is defined for both the operator and the operand and to which it is implicitly convertible. ((My Class has a widinning CType to Integer. This is applicable)) ◦If there is no such type, then the operand is converted to the next narrowest type that is defined for both the operator and the operand and to which it is implicitly convertible. ◦If there is no such type or the conversion cannot occur, a compile-time error occurs. •Otherwise, the operands are converted to the wider of the operand types and the operator for that type is used. If the narrower operand type cannot be implicitly converted to the wider operator type, a compile-time error occurs

So, the source of this bug (that clarely violates the specs) is :

AnthonyDGreen commented 3 years ago

So, to @CyrusNajmabadi 's point, at a minimum you're asking for a specification clarification. The spec may be wrong, or the spec might say what you think it says and intend to do so. But before any action can be taken on the implementation side the intent of the spec should be clarified, so we should discuss it in vblang. I could write some long pontification about the philosophy of conversions and why I think this particular pattern of behaviors is not allowed, but the place for that missive to live is on vblang and not buried here amongst clearer yes/no product defects. Traditionally, if this question were raised internally (in either language) it would be sent to the LDM first to make a determination before it would be actionable.

As for you points about the required operator pairings that is definitely a vblang suggestion and should be filed there and separately from the question about using widening conversion operators in this way.

VBAndCs commented 3 years ago

@AnthonyDGreen There is an old issue about this in VBlang: https://github.com/dotnet/vblang/issues/543