Draft: Add new __traits(isSame2) that mimics the sane parts of std.meta.isSame

[nordlow]

Naming isSame2 is obviously a temporary placeholder for the real name that is to be decided in some kind of democratic process.

This commit is just an implementation of isSame2 being a copy isSame. Adjustments to mimic phobos will happen in subsequent commits.

I had a discussion with @andralex about the discrepancies between std.meta.isSame and __traits(isSame) and he proposed the following:

1. Add a __traits(identical) (original proposed as __traits(isSame2)) that mimic std.meta.isSame
2. Use enum identical(T, U) = __traits(isSame, T, U);
3. Deprecate __traits(isSame)

[dlang-bot]

Thanks for your pull request and interest in making D better, @nordlow! 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 + dmd#13073"

[12345swordy]

This PR needs a rational.

[ljmf00]

Can you explain why this is being added? I can't see a reason for adding this as it can be done with normal code.

[maxhaton]

I can't give the full context right now as I can't be bothered to dig it up, but the rationale is that isSame as a template is extremely slow but existing use of said cannot be eliminated with the trait without changing a bunch of phobos code because IsSame can compare aliases to values IIRC.

[RazvanN7]

So why not change the phobos code?

[nordlow]

So why not change the phobos code?

Do you mean adjust std.meta.isSame to mimic __traits(isSame)? If so, that's an alternative because it's gonna cause too much breakage with changd behavior of template alias parameters.

@andralex has already proposed to the following plan

1. Add a __traits(isSame2) that mimic std.traits.isSame
2. Use enum isSame(T, U) = __traits(isSame2, T, U);
3. Deprecate __traits(isSame)

[nordlow]

So why not change the phobos code?

See motive at the top https://github.com/dlang/dmd/pull/13073#issue-734102064.

[nordlow]

Can you explain why this is being added? I can't see a reason for adding this as it can be done with normal code.

See motive at the top https://github.com/dlang/dmd/pull/13073#issue-734102064.

[nordlow]

This PR needs a rational.

See motive at the top https://github.com/dlang/dmd/pull/13073#issue-734102064.

[nordlow]

I can't make sense of why the CI fails. Afaict, I've just added a copy of the isSame trait renamed to isSame2. Do you have any clues, @RazvanN7?

[RazvanN7]

@nordlow maybe a rebase will fix the issue? Currently, it seems that std/path fails with an ICE, however, at a first glance, it seems unrelated.

[RazvanN7]

@nordlow do the phobos tests pass locally with this PR?

[nordlow]

@nordlow maybe a rebase will fix the issue? Currently, it seems that std/path fails with an ICE, however, at a first glance, it seems unrelated.

Rebased.

[nordlow]

@nordlow do the phobos tests pass locally with this PR?

Do you mean make -f posix.mak unittest under phobos using this dmd branch?

How do I build and run tests in dmd?

[RazvanN7]

Looking at posix.mak in phobos, it seems that it already picks the binary from dmd/generated/release/linux/x86, so all you need to do is build dmd (go to dmd/src and run ./build.d).

[nordlow]

Looking at posix.mak in phobos, it seems that it already picks the binary from dmd/generated/release/linux/x86, so all you need to do is build dmd (go to dmd/src and run ./build.d).

Ok, thanks. Btw, should I be able to do

cd test && ./run.d

? I'm asking because it fails for me as

./run.d AUTO_UPDATE=1
... compilable/ddocYear.d          -D -Dd/home/per/Work/dmd/test/test_results/compilable -o-  -fPIC ()==============================
Test 'compilable/ddocYear.d' failed. The logged output:
Executing post-test script: tools/postscript.sh compilable/extra-files/ddocYear-postscript.sh compilable ddocYear /home/per/Work/dmd/test/test_results/compilable/ddocYear.d.out0
tools/postscript.sh: line 18: /home/per/Work/dmd/test/tools
/home/per/Work/dmd/test/tools/exported_vars.sh: No such file or directory
/home/per/Work/dmd/generated/linux/release/64/dmd -conf= -m64 -Icompilable -D -Dd/home/per/Work/dmd/test/test_results/compilable -o-  -fPIC  -od/home/per/Work/dmd/test/test_results/compilable -of/home/per/Work/dmd/test/test_results/compilable/ddocYear_0.o  -c compilable/ddocYear.d 

==============================
Test 'compilable/ddocYear.d' failed: Expected rc == 0, but exited with rc == 1

