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

Discussion: tuple projection initializers #370

Closed
alrz opened this issue Mar 28, 2017 · 17 comments
Closed

Discussion: tuple projection initializers #370

alrz opened this issue Mar 28, 2017 · 17 comments
Assignees
Milestone

Comments

@alrz
Copy link
Member

alrz commented Mar 28, 2017

(int, int) t = (x, y);		// (int, int)
(int a, int b) t = (x, y);	// (int a, int b)
var t = (x, y);			// (int x, int y)
var t = (x, x);			// (int, int)
var t = (n: x, x);		// (int n, int x)
var t = (x: x, x);		// (int x, int)
var t = (x, 5);			// (int x, int)
var t = (x, x, y);		// (int, int, int y)

This is not a breaking change, as per dotnet/roslyn#13319.

@alrz
Copy link
Member Author

alrz commented Mar 28, 2017

/cc @jcouv

@jnm2
Copy link
Contributor

jnm2 commented Mar 28, 2017

Yes. I haven't been pleased with the number of times I've had to write (Name1: x.Name1, Name2: y.Name2) already.

@aluanhaddad
Copy link

aluanhaddad commented Apr 1, 2017

The use case comes up almost immediately

var indexedAndFiltered = elements
    .Select((element, index) => (element, index))
    .Where(x => x.index % 2 == 0) // error
    .Select(x => x.element); // error

The above should work.

Currently, using a tuple results in code that is more performant and easier to write than using an anonymous type but is less readable. That should not be the case and it does not need to be here.

@jcouv jcouv self-assigned this Apr 1, 2017
@jcouv
Copy link
Member

jcouv commented Apr 1, 2017

@alrz I like the idea (inferring names for tuple elements based on the expressions used), and @aluanhaddad's motivating example. More supporting examples would be nice.

If I understand your proposal, we would:

  1. try to infer a candidate name for each tuple element which does not have an explicit name
  2. if any candidate names are duplicates (case-sensitive in C#, case-insensitive in VB) within the entire tuple, we drop those candidates

Digging into the first part, some examples of expressions:

  • literals (1, null, "string") provides no candidate name
  • that includes tuple literals (`var t2 = (x, (y, z)); // tuple t2 has names "x" and null)
  • y, x.y and this.y gives "y" and System.Math.PI gives "PI"
  • M(x) and x.M() give "M"
  • this, Me, x + y, nameof(x), sizeof(x), default provide no candidate name (those could be added later if determined useful)

Although I don't see much of a problem with it, I want to point out that there would be no way of overriding this. You could choose another name ((a: x, b: y) uses names "a" and "b"), but you could not go back to the "no element name" behavior (null names). I can't think of a motivating example. If we really needed it, maybe that's a job for the discard ((_: x, _: y) would have no names).

Another important consideration is that when you use tuple.y, "Go To Definition" on y should take you to the right place, but rename should create an explicit name.

@alrz
Copy link
Member Author

alrz commented Apr 1, 2017

@jcouv

Digging into the first part, some examples of expressions

As for extracting names from the expression, I'm thinking that we should use the existing ExtractAnonymousTypeMemberName for that, so something like M() should not give M.

if any candidate names are duplicates (case-sensitive in C#, case-insensitive in VB) within the entire tuple, we drop those candidates

Not just duplication, ambigious names should be dropped as well, e.g. Item1.

want to point out that there would be no way of overriding this.

The first example demonstrates how we can override names. If we do not use those names, I don't see any reason to be able to explicitly "unname" an element (is there a case where this is an issue?), however, the inferred name shall not contribute to "name mismatch" warnings and such.

@jcouv
Copy link
Member

jcouv commented Apr 1, 2017

@alrz Thanks for the pointer on ExtractAnonymousTypeMemberName.

I took a quick stab to see what this would look like. I'll bubble the idea around to see how it ranks.
Your observation about inferred names not contributing to "name mismatch" warnings is important. It means we won't be able to just merge all names into the tuple symbol. We need to remember which names came from inference in BoundTupleLiteral (until conversion checks happen).

@gafter
Copy link
Member

gafter commented Apr 5, 2017

As has been noted elsewhere, this would be a breaking change. Doesn't mean it can't happen.

@alrz
Copy link
Member Author

alrz commented Apr 5, 2017

This demonstrates the breaking change.

static void M<T>(this (T, Action M) t) => (1, t.M).M();

What are the odds.

@jcouv
Copy link
Member

jcouv commented Apr 5, 2017

From discussion with LDM and compat council today, we will pursue this change in C# 7.1.

@jcouv jcouv added this to the 7.1 candidate milestone Apr 5, 2017
@jcouv jcouv changed the title Discussion: tuple projection initializers Proposal: tuple projection initializers Apr 6, 2017
@jcouv jcouv changed the title Proposal: tuple projection initializers Discussion: tuple projection initializers Apr 6, 2017
@jcouv
Copy link
Member

jcouv commented Apr 6, 2017

Promoted to a championed issue #415. Next step: I'll capture our notes in a proposal/spec and finish the implementation (in PR dotnet/roslyn#18374)
Thanks @alrz and others for the discussion.

@jcouv jcouv closed this as completed Apr 6, 2017
@AlgorithmsAreCool
Copy link

static void M<T>(this (T, Action M) t) => (1, t.M).M(); 

@alrz
Why doesn't GitHub have a 😱 reaction

@Grauenwolf
Copy link

In VB, this allows for additional cases, such as x.y().

Any chance of getting this in C# too?

@alrz
Copy link
Member Author

alrz commented Jun 3, 2017

@Grauenwolf this will be available in C# 7.1

@jcouv
Copy link
Member

jcouv commented Jun 3, 2017

@Grauenwolf This may be worth starting a new discussion on. It would be good to look at the cases supported by VB and consider each difference.
At first glance, the trade-off (small incremental gain compared to trouble of small compat issue for anonymous types and tuples) it not very appealing to me. I could be convinced otherwise.

@jcouv
Copy link
Member

jcouv commented Jun 3, 2017

@alrz Grauenwolf is asking about inferring names from invocation expressions, in C# as VB does. That's not in C# 7.1.

@Grauenwolf
Copy link

@jcouv As I see it, the change is now or never. We're breaking compatibility between 7.0 and 7.1 anyways, but I don't think that we'd be able to a second time.

@Grauenwolf
Copy link

It's definitely a nice to have, both for tuples and anonymous types, but I wouldn't cry if I didn't get it. I would consider it a bit of polish, just like tuple name inference in general.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants