Skip to content

Use RTM version of framework ref assemblies#51819

Merged
1 commit merged intodotnet:mainfrom
jaredpar:name
Mar 12, 2021
Merged

Use RTM version of framework ref assemblies#51819
1 commit merged intodotnet:mainfrom
jaredpar:name

Conversation

@jaredpar
Copy link
Member

@jaredpar jaredpar commented Mar 11, 2021

This changes our code to use the RTM version of the framework reference
assemblies rather than preview.

The RTM version though forces a reference to Microsoft.VisualBasic.dll
when building VB projects. That caused a compilation error in part of
our code because of the following pattern

For Each str in strings

In VB the identifier in the For Each is not necessarily a new
declaration but can also refer to existing identifiers. Hence this name
must go through standard name lookup.

Once the reference to Microsoft.VisualBasic.dll is added the
Microsoft.VisualBasic.Conversions module becomes available and the
method Str in that module is now a valid top level name. This causes
the above code to stop compiling. Fix is to use a non-conflicting
identifier.

closes #51711

This changes our code to use the RTM version of the framework reference
assemblies rather than preview.

The RTM version though forces a reference to Microsoft.VisualBasic.dll
when building VB projects. That caused a compilation error in part of
our code because of the following pattern

```vb
For Each str in strings
```

In VB the identifier in the `For Each` is not necessarily a new
declaration but can also refer to existing identifiers. Hence this name
must go through standard name lookup.

Once the reference to Microsoft.VisualBasic.dll is added the
`Microsoft.VisualBasic.Conversions` module becomes available and the
method `Str` in that module is now a valid top level name. This causes
the above code to stop compiling. Fix is to use a non-conflicting
identifier.

closes #517711
@jaredpar jaredpar requested review from a team as code owners March 11, 2021 23:42
@jaredpar
Copy link
Member Author

@dotnet/roslyn-compiler @RikkiGibson @jmarolf for review.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Auto-approval

@RikkiGibson
Copy link
Member

I neglected to ask when I first reviewed: are we fine with the implications of this new reference? Could this cause a reference to Microsoft.VisualBasic.dll to come along with Microsoft.CodeAnalysis.VisualBasic.dll when it previously did not?

@ghost ghost merged commit ecdb876 into dotnet:main Mar 12, 2021
@ghost ghost added this to the Next milestone Mar 12, 2021
@jaredpar jaredpar deleted the name branch March 12, 2021 21:08
@jaredpar
Copy link
Member Author

Yeah the extra ref is only occurring in a few places and it's fine there.

@allisonchou allisonchou modified the milestones: Next, 16.10.P2 Mar 29, 2021
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Release reference assemblies break overload resolution in VB on .NET Framework 4.7.2

6 participants