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

Local of unconstrained type parameter type is reported as NotAnnotated when using var #46236

Closed
cston opened this issue Jul 22, 2020 · 7 comments
Assignees
Labels
Area-Compilers Bug Feature - Nullable Type Parameters Unconstrained type parameters can now be used with generics
Milestone

Comments

@cston
Copy link
Member

cston commented Jul 22, 2020

The local returned from SemanticModel.GetDeclaredSymbol() for y1 has Type.NullableAnnotation == NullableAnnotation.NotAnnotated, but should be NullableAnnotation.Annotated instead.

Compare with the GetDeclaredSymbol() result for y2 which has Type.NullableAnnotation == NullableAnnotation.Annotated.

#nullable enable

class Program
{
    static T F1<T>(T x1)
    {
        var y1 = x1;
        y1 = default(T);
        return y1;
    }

    static T F2<T>(T x2) where T : class
    {
        var y2 = x2;
        y2 = default(T);
        return y2;
    }
}
@jaredpar jaredpar added Bug Feature - Nullable Type Parameters Unconstrained type parameters can now be used with generics labels Jul 27, 2020
@jaredpar jaredpar added this to the 16.8 milestone Jul 27, 2020
@RikkiGibson
Copy link
Contributor

RikkiGibson commented Sep 18, 2020

@333fred is this by design, since we tend to "lie" about the nullable annotation to give a consistent answer to what we give for flow state? e.g. we always give (NotAnnotated, NotNull) or (Annotated, MaybeNull) or "oblivious"?

follow-up: the "lie" only occurs for API like GetTypeInfo, which is intended to give you an rvalue type. The API in this issue description is GetDeclaredSymbol which is expected to provide the real annotation for the symbol.

@333fred
Copy link
Member

333fred commented Sep 18, 2020

It might have been by design in C# 8, but with T? in C# 9 it no longer feels like that's the case.

@RikkiGibson
Copy link
Contributor

The initial state of parameters, though, is MaybeNull for both unconstrained T and T?, and that is where the NullableAnnotation.Annotated is coming from in this public API, right?

@333fred
Copy link
Member

333fred commented Sep 19, 2020

I don't believe we're talking about parameters here. We're talking about var. And var should be annotated, with whatever flow state it gets from the initializer.

@RikkiGibson
Copy link
Contributor

RikkiGibson commented Sep 19, 2020

Follow-up to offline discussion: this API is supposed to tell the truth 😉 so this is a bug. Thanks for helping me understand the scenario.

I think that even if the API were supposed to give an annotation reflecting the "rvalue type", this would still be the wrong behavior--I must have just gotten mixed up when reviewing the description.

@jaredpar jaredpar modified the milestones: 16.8, 16.9 Oct 12, 2020
@jcouv jcouv modified the milestones: 16.9, 17.0 May 28, 2021
@jcouv jcouv self-assigned this May 28, 2021
@jcouv
Copy link
Member

jcouv commented May 28, 2021

Fixed and verified for 17.0 (p1 or p2) by #53691

@jcouv jcouv closed this as completed May 28, 2021
@jcouv
Copy link
Member

jcouv commented Jun 2, 2021

Also backported fix to 16.11

@jcouv jcouv modified the milestones: 17.0, 16.11 Jun 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Bug Feature - Nullable Type Parameters Unconstrained type parameters can now be used with generics
Projects
None yet
Development

No branches or pull requests

5 participants