Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.

Commit cc1ed93

Browse files
kinkedlang-bot
kinke
authored and
dlang-bot
committed
Revise core.internal.array.equality
* Use memcmp for more types (incl. pointers and some static arrays), and don't require both element types to be equivalent (e.g., use it for comparing wchar[] and ushort[] too). * Simplify the non-memcmp branch - the previous code seemed to try to do what the compiler already does. * The previous code for structs without custom opEquals basically enforced -preview=fieldwise via .tupleof comparison. This breaking change with 2.078 almost certainly wasn't intended (and not mentioned in the changelog) and led to undesirable inconsistent behavior, see #3142 (comment). This reverts that change in semantics. * Avoid superfluously nested templates and convert the remaining `at()` helper to a module-level template to reduce the number of redundant instantiations.
1 parent d19c70b commit cc1ed93

File tree

2 files changed

+120
-82
lines changed

2 files changed

+120
-82
lines changed

‎changelog/array_equality.dd

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
Equality of arrays of structs is consistent again, as before v2.078
2+
3+
Since v2.078, some array equality comparisons (e.g., if both arrays are
4+
dynamic arrays) have been wrongly using a `.tupleof` comparison for
5+
structs without custom `opEquals`, basically enforcing
6+
`-preview=fieldwise` for these array comparisons.
7+
8+
---
9+
union U
10+
{
11+
string s;
12+
}
13+
14+
void main()
15+
{
16+
static immutable from = "from", from2 = "from2";
17+
U[] a = [{ s : from }];
18+
U[1] b = [{ s : from2[0..4] }];
19+
assert(a[0] != b[0]);
20+
assert(a != b);
21+
assert(a != b[]); // worked before v2.078, been failing since, now working again
22+
}
23+
---

‎src/core/internal/array/equality.d

