Skip to content

Comments

Expose a copy of std.conv with the DUB package of stdx.allocator#6613

Merged
dlang-bot merged 1 commit intodlang:masterfrom
wilzbach:conv-allocator
Jun 26, 2018
Merged

Expose a copy of std.conv with the DUB package of stdx.allocator#6613
dlang-bot merged 1 commit intodlang:masterfrom
wilzbach:conv-allocator

Conversation

@wilzbach
Copy link
Contributor

This little trick is needed as emplaceRef isn't public (imho it should be, but that's an issue for a different day).
With this patch, the collections library builds with a stable DMD/LDC compiler as it now can use the phobos:allocator package.

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @wilzbach!

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

@PetarKirov
Copy link
Member

We should probably split std.conv into a package where to and emplace are different modules. That way we could expose only std.conv.to on dub, as the rest of std.conv has many dependencies that would also need to be exposed.

pkgDir.buildPath("std", "conv.d")
.readText
.replace("std.conv", "stdx.allocator.conv")
.toFile(destDir.buildPath("conv.d"));
Copy link
Member

Choose a reason for hiding this comment

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

I'm surprised that this works, and I think this can be quite brittle in the future - e.g. emplace may depend on a fix or new feature in some other module. But given that we have a CI to check for such issues, I'd say we're fine for this experimental feature. But these sorts of hacks are not going to be sustainable, so sooner or later we should look into a more future-proof solution.

@dlang-bot dlang-bot merged commit 0de75ca into dlang:master Jun 26, 2018
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.

3 participants