Partially revert some changes to object.d: destroy and array capacity functions

[JinShil]

After the fix in dlang/dmd#10365 and accompanying druntime PRs #2772 and #2783, most of the core.internal API is undocumented (as it should be). Because the core.internal modules do not appear in the documentation, they are no longer a good place to put the destroy and array capacity functions because they are intended to be used by the user. This PR partially reverts #2772 and #2647 so function intended for the user are all in object.d and functions that are intended to be used only by the compiler remain in core.internal.

This is the last PR I intend to submit to clean up object.d. I'm fairly happy with the result. More could be done, such as making object a package instead of a file, but I think I've done enough for now and set a good precedent distinguishing functions intended for the user and functions intended only for the compiler.

[JinShil]

It might be easiest to review each commit separately.

[PetarKirov]

core.lifetime is good candidate for placing destroy. If you also want to keep capacity, etc. out of object.d, then I would suggest core.array.

[JinShil]

@ZombineDev I thought about that, but these are language-defined symbols not library-defined symbols. Right or wrong, they're available to all programs without having to import anything. I worry that by placing them in a public module like core.lifetime or core.array, they'd have to be public, and that makes the distinction between language and library ambiguous.

I think if we want to go further than what I've already done we should split object.d into object/object.d, object/lifetime.d, object/array.d, etc. and then pull them all together into one object/package.d. The language would still implicitly do import object; and all should work. I wouldn't mind doing that work today, but I worry that I'm exercising the patience of reviewers with all this refactoring.

[JinShil]

That being said, I'd be ok with moving things to core.lifetime and core.array, if that is preferred.

[PetarKirov]

I thought about that, but these are language-defined symbols not library-defined symbols.

I don't want to get into nitpicking, but IMO language-implementation symbols are all those __* and _d_* symbols that you recently moved to core.internal.*. They're not supposed to be called by the users, but by the compiler. If they're not defined certain language features won't work.

On the other hand destroy and capacity are purely library defined symbols meant for use by the user and obviously this is why they're public. Even if destroy is not defined, the rest of of the language will continue to work just, fine, because the actual language implementation is typeid(obj).destroy(&obj).
Same for capacity - all uses of arr.capacity could be replaced with arr.length, but with a performance degradation. I.e. capacity is runtime API (not language one) used for optimization purposes, when working with dynamic arrays backed by the GC. In a sense, .capacity, .length, etc. are APIs of the GC [dynamic array implementation].

I thought about turning object.d into a package and that's sounds nice, but actually I don't see a reason, when we already have the core package, where all public user or compiler facing symbols are defined. Just like core.atomic is part of the language and hence it would be a legit design to have it included in object.d, but it's much better to have it separate in core., I think that all the rest of the public symbols in object.d could be moved (and perhaps should) to core.*. One obvious candidate IMO is core.typeinfo (or core.rtti) for TypeInfo*.

[PetarKirov]

That being said, I'd be ok with moving things to core.lifetime and core.array, if that is preferred.

Let's wait for other reviewers to chime in.

[JinShil]

I want to leave reviewers with something to consider:

If, for example, length is moved to core.array, then that means users will be able to do something like this:

static import core.array: length;

writeln(core.array.length(myArray));

That isn't idiomatic D, but by moving the symbols to core.array we'd open that possibility. If users do such a thing, no matter how unlikely, we will no longer have the liberty to organize length and other similar symbols as we choose within druntime. They will need to remain in core.array indefinitely or be subject to a deprecation process.

If you'd like me to continue cleaning up object.d, my recommendation would be to split object.d into a package (e.g. object/object.d, object/lifetime.d, object/array.d, with object/package.d publicly importing the latter) That would accomplish the task of breaking up object.d into logical, encapsulated parts, but would prevent the language-defined properties from appearing in multiple public APIs (implicitly imported object and explicitly imported core.array) and also prevent the situation described in the prior paragraph.

If you'd like me to put an end to the refactoring (at least for now), I recommend pulling this PR.

But, again, I'll do whatever the community decides.

[thewilsonator]

That being said, I'd be ok with moving things to core.lifetime and core.array, if that is preferred.

That seems fine to me.

[CyberShadow]

If, for example, length is moved to core.array, then that means users will be able to do something like this:

You could declare length in core.array as __length, then in object.d, do public import core.array : length = __length; or such.

[JinShil]

You could declare length in core.array as __length, then in object.d, do public import core.array : length = __length; or such.

That won't work well due to documentation. When publicly imported the import declaration will contain a link to the core.array.__length declaration. Then the all documentation, including documented unit tests, will be in terms of __length instead of length.

Example:

/// See $(REF __length, core,array)  --> redirects user to `core.array.__length` documentation
public import core.array : length = __length;

[CyberShadow]

Well, if length is meant to be used through object, then I guess it should also be documented in object. You can't really attach documentation to a public renamed import, so, aside from the usual version(StdDdoc) approach, I guess the alternative would be a forwarding function.

[JinShil]

Right. so then I don't see moving it to core.array much better than just leaving it in object.d.

[WalterBright]

Can we please stop with all these endless code reshufflings?

I don't think they matter at all, for the simple reason that I can never predict where any function is. I use grep -r to find them.

object.d is special because the more imports it does, the slower compiles will be. File lookups are very slow. An object.d that consists of lots of imports is a slow idea. I.e. making it a package is not going to fly.

[JinShil]

Ping.

Please understand why these refactorings were done. We have an ongoing effort to convert some runtime hooks to templates to avoid dependency on runtime type information and to make D a more pay-as-you-go language. If we keep moving said converted templates to object.d, we'll eventually have the entire runtime in object.d. Furthermore, object.d is currently unwieldy causing merge conflicts and horrendous diffs for simple changes.

This PR is the last of the refactorings bringing that effort to a close. More could be done, but the tolerance for these refactorings has faded, and with the refactorings done recently, moving templated runtime hooks to object.d is no longer an escalating problem.

Please merge this PR so we can bring this refactoring effort to a close. It's the last one for this stage of work. Thank you.

[CyberShadow]

So what about the forwarding function idea?

Also, if you put the import inside the function, it will speed up compiling programs that never call these functions.

[JinShil]

So what about the forwarding function idea? Also, if you put the import inside the function, it will speed up compiling programs that never call these functions.

By "forwarding function idea", I think you're referring to this...

size_t hashOf(T)(auto ref T arg)
{
    static import core.internal.hash;
    return core.internal.hash.hashOf(arg);
}

... instead of...

public import core.internal.hash : hashOf;

Yes, that would be nice to avoid the import until the template is instantiated. However, there are a couple of issues with it.

1. For reasons I don't yet understand, it doesn't always work. I tried it with hashOf and dup and it caused failures. Either the compiler is giving these symbols special treatment or there are bugs in the language preventing the forwarding of templates in this manner. I wasn't able to figure it out in the time I allotted to it.
2. I don't see enough evidence that the public imports significantly slows compilation to justify the change. What I've seen is a few ms increase that does not scale with code size (i.e. the same few ms is paid regardless of how large the project being compiled is)
3. I think there might actually be a runtime performance cost if one does not compile with -inline due to the extra level of indirection and the fact that @WalterBright doesn't want to always inline pragma(inline, true) despite the fact that the programmer explicitly told the compiler to do it.
4. It's out of scope of my original goal. My primary goal was to put an end to the escalating trend to move runtime hook templates to object.d, and that was achieved. I would like to take things further, but given @WalterBright's comments, all I see are obstacles, and I have to pick my battles. I would like to move on to other things now.

[CyberShadow]

2\. What I've seen is a few ms increase that _does not_ scale with code size (i.e. the same few ms is paid regardless of how large the project being compiled is)

But it does scale with the number of imports avoided! If one repeats an "it's just one millisecond" argument a thousand times, it's no longer just one millisecond.

I agree with everything else, thanks.