Lines changed: 97 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/**
2-
* This module contains compiler support determining equality of dynamic arrays.
2+
* This module contains compiler support determining equality of arrays.
33
*
4-
* Copyright: Copyright Digital Mars 2000 - 2019.
4+
* Copyright: Copyright Digital Mars 2000 - 2020.
55
* License: Distributed under the
66
* $(LINK2 http://www.boost.org/LICENSE_1_0.txt, Boost Software License 1.0).
77
* (See accompanying file LICENSE)
@@ -10,108 +10,47 @@
1010

1111
module core.internal.array.equality;
1212

13-
// `lhs == rhs` lowers to `__equals(lhs, rhs)` for dynamic arrays
14-
bool __equals(T1, T2)(T1[] lhs, T2[] rhs)
13+
// The compiler lowers `lhs == rhs` to `__equals(lhs, rhs)` for
14+
// * dynamic arrays,
15+
// * (most) arrays of different (unqualified) element types, and
16+
// * arrays of structs with custom opEquals.
17+
bool __equals(T1, T2)(scope T1[] lhs, scope T2[] rhs)
1518
{
16-
import core.internal.traits : Unqual;
17-
alias U1 = Unqual!T1;
18-
alias U2 = Unqual!T2;
19-
20-
static @trusted ref R at(R)(R[] r, size_t i) { return r.ptr[i]; }
21-
static @trusted R trustedCast(R, S)(S[] r) { return cast(R) r; }
22-
2319
if (lhs.length != rhs.length)
2420
return false;
2521

26-
if (lhs.length == 0 && rhs.length == 0)
22+
if (lhs.length == 0)
2723
return true;
2824

29-
static if (is(U1 == void) && is(U2 == void))
30-
{
31-
return __equals(trustedCast!(ubyte[])(lhs), trustedCast!(ubyte[])(rhs));
32-
}
33-
else static if (is(U1 == void))
34-
{
35-
return __equals(trustedCast!(ubyte[])(lhs), rhs);
36-
}
37-
else static if (is(U2 == void))
38-
{
39-
return __equals(lhs, trustedCast!(ubyte[])(rhs));
40-
}
41-
else static if (!is(U1 == U2))
25+
static if (useMemcmp!(T1, T2))
4226
{
43-
foreach (const u; 0 .. lhs.length)
44-
{
45-
if (at(lhs, u) != at(rhs, u))
46-
return false;
47-
}
48-
return true;
49-
}
50-
else static if (__traits(isIntegral, U1))
51-
{
52-
5327
if (!__ctfe)
5428
{
55-
import core.stdc.string : memcmp;
56-
return () @trusted { return memcmp(cast(void*)lhs.ptr, cast(void*)rhs.ptr, lhs.length * U1.sizeof) == 0; }();
29+
static bool trustedMemcmp(scope T1[] lhs, scope T2[] rhs) @trusted @nogc nothrow pure
30+
{
31+
pragma(inline, true);
32+
import core.stdc.string : memcmp;
33+
return memcmp(cast(void*) lhs.ptr, cast(void*) rhs.ptr, lhs.length * T1.sizeof) == 0;
34+
}
35+
return trustedMemcmp(lhs, rhs);
5736
}
5837
else
5938
{
60-
foreach (const u; 0 .. lhs.length)
39+
foreach (const i; 0 .. lhs.length)
6140
{
62-
if (at(lhs, u) != at(rhs, u))
41+
if (at(lhs, i) != at(rhs, i))
6342
return false;
6443
}
6544
return true;
6645
}
6746
}
6847
else
6948
{
70-
foreach (const u; 0 .. lhs.length)
49+
foreach (const i; 0 .. lhs.length)
7150
{
72-
static if (__traits(compiles, __equals(at(lhs, u), at(rhs, u))))
73-
{
74-
if (!__equals(at(lhs, u), at(rhs, u)))
75-
return false;
76-
}
77-
else static if (__traits(isFloating, U1))
78-
{
79-
if (at(lhs, u) != at(rhs, u))
80-
return false;
81-
}
82-
else static if (is(U1 : Object) && is(U2 : Object))
83-
{
84-
if (!(cast(Object)at(lhs, u) is cast(Object)at(rhs, u)
85-
|| at(lhs, u) && (cast(Object)at(lhs, u)).opEquals(cast(Object)at(rhs, u))))
86-
return false;
87-
}
88-
else static if (__traits(hasMember, U1, "opEquals"))
89-
{
90-
if (!at(lhs, u).opEquals(at(rhs, u)))
91-
return false;
92-
}
93-
else static if (is(U1 == delegate))
94-
{
95-
if (at(lhs, u) != at(rhs, u))
96-
return false;
97-
}
98-
else static if (is(U1 == U11*, U11))
99-
{
100-
if (at(lhs, u) != at(rhs, u))
101-
return false;
102-
}
103-
else static if (__traits(isAssociativeArray, U1))
104-
{
105-
if (at(lhs, u) != at(rhs, u))
106-
return false;
107-
}
108-
else
109-
{
110-
if (at(lhs, u).tupleof != at(rhs, u).tupleof)
111-
return false;
112-
}
51+
if (at(lhs, i) != at(rhs, i))
52+
return false;
11353
}
114-
11554
return true;
11655
}
11756
}
@@ -186,3 +125,79 @@ bool __equals(T1, T2)(T1[] lhs, T2[] rhs)
186125
assert(!__equals(a1, a2));
187126
assert(a1 != a2);
188127
}
128+
129+
130+
private:
131+
132+
// - Recursively folds static array types to their element type,
133+
// - maps void to ubyte, and
134+
// - pointers to size_t.
135+
template BaseType(T)
136+
{
137+
static if (__traits(isStaticArray, T))
138+
alias BaseType = BaseType!(typeof(T.init[0]));
139+
else static if (is(immutable T == immutable void))
140+
alias BaseType = ubyte;
141+
else static if (is(T == E*, E))
142+
alias BaseType = size_t;
143+
else
144+
alias BaseType = T;
145+
}
146+
147+
// Use memcmp if the element sizes match and both base element types are integral.
148+
// Due to int promotion, disallow small integers of diverging signed-ness though.
149+
template useMemcmp(T1, T2)
150+
{
151+
static if (T1.sizeof != T2.sizeof)
152+
enum useMemcmp = false;
153+
else
154+
{
155+
alias B1 = BaseType!T1;
156+
alias B2 = BaseType!T2;
157+
enum useMemcmp = __traits(isIntegral, B1) && __traits(isIntegral, B2)
158+
&& !( (B1.sizeof < 4 || B2.sizeof < 4) && __traits(isUnsigned, B1) != __traits(isUnsigned, B2) );
159+
}
160+
}
161+
162+
unittest
163+
{
164+
enum E { foo, bar }
165+
166+
static assert(useMemcmp!(byte, byte));
167+
static assert(useMemcmp!(ubyte, ubyte));
168+
static assert(useMemcmp!(void, const void));
169+
static assert(useMemcmp!(void, immutable bool));
170+
static assert(useMemcmp!(void, inout char));
171+
static assert(useMemcmp!(void, shared ubyte));
172+
static assert(!useMemcmp!(void, byte)); // differing signed-ness
173+
static assert(!useMemcmp!(char[8], byte[8])); // ditto
174+
175+
static assert(useMemcmp!(short, short));
176+
static assert(useMemcmp!(wchar, ushort));
177+
static assert(!useMemcmp!(wchar, short)); // differing signed-ness
178+
179+
static assert(useMemcmp!(int, uint)); // no promotion, ignoring signed-ness
180+
static assert(useMemcmp!(dchar, E));
181+
182+
static assert(useMemcmp!(immutable void*, size_t));
183+
static assert(useMemcmp!(double*, ptrdiff_t));
184+
static assert(useMemcmp!(long[2][3], const(ulong)[2][3]));
185+
186+
static assert(!useMemcmp!(float, float));
187+
static assert(!useMemcmp!(double[2], double[2]));
188+
static assert(!useMemcmp!(Object, Object));
189+
static assert(!useMemcmp!(int[], int[]));
190+
}
191+
192+
// Returns a reference to an array element, eliding bounds check and
193+
// casting void to ubyte.
194+
pragma(inline, true)
195+
ref at(T)(T[] r, size_t i) @trusted
196+
// exclude opaque structs due to https://issues.dlang.org/show_bug.cgi?id=20959
197+
if (!(is(T == struct) && !is(typeof(T.sizeof))))
198+
{
199+
static if (is(immutable T == immutable void))
200+
return (cast(ubyte*) r.ptr)[i];
201+
else
202+
return r.ptr[i];
203+
}

0 commit comments

Comments
 (0)