ToFrozenDictionary overloads differ in duplicate handling #127798

Activity

1. dotnet-policy-service

   added

   untriagedNew issue has not been triaged by the area ownerNew issue has not been triaged by the area owner

   on May 5, 2026
2. Timovzl commented on May 5, 2026

   

   Timovzl

   on May 5, 2026

   Author

   More actions

   Somewhat related to #113096.
3. github-actions

   added

   area-System.Collections

   on May 5, 2026
4. dotnet-policy-service commented on May 5, 2026

   

   dotnet-policy-servicebot

   on May 5, 2026 – with Dotnet Policy Service

   Contributor

   More actions

   Tagging subscribers to this area: @dotnet/area-system-collections
   See info in area-owners.md if you want to be subscribed.
5. jeffhandley commented on Aug 17, 2026

   

   jeffhandley

   on Aug 17, 2026

   Member

   More actions

   We're open to suggestions for how best to address this while taking #113096 into consideration as well.
6. jeffhandley

   added

   help wanted[up-for-grabs] Good issue for external contributors[up-for-grabs] Good issue for external contributors

   and removed

   untriagedNew issue has not been triaged by the area ownerNew issue has not been triaged by the area owner

   on Aug 17, 2026
7. jeffhandley

   added this to the Future milestone on Aug 17, 2026
8. caraioniurie47 commented on Sep 11, 2026

   

   caraioniurie47

   on Sep 11, 2026

   Contributor

   More actions

   Summary

   • The inconsistency looks incidental rather than designed. The two selector overloads are
     one-liners that call Enumerable.ToDictionary first and freeze second, so the ArgumentException
     is LINQ's and is thrown before any code in the Frozen namespace runs. The last-wins path, by
     contrast, is a deliberate choice with a comment in the source saying exactly that.
   • Suggested split: document duplicate handling on all five factories now — in <summary>, not
     <remarks> — and defer any behavioural alignment to [API Proposal]: Enhance dictionary and set constructors/factories with Duplicate Key Handling Options #113096.
   • Both defaults have a real complainant: this issue is from someone surprised that the selector
     overloads throw, and datvm's comment on [API Proposal]: Enhance dictionary and set constructors/factories with Duplicate Key Handling Options #113096 is from someone surprised that the
     KeyValuePair ones do not. That argues for making the handling selectable rather than picking a
     winner now.

   Detail below, read against main at d84e42c; the file itself last changed in #128300 back in May,
   so it should still hold.

   Where the five factories stand today, all in
   src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenDictionary.cs:

   factory	duplicate keys	documented?
   Create(params ReadOnlySpan<KeyValuePair<TKey, TValue>>)	last wins	yes, in <remarks>
   Create(IEqualityComparer<TKey>?, params ReadOnlySpan<KeyValuePair<TKey, TValue>>)	last wins	yes, in <remarks>
   ToFrozenDictionary(IEnumerable<KeyValuePair<TKey, TValue>>, IEqualityComparer<TKey>?)	last wins	yes, in <remarks>
   ToFrozenDictionary(IEnumerable<TSource>, Func<TSource, TKey>, ...)	throws	no
   ToFrozenDictionary(IEnumerable<TSource>, Func<TSource, TKey>, Func<TSource, TElement>, ...)	throws	no

   The last-wins behaviour is a deliberate choice, and GetExistingFrozenOrNewDictionary says so in as
   many words:

   // Dictionary's constructor uses Add, which will throw on duplicates.
   // This implementation uses the indexer to avoid throwing and to overwrite
   // existing entries such that last one wins.
   newDictionary[pair.Key] = pair.Value;

   The throwing behaviour is not a choice — it is inherited. Both selector overloads are one-liners
   that convert with LINQ first and freeze second:

   source.ToDictionary(keySelector, comparer).ToFrozenDictionary(comparer);
   source.ToDictionary(keySelector, elementSelector, comparer).ToFrozenDictionary(comparer);

   Enumerable.ToDictionary throws on duplicates, so the exception is raised before any code in the
   Frozen namespace runs. The stack from the two-argument overload on 10.0.12 contains no Frozen
   frames at all:

   System.ArgumentException: An item with the same key has already been added. Key: 1
      at System.Collections.Generic.Dictionary`2.TryInsert(TKey key, TValue value, InsertionBehavior behavior)
      at System.Collections.Generic.Dictionary`2.Add(TKey key, TValue value)
      at System.Linq.Enumerable.SpanToDictionary[TSource,TKey](ReadOnlySpan`1 source, Func`2 keySelector, IEqualityComparer`1 comparer)
      at System.Linq.Enumerable.ToDictionary[TSource,TKey](IEnumerable`1 source, Func`2 keySelector, IEqualityComparer`1 comparer)
      at Program.<Main>$(String[] args)
   

   Running all five against 10.0.12, with a bare Enumerable.ToDictionary as a control:

   Runtime: .NET 10.0.12
   
   FrozenDictionary.Create(ROS<KVP>)                       -> OK   Count=1 d[1]=b
   FrozenDictionary.Create(comparer, ROS<KVP>)             -> OK   Count=1 d[1]=b
   ToFrozenDictionary(IEnumerable<KVP>)                    -> OK   Count=1 d[1]=b
   ToFrozenDictionary(source, keySelector)                 -> ArgumentException: An item with the same key has already been added. Key: 1
   ToFrozenDictionary(source, keySelector, elementSelector) -> ArgumentException: An item with the same key has already been added. Key: 1
   Enumerable.ToDictionary (control)                       -> ArgumentException: An item with the same key has already been added. Key: 1
   

   Same type, same message as the control.

   Code for the above

   Each call is listed on its own; the run that produced the output wrapped them in separate
   try/catch blocks so that one failure did not stop the rest.

   using System.Collections.Frozen;
   
   var kvps = new[] { KeyValuePair.Create(1, "a"), KeyValuePair.Create(1, "b") };
   
   FrozenDictionary.Create(kvps);                                  // OK, last wins
   FrozenDictionary.Create((IEqualityComparer<int>)null, kvps);    // OK, last wins
   kvps.ToFrozenDictionary();                                      // OK, last wins
   
   new[] { "a", "b" }.ToFrozenDictionary(s => 1);                  // ArgumentException
   new[] { "a", "b" }.ToFrozenDictionary(s => 1, s => s);          // ArgumentException
   new[] { "a", "b" }.ToDictionary(s => 1);                        // control: same exception

   So nobody actually picked this inconsistency — which is also why no documentation describes it.
   The existing remark sharpens the point: it warns that the
   behaviour "differs from Enumerable.ToDictionary, with which multiple duplicate keys will result in
   an exception" — and Enumerable.ToDictionary is exactly what the other two overloads call. So the
   contrast is spelled out on the three factories that do not exhibit it, and left unmentioned on the
   two that do.

   Suggestion

   Split it, because the documentation half and the behaviour half carry very different risk.

   1. Now — documentation only. State duplicate handling explicitly on all five factories, and
   state it in <summary> rather than <remarks>. datvm's point in
   #113096 is that the
   existing remark "does not even show up on the IntelliSense tooltip", and that is precisely how this
   trap survives: the behaviour is technically documented, in the one place a caller is least likely to
   be looking at the moment they make the mistake. Adding a <remarks> to the selector overloads would
   reproduce the reported failure mode rather than fix it. I am happy to send this PR if it is wanted.

   2. Behavioural alignment — defer to #113096. Changing either side silently is unattractive.
   Making the selector overloads last-wins would stop throwing for callers who rely on the
   ArgumentException as an integrity check, with no compile-time signal; changing the
   KeyValuePair overloads would break documented, shipped, intentional behaviour. An explicit
   DuplicateKeyHandling argument resolves it in both directions without breaking anyone, and
   eiriktsarpalis's direction on that issue — "augment collection constructors accepting IEnumerable
   with overloads accepting this enum in dictionary and set types" — would appear to cover these
   factories already.

   On the opposite-defaults point above: that pair may be directly useful to #113096 as well.
   GrabYourPitchforks asked there for concrete scenarios to strengthen the case for the API, and this
   is two real ones pulling in opposite directions on the same type — which argues for selectable
   handling rather more strongly than either would on its own.

   For completeness, FrozenSet does not have the equivalent problem. None of its three factories
   document duplicate handling either, but all three collapse duplicates through HashSet<T>, none
   throws, and so there is no pair of overloads that disagree — and for a set, collapsing duplicates is
   the defining behaviour rather than a surprise.

   Note

   AI-generated, written at my direction and reviewed by me before posting. The source references
   were read against main at d84e42c, and the stack trace and sample output above come from running
   the sample on .NET 10.0.12.