Skip to content

Add more test coverage (runtime lowerings) for betterC#8308

Merged
dlang-bot merged 1 commit intodlang:masterfrom
JinShil:betterc_tests
May 30, 2018
Merged

Add more test coverage (runtime lowerings) for betterC#8308
dlang-bot merged 1 commit intodlang:masterfrom
JinShil:betterc_tests

Conversation

@JinShil
Copy link
Copy Markdown
Contributor

@JinShil JinShil commented May 30, 2018

This adds test coverage to betterC to ensure dlang/druntime#2194 doesn't break code. It will probably need to be expanded, but this is what I have for now.

@dlang-bot
Copy link
Copy Markdown
Contributor

Thanks for your pull request, @JinShil!

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#8308"

@JinShil JinShil added Review:Easy Review Review:Vision Vision Plan https://wiki.dlang.org/Vision/2018H1 labels May 30, 2018
@JinShil JinShil added Review:WIP Work In Progress - not ready for review or pulling and removed Review:Easy Review labels May 30, 2018
@JinShil JinShil removed the Review:WIP Work In Progress - not ready for review or pulling label May 30, 2018
@jacob-carlborg
Copy link
Copy Markdown
Contributor

We can’t do this in druntime? Didn’t you already create a betterC test in druntime?

@JinShil
Copy link
Copy Markdown
Contributor Author

JinShil commented May 30, 2018

This is not actually testing the druntime implementation. What this test does is ensure DMD lowers the expression to the runtime implementation and calls it properly. Well, actually, this does test the runtime implementation too, but that is by accident.

IMO this is the correct spot for this test as -betterC is a compiler flag, and we want to make sure -betterC code is making the right calls to the runtime implementations.

That being said, we do need runtime implementation tests as well, but those should be in the druntime repository in the form of unittests, and they should call the runtime implementations directly (e.g. __equals(x, y), as opposed to x == y). I've asked @wilzbach, privately, for help to modify the druntime makefiles so we can run the druntime unittests under the -betterC compiler flag. So, the druntime CIs would then run the unittests twice: once without -betterC, and once with. Then we can add version(D_BetterC) conditional compilations to the druntime unittests to verify the druntime PRs.

I hope that make sense.

@JinShil JinShil added the Review:Blocking Other Work review and pulling should be a priority label May 30, 2018
@dlang-bot dlang-bot merged commit ff9518d into dlang:master May 30, 2018
@JinShil JinShil deleted the betterc_tests branch June 3, 2018 22:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Merge:auto-merge Review:Blocking Other Work review and pulling should be a priority Review:Vision Vision Plan https://wiki.dlang.org/Vision/2018H1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants