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

[VB.NET] Wrong assignments to interfaces do compile #48387

Closed
VBAndCs opened this issue Oct 7, 2020 · 36 comments
Closed

[VB.NET] Wrong assignments to interfaces do compile #48387

VBAndCs opened this issue Oct 7, 2020 · 36 comments
Labels

Comments

@VBAndCs
Copy link

VBAndCs commented Oct 7, 2020

In design time, when Option Strict off, all assignments to interfaces are OK, even it is obviously impossible because the class doesn't implement the interface. I tested with IEnumerable, IQueryable, and IList. This code sample compiles correctly (and it must not) but gives runtime errors of course!

Class Foo1
End Class

Class foo2
End Class


Sub main()
   Dim x As IList(Of Foo1)
   x = New foo2 ' compiles!
   x = New List(Of foo2) ' compiles
End Sub

Note:
This code will not compile as expected:

  x.Add(New foo2)
  x.Add(New foo2 List(Of foo2))

Is there any case that a class that doesn't implement the interface can be cast to it? I have no knowledge of that.
Note that this issue applies also when assigning IQueryable(Of Foo2) to IQueryable(Of Foo1). VB refuse Dim y As Foo1 = New foo2 even when Option Strict off, so, why it accepts:

    Function Test() As IQueryable(Of Foo1)
        Dim x As IQueryable(Of foo2)
        Return x
    End Function
@CyrusNajmabadi
Copy link
Member

I don't see anything in the spec that would cause errors here in the cases you've mentioned. option strict is covered here:

https://github.com/dotnet/vblang/blob/master/spec/source-files-and-namespaces.md#option-strict-statement

There it says:

Under strict semantics, the following are disallowed:
Narrowing conversions without an explicit cast operator.

Which indicates that non-strict semantics narrowing conversions without an explicit cast are allowed.

The conversions spec says this about narrowing conversions:

A narrowing conversion is a conversion from a type to another type whose value domain is either smaller than the original type's value domain or sufficiently unrelated that extra care must be taken when doing the conversion (for example, when converting from Integer to String). Narrowing conversions, which may entail loss of information, can fail.

So this seems by design. You have option-strict off. So narrowing conversions without an explicit cast are not disallowed.

@CyrusNajmabadi
Copy link
Member

I'm going to close this out as Roslyn seems to be working as expected. If you disagree please let me know, including links to the appropriate places in the spec that you think the impl contradicts.

If you do not like how the specification works here, then the appropriate thing to do is open an issue at dotnet/vblang. Thanks!

@VBAndCs
Copy link
Author

VBAndCs commented Oct 7, 2020

S, why VB refused this:
Dim y As Foo1 = New foo2

Option Strict Off doesn't allow converting between types without a CType. It allows narrowing (and the risk on the coder). Other cases include base classes and children, Interfaces and implementing classes. Here there is nothing of those cases. It is obviously clear that a runtime error will happen.
So, I think this Interface behaviour should be fixed. I am not aware of any situation that can make IQuerable(Of Foo1) accept a value of type IQuerable(Of Foo2).
Note that VB refuses this:

        Dim a As List(Of Integer)
        a = New List(Of Short)

despite that short can be safely widened to integer. This applies also even if one of the two types inherits the other.
I even made Foo1 implement IFoo, and tries:

       Dim a As List(Of Ifoo)
        a = New List(Of Foo1)

and VB refused it!
That said, what is the possible situation that can make IQuerable(Foo1) accept a value of type IQuerable(Foo2)?

@VBAndCs
Copy link
Author

VBAndCs commented Oct 7, 2020

Please Don't close issues before hearing the response on your answer. This is defiantly a bug or at least misbehaviour.

@VBAndCs
Copy link
Author

VBAndCs commented Oct 7, 2020

@AdamSpeight2008 @paul1956 @Happypig375 @pricerc @hartmair @franzalex @gilfusion
What do you thing about this?

@CyrusNajmabadi
Copy link
Member

Dim y As Foo1 = New foo2

That doesn't appear to be a narrowing conversion. Afaict, there is no conversion that would allow that. So this isn't allowed because the language has no conversion that for at all.

Option strict on/off is about if narrowing conversions are allowed are not.

@CyrusNajmabadi
Copy link
Member

This is defiantly a bug or at least misbehaviour.

If you can point to the part of the spec that you feel demonstrate that, please let me know and i can reopen. Thanks! :)

@CyrusNajmabadi
Copy link
Member

Here there is nothing of those cases. It is obviously clear that a runtime error will happen.

That's literally what the spec on conversions says:

Narrowing conversions, which may entail loss of information, can fail.

:)

And option-strict-on is what causes you to get errors if you have a narrowing conversion without an explicit cast. Since you are in option-strict-off, i'm not seeing what part of the spec says this should be disallowed. Please point to what you think would disallow this. Thank you :)

@VBAndCs
Copy link
Author

VBAndCs commented Oct 7, 2020

That doesn't appear to be a narrowing conversion. Afaict, there is no conversion that would allow that. So this isn't allowed because the language has no conversion that for at all.
Option strict on/off is about if narrowing conversions are allowed are not.

This is exactly the case in other wrongly accepted codes!

@CyrusNajmabadi
Copy link
Member

This is exactly the case in other wrongly accepted codes!

What do you mean? The spec here says: https://github.com/dotnet/vblang/blob/master/spec/conversions.md#reference-conversions

Class and interface types can be cast to and from any interface type. Conversions between a type and an interface type only succeed at run time if the actual types involved have an inheritance or implementation relationship. Because an interface type will always contain an instance of a type that derives from Object, an interface type can also always be cast to and from Object.

There is no defined conversion between Foo1 and foo2. There are defined conversions between IList(Of Foo1) and foo2 and between IList(Of Foo1) and List(Of foo2).

If you believe otherwise point out why using the specification. I have linked exactly to the points in the spec i think are relevant here. You cannot simply state you disagree. You have to actually give the exact sspec reasons why this is incorrect.

@VBAndCs
Copy link
Author

VBAndCs commented Oct 7, 2020

You seem to totally miss the point:
Regardless of Option Strict and Ctype operators, VB doesn't allow any sort of conversion whatever between collections if their elements have different types (even these types are related or can be casted).
IList, IQueryable and IEnumarble must be treated as Collections here. If there is no specification for that, please add it. :)

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Oct 7, 2020

VB doesn't allow any sort of conversion whatever between collections if their elements have different types (even these types are related or can be casted).

Yes it does. I just pointed to the part of the spec that allows this. Again, it is right here:

https://github.com/dotnet/vblang/blob/master/spec/conversions.md#reference-conversions

Class and interface types can be cast to and from any interface type.

https://github.com/dotnet/vblang/blob/master/spec/conversions.md#narrowing-conversions

From a class type to an interface type, provided the class type does not implement the interface type or an interface type variant compatible with it.

It says so right there. Any class or interface type can be converted to and from any interface type. That is exactly the case we have here. And it's a narrowing conversion when the class type does not implement the interface type.

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Oct 7, 2020

IList, IQueryable and IEnumarble must be treated as Collections here.

IList/IQueryable/IEnumerable are all interfaces. The specification says that any class/interface type can be converted to/from them.

If there is no specification for that, please add it. :)

Specification requests are not filed at dotnet/roslyn. If you want the specification changed, please file an issue at dotnet/vblang. :)

@VBAndCs
Copy link
Author

VBAndCs commented Oct 7, 2020

And here is another point to check in specs:
Generic interfaces have a type param, and should be checked in design time. IQueryable(Of Foo1) is not related to object only, but also to Foo1. So, there is no way to accept classes of type params other than Foo1. This is a crystal clear situation, and specs should deal with it.

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Oct 7, 2020

This is a crystal clear situation, and specs should deal with it.

The spec does deal with it. I have linked to where it is dealt with. :)

If you do not like how the specification deals with it, then the appropriate thing to do is to open an issue at dotnet/vblang where you can put forth a design on how the language should change. Right now the compiler (dotnet/roslyn) seems to be implementing the language properly.

@CyrusNajmabadi
Copy link
Member

and should be checked in design time.

option strict on will check at compile time and will produce an error if you are using a narrowing conversion without a cast. If you have option strict off then it will not. This seems like the easy thing to do here to get hte behavior you are asking for :)

@VBAndCs
Copy link
Author

VBAndCs commented Oct 7, 2020

Let me rephrase:
My practical case involves a LinQ query that returns IQueryable(Of Fee), but in a copy and paste operation I forgot to change the return type of the function from IQueryable(Of Operation) to IQueryable(Of Fee), and got no error in compile time. Here is the conversion is between two incompatible interfaces not a class and an interface, and I think the specs says something about when generic interfaces are compatible.

@CyrusNajmabadi
Copy link
Member

and I think the specs says something about when generic interfaces are compatible.

Feel free to point to where you think the spec says this. If you are correct, i will reactivate the issue. Absent that, i'm going to keep the issue closed as i can't find anything in the spec supporting that statement.

@CyrusNajmabadi
Copy link
Member

and got no error in compile time.

Yes. You have not enabled option strict on. If you don't enable that then narrowing conversions are allowed. As the spec says (as i've linked to numerous times), there is a narrowing conversion:

From an interface type to another interface type, provided there is no inheritance relationship between the two types and provided they are not variant compatible.

With IQueryable(Of Operation) to IQueryable(Of Fee) there is no inheritance relationship between either of those types, and those types are not variant compatible. As such, it's a narrowing conversion, and as such there is no problem.

Here is the conversion is between two incompatible interfaces not a class and an interface

As i've pointed out, the spec says: Class and interface types can be cast to and from any interface type.

It explicitly calls out that there is a reference conversion between any two interfaces. And it points out when this is a narrowing conversion. If you do not enable option strict on, then this appears to be the behavior that is expected.

This is why option strict exists so you can enable that and say: "tell me when i have narrowing conversions that may fail at runtime, and i do not have an explicit cast in source".

@VBAndCs

This comment has been minimized.

@CyrusNajmabadi
Copy link
Member

@VBAndCs i think you really should read through the spec. Of high relevance is this part:

The set of implicit conversions depends on the compilation environment and the Option Strict statement. If strict semantics are being used, only widening conversions may occur implicitly. If permissive semantics are being used, all widening and narrowing conversions (in other words, all conversions) may occur implicitly.

You are using permissive semantics. So all narrowing conversions may occur implicitly. The examples you have given are all narrowing conversions. So they are allowed. If you don't want that, use option strict on.

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Oct 7, 2020

@VBAndCs I have hidden your question for being off topic. If you have questions about that issue, please use that issue to discuss them. Thanks. Alternatively, you can discuss it on gitter or discord. Please don't use other issues to discuss things unrelated to them. Thanks! :)

@Happypig375
Copy link
Member

@VBAndCs

Imports System

Class Foo1
    Shared Sub Main()
        Dim x As IDisposable
        Dim y As Foo1 = New Foo2
        x = y ' Perfectly acceptable
        Console.WriteLine(x)
        
        y = New Foo1
        x = y ' Try again: InvalidCastException
    End Sub
End Class

Class Foo2
    Inherits Foo1
    Implements IDisposable
    Sub Dispose() Implements IDisposable.Dispose
    End Sub
End Class

@VBAndCs
Copy link
Author

VBAndCs commented Oct 7, 2020

The examples you have given are all narrowing conversions

The compiler may expect to have a narrowing in runtime, if only the opposite conversion is widening. Dealing with generic interfaces, must force VB to check the generic types, not only the interfaces types, and here Foo1 and Foo2 doesn't have a widening operation to expect they may have a narrowing one in runtime. VB refuses to cast each of them to each other, so, defiantly it must do the same with a generic interface having them as generic types.
Please, fix that.

@CyrusNajmabadi
Copy link
Member

Please, fix that.

I honestly don't know what you're asking for. As i've pointed out, the compiler seems to be operating as per the spec. If you disagree you must point out the particular parts of the spc that you think it is violating. Absent that there will be no further movement on this topic.

Again, if you do not actually point out the spec violation that is occurring, nothing will change here. Also, when i say "point out" i mean: provide an actual hyperlink to the section of the spec in question, or an actual quote from the spec. Saying you disagree or that you don't like this does not suffice.

