Nullable feature into master

[danmoseley]

Rolling PR to check for bugs introduced in the nullable annotations work.

As new changes merge into the nullable branch, this will re-run tests automatically.

We have not yet determined when to do the merge, so this is no-merge.

[danmoseley]

@safern @joshfree @buyaa-n @krwq @tannergooding @stephentoub

[danmoseley]

@stephentoub build break:

shared/System/Threading/CancellationTokenSource.cs(685,45): error CS8600: Converting null literal or possible null value to non-nullable type. [/mnt/j/workspace/dotnet_coreclr/master/arm_cross_checked_ubuntu_crossgen_comparison_prtest/src/System.Private.CoreLib/System.Private.CoreLib.csproj]

https://ci.dot.net/job/dotnet_coreclr/job/master/job/arm_cross_checked_ubuntu_crossgen_comparison_prtest/6961/consoleFull#705971828a82fefab-f698-416f-8fca-58544c94cd4e

[stephentoub]

I'm amazed they don't happen more often with all these concurrent PRs. Will fix.

[danmoseley]

@safern is going to add build -linuxmscorlib -arm to the branch CI.

[danmoseley]

shared\System\IO\FileStream.Windows.cs(317,30): error CS8604: Possible null reference argument for parameter 'tasks' in 'Task Task.WhenAll(params Task[] tasks)'. [D:\j\workspace\x64_checked_w---43686a05\src\System.Private.CoreLib\System.Private.CoreLib.csproj]
22:59:05 shared\System\IO\Stream.cs(259,52): error CS8600: Converting null literal or possible null value to non-nullable type. [D:\j\workspace\x64_checked_w---43686a05\src\System.Private.CoreLib\System.Private.CoreLib.csproj]
22:59:05 shared\System\IO\Stream.cs(259,52): error CS8602: Possible dereference of a null reference. [D:\j\workspace\x64_checked_w---43686a05\src\System.Private.CoreLib\System.Private.CoreLib.csproj]
22:59:05 shared\System\IO\Stream.cs(515,31): error CS8600: Converting null literal or possible null value to non-nullable type. [D:\j\workspace\x64_checked_w---43686a05\src\System.Private.CoreLib\System.Private.CoreLib.csproj]
22:59:05 shared\System\IO\Stream.cs(516,34): error CS8602: Possible dereference of a null reference. [D:\j\workspace\x64_checked_w---43686a05\src\System.Private.CoreLib\System.Private.CoreLib.csproj]
22:59:05 shared\System\IO\FileStream.Windows.cs(1592,31): error CS8600: Converting null literal or possible null value to non-nullable type. [D:\j\workspace\x64_checked_w---43686a05\src\System.Private.CoreLib\System.Private.CoreLib.csproj]
22:59:05 shared\System\IO\FileStream.Windows.cs(1592,31): error CS8602: Possible dereference of a null reference. [D:\j\workspace\x64_checked_w---43686a05\src\System.Private.CoreLib\System.Private.CoreLib.csproj]
22:59:05

@stephentoub

[danmoseley]

I would have expected to see a merge conflict with the move of delegate. #23552

[safern]

@stephentoub @jkotas -- let's merge this as is if we get a green build into master so that we can get annotations in reference assemblies for preview5 so we can start collecting feedback -- also this way we could catch any perf regression and fix it in the next preview.

We have 1 things to follow that are fixed on the next preview:

• ILLinker should strip Nullable attributes from internal/private APIs. (https://github.com/dotnet/coreclr/issues/23694)

Does that sound good?

[jkotas]

Fine with me

[stephentoub]

I'm ok with it, too. I don't believe the first issue you call out is actually an issue. And the second one is just something we'll want to address prior to actually shipping but shouldn't block work making it to master.

[safern]

I don't believe the first issue you call out is actually an issue

Yeah I thought it was an issue, but I just spoke with @tannergooding and he confirmed it is not an issue. I'll update my comment.

[tannergooding]

Is it that much infrastructure work to instead ship a SxS preview with nullable annotations (similar to what the compiler team did for their initial nullable previews - CC. @jaredpar).

My concern is that we are still working on annotating the framework and it will take some time and churn before it is complete. By shipping the framework as annotated by default, we are going to be forcing all users who are annotating their libraries to deal with our own churn and partial annotations, while simultaneously dealing with their own.

I think that shipping a SxS feature branch would be an overall better solution as it would let users opt-in to this dual-churn scenario or to decide to only deal with it after we have decided our own annotations are stable.

I believe the infrastructure work would largely resolve down to creating a NullableFeature branch in CoreFX and CoreSetup and then publishing those bits to a well-known location for people to pull down.

[stephentoub]

I don't believe merging into master affects that one way or the other; public consumption of these annotations only happens via reference assemblies, which are not directly impacted by any changes in corelib.

We could have a separate conversation about your question, but this PR doesn't seem like the right place for it.

[safern]

@dotnet-bot test Ubuntu x64 Checked CoreFX Tests

[safern]

Failures seem to be happening in all other PRs. I will merge to avoid potential merge conflicts and so that we can merged approved PRs into the feature branch.

[safern]

I'll open a new draft PR once we merge a new change into the nullable branch.