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

State that the set of ValueTuple types provided is implementation-defined #798

Closed
wants to merge 1 commit into from

Conversation

jskeet
Copy link
Contributor

@jskeet jskeet commented Jun 2, 2023

Part of #768.

@Nigel-Ecma I'm not sure this fully addresses your concern of "Do we need to say anything about how one implementation handles tuple values created by another with a different m?" from #664, but I couldn't think of a simple way of stating that. I'm hoping you'll agree that saying that the set of types available is implementation-defined implies that you shouldn't expect to be able to use code compiled for one implementation on another implementation, but other suggestions are welcome :)

@@ -389,7 +389,7 @@ An enumeration type is a distinct type with named constants. Every enumeration t

### 8.3.11 Tuple types

A tuple type represents an ordered, fixed-length sequence of values with optional names and individual types. The number of elements in a tuple type is referred to as its ***arity***. A tuple type is written `(T1 I1, ..., Tn In)` with n ≥ 2, where the identifiers `I1...In` are optional ***tuple element names***. This syntax is shorthand for a type constructed with the types `T1...Tn` from `System.ValueTuple<...>`, which shall be a set of generic struct types capable of expressing tuple types of any arity greater than one.
A tuple type represents an ordered, fixed-length sequence of values with optional names and individual types. The number of elements in a tuple type is referred to as its ***arity***. A tuple type is written `(T1 I1, ..., Tn In)` with n ≥ 2, where the identifiers `I1...In` are optional ***tuple element names***. This syntax is shorthand for a type constructed with the types `T1...Tn` from `System.ValueTuple<...>`, which shall be a set of generic struct types capable of expressing tuple types of any arity greater than one. The precise set of `System.ValueTuple<...>` types provided is implementation-defined.

> *Note*: There does not need to exist a `System.ValueTuple<...>` declaration that directly matches the arity of any tuple type with a corresponding number of type parameters. Instead, larger tuples can be represented with a special overload `System.ValueTuple<..., TRest>` that in addition to tuple elements has a `Rest` field containing a nested tuple value of the remaining elements. Such nesting may be observable in various ways, e.g. via the presence of a `Rest` field. *end note*
Copy link
Contributor

@KalleOlaviNiemitalo KalleOlaviNiemitalo Jun 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading this made me worry about possible conflicts in overloaded methods, if one method takes an (int, int, int, int, int, int, int, char, char) parameter and another overload takes (int, int, int, int, int, int, int, (char, char)) -- does the compiler translate both of those to ValueTuple<int, int, int, int, int, int, int, ValueTuple<char, char>>? But no, the latter becomes ValueTuple<int, int, int, int, int, int, int, ValueTuple<ValueTuple<char, char>>> instead. Here, TRest is a ValueTuple<T1> with only one type argument, and does not correspond to any C# tuple type. The wording "a nested tuple value" is a bit misleading, then, as it could be taken to mean an instance of a tuple type.

Perhaps "shall be a set of generic struct types capable of expressing tuple types of any arity greater than one" should also be changed to require ValueTuple<T1>, at least in those implementations that translate large-arity tuple types to System.ValueTuple<..., TRest>. If the implementation does generate a dedicated generic type for tuple types of each arity, instead of using TRest, then I suppose it need not support ValueTuple<T1>, because:

  • The non-generic struct ValueTuple and its public static ValueTuple<T1> Create\<T1>(T1 item1) method is not part of Annex C Standard library.
  • struct ValueTuple<T1> is part of Annex C Standard library, but its omission can be excused because "The precise set of System.ValueTuple<...> types provided is implementation-defined."

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@KalleOlaviNiemitalo – I see your point but rather than trying to improve the wording I would suggest a different tack.

The C# Standard does not specify a “wire format” for interoperability of binary data, or how arrays should be stored, and a myriad of other things. Why is it specifying how a tuple should be stored? Why even mention ValueTuple? Are these not implementation details of the kind the Standard doesn’t usually cover?

Rather than add a sentence to the end of the paragraph, and the following note, why not delete ”This syntax is shorthand for … arity greater than one” instead?

@jskeet jskeet added this to the C# 7.x milestone Jun 5, 2023
Copy link
Contributor

@Nigel-Ecma Nigel-Ecma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the translation from language-level tuples to structures built from ValueTuple is to be included, as implementation-defined or not, then the issue raise by @KalleOlaviNiemitalo needs to be addressed.

Now while I might like the idea in my response of classing the whole translation as an implementation issue, and not discussing it past something like “the underlying storage structure for tuples, like for arrays, is an implementation issue”, I expect that is not going to happen. That leaves addressing the 1 “tuples” to address, somehow…

@jskeet
Copy link
Contributor Author

jskeet commented Jul 12, 2023

Actions from meeting 2023-07-12:

  • @Nigel-Ecma to write a suggested paragraph that removes all reference to System.ValueTuple
  • @jskeet to write a suggested paragraph that addresses the womple concern

@jskeet
Copy link
Contributor Author

jskeet commented Jul 12, 2023

Alternative suggestion from @gafter: we take the standard in the other direction, specifying exactly what set of types we have.

@jskeet
Copy link
Contributor Author

jskeet commented Jul 12, 2023

... and now we're considering whether womples are actually tuple types despite not having syntax.

New proposal:

  • Specify the complete set of ValueTuple<...> types
  • Specify that womples exist, but cannot be written using tuple type or tuple literal syntax (do this bit properly!)

Thing to check: whether == is handled "appropriately" (in the compiler) by ValueTuple<T> and ValueTuple. If it is, we should describe these as tuple types.

@jskeet
Copy link
Contributor Author

jskeet commented Jul 12, 2023

Nope, it looks like ValueTuple and ValueTuple<T> are not tuple types in terms of ==:

ValueTuple<int> x = new ValueTuple<int>(5);
ValueTuple<int> y = new ValueTuple<int>(5);
// Operator '==' cannot be applied to operands of type 'ValueTuple<int>' and 'ValueTuple<int>'
Console.WriteLine(x == y);

ValueTuple v1 = new ValueTuple();
ValueTuple v2 = new ValueTuple();
// Operator '==' cannot be applied to operands of type 'ValueTuple' and 'ValueTuple'
Console.WriteLine(v1 == v2);

(We are somewhat surprised by that.)

@jskeet
Copy link
Contributor Author

jskeet commented Jul 12, 2023

@jskeet to try to create a new PR that specifies all System.ValueTuple types, and rewrites the representation to only go one way (to System.ValueTuple without coming back out in terms of "tuples types" for the continuation part).

(This will be in a separate PR.)

@jskeet jskeet self-assigned this Jul 12, 2023
jskeet added a commit to jskeet/csharpstandard that referenced this pull request Aug 7, 2023
jskeet added a commit to jskeet/csharpstandard that referenced this pull request Aug 7, 2023
@jskeet
Copy link
Contributor Author

jskeet commented Aug 7, 2023

Closing in favor of #876.

@jskeet jskeet closed this Aug 7, 2023
jskeet added a commit that referenced this pull request Aug 14, 2023
@jskeet jskeet deleted the tuple-encoding branch August 14, 2023 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants