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

Allow deconstruction without ValueTuple type #22382

Closed
jcouv opened this issue Sep 27, 2017 · 2 comments
Closed

Allow deconstruction without ValueTuple type #22382

jcouv opened this issue Sep 27, 2017 · 2 comments
Assignees
Labels
Area-Compilers Test Test failures in roslyn-CI
Milestone

Comments

@jcouv
Copy link
Member

jcouv commented Sep 27, 2017

From discussion with Neal and Mads today, it would be nice if (x, y) = (1, 2); didn't require the System.ValueTuple types.

Presently, the only deconstruction where ValueTuple is not referenced is when the right-hand-side is a non-tuple expression and the result of the deconstruction-assignment expression is not used (see #18629).

One difficulty with this is, what will the semantic model show for the right-hand-side?

Also, when we do this, we should think about safe-to-escape rules. We may want to do element-wise checks (including nested deconstructions and foreach-deconstructions). The scenario is (global1, local1) = (global2, local2);.

FYI @gafter

@jcouv
Copy link
Member Author

jcouv commented Oct 9, 2017

Note: although ValueTuple is required to bind, not reference to it ends up emitted.
From discussion with Aleksey that makes sense: the language says there is an expression on the right-hand-side of the deconstruction, and it is a tuple.
I need to check with Neal and Mads whether that is sufficient/acceptable.

        [Fact]
        public void ValueTupleRequiredWhenRightHandSideIsTupleButNoReferenceEmitted()
        {
            string source = @"
class C
{
    public static void Main()
    {
        int x, y;
        (x, y) = (1, 2);
    }
}
";
            var comp = CreateStandardCompilation(source, parseOptions: TestOptions.Regular7, references: s_valueTupleRefs);
            comp.VerifyDiagnostics();

            Action<PEAssembly> assemblyValidator = assembly =>
            {
                var reader = assembly.GetMetadataReader();
                var names = reader.GetAssemblyRefNames().Select(name => reader.GetString(name));
                Assert.Empty(names.Where(name => name.Contains("ValueTuple")));
            };

            CompileAndVerify(comp, assemblyValidator: assemblyValidator);
        }

Note: this test should be checked in before closing this issue.

@jcouv jcouv added 4 - In Review A fix for the issue is submitted for review. Test Test failures in roslyn-CI and removed Feature Request labels Oct 17, 2017
@jcouv jcouv self-assigned this Oct 17, 2017
@jcouv
Copy link
Member Author

jcouv commented Oct 26, 2017

Confirmed with Neal that the current behavior makes sense.

  • (x, y) = (1, 2) requires SVT for binding, although it's not emitted.
  • (x, y) = e doesn't require SVT for binding (or emitting)

Closing the issue.

@jcouv jcouv closed this as completed Oct 26, 2017
@jcouv jcouv removed the 4 - In Review A fix for the issue is submitted for review. label Dec 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Test Test failures in roslyn-CI
Projects
None yet
Development

No branches or pull requests

1 participant