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.92k stars 4.02k forks source link

Struct initialization to null uses Implicit operator type(string value) #49029

Closed ghost closed 3 years ago

ghost commented 3 years ago

Hi,

not sure if this is a bug or not, but doesn't looks right to me (probably due my lack of knowledge).

I have an struct type like

    [Serializable]
    [StructLayout(LayoutKind.Auto)]
    [ComVisible(true)]
    public readonly struct MyNewType: IComparable, IFormattable, IConvertible
        , IComparable<MyNewType>, IEquatable<MyNewType>
    {
        #region Members
        private const string _symbol = "MB";
        private const long _bytesValue = 1024;
        private readonly long m_value;
        private const long MinValue = 0;
        #endregion

        #region CTOR's
        private MyNewType(long mb)
        {
            m_value = mb;
        }
        #endregion

        #region Operators
        public static implicit operator long(MyNewTypemb) => mb.m_value;
        public static implicit operator MyNewType(long mb) => new MyNewType(mb);
        public static implicit operator MyNewType(int mb) => new MyNewType(mb);
        public static implicit operator MyNewType(short mb) => new MyNewType(mb);
        public static implicit operator MyNewType(string mb) 
        {
           .... some code ...
        }
        public void DoSomething() => doing something

When i run the following code

MyNewType a = null;
a.DoSomething

it enters into method public static implicit operator MyNewType(string mb) like the null initialisation was an string initialisation.

Is this right?

BTW, how i can make my a.DoSomething to raise a regular Null Exception when i use it?

Thank you very much in advance

Dotnet-GitSync-Bot commented 3 years ago

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

PathogenDavid commented 3 years ago

not sure if this is a bug or not, but doesn't looks right to me (probably due my lack of knowledge).

This is expected. Value types (structs) cannot be null by default, so the compiler has to search for user-defined conversions from null to the struct. Since strings can be null, it chooses the implicit string conversion operator. It might make more sense if you reword your code like this:

string s = null;
MyNewType a = s;

(Note that if it's ambiguous, you'll get an error and have to disambiguate the null.)

As an aside, implicit operators should generally now throw exceptions and shouldn't cause information to be lost. You didn't provide your string conversion code, but based on the others I assume it is parsing the string for a long which would violate one or both of these. (This isn't a hard rule of course, but I thought I'd mention it.)

BTW, how i can make my a.DoSomething to raise a regular Null Exception when i use it?

This makes me wonder if you should actually be using a struct here.

If you want a true null value you need to use nullable value types.

If you don't want to use nullable value types everywhere then you need to define some special value of m_value that is considered "null" or invalid. A lot of people use 0 because this also applies to a defaulted struct. (IE: MyNewType a = default;)

Also personally I'd recommend something like InvalidOperationException over NullReferenceException.

ghost commented 3 years ago

Thank you very much for all this info. I really appreciate it

PathogenDavid commented 3 years ago

No problem!