Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Nullable: CancellationToken#23609

Merged
stephentoub merged 1 commit intodotnet:NullableFeaturefrom
stephentoub:nullablecancellation
Apr 1, 2019
Merged

Nullable: CancellationToken#23609
stephentoub merged 1 commit intodotnet:NullableFeaturefrom
stephentoub:nullablecancellation

Conversation

@stephentoub
Copy link
Copy Markdown
Member

No description provided.

if (_kernelEvent != null)
{
ManualResetEvent mre = Interlocked.Exchange(ref _kernelEvent, null);
ManualResetEvent? mre = Interlocked.Exchange<ManualResetEvent?>(ref _kernelEvent!, null);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@jaredpar, I'm confused by the changes that were necessary on this line. Here _kernelEvent is declared as a ManualResetEvent?. As written here:

ManualResetEvent? mre = Interlocked.Exchange<ManualResetEvent?>(ref _kernelEvent!, null)

it compiles without warning.

However, this:

ManualResetEvent? mre = Interlocked.Exchange(ref _kernelEvent, null);

warns on the null argument that Cannot convert null literal to non-nullable reference or unconstrained type parameter and hovering over Exchange to get IntelliSense for it shows that it inferred the type of T to be ManualResetEvent rather than ManualResetEvent?, even though _kernelEvent is ManualResetEvent?. Is it somehow getting messed up by the _kernelEvent != null guard above it?

Similarly, if I change it to be:

ManualResetEvent? mre = Interlocked.Exchange<ManualResetEvent?>(ref _kernelEvent, null);

that warns on the _kernelEvent argument "Possible null reference assignment", which should be fine, given that the type of T here is nullable.

Is this by design or a bug?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, the null check is causing ref _kernelEvent to be inferred as non-nullable.

/// be used to unregister the callback.</returns>
/// <exception cref="T:System.ArgumentNullException"><paramref name="callback"/> is null.</exception>
public CancellationTokenRegistration Register(Action<object> callback, object state) =>
public CancellationTokenRegistration Register(Action<object?> callback, object? state) =>
Copy link
Copy Markdown
Member Author

@stephentoub stephentoub Apr 1, 2019

Choose a reason for hiding this comment

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

@jcouv, there are a bunch of places in the framework where we take an Action<object> and an object state to pass to it. Thus, the object in Action<object> callback will be null if and only if state is null. Do we have any attribution planned that will allow for that relationship to be expressed? I don't yet know if it would actually matter much, so just asking right now.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We have not discussed any such attribute yet.
Feel free to add a comment with scenario details in dotnet/roslyn#26761

@stephentoub stephentoub merged commit 0efef3e into dotnet:NullableFeature Apr 1, 2019
@stephentoub stephentoub deleted the nullablecancellation branch April 1, 2019 23:35
Dotnet-GitSync-Bot pushed a commit to Dotnet-GitSync-Bot/corefx that referenced this pull request Apr 5, 2019
Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
stephentoub added a commit to dotnet/corefx that referenced this pull request Apr 6, 2019
Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
Dotnet-GitSync-Bot pushed a commit to Dotnet-GitSync-Bot/corert that referenced this pull request Apr 9, 2019
Signed-off-by: dotnet-bot <anirudhagnihotry098@gmail.com>
Dotnet-GitSync-Bot pushed a commit to Dotnet-GitSync-Bot/corert that referenced this pull request Apr 9, 2019
Signed-off-by: dotnet-bot <anirudhagnihotry098@gmail.com>
Dotnet-GitSync-Bot pushed a commit to Dotnet-GitSync-Bot/mono that referenced this pull request Apr 9, 2019
Signed-off-by: dotnet-bot <anirudhagnihotry098@gmail.com>
Anipik pushed a commit to Dotnet-GitSync-Bot/corert that referenced this pull request Apr 9, 2019
Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
jkotas pushed a commit to dotnet/corert that referenced this pull request Apr 10, 2019
Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants