Do not import a module you don't need

[ryuukk]

I'd personally not call this function at all

[dlang-bot]

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

[CyberShadow]

Sorry, would you mind explaining in more detail why this an improvement? I'm confused because it deletes one line and adds twenty.

[ryuukk]

Sorry, would you mind explaining in more detail why this an improvement? I'm confused because it deletes one line and adds twenty.

why import std.exception only just to do:

array = null;

if i want to import std.path, i only want the compiler to know about std.path, not std.exception, unnecessary lookup, unnecessary work for the compiler

As i said, i'd rather not call this function to begin with

"why my compile time becomes 2seconds whenever i import std.x ??"

[CyberShadow]

I think your argument will be stronger if you include some benchmarks which show the improvement in compilation time brought by this change.

Either way, I really don't think this is a good approach to this general problem. If we take that logic to the extreme, we'd just copy-paste all dependencies into every Phobos module and not have anything import anything.

Such changes may give us small improvements in compilation time, but we will need to keep paying the cost in maintaining duplicate or overcomplicated code.

A perfect solution would be to make the compiler delay parsing of declarations only until it knows it needs them; then, importing big modules just to use one small function would not be a problem at all. In the interim, a better solution may be to move commonly used declarations into tiny modules in an internal namespace, and import those instead.

[ryuukk]

I can't believe what i am reading

Do you understand the problem?

import std.exception

only just to do:

array = null

We are not yet april 1st right?

[CyberShadow]

I don't think array = null here is actually needed at all, you can remove it from the implementations you added.

The problem is that the alternative is a cast, which loses the semantic meaning of the original code. It's a good idea to define correspondingly named functions which do something simple but for a not-entirely-obvious reason (why are we casting away constness? -> because we are moving the only reference (trust me), so it's safe to do so). It's even a good idea to define multiple functions with the exact same body, if the circumstances of using them are different.

Such changes decrease code readability and make the code quality of Phobos worse overall, which is not something we should take lightly.

We are not yet april 1st right?

I'm trying to hold this conversation in good faith, but, if you're going to be like that, you'll have to convince an organization admin, because I'm out.

[dkorpel]

The result of array can be implicitly converted to immutable, no need for assumeUnique here.

[dkorpel]

Suggested change

	return () @trusted { return assumeUnique(result.array); } ();
	return result.array;

[dkorpel]

Superseded by #8726