Skip to content

Comments

test name mangling of core.stdc.stdint types#8252

Merged
dlang-bot merged 1 commit intodlang:masterfrom
WalterBright:teststdint
May 22, 2018
Merged

test name mangling of core.stdc.stdint types#8252
dlang-bot merged 1 commit intodlang:masterfrom
WalterBright:teststdint

Conversation

@WalterBright
Copy link
Member

Types from core.stdc.stdint should mangle to the same as the corresponding C++ compiler does.

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @WalterBright!

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 fetch digger
dub run digger -- build "master + dmd#8252"

@WalterBright
Copy link
Member Author

Blocked by dlang/druntime#2183

@Geod24
Copy link
Member

Geod24 commented May 16, 2018

Couldn't we put this in druntime ?

@WalterBright
Copy link
Member Author

Couldn't we put this in druntime ?

I thought about it, but core.stdc.stdint is really a "header only" module, and it just didn't seem right to put tests in it. Besides, getting the mangling right isn't the responsibility of druntime, it's the compiler.

@rainers
Copy link
Member

rainers commented May 17, 2018

Shouldn't this actually test against a C++ compiler? Could be placed into externmangle.d/externmangle.cpp.

I thought about it, but core.stdc.stdint is really a "header only" module, and it just didn't seem right to put tests in it.

I agree, but you can also put tests into druntime/test.

@WalterBright
Copy link
Member Author

Shouldn't this actually test against a C++ compiler?

It's not really necessary since C++ compilers pretty much never change their mangling. The test cases were made from cut&pasting the output of the C++ compiler. Testing .mangleof is simpler (all in one file), and easier to debug when it goes wrong (no linker errors to figure out - the error prints what the string is and what it should be).

I agree, but you can also put tests into druntime/test.

It could, but I like it here.

@JinShil
Copy link
Contributor

JinShil commented May 18, 2018

It could, but I like it here.

The problem with the tests being here, is that dlang/druntime#2183 can't be fully verified. It would be best to add the tests to dlang/druntime#2183, show that it's green, and then after that is merged, move them here to DMD.

@WalterBright
Copy link
Member Author

Getting the name mangling correct is the job of the compiler, not druntime.

Inevitably there isn't a sharp line separating druntime from dmd, and some judgement will be necessary now and then. This is one such case.

@JinShil
Copy link
Contributor

JinShil commented May 18, 2018

Druntime PR was merged. Should no longer be blocked.

@WalterBright
Copy link
Member Author

Blocked by dlang/druntime#2188

@WalterBright WalterBright added Review:Vision Vision Plan https://wiki.dlang.org/Vision/2018H1 and removed Merge:Blocked labels May 20, 2018
@dlang-bot dlang-bot merged commit 09f81bb into dlang:master May 22, 2018
@WalterBright WalterBright deleted the teststdint branch May 22, 2018 18:04
@wilzbach
Copy link
Contributor

FYI this test is now failing on the self-bootstrapped SemaphoreCI:

Test compilable/teststdint.d failed.  The logged output:
../generated/linux/release/64/dmd -conf= -m64 -Icompilable  -fPIC  -odtest_results/compilable -oftest_results/compilable/teststdint_0.o  -c compilable/teststdint.d 
compilable/teststdint.d(211): Error: static assert:  `"_Z10int_fast16i" == "_Z10int_fast16l"` is false

@WalterBright
Copy link
Member Author

On my Ubuntu linux box, the mangling from the C version of the code is _Z10int_fast16l. The line in core.stdc.stdint for fast16 is:

        alias int_fast16_t  = cpp_long;

which all looks right. Anyhow, why would it pass all the other linux tests? What is different about semaphoreci?

@wilzbach
Copy link
Contributor

SemaphoreCI uses a compiler built from the PR, to build itself (instead of just using the host compiler as the other CIs do):

Use host DMD to build DMD ->use this built DMD with fresh DMD -> run tests

This means that one should be able to reproduce this if with dmd-nightly or using a freshly built host compiler via HOST_DMD.

I'm sorry that I can't be of more help for the next days as I am still traveling with my phone only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Merge:auto-merge Review:Vision Vision Plan https://wiki.dlang.org/Vision/2018H1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants