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

Type inference should be able to infer U? for T when T? (and inference differs when a discard is used) #50782

Closed
MrJul opened this issue Jan 26, 2021 · 4 comments
Assignees
Labels
Area-Compilers Bug Feature - Nullable Reference Types Nullable Reference Types help wanted The issue is "up for grabs" - add a comment if you are interested in working on it
Milestone

Comments

@MrJul
Copy link
Contributor

MrJul commented Jan 26, 2021

Version Used:
VisualStudio.16.Release/16.8.4+30907.101
C# Tools 3.8.0-5.20604.10+9ed4b774d20940880de8df1ca8b07508aa01c8cd

Steps to Reproduce:
<LangVersion>9</LangVersion>

using System.Diagnostics.CodeAnalysis;

#nullable enable

interface IOperation<T> { }
class StringOperation : IOperation<string?> { }

class C {   
    void M() {
        TestA(new StringOperation(), out string? discard1); // 1: no warning
        TestA(new StringOperation(), out string? _);        // 2: CS8620
        TestA(new StringOperation(), out _);                // 3: CS8620
        
        TestB(new StringOperation(), out string? discard2); // 4: CS8620
        TestB(new StringOperation(), out string? _);        // 5: CS8620
        TestB(new StringOperation(), out _);                // 6: CS8620
    }

    void TestA<T>(IOperation<T> operation, [MaybeNull] out T result) => result = default;
    void TestB<T>(IOperation<T> operation, out T? result) => result = default;
}

Expected Behavior:

  • Ideally, no warnings at all, I would expect the compiler to see that T?/[MaybeNull] T can be matched by string? (in effect making it string??) without warnings. Currently T is always inferred to be string instead of string?, resulting in warnings.
  • At the very least, 1 and 2 should have same behavior: the specified types are the same, only the parameter name is missing in 2.

Actual Behavior:
Warnings everywhere but on 1.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Jan 26, 2021
@MrJul MrJul changed the title Nullability inference should be able to infer U? for T when T? (and inference differs when a discard is used) Type inference should be able to infer U? for T when T? (and inference differs when a discard is used) Jan 26, 2021
@jaredpar
Copy link
Member

At the very least, 1 and 2 should have same behavior: the specified types are the same, only the parameter name is missing in 2.

Agree. Interested to see what the bug appears to be here, that's very strange.

@jaredpar jaredpar added Bug Feature - Nullable Reference Types Nullable Reference Types and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Jan 26, 2021
@jaredpar jaredpar added this to the 16.10 milestone Jan 26, 2021
@jinujoseph jinujoseph modified the milestones: 16.10, 17.0 Jul 16, 2021
@jaredpar jaredpar modified the milestones: 17.0, 17.1 Aug 3, 2021
@jcouv jcouv modified the milestones: 17.1, 17.2 Mar 17, 2022
@jcouv jcouv modified the milestones: 17.2, 17.3 May 14, 2022
@jaredpar jaredpar modified the milestones: 17.3, C# 11.0, 17.4 Jun 24, 2022
@jaredpar jaredpar modified the milestones: 17.4, Backlog Jul 20, 2022
@RikkiGibson RikkiGibson added the help wanted The issue is "up for grabs" - add a comment if you are interested in working on it label Jul 20, 2022
@TomatorCZ
Copy link
Contributor

Hi, I will try to investigate the issue as an opportunity to get familiar with the platform. @RikkiGibson

@TomatorCZ
Copy link
Contributor

Hi!

I made a little investigation of the issue. I managed to figure out why there is the different behaviour. See PR.

During the investigation, I noticed an additional strange behaviour in several parts of the code. I think that this causes a warning in case of the discard delegate out _ . IMO this should be without the warning because the compiler should determine that _ is a type of string or string?, since there is no user defined type or annotation.

So, the first weird thing is during resolving actual type parameter here. This code is called every time when there are candidates for the type parameter. You can see, we ignore nullability information here whenever the conversions don't include nullability. This looks fine, but the conversions never includes nullability(I tried to debug the example. The first and the second phase of inferring contains conversions without nullability resolving in the type parameter never being sth?).

The second weird thing is the type of parameter argument out _. After the first phase, the parameter types are resolved based on type parameters of generic method. In case of out _, the execution continues here, where we lose information about inferred nullability. It results in the second phase, where the parameter looks like this out string _ instead of this out string? _.

The third thing is ignoring TypeAnnotations of discard expressions here. The compiler ignores user defined ? which is a mistake IMO.

@RikkiGibson
Copy link
Contributor

I think the issue is resolved by #66309. Thanks for reporting it @MrJul!

jaredpar added a commit to jaredpar/runtime that referenced this issue Mar 1, 2023
Once runtime merges to a Roslyn toolset that has the fix for [issue
50782](dotnet/roslyn#50782) these annotations
will be necessary. Front loading the work here with this change.
jaredpar added a commit to dotnet/runtime that referenced this issue Mar 3, 2023
* Nullable annotation fixes

Once runtime merges to a Roslyn toolset that has the fix for [issue
50782](dotnet/roslyn#50782) these annotations
will be necessary. Front loading the work here with this change.

* Apply suggestions from code review

Co-authored-by: Stephen Toub <stoub@microsoft.com>

---------

Co-authored-by: Stephen Toub <stoub@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Bug Feature - Nullable Reference Types Nullable Reference Types help wanted The issue is "up for grabs" - add a comment if you are interested in working on it
Projects
None yet
Development

No branches or pull requests

6 participants