@CyrusNajmabadi
Copy link
Member

and here Foo1 and Foo2 doesn't have a widening operation to expect they may have a narrowing one in runtime.

I see nothing in the spec that says this is how things work. The spec is explicit here and refers to "interfaces" as a whole. That means it applies to both normal interfaces and generic interfaces. So the same rules apply for those. If you feel otherwise, you must link/quote the section of the specification you feel supports your position.

Further comments that do not do this will not be acted on.

@pricerc
Copy link

pricerc commented Oct 12, 2020

... @pricerc ...
What do you thing about this?

Since I was asked for an opinion.

This is an excellent example of why option strict on is so important. If you code with option strict off, you're just asking for trouble, and you'll get no sympathy from me for any trouble you get as a result.

As far as I'm concerned option strict off is for 'backward compatibility', and should not be used in new code. So you have to add some explicit casts here-and-there. It's a small price to pay for more reliable and predictable code.

@VBAndCs if VB was being re-imagined today, I would expect there to be no option strict off available at all - it would just be always strict on.

@VBAndCs
Copy link
Author

VBAndCs commented Oct 12, 2020

@pricerc
I totally disagree.
VB can work in three modes:

  1. Fully static typing: Option explicit on & Option strict on
  2. Fully dynamic typing: Option explicit off & Option strict off
  3. Hybrid: Option explicit on & Option strict off

The third mode is the most useful and make coding faster but still safe with less errors. This is not a legacy as you think. This is the default settings for the language, meaning that it is encouraged or at least asked for by most developers.
In fact, MS adopted the 2nd mode in Small basic, to work without types or declaration statements, as a fully dynamic language, which means this is the easiest and fastest mode for beginners.
The only situation I find the first mode useful, is when I use a tool to convert C# code to VB. It helps in detecting translation mistakes.
I love VB this way, and I hate to convert it to a verbose version of C#.

@pricerc
Copy link

pricerc commented Oct 12, 2020

@VBAndCs

You're welcome to totally disagree. I'm just giving you my opinion :) (which you asked for :) )

Which comes from my personal experience dealing with hundreds of VB/VBA programs from different people, for over 20 years.

I'm sure there was a discussion somewhere about changing the VB defaults for Strict. I wish they would. Because of the environments I support, I have about 10 different active virtual environments with Visual Studio installed, and I regularly have to retire and replace them with new ones. So I always pretty much have VS in its default settings. But changing the option strict is something I do on all my projects, and one of the first things I do if I'm asked to take over someone else's work. It is one of the quickest ways to find weird problems.

For me, it's option explicit on, option strict on, option infer on, option compare binary.

(side note: for text comparisons, I have gotten into the habit of using String.Compare in both C# and VB, because that's what StyleCop/FxCop (can't remember which) typically recommends, so the option compare is largely redundant for me).

@CyrusNajmabadi
Copy link
Member

The third mode is the most useful and make coding faster but still safe with less errors.

You say it's safe with less errors. But you also don't like that it's unsafe and make these conversion errors. If you don't like that conversion errors then turn on option strict.

@VBAndCs
Copy link
Author

VBAndCs commented Oct 12, 2020

@CyrusNajmabadi
No. I say I am using this mode for decades, but this is an odd situation that works against my expectations. If is easy to be detected in design time, so, no need to get a runtime error. The powerfulness of the 3rd mod is what makes me ask for this, because forcing developers to use the first mode, will make them hate vb and prefer C#. In the first mode, VB is too verbose with no advantages over C# at all! So, VB should be best tuned to work in the 3rd mode.

@CyrusNajmabadi
Copy link
Member

but this is an odd situation that works against my expectations.

It's literally why this option exists :-)

If is easy to be detected in design time,

It is. But that's why you can turn on this option. It is literally the option you can turn in to say you want this behavior. We can't make this behavior the default because it would literally break customers all over the ecosystem.

@pricerc
Copy link

pricerc commented Oct 13, 2020

will make them hate vb and prefer C#.

Ironically, I'm sure I've seen the exact opposite argument somewhere, where someone was complaining about VB's default 'strict off' behaviour, and why that made VB an inferior language.

Which goes to show, you can't please all of the people all of the time.

@paul1956
Copy link
Contributor

@pricerc LOVE VB, HATE Strict Off you can believe both.

@AnthonyDGreen
Copy link
Contributor

Consider this:

Module Module1

    Sub Main()

        Dim x As IList(Of Foo1)
        Dim y As Foo2 = New Foo2()

        Try
            x = y ' compiles!

        Catch ex As Exception
            Console.WriteLine(ex.Message)
        End Try

        y = New Foo2Derived()
        x = y

        x = New List(Of Foo2) ' compiles

    End Sub

End Module

Class Foo1

End Class

Class Foo2

End Class

Class Foo2Derived
    Inherits Foo2

    Implements IList(Of Foo1)

    Public Function IndexOf(item As Foo1) As Integer Implements IList(Of Foo1).IndexOf
        Throw New NotImplementedException()
    End Function

    Public Sub Insert(index As Integer, item As Foo1) Implements IList(Of Foo1).Insert
        Throw New NotImplementedException()
    End Sub

    Public Sub RemoveAt(index As Integer) Implements IList(Of Foo1).RemoveAt
        Throw New NotImplementedException()
    End Sub

    Default Public Property Item(index As Integer) As Foo1 Implements IList(Of Foo1).Item
        Get

        End Get
        Set(value As Foo1)

        End Set
    End Property

    Public Sub Add(item As Foo1) Implements ICollection(Of Foo1).Add
        Throw New NotImplementedException()
    End Sub

    Public Sub Clear() Implements ICollection(Of Foo1).Clear
        Throw New NotImplementedException()
    End Sub

    Public Function Contains(item As Foo1) As Boolean Implements ICollection(Of Foo1).Contains
        Throw New NotImplementedException()
    End Function

    Public Sub CopyTo(array() As Foo1, arrayIndex As Integer) Implements ICollection(Of Foo1).CopyTo
        Throw New NotImplementedException()
    End Sub

    Public Function Remove(item As Foo1) As Boolean Implements ICollection(Of Foo1).Remove
        Throw New NotImplementedException()
    End Function

    Public ReadOnly Property Count As Integer Implements ICollection(Of Foo1).Count
    Public ReadOnly Property IsReadOnly As Boolean Implements ICollection(Of Foo1).IsReadOnly

    Public Function GetEnumerator() As IEnumerator(Of Foo1) Implements IEnumerable(Of Foo1).GetEnumerator
        Throw New NotImplementedException()
    End Function

    Private Function IEnumerable_GetEnumerator() As IEnumerator Implements IEnumerable.GetEnumerator
        Throw New NotImplementedException()
    End Function
End Class

In short, forget about the expression being a New expression for a moment.

  • Imagine it's just a variable of type Foo2 (or List(Of Foo2)).
  • Because the variable has type Foo2 or List(Of Foo2), it can refer to an instance of a type derived from type Foo2 or List(Of Foo2).
  • That derived type could implement IList(Of Foo1); this is why it's not provable that the cast will fail.
  • And, even an explicitly written cast will compile.

A cast that truly is provably impossible will still be an error regardless of the Option Strict setting. Now, if you were to change the example to mark Foo2 as NotInheritable the compiler will start reporting a warning on the assignment because the above scenario--a derived type that implements the target interface--is no longer possible.

However, because of COM interop (and something else I think) it's still technically possible for a NotInheritable type to implement at runtime an interface which it does not implement at compile time which is why this cast will still not be an error. Rather, it would be incorrect for such code to not compile. So at that point the issue isn't really about Strict On or Strict Off, as @CyrusNajmabadi points out Strict On is behaving as designed in that it requires such a conversion to be explicit. I do not believe there is any defect here.

NOTE: And to be clear, Strict Off isn't a feature. It's never behaving as designed. The language is permissive. Strict On is some stuff that got bolted on.

@VBAndCs
Copy link
Author

VBAndCs commented Nov 4, 2020

@AnthonyDGreen
Thanks man. You made it crystal clear. :)
I hope you provide some insight about this too:
#49095 (comment)

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

6 participants