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

C# Design Notes for Jul12, 2016 #13022

Closed
MadsTorgersen opened this issue Aug 9, 2016 · 30 comments
Closed

C# Design Notes for Jul12, 2016 #13022

MadsTorgersen opened this issue Aug 9, 2016 · 30 comments

Comments

@MadsTorgersen
Copy link
Contributor

C# Language Design Notes for Jul 12, 2016

Agenda

Several design details pertaining to tuples and deconstruction resolved.

All or nothing with element names?

There's certainly a case for partial names on literals - sometimes you want to specify the names of some of the elements for clarity. These names have to fit names in the tuple type that the literal is being converted to:

List<(string firstName, string lastName, bool paid)> people = ...;

people.Add(("John", "Doe", paid: true)); 

Do we also want to allow partial names in types?

Yes we do. We don't have strong arguments against it, and generality calls for it. It's likely that there are scenarios, and not allowing will feel arbitrary. "The bool deserves a clarifying name, but the TypeSymbol is clear".

var t = (1, y: 2); // infers (int, int y)
(int x, int) t = (1, 2);

As a reminder, we distinguish between leaving out the name and explicitly specifying Item1, Item2, etc. You get a warning if you use Item1... in literals if they are not explicitly there in the target type. (Warning)`

(int, int) t = (Item1: 1, Item2: 2); // Warning, names are lost

For the editor experience we imagine offering completion of a named element in the corresponding position of a tuple literal.

ITuple

interface ITuple
{
    int Size;
    object this[int i] { get; }
}

ITuple is really an interface for dynamically checking for deconstructability. Whether we should add ITuple now depends on whether we think we want to allow recursive patterns to do that, once we add them.

We do think we want that.

We cannot use an existing collection interface (such as IReadOnlyCollection) because we want to be able to distinguish between deconstructable objects and collections (which might one day get their own pattern). It is fine for a class to implement both.

ValueTuple<...> should of course implement ITuple, and the translation of long tuples should work.

The Framework team should chime in on the specific shape and names on the interface. Is ITuple the right name, or is it too narrow? Is IDeconstructable more to the point or is it too long? Should it be Size or Length or Count?

var when var type exists

class var {}
var (x, y) = e;

People use this trick to block the use of var, and we should let them, even though we disagree with the practice. So even though a type isn't valid in that position, we will let the presence of a var type turn this into an error.

var method

What if there's a method called var?

ref int var(int x, int y);
var(x, y) = e; // deconstruction or call?

Here there is no prior art, so we will always interpret the var to mean a deconstructing declaration. If you wanted the method call, parenthesize the call or use @.

@dsaf
Copy link

dsaf commented Aug 9, 2016

ValueTuple<...> should of course implement ITuple, and the translation of long tuples should work.

@MadsTorgersen Will existing Tuple<...> hopefully also implement this? The ITuple with an indexed property accessor should definitely exist even simply for advanced reflection scenarios. I had to implement a custom one in one of the dynamic coding scenarios I had.

@vbcodec
Copy link

vbcodec commented Aug 9, 2016

@MadsTorgersen
Few questions:

Every type that implement ITuple can be deconstructed ?

Which level of all specified in #11031 (Tuple syntax for other types) will be available ?
var (int s, string n) c = new Car(); // assuming that car have speed and name properties
Is this code correct ?

Is it possible to add names for items (by fields (ItemName1, ItemName2, etc) or by function GetItemName(Index)) to ValueTuple ? This can be very helpful for reading data using reflection (for example databinding).

@alrz
Copy link
Member

alrz commented Aug 9, 2016

What is the recommended naming convention for tuple item names, specially in case of a partially named one? If (2, x: 3) is going to be used like t.Item1 and t.x it wouldn't be consistent.

Are named tuples allowed in an is expression? Issues arise when recursive patterns are introduced in the language (#11744).

@gafter
Copy link
Member

gafter commented Aug 9, 2016

@vbcodec

Every type that implement ITuple can be deconstructed ?

No. ITuple is to support pattern-matching when the object to be matched isn't statically known to be a tuple. This interface will not affect the deconstruction declaration or the deconstruction assignment.

Which level of all specified in #11031 (Tuple syntax for other types) will be available ?

That is what the Deconstruct method is for. When you put a deconstruct method on a type (with out parameters), that type becomes usable in deconstruction.

Is it possible to add names for items (by fields (ItemName1, ItemName2, etc) or by function GetItemName(Index)) to ValueTuple ?

I don't understand the question.

@gafter
Copy link
Member

gafter commented Aug 9, 2016

@alrz

What is the recommended naming convention for tuple item names, specially in case of a partially named one? If (2, x: 3) is going to be used like t.Item1 and t.x it wouldn't be consistent.

We don't have recommendations at this time.

Are named tuples allowed in an is expression? Issues arise when recursive patterns are introduced in the language (#11744).

You mean on the left-hand-side? #11744 is about tuple patterns.

@miloush
Copy link

miloush commented Aug 9, 2016

Just for reference, I found #11205 useful in answering the questions about Deconstruct methods I wanted to ask.

@vbcodec
Copy link

vbcodec commented Aug 9, 2016

@gafter

As for tuples with names I mean that code:

var t = (name: "John", age: 5);

will create instance of ValueTuple with: Item1 = "John", Item2 = 5, ItemName1 = "name", ItemName2 = "age". For now ValueTuple type do not have fields ItemName1 and ItemNAme2. So I ask if you can consider and eventually add and use these fields ?

@gafter
Copy link
Member

gafter commented Aug 9, 2016

@vbcodec We explicitly decided not to have names recorded at runtime. There is an identity conversion between tuple types for which only the names differ, and that would interfere with the identity conversion.

@iam3yal
Copy link

iam3yal commented Aug 10, 2016

The Framework team should chime in on the specific shape and names on the interface. Is ITuple the right name, or is it too narrow? Is IDeconstructable more to the point or is it too long? Should it be Size or Length or Count?

Just my opinion about ITuple if the whole point is deconstruction then it really needs to be IDeconstructable or ICanDeconstruct when you think about interfaces it's much better to think about features rather than representation.

The reason I think it's better to think about features is because then the intent is well documented, it opens the door for future improvements, it's more reusable and overall in most cases it's more robust.

However, if there's going to be more methods/properties on it like Length then IDeconstructable doesn't make much sense and it should be ITuple or maybe both, i.e, ITuple inherits IDeconstructable but this seems like over-engineering to me, IDeconstructable can be introduced when there's a need for more types to support deconstruction so maybe ITuple is in fact more suitable here.

The Framework team should chime in on the specific shape and names on the interface. Is ITuple the right name, or is it too narrow? Is IDeconstructable more to the point or is it too long? Should it be Size or Length or Count?

Again, my opinion, I don't think that that having a long name should be a problem, I mean IReadOnlyCollection isn't exactly shorter than IDeconstructable and my guess is it should be Length.

What is the recommended naming convention for tuple item names, specially in case of a partially named one? If (2, x: 3) is going to be used like t.Item1 and t.x it wouldn't be consistent.

My suggestion is don't use tuples in your public API and then you will save yourself a lot of headaches, if it's private or at least internal the convention isn't important but to you and your team, you can use whatever you feel is better and works for you.

I feel like tuples should have been designed with limited access and it was much better if they disallowed exposing them to the outside world, aka, other assemblies but I guess there's a good reason for it, although so far I haven't seen a good use-case for this.

@miloush
Copy link

miloush commented Aug 10, 2016

ITuple is really an interface for dynamically checking for deconstructability.

But (#11205):

Deconstruction should be specified with an instance (or extension) method. This is in keeping with other API patterns added throughout the history of C#, such as GetEnumerator, Add, and GetAwaiter. The benefit is that this leads to a relatively natural kind of member to have, and it can be specified with an extension method so that existing types can be augmented to be deconstructable outside of their own code.

Does it mean the dynamic checking for deconstructability scenario would work differently than how the compiler checks it? Wouldn't it make more sense to have something like Type.IsDeconstructible and Type.Deconstruct (although not sure how that would handle the extension method case)?

@gafter
Copy link
Member

gafter commented Aug 10, 2016

@miloush We do not have plans to make any changes to System.Type in support of the tuple feature.

@miloush
Copy link

miloush commented Aug 10, 2016

@gafter I see. And it's true that it is a language feature, not CLR feature, so that actually makes sense.

So ITuple would basically work like IEnumerable - you can detect explicit support for it in the type, but not match the compiler in duck-typing the actually needed method. That works for me, and it would appear to me that it should be IDeconstructible indeed. Unlike IEnumerable though, is there a reason why it enforces an indexer rather than a method?

@iam3yal
Copy link

iam3yal commented Aug 10, 2016

@miloush For the current design there's no need for CLR support but broadly speaking, it can easily be a feature that C# needs CLR support, it really depends on cons and pros of things and how you want to design it, therefor, it is a language feature by choice.

@gafter
Copy link
Member

gafter commented Aug 11, 2016

That works for me, and it would appear to me that it should be IDeconstructible indeed.

That would be a suitable name if its method were named Deconstruct, but that doesn't work.

@miloush
Copy link

miloush commented Aug 11, 2016

@gafter yes that was my concern, why is it realized through indexer rather than a Deconstruct method?

@alrz
Copy link
Member

alrz commented Aug 11, 2016

@gafter

You mean on the left-hand-side?

No, currently the following code produces three errors on master, namely, CS1525 twice and CS0150.

class C {
    bool M(object o) => o is (int, int); // tuple type
}

Is this reserved to be parsed as a tuple pattern?

@vbcodec
Copy link

vbcodec commented Aug 11, 2016

Current Deconstruct method can degrade performance, as it read all data regardless what is needed. Indexer (like in ITuple) could be faster, although multiple calls also may not be fast. Maybe better is to modify Deconstruct method to provide additional integer parameter as bitfield to specify which members are necessary..

@gafter
Copy link
Member

gafter commented Aug 11, 2016

@miloush There is no signature for a Deconstruct method that could be placed in the interface suitable for the deconstruction pattern.

@alrz Yes, the "is type" expression is not syntactically allowed to have a tuple type on the right-hand-side, as that is reserved for a tuple pattern.

@vbcodec Deconstruction is not specified to be linked to any "members". If you want some particular data members rather than the result of Deconstruct, use the . operator.

@miloush
Copy link

miloush commented Aug 11, 2016

@gafter Well, there is obviously one equivalent to the indexer, object Deconstruct(int i). What does indexer bring better? It seems to me it just limits what the only indexer a type can have is intended for.

EDIT: not if implemented explicitly indeed

@gafter
Copy link
Member

gafter commented Aug 11, 2016

@miloush Although your method is named Deconstruct, a method that obeys the deconstruct pattern (and therefore provides support for deconstruction on values of its static type) has a void return type and a series of out parameters for the individual values. So the method that you propose has nothing to do with deconstruction.

@miloush
Copy link

miloush commented Aug 11, 2016

@gafter I am aware of that, I am not claiming it should match the actual Deconstruct method. As a type owner, if I wanted my type to be deconstructible, I imagined it would be more natural for both approaches to have a Deconstruct method, rather than grouping a method with an indexer. I am nowhere strongly opinioned about this, I was just curious in case there was an obvious reason for the decision I was missing.

@DavidArno
Copy link

DavidArno commented Aug 17, 2016

People use this trick to block the use of var, and we should let them, even though we disagree with the practice. So even though a type isn't valid in that position, we will let the presence of a var type turn this into an error.

var was introduced with C# 3. The idea that you are still allowing these luddites to continue to fight progress with a new feature in v7 is depressing beyond words. What you should be doing is making the existance of a type called var a compilation error and screw backward-compatibility, because sometimes language features should not be compatible with those with backward thinking.

@markhurd
Copy link

@DavidArno Except that only some of the users of existing code with a type var are doing so to block its use. Others might be compiler or interpreter writers, for example, that quite validly had a var type pre-C# 3 :-)

@DavidArno
Copy link

@markhurd,

To a certain extent, my comment was tongue-in-cheek. But there is a serious side to it. C# 3 came out nine years ago. Quite frankly, if someone still has a class called var after all this time in their, eg compiler app, then they only have themselves to blame if C# 7 then broke their code. Pandering to such laziness is ridiculous.

@stepanbenes
Copy link

@DavidArno The point is that someone deliberately defines class named var to prevent using var as a placeholder for type in their codebase.

@alrz
Copy link
Member

alrz commented Aug 18, 2016

@stepanbenes I think that should be enforced via a code style analyzer, not an actual hack in the codebase.

@stepanbenes
Copy link

@alzr I agree. However, changing these rules for the new var-related features would be inconsistent.

@orthoxerox
Copy link
Contributor

@alrz I wouldn't trust people who think type inference is a dangerous newfangled frivolity to know how to use a style analyzer.

@Joe4evr
Copy link

Joe4evr commented Aug 21, 2016

@orthoxerox Just make the style analyzer a dependency of the project and they don't need to know how to use it. 😝

@jcouv
Copy link
Member

jcouv commented Jul 29, 2017

The LDM notes for July 12 2016 are now available in the csharplang repo: https://github.com/dotnet/csharplang/blob/master/meetings/2016/LDM-2016-07-12.md

I'll close the present issue. Thanks

@jcouv jcouv closed this as completed Jul 29, 2017
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