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

Adjust for roslyn change on ref parameters and NotNullIfNotNull #42707

Merged
merged 2 commits into from
Sep 25, 2020

Conversation

jcouv
Copy link
Member

@jcouv jcouv commented Sep 24, 2020

Roslyn now uses the R-value/flow-state for ref parameters, instead of the declared type (L-value). (PR dotnet/roslyn#47952)
This affected call to EnsureInitializedCore, which now needs explicit type parmaeters.

Roslyn now enforced NotNullIfNotNull attribute within method bodies. This affected CreateWrapperOfType, which now needs a suppression.
Fixes D:\repos\runtime\src\coreclr\src\System.Private.CoreLib\src\System\Runtime\InteropServices\Marshal.CoreCLR.cs(592,13): error CS8825: Return value must be non-null because parameter 'o' is non-null. [D:\repos\runtime\src\coreclr\src\System.Private.CoreLib\System.Private.CoreLib.csproj]

FYI, more fixes will be need to integrate a new version of roslyn, mostly due to UnmanagedCallersOnly:

D:\repos\runtime\src\coreclr\src\System.Private.CoreLib\src\System\Runtime\CompilerServices\CastHelpers.cs(140,32): error CS0030: Cannot convert type 'nuint' to 'System.Runtime.CompilerServices.CastHelpers.CastResult' [D:\repos\runtime\src\coreclr\src\System.Private.CoreLib\System.Private.CoreLib.csproj]

D:\repos\runtime\src\libraries\System.Private.CoreLib\src\System\Globalization\CalendarData.Icu.cs(428,59): error CS1503: Argument 1: cannot convert from '&method group' to 'delegate*<char*, IntPtr, void>' [D:\repos\runtime\src\coreclr\src\System.Private.CoreLib\System.Private.CoreLib.csproj]
D:\repos\runtime\src\libraries\System.Private.CoreLib\src\System\Globalization\CalendarData.Windows.cs(269,51): error CS1503: Argument 1: cannot convert from '&method group' to 'delegate*<char*, uint, IntPtr, void*, Interop.BOOL>' [D:\repos\runtime\src\coreclr\src\System.Private.CoreLib\System.Private.CoreLib.csproj]
D:\repos\runtime\src\libraries\System.Private.CoreLib\src\System\Globalization\CalendarData.Windows.cs(421,56): error CS8786: Calling convention of 'CalendarData.EnumCalendarsCallback(char*, uint, IntPtr, void*)' is not compatible with 'Default'. [D:\repos\runtime\src\coreclr\src\System.Private.CoreLib\System.Private.CoreLib.csproj]
D:\repos\runtime\src\libraries\System.Private.CoreLib\src\System\Globalization\CultureData.Nls.cs(120,55): error CS8786: Calling convention of 'CultureData.EnumSystemLocalesProc(char*, uint, void*)' is not compatible with 'Default'. [D:\repos\runtime\src\coreclr\src\System.Private.CoreLib\System.Private.CoreLib.csproj]
D:\repos\runtime\src\libraries\System.Private.CoreLib\src\System\Globalization\CultureData.Nls.cs(407,49): error CS8786: Calling convention of 'CultureData.EnumTimeCallback(char*, void*)' is not compatible with 'Default'. [D:\repos\runtime\src\coreclr\src\System.Private.CoreLib\System.Private.CoreLib.csproj]
D:\repos\runtime\src\libraries\System.Private.CoreLib\src\System\Globalization\CultureData.Nls.cs(493,55): error CS8786: Calling convention of 'CultureData.EnumAllSystemLocalesProc(char*, uint, void*)' is not compatible with 'Default'. [D:\repos\runtime\src\coreclr\src\System.Private.CoreLib\System.Private.CoreLib.csproj]
D:\repos\runtime\src\libraries\System.Private.CoreLib\src\System\Globalization\CultureData.Nls.cs(521,59): error CS8786: Calling convention of 'CultureData.EnumAllSystemLocalesProc(char*, uint, void*)' is not compatible with 'Default'. [D:\repos\runtime\src\coreclr\src\System.Private.CoreLib\System.Private.CoreLib.csproj]

@jcouv jcouv marked this pull request as ready for review September 24, 2020 23:59
@stephentoub
Copy link
Member

@danmosemsft, assuming we're going to take an updated compiler into the release branch for .NET 5, we'll need this change, plus whatever change addresses the unmanaged callers updates @jcouv mentions.

@danmoseley
Copy link
Member

@jeffschw where did we land on compiler updates? I think we have a principle that we build release with release so we must keep taking updates? Any matching fixes would likely be tell mode.

@am11
Copy link
Member

am11 commented Sep 25, 2020

I think we have a principle that we build release with release so we must keep taking updates?

There was some related discussion about it in this thread for SDK 3.1 release, still using beta version of csc: dotnet/installer#5857.

@jcouv
Copy link
Member Author

jcouv commented Sep 25, 2020

@stephentoub I'm not sure the approval policy for runtime repo. If one approval suffices and there is no special ask-mode now, feel free to merge/squash. Thanks

@stephentoub stephentoub merged commit 91d2937 into dotnet:master Sep 25, 2020
@stephentoub
Copy link
Member

Thanks, @jcouv.

@danmoseley
Copy link
Member

I noticed @ViktorHofer did #42269 which I guess (?) means we will continue to get new compilers.

@ViktorHofer FYI that this change will need to get into 5.0 when that happens.

BTW, is there an @ handle for the dotnet/runtime infra crew? I think that might be useful.

@ViktorHofer
Copy link
Member

I noticed @ViktorHofer did #42269 which I guess (?) means we will continue to get new compilers.

Depends on if Arcade will continue to update the compiler in their release/5.x branch (which doesn't exist yet).

BTW, is there an @ handle for the dotnet/runtime infra crew? I think that might be useful.

@dotnet/runtime-infrastructure

@ViktorHofer
Copy link
Member

@ViktorHofer FYI that this change will need to get into 5.0 when that happens.

if not necessary for 5.0 RTW, we could/should port this into 5.0 for servicing readiness.

jkotas pushed a commit that referenced this pull request Oct 9, 2020
* Adjust for roslyn change on ref parameters

* Adjust for roslyn change on NotNullIfNotNull
ViktorHofer added a commit that referenced this pull request Oct 10, 2020
* Update dependencies from https://github.com/dotnet/arcade build 20201006.7

Microsoft.DotNet.Build.Tasks.Feed , Microsoft.DotNet.Build.Tasks.Packaging , Microsoft.DotNet.Build.Tasks.SharedFramework.Sdk , Microsoft.DotNet.Build.Tasks.TargetFramework.Sdk , Microsoft.DotNet.CodeAnalysis , Microsoft.DotNet.GenAPI , Microsoft.DotNet.GenFacades , Microsoft.DotNet.XUnitExtensions , Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Helix.Sdk , Microsoft.DotNet.RemoteExecutor , Microsoft.DotNet.VersionTools.Tasks , Microsoft.DotNet.XUnitConsoleRunner , Microsoft.DotNet.ApiCompat
 From Version 5.0.0-beta.20474.4 -> To Version 5.0.0-beta.20506.7

* React to TargetFramework.Sdk change

* Roslyn update response (#43056)

* Update function pointer syntax usage to official.

* Fix warnings with new Roslyn

Co-authored-by: Jan Kotas <jkotas@microsoft.com>

* Adjust for roslyn change on ref parameters and NotNullIfNotNull (#42707)

* Adjust for roslyn change on ref parameters

* Adjust for roslyn change on NotNullIfNotNull

* Fix all configurations build

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>
Co-authored-by: Aaron Robinson <arobins@microsoft.com>
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
Co-authored-by: Julien Couvreur <jcouv@users.noreply.github.com>
Co-authored-by: Santiago Fernandez Madero <safern@microsoft.com>
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants