Skip to content

Commit 56dc7a8

Browse files
committed
fix(SAMPLE-020): allow List<T> for an element type without equality
SAMPLE-020's TransformedCollision stores `List<Block>`, and `Block` declares no equality of any kind. `List<T>::Contains`, `IndexOf` and `Remove` are overrides, so they are instantiated with the class, and the `findValue` they share required `operator==` on `T`. The container could therefore not be instantiated at all -- a restriction .NET's `List<T>` does not have, because the equality it needs comes from `EqualityComparer<T>.Default` rather than from `T`. `findValue` now refuses at the call instead of at instantiation: for a `T` that is not `equality_comparable` it throws `NotSupportedException`, and every type that does declare equality keeps the `std::find` path (and the NaN special case) unchanged. Storage, index-based mutation and enumeration work for any `T`, which is what the sample uses -- it removes blocks by index. Seven tests cover storage, index-based mutation, enumeration, the three refusals and the unchanged searching behaviour when equality does exist. Full suite after the change: 17847/17847.
1 parent 5457859 commit 56dc7a8

2 files changed

Lines changed: 134 additions & 5 deletions

File tree

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
// SPDX-License-Identifier: MIT
2+
// Copyright (c) Robert Vokac and contributors
3+
// Portions based on .NET runtime API (MIT License, Copyright .NET Foundation and Contributors)
4+
#include <gtest/gtest.h>
5+
#include "System/Collections/Generic/List.hpp"
6+
#include "System/NotSupportedException.hpp"
7+
8+
using namespace System::Collections::Generic;
9+
10+
namespace {
11+
12+
// A element type with no equality of any kind, the C++ counterpart of a plain C# class
13+
// that overrides neither Equals nor operator==.
14+
struct NoEquality {
15+
float x = 0.0f;
16+
int tag = 0;
17+
};
18+
19+
} // namespace
20+
21+
// A .NET List<T> is instantiable for every T, because the equality it needs comes from
22+
// EqualityComparer<T>.Default rather than from T itself. Everything that does not compare
23+
// elements must therefore work here too.
24+
TEST(ListNonComparableElementTests, StoresAndRetrievesElementsWithoutEquality) {
25+
List<NoEquality> list;
26+
list.Add(NoEquality{1.0f, 10});
27+
list.Add(NoEquality{2.0f, 20});
28+
list.Add(NoEquality{3.0f, 30});
29+
30+
EXPECT_EQ(list.getCountProperty(), 3);
31+
EXPECT_EQ(list.getItem(0).tag, 10);
32+
EXPECT_EQ(list.getItem(2).tag, 30);
33+
}
34+
35+
TEST(ListNonComparableElementTests, IndexBasedMutationWorksWithoutEquality) {
36+
List<NoEquality> list;
37+
list.Add(NoEquality{1.0f, 10});
38+
list.Add(NoEquality{2.0f, 20});
39+
40+
list.setItem(0, NoEquality{9.0f, 90});
41+
EXPECT_EQ(list.getItem(0).tag, 90);
42+
43+
list.Insert(1, NoEquality{5.0f, 50});
44+
EXPECT_EQ(list.getCountProperty(), 3);
45+
EXPECT_EQ(list.getItem(1).tag, 50);
46+
47+
list.RemoveAt(1);
48+
EXPECT_EQ(list.getCountProperty(), 2);
49+
EXPECT_EQ(list.getItem(1).tag, 20);
50+
51+
list.Clear();
52+
EXPECT_EQ(list.getCountProperty(), 0);
53+
}
54+
55+
TEST(ListNonComparableElementTests, EnumerationWorksWithoutEquality) {
56+
List<NoEquality> list;
57+
list.Add(NoEquality{1.0f, 10});
58+
list.Add(NoEquality{2.0f, 20});
59+
60+
int sum = 0;
61+
for (const NoEquality& item : list) {
62+
sum += item.tag;
63+
}
64+
EXPECT_EQ(sum, 30);
65+
}
66+
67+
// The members that do compare elements have no counterpart for such a T, and say so
68+
// instead of silently answering with something arbitrary.
69+
TEST(ListNonComparableElementTests, ContainsThrowsNotSupportedWithoutEquality) {
70+
List<NoEquality> list;
71+
list.Add(NoEquality{1.0f, 10});
72+
73+
EXPECT_THROW((void)list.Contains(NoEquality{1.0f, 10}), System::NotSupportedException);
74+
}
75+
76+
TEST(ListNonComparableElementTests, IndexOfThrowsNotSupportedWithoutEquality) {
77+
List<NoEquality> list;
78+
list.Add(NoEquality{1.0f, 10});
79+
80+
EXPECT_THROW((void)list.IndexOf(NoEquality{1.0f, 10}), System::NotSupportedException);
81+
}
82+
83+
TEST(ListNonComparableElementTests, RemoveThrowsNotSupportedWithoutEquality) {
84+
List<NoEquality> list;
85+
list.Add(NoEquality{1.0f, 10});
86+
87+
EXPECT_THROW((void)list.Remove(NoEquality{1.0f, 10}), System::NotSupportedException);
88+
EXPECT_EQ(list.getCountProperty(), 1);
89+
}
90+
91+
// An element type that does declare equality keeps the ordinary searching behaviour.
92+
namespace {
93+
94+
struct WithEquality {
95+
int tag = 0;
96+
bool operator==(const WithEquality& other) const { return tag == other.tag; }
97+
};
98+
99+
} // namespace
100+
101+
TEST(ListNonComparableElementTests, SearchingStillWorksWhenEqualityExists) {
102+
List<WithEquality> list;
103+
list.Add(WithEquality{10});
104+
list.Add(WithEquality{20});
105+
106+
EXPECT_TRUE(list.Contains(WithEquality{20}));
107+
EXPECT_EQ(list.IndexOf(WithEquality{20}), 1);
108+
EXPECT_TRUE(list.Remove(WithEquality{10}));
109+
EXPECT_EQ(list.getCountProperty(), 1);
110+
EXPECT_EQ(list.getItem(0).tag, 20);
111+
}

modules/core/include/System/detail/ComparisonPolicy.hpp

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#pragma once
55
#include <algorithm>
66
#include <cmath>
7+
#include <concepts>
78
#include <cstddef>
89
#include <functional>
910
#include <iterator>
@@ -12,6 +13,8 @@
1213
#include <type_traits>
1314
#include <utility>
1415

16+
#include "System/NotSupportedException.hpp"
17+
1518
namespace System::detail {
1619

1720
/**
@@ -185,13 +188,28 @@ template<typename T>
185188
*/
186189
template<typename It, typename T>
187190
[[nodiscard]] It findValue(It first, It last, const T& value) {
188-
if constexpr (std::is_floating_point_v<typename std::iterator_traits<It>::value_type>) {
189-
if (std::isnan(value)) {
190-
return std::find_if(first, last,
191-
[](const auto& v) { return std::isnan(v); });
191+
if constexpr (!std::equality_comparable<T>) {
192+
// .NET reaches `EqualityComparer<T>.Default` here, which exists for every T: a
193+
// reference type with no Equals override falls back to reference equality. This
194+
// port stores elements by value, so a type that declares no equality has nothing
195+
// to compare and no reference identity to fall back on. Refusing at the call is
196+
// what keeps `List<T>` and `Array` usable for such a T at all -- a hard
197+
// requirement on `operator==` would make the container itself uninstantiable,
198+
// which C#'s never is.
199+
(void)first; (void)last; (void)value;
200+
throw System::NotSupportedException(
201+
"This element type declares no equality, so the .NET EqualityComparer<T>.Default "
202+
"search this method performs has no counterpart. Give the type an operator== to "
203+
"make Contains/IndexOf/Remove meaningful for it.");
204+
} else {
205+
if constexpr (std::is_floating_point_v<typename std::iterator_traits<It>::value_type>) {
206+
if (std::isnan(value)) {
207+
return std::find_if(first, last,
208+
[](const auto& v) { return std::isnan(v); });
209+
}
192210
}
211+
return std::find(first, last, value);
193212
}
194-
return std::find(first, last, value);
195213
}
196214

197215
/**

0 commit comments

Comments
 (0)