Skip to content

Commit 8edc13f

Browse files
xperiandriclaude
andcommitted
fixup! feat: HashSet and ISet modules with the vocabulary of the FSharp.Core Set module
Every `seq<'T>` parameter rejects `null` and a default flat list that is given as a sequence with an `ArgumentNullException` that names the parameter of our function (ADR 0001, D6). A default flat list is a boxed, non-null struct, so the check for `null` alone let it reach the BCL call, which failed with `InvalidOperationException` when it enumerated the sequence. - `HashSet.ofSeq` and the member `ofSeq` of `CustomComparerHashSet` (`elements`), `addRange` and `removeRange` of `HashSet` and `ISet` (`elements`), and `intersectMany` and `unionMany` of `HashSet` and `ISet` (`sets`) call `checkNotNullOrDefaultSeq` first. The check of each set of `sets` stays. `ofList` and `ofArray` take another type and keep their check for `null`. - The `yield!` overloads of `hashSet { }` for a sequence, an array and a hash set (`items`) and its `for` (`source`) check their input before the buffer is returned. A `null` array or hash set reached `UnionWith`, so the exception named `other`, and a `null` source made `for` throw `NullReferenceException`. - The exception documentation of the affected functions says "null or a default flat list", as the documentation of `FlatList` does. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
1 parent 6309937 commit 8edc13f

8 files changed

Lines changed: 40 additions & 22 deletions

File tree

‎src/FSharp.Collections.Immutable/HashSet/HashSet.CustomComparer.fs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ type CustomComparerHashSet<'T> =
4646

