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

Analyze tuple conversions element-wise with W-warnings #33035

Closed
jcouv opened this issue Feb 1, 2019 · 1 comment
Closed

Analyze tuple conversions element-wise with W-warnings #33035

jcouv opened this issue Feb 1, 2019 · 1 comment
Labels
Area-Compilers New Language Feature - Nullable Reference Types Nullable Reference Types Resolution-By Design The behavior reported in the issue matches the current design
Milestone

Comments

@jcouv
Copy link
Member

jcouv commented Feb 1, 2019

Currently, we treat tuples like generic Pair<T1, T2> during the conversion/assignment. This results in a redundant safety warning, since we already track the initial state of Item1/Item2 in the case of tuples.

image

#nullable enable
public class UnitTest1
{
    void M(string? s, string s2)
    {
        (string, string) t = (s, s2); // should be W-warning on elements
        t.Item1.ToString();

        // Motivations to change this:
        // - redundant safety warning
        // - generic type inference
        // - deconstruction

        Pair<string, string> p = Pair.Create(s, s2);
        p.Item1.ToString();

        var t2 = ((string, string))(s, s2); // should be W-warning on elements. t2 is (string, string), but state is Item1 is nullable 
        t2.Item1.ToString(); // safety
    }
}
public class Pair<T1, T2>
{
    public T1 Item1;
    public T2 Item2;
}
static class Pair
{
    public static Pair<T1, T2> Create<T1, T2>(T1 x, T2 y) { throw null; }
}

FYI @cston

@jcouv
Copy link
Member Author

jcouv commented Mar 14, 2019

Closing from discussion with the team.

We’ve explored the tuple scenario in more details with the crew and we still feel the current behavior is correct.
In short, I’m happy to close this issue without further discussion in LDM.

For the record, the alternative was clarified as (roughly): whenever you read a local of tuple type, we would give the result a type based on the state of the tuple.

But this involves complexity:

  • We would do this for tuples in locals, but probably not those nested in other types.
  • We’d have to do special checks for passing tuples as ref arguments.

And the benefits aren’t clear:

  • Users probably don’t expect to see the type of a tuple local change as they do for simple locals.
  • Inferring (string?, string) in the scenario below doesn’t seem a major advantage.

@jcouv jcouv closed this as completed Mar 14, 2019
@jcouv jcouv added the Resolution-By Design The behavior reported in the issue matches the current design label Mar 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers New Language Feature - Nullable Reference Types Nullable Reference Types Resolution-By Design The behavior reported in the issue matches the current design
Projects
None yet
Development

No branches or pull requests

2 participants