Skip to content

VB: Report access to members of an interface that are not supported by runtime.#38288

Merged
AlekseyTs merged 2 commits intodotnet:masterfrom
AlekseyTs:Issue35834
Aug 30, 2019
Merged

VB: Report access to members of an interface that are not supported by runtime.#38288
AlekseyTs merged 2 commits intodotnet:masterfrom
AlekseyTs:Issue35834

Conversation

@AlekseyTs
Copy link
Copy Markdown
Contributor

Fixes #35834. Fixes #35885.
Also addresses some scenarios related to #35824.

…y runtime.

Fixes dotnet#35834. Fixes dotnet#35885.
Also addresses some scenarios related to dotnet#35824.
@AlekseyTs AlekseyTs requested a review from a team August 26, 2019 02:34
@AlekseyTs
Copy link
Copy Markdown
Contributor Author

@dotnet/roslyn-compiler Please review

<value>Type '{0}' cannot be embedded because it has a re-abstraction of a member from base interface. Consider setting the 'Embed Interop Types' property to false.</value>
</data>
<data name="ERR_RuntimeDoesNotSupportDefaultInterfaceImplementation" xml:space="preserve">
<value>Target runtime doesn't support default interface implementation.</value>
Copy link
Copy Markdown
Member

@333fred 333fred Aug 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

implementation [](start = 60, length = 14)

Should this be pluralized? #Resolved

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be pluralized?

Either way makes no difference to me. The error message is copied from C# compiler , BTW.


In reply to: 317831806 [](ancestors = 317831806)


If symbol.Kind <> SymbolKind.Property AndAlso
Compilation.SourceModule IsNot symbol.ContainingModule AndAlso
symbol.ContainingType?.IsInterface AndAlso
Copy link
Copy Markdown
Member

@333fred 333fred Aug 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

symbol.ContainingType?.IsInterface [](start = 15, length = 34)

Nit: This nullable bool will cause the whole condition to get thrown into a bool?, consider explicitly comparing with true. (Side note: VB allows this?) #Pending

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VB allows this?

Yes. See section "11.19 Boolean Expressions" in the language specification

This nullable bool will cause the whole condition to get thrown into a bool?, consider explicitly comparing with true.

This wouldn't make any difference in terms of types in VB because comparing anything with Null is Null in VB, therefore the type of the comparison would be Boolean? as well.
However, the code generated might be slightly better if I call GetValueOrDefault explicitly here.


In reply to: 317833083 [](ancestors = 317833083)

Copy link
Copy Markdown
Member

@333fred 333fred left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (commit 1), other than noted nits.

@AlekseyTs
Copy link
Copy Markdown
Contributor Author

@dotnet/roslyn-compiler Please review, need a second sign-off.

5 similar comments
@AlekseyTs
Copy link
Copy Markdown
Contributor Author

@dotnet/roslyn-compiler Please review, need a second sign-off.

@AlekseyTs
Copy link
Copy Markdown
Contributor Author

@dotnet/roslyn-compiler Please review, need a second sign-off.

@AlekseyTs
Copy link
Copy Markdown
Contributor Author

@dotnet/roslyn-compiler Please review, need a second sign-off.

@AlekseyTs
Copy link
Copy Markdown
Contributor Author

@dotnet/roslyn-compiler Please review, need a second sign-off.

@AlekseyTs
Copy link
Copy Markdown
Contributor Author

@dotnet/roslyn-compiler Please review, need a second sign-off.

@AlekseyTs AlekseyTs merged commit ce40ec5 into dotnet:master Aug 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

3 participants