4747
[<CompiledName "OfSeq">]
4848
member this.ofSeq (elements : seq<'T>) : HashSet<'T> =
49-
checkNotNull (nameof elements) elements
49+
// A default flat list given as a sequence would fail with InvalidOperationException in the BCL call (ADR 0001, D6)
50+
checkNotNullOrDefaultSeq (nameof elements) elements
5051
// A hash set gets the comparer through WithComparer, which returns the hash set itself for the same comparer
5152
// instance (the test is reference equality, ADR 0001, D14)
5253
ImmutableHashSet.CreateRange<'T>(this.Comparer, elements)

‎src/FSharp.Collections.Immutable/HashSet/HashSet.CustomComparer.fsi‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ type CustomComparerHashSet<'T> =
102102
/// The hash set of the elements that are distinct in the comparer; of the elements that the comparer considers equal,
103103
/// it holds the first one.
104104
/// </returns>
105-
/// <exception cref="T:System.ArgumentNullException">Thrown when <paramref name="elements"/> is <see langword="null"/>.</exception>
105+
/// <exception cref="T:System.ArgumentNullException">Thrown when <paramref name="elements"/> is <see langword="null"/> or a default flat list.</exception>
106106
/// <remarks>
107107
/// <para>
108108
/// O(n log n). The member wraps

‎src/FSharp.Collections.Immutable/HashSet/HashSet.fs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ let add (value : 'T) (set : HashSet<'T>) =
190190

191191
[<CompiledName "AddRange">]
192192
let addRange (elements : seq<'T>) (set : HashSet<'T>) =
193-
checkNotNull (nameof elements) elements
193+
checkNotNullOrDefaultSeq (nameof elements) elements
194194
checkNotNull (nameof set) set
195195
let result = set.Union elements
196196

@@ -235,7 +235,7 @@ let intersect (set1 : HashSet<'T>) (set2 : HashSet<'T>) =
235235
[<CompiledName "IntersectMany">]
236236
let intersectMany (sets : seq<HashSet<'T>>) =
237237
// Set.intersectMany is Seq.reduce over Set.intersect, so its checks are those of Seq.reduce
238-
checkNotNull (nameof sets) sets
238+
checkNotNullOrDefaultSeq (nameof sets) sets
239239

240240
// The type of a set is annotated, because the sequence that fixes it comes last
241241
let checkSet (set : HashSet<'T>) = checkNotNull (nameof sets) set
@@ -256,7 +256,7 @@ let remove (value : 'T) (set : HashSet<'T>) =
256256

257257
[<CompiledName "RemoveRange">]
258258
let removeRange (elements : seq<'T>) (set : HashSet<'T>) =
259-
checkNotNull (nameof elements) elements
259+
checkNotNullOrDefaultSeq (nameof elements) elements
260260
checkNotNull (nameof set) set
261261
set.Except elements
262262

@@ -274,7 +274,7 @@ let union (set1 : HashSet<'T>) (set2 : HashSet<'T>) =
274274

275275
[<CompiledName "UnionMany">]
276276
let unionMany (sets : seq<HashSet<'T>>) =
277-
checkNotNull (nameof sets) sets
277+
checkNotNullOrDefaultSeq (nameof sets) sets
278278

279279
let checkSet (set : HashSet<'T>) = checkNotNull (nameof sets) set
280280

‎src/FSharp.Collections.Immutable/HashSet/HashSet.fsi‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ val ofReadOnlySpan : span : ReadOnlySpan<'T> -> HashSet<'T>
123123
/// </summary>
124124
/// <param name="elements">The input sequence.</param>
125125
/// <returns>The hash set of the distinct elements of the sequence.</returns>
126-
/// <exception cref="T:System.ArgumentNullException">Thrown when <paramref name="elements"/> is <see langword="null"/>.</exception>
126+
/// <exception cref="T:System.ArgumentNullException">Thrown when <paramref name="elements"/> is <see langword="null"/> or a default flat list.</exception>
127127
/// <remarks>
128128
/// O(n log n), or O(1) for a hash set that compares with the default comparer: the function returns such a hash set
129129
/// without a copy, and gives a hash set with another comparer the default one. The function wraps
@@ -438,7 +438,7 @@ val add : value : 'T -> set : HashSet<'T> -> HashSet<'T>
438438
/// A hash set of the elements and the elements of the sequence, with the comparer of the input; the input itself when the
439439
/// sequence adds no element.
440440
/// </returns>
441-
/// <exception cref="T:System.ArgumentNullException">Thrown when <paramref name="elements"/> or <paramref name="set"/> is <see langword="null"/>.</exception>
441+
/// <exception cref="T:System.ArgumentNullException">Thrown when <paramref name="elements"/> is <see langword="null"/> or a default flat list, or <paramref name="set"/> is <see langword="null"/>.</exception>
442442
/// <remarks>
443443
/// <para>
444444
/// O(m log (n + m)), where n and m are the numbers of elements of the hash set and the sequence. The function wraps
@@ -526,7 +526,7 @@ val intersect : set1 : HashSet<'T> -> set2 : HashSet<'T> -> HashSet<'T>
526526
/// </summary>
527527
/// <param name="sets">The hash sets.</param>
528528
/// <returns>The intersection, with the comparer of the first hash set.</returns>
529-
/// <exception cref="T:System.ArgumentNullException">Thrown when <paramref name="sets"/> or one of its hash sets is <see langword="null"/>.</exception>
529+
/// <exception cref="T:System.ArgumentNullException">Thrown when <paramref name="sets"/> is <see langword="null"/> or a default flat list, or one of its hash sets is <see langword="null"/>.</exception>
530530
/// <exception cref="T:System.ArgumentException">Thrown when the sequence is empty.</exception>
531531
/// <remarks>
532532
/// O(N log n), where N is the total number of elements of the other hash sets and n the number of elements of the first
@@ -573,7 +573,7 @@ val remove : value : 'T -> set : HashSet<'T> -> HashSet<'T>
573573
/// A hash set of the other elements, with the comparer of the input; the input itself when it has no element equal to an
574574
/// element of the sequence.
575575
/// </returns>
576-
/// <exception cref="T:System.ArgumentNullException">Thrown when <paramref name="elements"/> or <paramref name="set"/> is <see langword="null"/>.</exception>
576+
/// <exception cref="T:System.ArgumentNullException">Thrown when <paramref name="elements"/> is <see langword="null"/> or a default flat list, or <paramref name="set"/> is <see langword="null"/>.</exception>
577577
/// <remarks>
578578
/// O(m log n), where n and m are the numbers of elements of the hash set and the sequence. The function wraps
579579
/// <see cref="M:System.Collections.Immutable.ImmutableHashSet`1.Except(System.Collections.Generic.IEnumerable{`0})"/>
@@ -620,7 +620,7 @@ val union : set1 : HashSet<'T> -> set2 : HashSet<'T> -> HashSet<'T>
620620
/// The union, with the comparer of the first hash set; the empty hash set with the default comparer for an empty
621621
/// sequence.
622622
/// </returns>
623-
/// <exception cref="T:System.ArgumentNullException">Thrown when <paramref name="sets"/> or one of its hash sets is <see langword="null"/>.</exception>
623+
/// <exception cref="T:System.ArgumentNullException">Thrown when <paramref name="sets"/> is <see langword="null"/> or a default flat list, or one of its hash sets is <see langword="null"/>.</exception>
624624
/// <remarks>
625625
/// O(N log N), where N is the total number of elements of the hash sets. The function unites the hash sets from the first
626626
/// one to the last one.

‎src/FSharp.Collections.Immutable/HashSet/HashSetCE.fs‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,20 @@ type HashSetCE<'T> (comparer : System.Collections.Generic.IEqualityComparer<'T>)
1818

1919
member inline _.Yield (item : 'T) : HashSetBuffer<'T> = fun builder -> builder.Add item |> ignore
2020

21-
member inline _.YieldFrom (items : seq<'T>) : HashSetBuffer<'T> = fun builder -> builder.UnionWith items
21+
// Each input is checked before the buffer is returned, so that the exception names the parameter of the expression and
22+
// not the parameter of UnionWith (other); a default flat list given as a sequence is rejected as FlatListCE rejects it
23+
// (ADR 0001, D6)
24+
member inline _.YieldFrom (items : seq<'T>) : HashSetBuffer<'T> =
25+
checkNotNullOrDefaultSeq (nameof items) items
26+
fun builder -> builder.UnionWith items
2227

23-
member inline _.YieldFrom (items : 'T array) : HashSetBuffer<'T> = fun builder -> builder.UnionWith items
28+
member inline _.YieldFrom (items : 'T array) : HashSetBuffer<'T> =
29+
checkNotNull (nameof items) items
30+
fun builder -> builder.UnionWith items
2431

25-
member inline _.YieldFrom (items : HashSet<'T>) : HashSetBuffer<'T> = fun builder -> builder.UnionWith items
32+
member inline _.YieldFrom (items : HashSet<'T>) : HashSetBuffer<'T> =
33+
checkNotNull (nameof items) items
34+
fun builder -> builder.UnionWith items
2635

2736
member inline _.YieldFrom (buffer : HashSetBuffer<'T>) : HashSetBuffer<'T> = buffer
2837

@@ -35,6 +44,10 @@ type HashSetCE<'T> (comparer : System.Collections.Generic.IEqualityComparer<'T>)
3544
fun builder -> delayed () builder
3645

3746
member inline _.For (source : seq<'U>, [<InlineIfLambda>] body : 'U -> HashSetBuffer<'T>) : HashSetBuffer<'T> =
47+
// A null source would throw NullReferenceException and a default flat list InvalidOperationException when the loop
48+
// enumerates; the expression rejects both as yield! does
49+
checkNotNullOrDefaultSeq (nameof source) source
50+
3851
fun builder ->
3952
for item in source do
4053
body item builder

‎src/FSharp.Collections.Immutable/HashSet/HashSetCE.fsi‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,15 @@ type HashSetCE<'T> =
4040
member inline Yield : item : 'T -> HashSetBuffer<'T>
4141

4242
/// <summary>Adds the elements of a sequence.</summary>
43+
/// <exception cref="T:System.ArgumentNullException">Thrown when the sequence is <see langword="null"/> or a default flat list.</exception>
4344
member inline YieldFrom : items : seq<'T> -> HashSetBuffer<'T>
4445

4546
/// <summary>Adds the elements of an array.</summary>
47+
/// <exception cref="T:System.ArgumentNullException">Thrown when the array is <see langword="null"/>.</exception>
4648
member inline YieldFrom : items : 'T array -> HashSetBuffer<'T>
4749

4850
/// <summary>Adds the elements of a hash set.</summary>
51+
/// <exception cref="T:System.ArgumentNullException">Thrown when the hash set is <see langword="null"/>.</exception>
4952
member inline YieldFrom : items : HashSet<'T> -> HashSetBuffer<'T>
5053

5154
/// <summary>Adds the elements of another expression body.</summary>
@@ -60,6 +63,7 @@ type HashSetCE<'T> =
6063
member inline Delay : [<InlineIfLambda>] delayed : (unit -> HashSetBuffer<'T>) -> HashSetBuffer<'T>
6164

6265
/// <summary>Adds the elements of the body for each item of a sequence.</summary>
66+
/// <exception cref="T:System.ArgumentNullException">Thrown when the sequence is <see langword="null"/> or a default flat list.</exception>
6367
member inline For : source : seq<'U> * [<InlineIfLambda>] body : ('U -> HashSetBuffer<'T>) -> HashSetBuffer<'T>
6468

6569
/// <summary>Adds the elements of the body while the guard holds.</summary>

‎src/FSharp.Collections.Immutable/HashSet/ISet.fs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ let add (value : 'T) (set : IImmutableSet<'T>) =
103103

104104
[<CompiledName "AddRange">]
105105
let addRange (elements : seq<'T>) (set : IImmutableSet<'T>) =
106-
checkNotNull (nameof elements) elements
106+
checkNotNullOrDefaultSeq (nameof elements) elements
107107
checkNotNull (nameof set) set
108108
let result = set.Union elements
109109

@@ -145,7 +145,7 @@ let intersect (set1 : IImmutableSet<'T>) (set2 : IImmutableSet<'T>) =
145145
[<CompiledName "IntersectMany">]
146146
let intersectMany (sets : seq<IImmutableSet<'T>>) =
147147
// Set.intersectMany is Seq.reduce over Set.intersect, so its checks are those of Seq.reduce
148-
checkNotNull (nameof sets) sets
148+
checkNotNullOrDefaultSeq (nameof sets) sets
149149

150150
// The type of a set is annotated, because the sequence that fixes it comes last
151151
let checkSet (set : IImmutableSet<'T>) = checkNotNull (nameof sets) set
@@ -166,7 +166,7 @@ let remove (value : 'T) (set : IImmutableSet<'T>) =
166166

167167
[<CompiledName "RemoveRange">]
168168
let removeRange (elements : seq<'T>) (set : IImmutableSet<'T>) =
169-
checkNotNull (nameof elements) elements
169+
checkNotNullOrDefaultSeq (nameof elements) elements
170170
checkNotNull (nameof set) set
171171
// ImmutableHashSet and ImmutableSortedSet return the input itself when nothing is removed (ADR 0001, D14), so unlike
172172
// in addRange no check of the result is needed
@@ -186,7 +186,7 @@ let union (set1 : IImmutableSet<'T>) (set2 : IImmutableSet<'T>) =
186186

187187
[<CompiledName "UnionMany">]
188188
let unionMany (sets : seq<IImmutableSet<'T>>) =
189-
checkNotNull (nameof sets) sets
189+
checkNotNullOrDefaultSeq (nameof sets) sets
190190

191191
let checkSet (set : IImmutableSet<'T>) = checkNotNull (nameof sets) set
192192

‎src/FSharp.Collections.Immutable/HashSet/ISet.fsi‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ val add : value : 'T -> set : IImmutableSet<'T> -> IImmutableSet<'T>
260260
/// A set of the elements and the elements of the sequence, with the implementation and the comparer of the input; the
261261
/// input itself when the sequence adds no element.
262262
/// </returns>
263-
/// <exception cref="T:System.ArgumentNullException">Thrown when <paramref name="elements"/> or <paramref name="set"/> is <see langword="null"/>.</exception>
263+
/// <exception cref="T:System.ArgumentNullException">Thrown when <paramref name="elements"/> is <see langword="null"/> or a default flat list, or <paramref name="set"/> is <see langword="null"/>.</exception>
264264
/// <remarks>
265265
/// <para>
266266
/// The complexity depends on the implementation of the set: O(m log (n + m)) for an
@@ -360,7 +360,7 @@ val intersect : set1 : IImmutableSet<'T> -> set2 : IImmutableSet<'T> -> IImmutab
360360
/// </summary>
361361
/// <param name="sets">The sets.</param>
362362
/// <returns>The intersection, with the implementation and the comparer of the first set.</returns>
363-
/// <exception cref="T:System.ArgumentNullException">Thrown when <paramref name="sets"/> or one of its sets is <see langword="null"/>.</exception>
363+
/// <exception cref="T:System.ArgumentNullException">Thrown when <paramref name="sets"/> is <see langword="null"/> or a default flat list, or one of its sets is <see langword="null"/>.</exception>
364364
/// <exception cref="T:System.ArgumentException">Thrown when the sequence is empty.</exception>
365365
/// <remarks>
366366
/// The complexity depends on the implementation of the first set: O(N log n) for an
@@ -406,7 +406,7 @@ val remove : value : 'T -> set : IImmutableSet<'T> -> IImmutableSet<'T>
406406
/// A set of the other elements, with the implementation and the comparer of the input; the input itself when it has no
407407
/// element equal to an element of the sequence.
408408
/// </returns>
409-
/// <exception cref="T:System.ArgumentNullException">Thrown when <paramref name="elements"/> or <paramref name="set"/> is <see langword="null"/>.</exception>
409+
/// <exception cref="T:System.ArgumentNullException">Thrown when <paramref name="elements"/> is <see langword="null"/> or a default flat list, or <paramref name="set"/> is <see langword="null"/>.</exception>
410410
/// <remarks>
411411
/// <para>
412412
/// The complexity depends on the implementation of the set: O(m log n) for an
@@ -465,7 +465,7 @@ val union : set1 : IImmutableSet<'T> -> set2 : IImmutableSet<'T> -> IImmutableSe
465465
/// <see cref="T:FSharp.Collections.Immutable.HashSet`1"/>, which compares with
466466
/// <see cref="P:System.Collections.Generic.EqualityComparer`1.Default"/>.
467467
/// </returns>
468-
/// <exception cref="T:System.ArgumentNullException">Thrown when <paramref name="sets"/> or one of its sets is <see langword="null"/>.</exception>
468+
/// <exception cref="T:System.ArgumentNullException">Thrown when <paramref name="sets"/> is <see langword="null"/> or a default flat list, or one of its sets is <see langword="null"/>.</exception>
469469
/// <remarks>
470470
/// The complexity depends on the implementation of the first set: O(N log N) for an
471471
/// <see cref="T:System.Collections.Immutable.ImmutableHashSet`1"/>, where N is the total number of elements of the sets.

0 commit comments

Comments
 (0)