... compilable/ddoc4.d             -D -Dd/home/per/Work/dmd/test/test_results/compilable -o-  -fPIC ()==============================
Test 'compilable/ddoc4.d' failed. The logged output:
Executing post-test script: tools/postscript.sh compilable/extra-files/ddocAny-postscript.sh compilable ddoc4 /home/per/Work/dmd/test/test_results/compilable/ddoc4.d.out0
tools/postscript.sh: line 18: /home/per/Work/dmd/test/tools
/home/per/Work/dmd/test/tools/exported_vars.sh: No such file or directory
/home/per/Work/dmd/generated/linux/release/64/dmd -conf= -m64 -Icompilable -D -Dd/home/per/Work/dmd/test/test_results/compilable -o-  -fPIC  -od/home/per/Work/dmd/test/test_results/compilable -of/home/per/Work/dmd/test/test_results/compilable/ddoc4_0.o  -c compilable/ddoc4.d 

[RazvanN7]

Ok, thanks. Btw, should I be able to do

Yes. It seems that you are missing that script. Is it really there?

Also, the pipeline seems to pass the dmd tests, the phobos ones are problematic. So I suggest to not waste any time with dmd tests.

[nordlow]

Ok, thanks. Btw, should I be able to do

Yes. It seems that you are missing that script. Is it really there?

Also, the pipeline seems to pass the dmd tests, the phobos ones are problematic. So I suggest to not waste any time with dmd tests.

Believe me, it is there. Something very strange is going on.

Hmm, when I insert

echo "$DIR"

before

source "$DIR/exported_vars.sh"

I get

/home/per/Work/dmd/test/tools
/home/per/Work/dmd/test/tools

instead of

/home/per/Work/dmd/test/tools

. Seems like the calculation

DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"

has different behaviour on my machine.

Moreover,

echo "${BASH_SOURCE[0]}"

gives

tools/postscript.sh

when doing ./run.d under test. That doesn't seem right either.

Ping, @RazvanN7

[nordlow]

Believe me, it is there. Something very strange is going on.

I had CDPATH defined. If CDPATH is set it echoes the directory passed to cd. For details see https://superuser.com/questions/90535/how-do-i-turn-of-auto-echo-in-bash-when-i-cd.

Created https://github.com/dlang/dmd/pull/13104 that fixes this issue.

[nordlow]

Ok, so it seems I'm ready to start adapting behavior of isSame2 to match std.traits.isSame..

[pbackus]

The behavior of std.meta.isSame is simply incorrect, and leads to absurd results like the following:

import std.meta;

int f() { return 123; }
int g() { return 456; }
int h() { return 789; }

alias functions = AliasSeq!(f, g, h);

// prints "0", but should print "-1" (not found)
pragma(msg, staticIndexOf!(123, functions));

If we're stuck keeping this behavior in Phobos for backwards compatibility, fair enough, but we should absolutely not allow it to metastasize into the language itself.

[nordlow]

The behavior of std.meta.isSame is simply incorrect, and leads to absurd results like the following:

I agree. I'll have that in mind, thanks.

[RazvanN7]

@nordlow any progress on this?

[RazvanN7]

ping @nordlow

[nordlow]

ping @nordlow

This got stalled because it wasn't clear from the comments what behavior we should have for isSame2. And if the naming was of the trait was sane.

For starters, shall we adjust behavior to not match the unsane behavior highlighted at https://github.com/dlang/dmd/pull/13073#issuecomment-939511045?

[nordlow]

Why hasn't is(T == U) been extended to cover most of the sane cases covered by std.traits.isSame(T, U)?

[pbackus]

The is() expression only works on types. isSame works on types, symbols, and values.

[nordlow]

The is() expression only works on types. isSame works on types, symbols, and values.

In the words of @andralex, isn't that a limitation that could/should be relaxed to cover the kinds of AST-nodes that isSame currently covers? That would remove the need for std.meta.isSame or this corresponding builtin trrait altogether. Ping, @WalterBright. I'm curious what packages what kind of breakage such a relaxation would lead to.

[pbackus]

isn't that a limitation that could/should be relaxed to cover the kinds of AST-nodes that isSame currently covers?

Given how confusing is() expressions already are for users learning D, I do not think adding even more special-case semantics to them is a good idea.

[andralex]

FWIW I'd personally very much like is to work on symbols and values as well.