Skip to content

Conversation

@schveiguy
Copy link
Member

@schveiguy schveiguy commented Apr 12, 2018

Having imports in version(unittest) blocks means that user code being unit tested will also import these files incurring a compile-time penalty.

Should help with dlang/dmd#8124 a bit, at least for the version(unittest) portion.

I will submit more PRs for the rest of Phobos, I wanted to make these PRs small and reviewable.

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @schveiguy!

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 + phobos#6450"

assert(r[0 .. lo].all!(x => !lt(p, x)));
assert(r[hi + 1 .. r.length].all!(x => !lt(x, p)));
// this used to import std.algorithm.all, but we want to save
// imports when unittests are enabled if possible.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the comment here to prevent regression.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be version (StdUnittest) anyway?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StdUnittest was reverted because apparently -deps includes all unittests and thus usage of StdUnittest caused breakage for projects using -deps

@schveiguy
Copy link
Member Author

ping @marler8997 here's the first one.

@JackStouffer
Copy link
Contributor

std/array.d(557:1)[warn]: A unittest should be annotated with at least @safe or @system
std/array.d(576:1)[warn]: A unittest should be annotated with at least @safe or @system

@schveiguy schveiguy force-pushed the version_unittest_p1 branch from 4cfe9bc to f2e5222 Compare April 12, 2018 19:31
@schveiguy
Copy link
Member Author

schveiguy commented Apr 12, 2018

@JackStouffer fixed

Edit: also fixed missing style space (grrr....)

@schveiguy schveiguy force-pushed the version_unittest_p1 branch from f2e5222 to adaba54 Compare April 12, 2018 19:47
@dlang-bot dlang-bot merged commit baed440 into dlang:master Apr 12, 2018
@schveiguy schveiguy deleted the version_unittest_p1 branch April 12, 2018 21:13
@schveiguy
Copy link
Member Author

FYI @marler8997 I have a few more modules to do, they are the tougher/more controversial ones. Will get to them next week.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants