Revise core.internal.array.equality

[kinke]

• 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.
  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.

[dlang-bot]

Thanks for your pull request and interest in making D better, @kinke! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + druntime#3142"

[kinke]

Azure CI issue for the OMF job:

D:/a/1/dmd/tools/dmd2/windows/bin/dmd.exe -m32  -g -lowmem -i -Itools -version=NoMain -odtest_results -oftest_results/d_do_test.exe tools/d_do_test.d

OPTLINK (R) for Win32  Release 8.00.17
Copyright (C) Digital Mars 1989-2013  All rights reserved.
http://www.digitalmars.com/ctg/optlink.html
test_results\d_do_test.obj(d_do_test) 
 Error 42: Symbol Undefined __D4core8internal5array8equality__T8__equalsTxkTxkZQqFNaNbNiNfAxkQdZb
Error: linker exited with status 1

AFAICT, that's the DMD host compiler failing to generate a linkable d_do_test tool. A druntime change shouldn't affect this at all.

The unittest failure in https://github.com/CyberShadow/ae/blob/ae2afb17cae70c8df739fb0dccd00b6eef212015/utils/regex.d#L345-L346 is somewhat worrying. I'll try to reproduce that later with LDC.

[kinke]

Those 2 lines in the CyberShadow/ae repo nicely show the current problem with diverging behavior for __equals (doing a field-wise comparison of a struct's fields (if the struct has no custom opEquals), i.e., basically enforcing -preview=fieldwise). This wrongly passes with master:

Transformation expected = { type : Transformation.Type.replace, replace : { search : "from", replacement : "to", flags : "" } };
auto actual = splitRETransformation("s/from/to/");

assert(actual[0] != expected);    // single element NOT equal (compiled without -preview=fieldwise)

Transformation[] expectedSlice = [expected];
assert(actual == expectedSlice);  // equal (lowered to __equals); unfortunately, this is tested

Transformation[1] expectedSarray = [expected];
assert(actual != expectedSarray); // NOT equal (not lowered)

[rainers]

AFAICT, that's the DMD host compiler failing to generate a linkable d_do_test tool. A druntime change shouldn't affect this at all.

That's probably the same issue as this one: #3141 (comment)

[kinke]

The ae project has been adapted (no tagged release available yet though). See the linked PR above for more context wrt. that issue, boiling down to:

union Transformation
{
    string search;
}

void main()
{
    static immutable from = "from", from2 = "from2";
    Transformation[] a = [{ search : from }];
    Transformation[] b = [{ search : from2[0..4] }];
    assert(a == b);
}

failing the assertion now with this PR (using memcmp (is) without -preview=fieldwise), as it used to before v2.078, presumably when __equals (with diverging semantics) was introduced (I haven't found any mention of this changed behavior in the changelog).

[schveiguy]

oops, didn't know there was already something for this instead of #3151.

Is there anything in this that still needs merging? I think it would need to at least be rebased now that the other PR is merged.

[kinke]

Yeah, a rebase is now necessary, but it's still blocked by the missing new tag for @CyberShadow's ae repo.

[n8sh]

Need to also check __traits(isUnsigned, T1) == __traits(isUnsigned, T2) or else:

ubyte[] a = [cast(ubyte) 0xff];
byte[] b = [cast(byte) 0xff];
assert(a != b); // Currently passes but will fail after this PR.

[kinke]

Right, thx. That only affects integers < 32 bit though, due to int promotion.

[n8sh]

From the perspective of the compiler frontend there anything to recommend this over is(immutable T == immutable void)?

[MoonlightSentinel]

What about shared?

[n8sh]

It works since immutable(shared T) == immutable T.

[kinke]

From the perspective of the compiler frontend there anything to recommend this over is(immutable T == immutable void)?

Absolutely not, thx for the suggestion.

[n8sh]

Those 2 lines in the CyberShadow/ae repo nicely show the current problem with diverging behavior for __equals (doing a field-wise comparison of a struct's fields (if the struct has no custom opEquals), i.e., basically enforcing -preview=fieldwise).

I think everyone would agree that a fix for something like this should appear in the changelog.

[kinke]

The ae repo is no blocker anymore (new tag available).

[kinke]

Thx @n8sh. I'm sure you'll update and re-evaluate #3152 (sorry, I haven't found the time to perform the promised tests yet).

[JohanEngelen]

Because it removed all the casting, this PR fixed this bug: https://issues.dlang.org/show_bug.cgi?id=21094.