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

[Proposal]: Dictionary expressions #7822

Open
1 of 4 tasks
CyrusNajmabadi opened this issue Jan 5, 2024 · 94 comments
Open
1 of 4 tasks

[Proposal]: Dictionary expressions #7822

CyrusNajmabadi opened this issue Jan 5, 2024 · 94 comments

Comments

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Jan 5, 2024

Dictionary Expressions

  • Proposed
  • Prototype: Not Started
  • Implementation: Not Started
  • Specification: Not Started

Summary

Collection Expressions were added in C# 12. They enabled a lightweight syntax [e1, e2, e3, .. c1] to create many types of linearly sequenced collections, with similar construction semantics to the existing [p1, p2, .. p3] pattern matching construct.

The original plan for collection expressions was for them to support "dictionary" types and values as well. However, that was pulled out of C# 12 for timing reasons. For C# 13 we would to bring back that support, with an initial suggested syntax of [k1: v1, k2: v2, .. d1]. As a simple example:

private readonly Dictionary<string, int> _nameToAge = [
    "Cyrus": 21,
    "Dustin": 22,
    "Mads": 23,
];

The expectation here would be that this support would closely track the design of collection expressions, with just additions to that specification to support this k:v element syntax, and to support dictionary-like types as the targets of these expressions.

Motivation

  • Collection-like values are hugely present in programming, algorithms, and especially in the C#/.NET ecosystem. Nearly all programs will utilize these values to store data and send or receive data from other components. Currently, almost all C# programs must use many different and unfortunately verbose approaches to create instances of such values.

  • An examination of the ecosystem (public repos, nuget packages, and private codebases we have access to), indicate that dictionaries are used as a collection type in APIs around 15% of the time. While not nearly as ever-present as the sequenced collections (like arrays, spans, lists, and so on), this is still substantial, and warrants the equivalently pleasant construction provided by collection-expressions.

  • Like with linear collections, there are numerous different types of dictionary-like types in the .net ecosystem. This includes, but is not limited to, simply constructed dictionaries like Dictionary<TKey, TValue> and ConcurrentDictionary<TKey, TValue>, but also interfaces like IDictionary<TKey, TValue> and IReadOnlyDictionary<TKey, TValue>, as well as immutable versions like `ImmutableDictionary<TKey, TValue>. Supporting all these common forms is a goal for these expressions.

Detailed design

Main specification here: https://github.com/dotnet/csharplang/blob/main/proposals/collection-expressions-next.md

The only grammar change to support these dictionary expressions is:
 

collection_literal_element
  : expression_element
+ | dictionary_element
  | spread_element
  ;

+ dictionary_element
  : expression ':' expression
  ;

Spec clarifications

  • dictionary_element instances will commonly be referred to as k1: v1, k_n: v_n, etc.

Conversions

The following implicit collection literal conversions exist from a collection literal expression:

  • ...

  • To a type that implements System.Collections.IDictionary where:

    • The type contains an applicable instance constructor that can be invoked with no arguments or invoked with a single argument for the 0-th parameter where the parameter has type System.Int32 and name capacity.
    • For each expression element Ei:
      • the type of Ei is dynamic and there is an applicable indexer setter that can be invoked with two dynamic arguments, or
      • the type of Ei is a type System.Collections.Generic.KeyValuePair<Ki, Vi> and there is an applicable indexer setter that can be invoked with two arguments of types Ki and Vi.
    • For each dictionary element Ki:Vi, there is an applicable indexer setter that can be invoked with two arguments of types Ki and Vi.
    • For each spread element Si:
      • the iteration type of Si is dynamic and there is an applicable indexer setter that can be invoked with two dynamic arguments, or
      • the iteration type is System.Collections.Generic.KeyValuePair<Ki, Vi> and there is an applicable indexer setter that can be invoked with two arguments of types Ki and Vi.
  • To an interface type I<K, V> where System.Collections.Generic.Dictionary<TKey, TValue> implements I<TKey, TValue> and where:

    • For each expression element Ei, the type of Ei is dynamic, or the type of Ei is a type System.Collections.Generic.KeyValuePair<Ki, Vi> and there is an implicit conversion from Ki to K and from Vi to V.
    • For each dictionary element Ki:Vi there is an implicit conversion from Ki to K and from Vi to V.
    • For each spread element Si, the iteration type of Si is dynamic, or the iteration type is System.Collections.Generic.KeyValuePair<Ki, Vi> and there is an implicit conversion from Ki to K and from Vi to V.

Syntax ambiguities

  • dictionary_element can be ambiguous with a conditional_expression. For example:

    var v = [ a ? [b] : c ];

    This could be interpreted as expression_element where the expression is a conditional_expression (e.g. [ (a ? [b] : c) ]). Or it could be interpreted as a dictionary_element "k: v" where a?[b] is k, and c is v.

Alternative designs

Several discussions on this topic have indicated a lot of interest and enthusiasm around exploring how close this feature is syntactically (not semantically) to JSON. Specifically, while we are choosing [k: v] for dictionary literals, JSON chooses { "key": value }. As "key" is already a legal C# expression, this means that [ "key": value ] would be nearly identical to JSON (except for the use of brackets instead of braces). While it would make it so we would have two syntaxes for collection versus dictionary expressions, we should examine this space to determine if the benefits we could get here would make up for the drawbacks.

Specifically, instead of reusing the collection_expression grammar, we would introduce:

primary_no_array_creation_expression
  ...
+ | dictionary_expression
  ;

+ dictionary_expression
  : '{' '}'
  | '{' dictionary_element ( ',' dictionary_element )* '}'
  ;

+ dictionary_element
  : expression_element
  | dictionary_element
  | spread_element
  ;

+ dictionary_element
  : expression ':' expression
  ;

You could then write code like:

private readonly Dictionary<string, int> _nameToAge = {
    "Cyrus": 21,
    "Dustin": 22,
    "Mads": 23,
};

Or:

Dictionary<int, string> combined = { .. ageToName1, .. ageToName2 };

Or

Dictionary<int, string> combined = { kvp1, key2: value2, .. ageToName1, .. ageToName2 };

The downside here is:

  1. The potential ambiguity with existing constructs.
  2. The potential conflicts with future constructs.
  3. The challenge in creating a corresponding pattern form.

In order:

First, the above syntax already conflicts with int[] a = { 1, 2, 3, 4 } (an array initializer). However, we could trivially sidestep this by saying that if the initializer contained a spread_element or dictionary_element it was definitely a dictionary. If it did not (it only contains normal expressions, or is empty), then it will be interpreted depending on what the target type is.

Second, this could definitely impact future work wanted in the language. For example, "block expressions" has long been something we've considered. Where one could have an expression-valued "block" that could allow statements and other complex constructs to be used where an expression is needed. That said, such future work is both speculative, and itself could take on some new syntax. For example, we could mandate that an expression block have syntax like @{ ... }.

Third, the pattern form here presents probably the hardest challenges. We would very much like patterns and construction to have a syntactic symmetry (with patterns in the places of expressions). Such symmetry would motivate having a pattern syntax of { p1: p2 }. However, this is already completely the completely legal pattern syntax for a property pattern. In other words, one can already write if (d is { Length: 0 }). Indeed, it was all the ambiguities with { ... } patterns in the first place that motivated us to use [ ... ] for list patterns (and then collection-expressions). We will end up having to resolve these all again if we were to want this to work. It is potentially possible, but will likely be quite subtle with various pitfalls. Alternatively, we can come up with some new syntax for dictionary-patterns, but we would then break our symmetry goal.

Regardless, even with the potential challenges above, there is a great attractiveness on aligning syntactically with JSON. This would make copying JSON into C# (particularly with usages in API like Json.net and System.Text.Json) potentially no work at all. However, we may decide the drawbacks are too high, and that it is better to use the original syntactic form provided in this specification.

That said, even the original syntactic form is not without its own drawbacks. Specifically, if we use [ ... ] (just with a new element type, then we will likely have to address ambiguities here when we get to the "natural type" question.

For example:

var v1 = []; // What if the user wants a dictionary?
var v2 = [.. dict1, .. dict2]; // should combining dictionaries produce a dictionary?

and so on.

Open Questions

  1. Should dictionary expressions have "Add" semantics or "Overwrite semantics". The working group is leaning toward the latter, specifically so you can write code like:
Dictionary<TKey, TValue> updated = [.. initialValues, k: v]; //or
Dictionary<TKey, TValue> merged = [.. d1, .. d2]; //or
Dictionary<TKey, TValue> overwritten = [k1: v1, k2: v2, .. augments]; // etc.

The space of allowing merging, with "last one wins" seems pretty reasonable to us. However, we want a read if people would prefer that throw, or if there should be different semantics for dictionaries without spreads for example.

  1. Unlike linear-collections, augmenting a dictionary is a much more common practice. For example, providing a suitable IEqualityComparer. Should that be something supported somehow in this syntax. Or would/should that require falling back out to a normal instantiation?

Design Meetings

@KennethHoff
Copy link

As noted by open question 2, I think losing support for IEqualityComparer - especially for string dictionaries - would be very unfortunate. For me personally equality comparisons is one of the primary reasons I use dictionaries over other data structures.

@HaloFour
Copy link
Contributor

HaloFour commented Jan 6, 2024

Would an extension method not suffice here?

var dictionary = [ "foo": "bar", "fizz": "buzz" ].WithComparer(StringComparer.OrdinalIgnoreCase);

// or

var dictionary  = StringComparer.OrdinalIgnoreCase.CreateDictionary([ "foo": "bar", "fizz": "buzz" ]);

@GabeSchaffer
Copy link

Would an extension method approach to specifying comparers be able work without creating a second dictionary?

@CyrusNajmabadi
Copy link
Member Author

@GabeSchaffer Likely yes (though we would still need to get extensions working with collection exprs). Specifically, the keys/values in the expr woudl likely be passed to the extension method as a ReadOnlySpan of KeyValuePairs. These would normally just be on the stack, and would be given as a contiguous chunk of data for the dictionary to create its internal storage from once.

@GabeSchaffer
Copy link

Apologies for my lack of understanding, but it seems like a builder approach to augmentation could be made to work, like with String.Create.

That seems like it could make a syntax like this possible:

var dictionary = Dictionary.Create(StringComparer.OrdinalIgnoreCase, [ "foo": "bar", "fizz": "buzz" ]);

Another options that seems feasible is to allow constructor arguments to be specified in parens after the collection expression:

[ "foo": "bar", "fizz": "buzz" ](StringComparer.OrdinalIgnoreCase) // function call syntax for passing ctor args

An uglier possibility is to use a semicolon:

[ "foo": "bar", "fizz": "buzz"; StringComparer.OrdinalIgnoreCase ] // ; delimits comparer

I think if we want to have hope of being able to specify a comparer in a pattern, the latter two are better.

@Perksey
Copy link
Member

Perksey commented Jan 9, 2024

Wow I had no idea the C# team were so young ;)

private readonly Dictionary<string, int> _nameToAge = [
    "Cyrus": 21,
    "Dustin": 22,
    "Mads": 23,
];

@hez2010
Copy link

hez2010 commented Jan 12, 2024

How about reusing the existing dictionary syntax?

Given that we already have

var dict = new Dictionary<K, V> { [key] = value }

We can somehow use a similar syntax:

var dict = { [key]: value }

This is also matching how TypeScript defines an interface with indexed field:

interface Foo {
    [key: string]: number;
}

And we can even support dictionary patterns along with existing patterns as well:

var obj = new C();

if (obj is {
    [string key]: 42,
    ["foo"]: int value,
    SomeProp: float x
}) ... // matching an object which has a indexed value 42 while its key is string, and a key "foo" whose value is int

class C
{
    public float SomeProp { get; }
    public int this[string arg] => ...
}

As well as the case where the indexer has multiple parameters:

if (obj is {
    ["foo", "bar"]: 42
})

@TahirAhmadov
Copy link

Specifically, the keys/values in the expr woudl likely be passed to the extension method as a ReadOnlySpan of KeyValuePairs.

What about spreads, those will have to be iterated and placed on the stack before calling the extension method? Or is there a way to pass them in somehow?

@GabeSchaffer
Copy link

Specifically, the keys/values in the expr woudl likely be passed to the extension method as a ReadOnlySpan of KeyValuePairs.

What about spreads, those will have to be iterated and placed on the stack before calling the extension method? Or is there a way to pass them in somehow?

I don't see how that can be implemented in any way other than to copy every element into a contiguous array in order to create a ReadOnlySpan. Because if the elements aren't contiguous, it's not a span, right?

@mpawelski
Copy link

@hez2010

How about reusing the existing dictionary syntax?

Given that we already have

var dict = new Dictionary<K, V> { [key] = value }

We can somehow use a similar syntax:

var dict = { [key]: value }

I think dictionary-like objects that have only one indexer are so common that it's worth to consider special, more succinct, syntax that doesn't require unnecessary [ and ]. Such as the one proposed in this thread.

Actually, your proposed syntax is not far from what we can do now in C#:

Dictionary<string, int> dict = new() { 
    ["aaa"] = 1, 
    ["bbb"] = 1
};

And we can even support dictionary patterns along with existing patterns as well:

var obj = new C();

if (obj is {
    [string key]: 42,
    ["foo"]: int value,
    SomeProp: float x
}) ... // matching an object which has a indexed value 42 while its key is string, and a key "foo" whose value is int

class C
{
    public float SomeProp { get; }
    public int this[string arg] => ...
}

As well as the case where the indexer has multiple parameters:

if (obj is {
    ["foo", "bar"]: 42
})

Actually I was kinda surprised that C# doesn't have pattern matching for indexers. This code doesn't work:

Dictionary<string, int> dict = new() { 
    ["aaa"] = 1, 
    ["bbb"] = 2
};

if (dict is { ["aaa"] : 1 })
{
}

But this feels looks like another "pattern matching improvements" feature, not strictly related to dictionary expressions we discuss in this thread.

@somecodingwitch
Copy link

I suggest using object notation, like TypeScript.

Like:

{
    Type = "Person",
    Name = "John"
}

@TahirAhmadov
Copy link

TahirAhmadov commented Jan 31, 2024

Can we make the "add/overwrite" be switchable? Say, we default to the safe "add" approach, and if overwrite is desired:

var d = [(k1: v1)!, (..d1)!];

Here, ! means "overwrite".
Frankly, I'm not crazy about this syntax, but I couldn't come up with anything better at the moment (admittedly, I thought about it for a few minutes only).
However, syntax aside, I'd still like to raise this as a suggestion.

PS. In addition to the above, if it should be "overwrite" for the whole list:

var d = ([k1: v1, ..d1])!;

@colejohnson66
Copy link

That syntax looks a bit weird. If an "or overwrite" operation was added, I'd prefer to augment the with syntax:

IDictionary<string, string> d = GetDictionary();
(string k, string v) = GetNewKVP();
IDictionary<string, string> d2 = d with { [k1]: v1 };

But maybe that would be confusing, as one would have to remember that spreading is adding and with is overwrite (or add if missing).

@TahirAhmadov
Copy link

@colejohnson66 with your approach, you would always have to create a temp dictionary variable to then "overwrite" things onto it to produce the final desired result, which effectively changes it from a "collection expression" to a "procedural code block".

@alrz
Copy link
Contributor

alrz commented Feb 1, 2024

You can do this today

Dictionary<string, int> _nameToAge = new(OptionalComparer) {
    ["Cyrus"] = 21,
    ["Dustin"] = 22,
    ["Mads"] = 23,
};

or just [new(key ,value)] given an extension Add(this Dictionary, KeyValuePair).

I'd rather see dictionary/indexer patterns to complete the matching side of the existing [e]=v initialization syntax. Meanwhile, I think stronger type inference could help a lot within the existing syntax for more concise maps.

var x = new Dictionary() { .. };

There's endless optimization possibilities for collection expressions, not so much with dictionaries, and spreads with maps are just confusing or so rare at best.

Having said that, I think immutable dictionaries could use some less awkward initialization API, if possible.

@Ai-N3rd
Copy link
Contributor

Ai-N3rd commented Feb 22, 2024

I would like to add my take on the IEqualtiyComparer argument:

var dict = 
(new SomeEqualityComparer())
[
    // Whatever syntax we decide on here
    ["Cyrus"] = 21,
    ["Dustin"] = 22,
    ["Mads"] = 23,
]

This could also work for constructor parameters on other types, if that is found necessary.

@glen-84
Copy link

glen-84 commented Feb 25, 2024

What about using the JSON-like syntax for declaration:

private readonly Dictionary<string, int> _nameToAge =
{
    "Cyrus": 21,
    "Dustin": 22,
    "Mads": 23
};

But for pattern matching, use the indexer syntax suggested by @hez2010:

// Match a KV pair in the dictionary.
if (_nameToAge is { ["Cyrus": 69] })

// Match a property of the dictionary.
if (_nameToAge is { Length: 0 })

@KennethHoff
Copy link

@glen-84 One of the goals of collection expressions were parity with pattern matching, so this (hopefully) won't happen.

@glen-84
Copy link

glen-84 commented Feb 25, 2024

This is probably as close to parity as you'll get when going with this JSON-like syntax. I think it's intuitive – you're matching elements of the dictionary, not the dictionary itself.

Otherwise it's back to option 1. 🙂

@Timovzl
Copy link

Timovzl commented Feb 27, 2024

At least when it comes to expressions for creating a dictionary, in order to avoid the confusion of having too many options (especially ones that are almost the same), I think it would help to stay close to this existing dictionary initializer syntax:

new Dictionary<string, int>()
{
   ["Cyrus"] = 21,
   ["Dustin"] = 22,
   ["Mads"] = 23,
};

For example:

[
   ["Cyrus"] = 21,
   ["Dustin"] = 22,
   ["Mads"] = 23,
]

This avoids the confusion of yet another style, and it separates key and value more clearly than the other existing collection initializer syntax:

new Dictionary<string, int>()
{
   // Meh for complex keys/values, especially when their expressions have commas and braces of their own
   { 111, new StudentName() { FirstName="Sachin", LastName="Karnik", ID=211 } },
   { 112, new StudentName() { FirstName="Dina", LastName="Salimzianova", ID=317 } },
   { 113, new StudentName() { FirstName="Andy", LastName="Ruth", ID=198 } },
};

@TahirAhmadov
Copy link

For example:

[
   ["Cyrus"] = 21,
   ["Dustin"] = 22,
   ["Mads"] = 23,
]

This adds a lot of annoying extra characters to type, and tries to address a non-existent goal - to maintain similarity to existing approaches. Collection expressions (including dictionary expressions) are meant to come up with a standardized new syntax for all (most) collections, not try to continue older approaches. With respect to the syntax above, you can already do:

new()
{
   ["Cyrus"] = 21,
   ["Dustin"] = 22,
   ["Mads"] = 23,
};

@colejohnson66
Copy link

Well, it's the same thing for lists and arrays. I can also currently do:

new() {1, 2, 3} // generates whatever collection target type is
new[] {1, 2, 3} // generates an array

But collection expressions were still added.

@Ai-N3rd
Copy link
Contributor

Ai-N3rd commented Feb 27, 2024

Just clarifying, which of these would we allow in pattern matching:

[
    // Type validation and accessing by value
    [string name] = 21,
    // Discarding
    [var _] = 22,
    // Accessing unknown values
    ["Mads"] = var age
]

@TahirAhmadov
Copy link

But collection expressions were still added.

It saves typing even in the simple [1, 2, 3] scenario, but collection expressions also support spreads, which the old style syntaxes don't. Also, the language needs the { } for other constructs, and it's better to use [ ] for collections. Plus ability to target type interfaces, etc.
However, for dictionaries specifically, the ["Cyrus"] = 21 style of element is a good amount more ceremony per element than "Cyrus": 21, and that difference doesn't exist for regular collections. That's the argument I was making - why add more typing to new syntax, just to maintain similarity with older syntax which is explicitly being (soft-)replaced with the new syntax?

@Ai-N3rd
Copy link
Contributor

Ai-N3rd commented Feb 27, 2024

@TahirAhmadov

Collection expressions (including dictionary expressions) are meant to come up with a standardized new syntax for all (most) collections, not try to continue older approaches.

This syntax is well known. We can create a standard without reinventing the wheel. It's not a rule that we cant reuse old syntax.

@TahirAhmadov
Copy link

TahirAhmadov commented Feb 27, 2024

This syntax is well known. We can create a standard without reinventing the wheel.

Personally, I never use the ["Cyrus"] = 21 "overwrite" syntax in my existing code specifically to keep my code safer - I prefer the Add semantics.
However, my personal preferences aside, the new "Cyrus": 21 syntax is not a difficult thing to learn in terms of introducing new syntax, and it is so intuitive, I'm frankly surprised by the amount of debate about it.
Also, I need to repeat - the new collection and dictionary expressions are meant to replace existing initializations, so it doesn't really matter that the old syntax is well known - think of it as something to forget going forward, really.

It's not a rule that we cant reuse old syntax.

There is no rule that we can't reuse old syntax, but there is zero reason to do so, and the old syntax is more typing. So, if it's worse than the proposed new one, and we are under no obligation to reuse old syntax, why reuse it?

@TahirAhmadov
Copy link

TahirAhmadov commented Feb 27, 2024

Another thought - perhaps the ["Cyrus"] = 21 syntax can be used going forward, to explicitly switch to the "overwrite" mode? Basically, ["Cyrus": 21, ["Cyrus"] = 22] would not throw and instead have 22 for "Cyrus", as opposed to ["Cyrus": 21, "Cyrus": 22], which would throw.
PS. However this is probably a bad approach, because it doesn't address the question of switching a spread to "overwrite" semantics, and doesn't allow switching the whole dictionary expression to "overwrite", either.

@Ai-N3rd
Copy link
Contributor

Ai-N3rd commented Feb 27, 2024

@TahirAhmadov What makes it worse? If you have autocomplete, it's 1 extra character you have to type, 2 without. It's not the end of the world. It also conveys how it really works with the override semantics, which is far more useful than saving typing. And again, it doesn't have to replace the current to create a standard. We do not have to reinvent the wheel. ["Cyrus"] = 21 is just as intuitive.

@TahirAhmadov
Copy link

@Ai-N3rd hmm that's incorrect. "Cyrus": 21 is one symbol - :. ["Cyrus"] = 21 is 3 symbols - [, ], and =. And auto-complete doesn't help here, because these are symbols - it would be very difficult for an IDE to predict when your key expression ends, at best it could suggest = for you after ], but even then you'd have to press Tab to accept it, so not really an improvement. So either way you look at it, it's 3 keystrokes per element instead of 1, multiply by say, 10 KVPs, and you're talking about 20 extra keystrokes for no good reason.

@mikernet
Copy link

Ah I see. Maybe problematic then, idk.

@HaloFour
Copy link
Contributor

With natural expressions and extension methods, why not just support a handful of helper methods?

var dict = ["a": 1, "b": 2].WithComparer(comparer); // name could be anything, and as short as you'd like.

@jnm2
Copy link
Contributor

jnm2 commented May 29, 2024

@HaloFour With an initial this IEnumerable<KeyValuePair<...>> parameter on the extension method, that should work great, assuming extension methods are eventually allowed.

@Mrxx99
Copy link
Contributor

Mrxx99 commented May 29, 2024

With natural expressions and extension methods, why not just support a handful of helper methods?

var dict = ["a": 1, "b": 2].WithComparer(comparer); // name could be anything, and as short as you'd like.

This would change behavior, as the comparer on Dictionary can currently only be set via constructor and the constructor taking KeyValuePairs and a Comparer does check the comparer while adding.

@HaloFour
Copy link
Contributor

@Mrxx99

This would change behavior, as the comparer on Dictionary can currently only be set via constructor and the constructor taking KeyValuePairs and a Comparer does check the comparer while adding.

I'm not suggesting that the extension method apply to Dictionary<TKey, TValue> and somehow affect the comparer after the fact, I'm suggesting that the extension method accept IEnumerable<KeyValuePair<TKey, TValue>> or ReadOnlySpan<KeyValuePair<TKey, TValue>> and it would be responsible for creating that dictionary with the comparer:

public static Dictionary<TKey, TValue> WithComparer<TKey, TValue>(
    this IEnumerable<KeyValuePair<TKey, TValue>> entries,
    IEqualityComparer<TKey> comparer) =>
        new Dictionary<TKey, TValue>(entries, comparer);

@jnm2
Copy link
Contributor

jnm2 commented May 29, 2024

Also, when dictionary expressions exist, this would become ambiguous:

[expr ? [constName: "b"] (comparer) : expr2]

That's already going to be true of this form too though, and we have a resolution for it:
[expr ? ["b"] : expr2]

@julealgon
Copy link

julealgon commented May 29, 2024

@Mrxx99

With natural expressions and extension methods, why not just support a handful of helper methods?

var dict = ["a": 1, "b": 2].WithComparer(comparer); // name could be anything, and as short as you'd like.

The method already exists:

Might as well just reuse it then:

var dict = ["a": 1, "b": 2].ToDictionary(comparer);

@HaloFour
Copy link
Contributor

@julealgon

The method already exists:
Might as well just reuse it then:

I believe the work to enable extension methods on collection literals would enable that method to be used. Hopefully a version that accepts ReadOnlySpan<KeyValuePair<TKey, TValue>> could also be exposed to reduce allocations and the cost of enumeration.

@GabeSchaffer
Copy link

With natural expressions and extension methods, why not just support a handful of helper methods?

var dict = ["a": 1, "b": 2].WithComparer(comparer); // name could be anything, and as short as you'd like.

@HaloFour my concern with the extension method approach is that it seems like the elements would have to be copied into a holding collection that provides the IEnumerable and then copied into the dictionary, while a standard dictionary doesn't need a holding collection.

In other words, I'd like to be able to add a comparer to the dictionary without a loss in efficiency. Do you see a way that can happen?

@HaloFour
Copy link
Contributor

@GabeSchaffer

my concern with the extension method approach is that it seems like the elements would have to be copied into a holding collection that provides the IEnumerable and then copied into the dictionary, while a standard dictionary doesn't need a holding collection.

I believe the team intends on supporting extension methods that accept Span<T>/ReadOnlySpan<T> so that at worst it would require a temporary stack allocation:

See: #7622 (reply in thread)

@mikernet
Copy link

mikernet commented May 29, 2024

Which, I think it will need anyway, no? It will ultimately be calling a collection initializer that takes a ROSpan<> input in any case I believe, at least in the general case. The compiler can specialize for Dictionary, but arbitrary dictionary-like collection types will only be required to provide a collection initializer method that accepts ROSpan, similar to regular collection literal expressions.

@GabeSchaffer
Copy link

my concern with the extension method approach is that it seems like the elements would have to be copied into a holding collection that provides the IEnumerable and then copied into the dictionary, while a standard dictionary doesn't need a holding collection.

I believe the team intends on supporting extension methods that accept Span<T>/ReadOnlySpan<T> so that at worst it would require a temporary stack allocation:

See: #7622 (reply in thread)

@HaloFour
It seems like that would be fine for ["a":1, "b": 2].ToDictionary(comparer) but how would that work for ["a":1, ..x, "b":2, ..y].ToDictionary(comparer)?

@HaloFour
Copy link
Contributor

@GabeSchaffer

but how would that work for ["a":1, ..x, "b":2, ..y].ToDictionary(comparer)?

That's a good question and I imagine that it's part of the conversation. I would expect that the buffer would be fully materialized before the extension method is invoked.

@jnm2
Copy link
Contributor

jnm2 commented May 30, 2024

but how would that work for ["a":1, ..x, "b":2, ..y].ToDictionary(comparer)

It would work as it currently does for regular collection literals with spreads, using an array instead of stack allocating (via inline array). There could be more nuance in the future to decide between stack allocation and arrays.

@GabeSchaffer
Copy link

but how would that work for ["a":1, ..x, "b":2, ..y].ToDictionary(comparer)

It would work as it currently does for regular collection literals with spreads, using an array instead of stack allocating (via inline array). There could be more nuance in the future to decide between stack allocation and arrays.

Currently T a = [1, ..x, 2, ..y]; is implemented like this:

T a = new T();
a.Add(1);
foreach (var temp in x)
    a.Add(temp);
a.Add(2);
foreach (var temp in y)
    a.Add(temp);

Ideally T a = [1, ..x, 2, ..y].With(comparer); is implemented like this:

T a = new T(comparer);
a.Add(1);
foreach (var temp in x)
    a.Add(temp);
a.Add(2);
foreach (var temp in y)
    a.Add(temp);
// or
TBuilder a_builder = new TBuilder(comparer);
a_builder.Add(1);
foreach (var temp in x)
    a_builder.Add(temp);
a_builder.Add(2);
foreach (var temp in y)
    a_builder.Add(temp);
T a = a_builder.AsT();

and not like this:

List<int> a_builder = new List<int>();
a_builder.Add(1);
foreach (var temp in x)
    a_builder.Add(temp);
a_builder.Add(2);
foreach (var temp in y)
    a_builder.Add(temp);
T a = new T(a, comparer);

@HaloFour
Copy link
Contributor

Here's some really uncooked spaghetti, but what if that natural expression type could support some kind of empty generic struct that is used specifically to allow the extension method to return an empty collection into which the items would then be added?

public static Dictionary<T, TKey> WithComparer<T, TKey>(
    this CollectionLiteral<KeyValuePair<TKey, TValue>> literal,
    IEqualityComparer<TKey> comparer)
{
    return new Dictionary<T, TKey>(comparer);
}


// then
var dict = ["a": 1, "b": 2, ..others].WithComparer(StringComparer.CurrentCultureIgnoreCase);

// lowers (very roughly) into
var dict = DictionaryExtensions.WithComparer<string, int>(
    default(CollectionLiteral<KeyValuePair<string, int>>),
    StringComparer.CurrentCultureIgnoreCase);
dict["a"] = 1;
dict["b"] = 2;
foreach (var item in others) {
    dict[item.Key] = item.Value;
}

@julealgon
Copy link

Here's some really uncooked spaghetti, but what if that natural expression type could support some kind of empty generic struct that is used specifically to allow the extension method to return an empty collection into which the items would then be added?

public static Dictionary<T, TKey> WithComparer<T, TKey>(
    this CollectionLiteral<KeyValuePair<TKey, TValue>> literal,
    IEqualityComparer<TKey> comparer)
{
    return new Dictionary<T, TKey>(comparer);
}


// then
var dict = ["a": 1, "b": 2, ..others].WithComparer(StringComparer.CurrentCultureIgnoreCase);

// lowers (very roughly) into
var dict = DictionaryExtensions.WithComparer<string, int>(
    default(CollectionLiteral<KeyValuePair<string, int>>),
    StringComparer.CurrentCultureIgnoreCase);
dict["a"] = 1;
dict["b"] = 2;
foreach (var item in others) {
    dict[item.Key] = item.Value;
}

Using extension methods (or extension methods syntax) for this feels misleading to me though... When using normal extension methods, one expects that it will apply "to the result of the expression to the left". Yet, here we are proposing some magical change of the order of operations, and the extension now applies to something that exist before the actual expression is computed.

What if now I want to create an extension that applies after the collection is created? I'd use the explicit interface of the final collection instead of this custom CollectionLiteral type. That feels incredibly weird to me.

Not a fan of that aspect at all. I'd rather have custom syntax at this point to avoid the confusion in behavior and special casing something that already exists.

@GabeSchaffer
Copy link

Here's some really uncooked spaghetti, but what if that natural expression type could support some kind of empty generic struct that is used specifically to allow the extension method to return an empty collection into which the items would then be added?

I think I prefer something akin to InterpolatedStringHandler. My suggestion is for something like this:

[CollectionExpressionHandler]
public ref struct DictionaryExpressionHandler<TKey, TValue>
{
    Dictionary<TKey, TValue> builder;
    public DictionaryExpressionHandler(IEqualityComparer<TKey> comparer) => builder = new(comparer);
    public void Add(KeyValuePair<TKey, TValue> pair) => builder[pair.Key] = pair.Value;
    internal Dictionary<TKey, TValue> GetCollection() => builder;
}

public static Dictionary<TKey, TValue> WithComparer<TKey, TValue>(
    this [CollectionExpressionHandlerArgument("comparer")] DictionaryExpressionHandler<TKey, TValue> handler,
    StringComparer comparer) => handler.GetCollection();

// then
var dict = ["a": 1, "b": 2, ..others].WithComparer(StringComparer.CurrentCultureIgnoreCase);

// would lower to something like this:
var handler = new DictionaryExpressionHandler<string, int>(StringComparer.CurrentCultureIgnoreCase);
handler.Add(new KeyValuePair<string, int>("a", 1));
handler.Add(new KeyValuePair<string, int>("b", 2));
foreach (var pair in others)
    hander.Add(pair);
var dict = handler.WithComparer(StringComparer.CurrentCultureIgnoreCase);

@BreyerW
Copy link

BreyerW commented Jun 2, 2024

From what i see dictionary already has copy constructors with comparer variant will these be optimized whenever used directly with dict expressions? Like:

var dict = new Dictionary<string, string>(["prop1" : "value", "prop2" : "value"], comparer);

If yes you could use real alias or implicit extensions as fake alias to make it decently short and still equally performant to hand-written code. Although this has a downside of generics not being inferred and either requiring them to be spelled on each use site or making 1 alias per generic combination used. The upside is there no new pattern since it uses preexisting copy constructors

@Timovzl
Copy link

Timovzl commented Jun 4, 2024

This one seems to have gotten snowed under by new suggestions, but it was quite intuitive and seemed to attract no opposition:

Dictionary<string,string> d1 = [new: (comparer), "a": "b"];
FrozenDictionary<string,string> d2 = [new: (comparer, true), "a": "b"];
// maybe use "init" so that it's clear we're not calling the collection's ctor?
FrozenDictionary<string,string> d2 = [init: (comparer, true), "a": "b"]; 

It also features the nice property of reducing cognitive load by putting the constructor parameters first. Before we get to the meat, which could span many lines, it gets the collection's configuration out of the way. (And that configuration may well determine how we read the keys, e.g. whether the presence of both "a" and "A" sets off alarm bells in our heads.)

Conversely, imagine code reviewing an initializer without this property, needing to scrutinize the elements, all the while juggling in your head the thought of "shouldn't this need a case-insensitive comparer"...

@julealgon
Copy link

@Timovzl

...but it was quite intuitive and seemed to attract no opposition

I'm opposed to it on the grounds that it is special cased to collection literals.

I don't see a need for the syntax to be exclusive to only collection literals. It makes the language unnecessarily complex by introducing lots of equivalent constructs that are not orthogonal later when similar needs are seen with other types of literals.

A simple modification would allow for a more reusable syntax:

Dictionary<string,string> d1 = new: (comparer) ["a": "b"];
FrozenDictionary<string,string> d2 = new: (comparer, true) ["a": "b"];
// maybe use "init" so that it's clear we're not calling the collection's ctor?
FrozenDictionary<string,string> d2 = init: (comparer, true) ["a": "b"]; 

@CyrusNajmabadi
Copy link
Member Author

I'm opposed to it on the grounds that it is special cased to collection literals.

Collection literals are not trying to create composable syntax for all features. In a very real sense, a major pillar of them is that they are not trying to be built out of orthogonal concepts (which generally need to be much more verbose to meet generalized needs). Collectoin literals realize that collections are important enough to warrant specialized syntax for the collections-domain.

@julealgon
Copy link

@CyrusNajmabadi

I'm opposed to it on the grounds that it is special cased to collection literals.

Collection literals are not trying to create composable syntax for all features. In a very real sense, a major pillar of them is that they are not trying to be built out of orthogonal concepts (which generally need to be much more verbose to meet generalized needs). Collectoin literals realize that collections are important enough to warrant specialized syntax for the collections-domain.

I don't quite follow how a major pillar in the design is to actively avoid using orthogonal syntax... that's just crazy to me, but fair enough.

Given two equally expressive and intuitive syntaxes, one that is orthogonal/general purpose, and the other that is completely coupled to collection literals only, I fail to see the appeal of picking the latter. To me, that's just shooting oneself in the foot as you just purposefully put yourself in a corner that will force you to come up with yet another initialization syntax later down the line making the language worse to digest.

Specifically with the example from @TahirAhmadov above, is my suggestion that less intuitive or harder to use? It surely enough feels basically identical to me, it just moves some characters out of the [...] while preserving everything else: one is tied to collection expressions and the other isn't.

Specialized syntax should only win if it really is vastly more intuitive/simple/easy to use than the general purpose one IMHO, otherwise there is just no reason to go with it. Yet, you are apparently locked in the idea that it will be specific... even though it hasn't been defined yet.

@CyrusNajmabadi
Copy link
Member Author

I don't quite follow how a major pillar in the design is to actively avoid using orthogonal syntax

A core virtue here is brevity. We already have verbose options available to us today:-)

I fail to see the appeal of picking the latter.

Only if they are truly equal on every other dimension. That remains to be seen. A better syntax, for collections only, would likely still be preferred.

is my suggestion that less intuitive or harder to use

To me, it is. Especially given that we already have a new(...) form. Now we'd have new:(...) form as well. This is not a positive for me.

@julealgon
Copy link

I fail to see the appeal of picking the latter.

Only if they are truly equal on every other dimension. That remains to be seen. A better syntax, for collections only, would likely still be preferred.

Fair.

is my suggestion that less intuitive or harder to use

To me, it is. Especially given that we already have a new(...) form. Now we'd have new:(...) form as well. This is not a positive for me.

Oh, just to be clear, I was asking that question when comparing it to @TahirAhmadov's original proposed syntax, not in general. I also dislike the new: syntax. The point was that "if you are doing it that way, at least move the elements outside of the array literal so it could be reused for other use cases later".

@tzographos
Copy link

Currently when we want to create e.g. a Dictionary<string, double> we do something like this:

Dictionary<string, double> airComponents = new(capacity, comparer) {
        {  "CO", 10.0 },
        {  "CO2", 200.0 },
}

How will we be able to utilize capacity and comparer parameters in the proposed syntax?

Something like this maybe in order to be as close to the existing collection initialization?

Dictionary<string, double> airComponents = [
    new("CO",  10.0),
    new("CO2", 200.0)
].WithCapacity(...).WithComparer(....)

@TahirAhmadov
Copy link

@tzographos see these:
#7822 (comment)
#7822 (comment)
It's based on a proposed syntax I noticed in LDM notes but I can't find that link now.

@slang25
Copy link
Contributor

slang25 commented Jul 2, 2024

Given that there is no branch in Roslyn for this (that I can see), is it safe to say this won't be a C# 13 feature?

@333fred
Copy link
Member

333fred commented Jul 2, 2024

Correct.

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

No branches or pull requests