dlang.org icon indicating copy to clipboard operation
dlang.org copied to clipboard

Fix Issue 22141 - Property .capacity is not listed in the array prope…

Open RazvanN7 opened this issue 1 year ago • 9 comments

…rties section

RazvanN7 avatar Jul 29 '22 08:07 RazvanN7

@thewilsonator done. It was a copy paste error on my part.

RazvanN7 avatar Jul 29 '22 08:07 RazvanN7

Is capacity actually part of the language spec, or just a runtime library feature? It is documented in druntime's docs: http://druntime.dpldocs.info/object.capacity.html

adamdruppe avatar Jul 29 '22 11:07 adamdruppe

Is capacity actually part of the language spec, or just a runtime library feature? It is documented in druntime's docs: http://druntime.dpldocs.info/object.capacity.html

I think that this distinction is rather moot. It's a property that users may use, therefore it should be present in the docs. Since the compiler frontend specifically knows about capacity, I would say that it should be included, if that's not the case, in the language spec. Any alternative runtime should implement capacity otherwise you will end up with linker errors.

RazvanN7 avatar Jul 29 '22 12:07 RazvanN7

Is capacity actually part of the language spec, or just a runtime library feature? It is documented in druntime's docs: http://druntime.dpldocs.info/object.capacity.html

I think that this distinction is rather moot. It's a property that users may use, therefore it should be present in the docs. Since the compiler frontend specifically knows about capacity, I would say that it should be included, if that's not the case, in the language spec. Any alternative runtime should implement capacity otherwise you will end up with linker errors.

I agree. It's not hard to dummy the capacity to be the same as length, in case some runtime doesn't support querying the BlkInfo, so I would say we should enforce this to exist to increase portability.

ljmf00 avatar Jul 29 '22 13:07 ljmf00

It's a property that users may use, therefore it should be present in the docs.

It is present in the docs.

Since the compiler frontend specifically knows about capacity,

Where? I just grepped it and the only time I see it appearing is

    counter += l.capacity; // Make sure l is not optimized out.

in a test file, where it is just used as a hack to the optimizer instead of any actual spec test.

There are no references to the _d_arraysetcapacity implementation

Any alternative runtime should implement capacity otherwise you will end up with linker errors.

It would be compile errors, not linker errors, if and only if the function is actually called. Which is the same as any other function or module in there.

What about assumeSafeAppend? Or core.thread and core.sync which are mentioned as "built-in" features elsewhere in the spec (see the betterC page for example)?

.dup and .idup used to be built in, just like .ptr and .length. But they aren't anymore either. So really, why are there some druntime functions listed here and other ones not? It seems completely arbitrary.

BTW speaking of betterC (which is terrible and should be removed), if this PR is merged, dmd -betterC would no longer be compliant with the spec. Which is fine by me, again, it is terrible and should be removed, but you're introducing an inconsistency.

void main() {
        int[5] s;
        assert(s.capacity == 0);
}

$ dmd sppp -betterC
/home/me/d/dmd2/linux/bin64/../../src/druntime/import/object.d(3661): Error: `TypeInfo` cannot be used with -betterC

adamdruppe avatar Jul 29 '22 13:07 adamdruppe

Where? I just grepped it and the only time I see it appearing is

Wow, you are right. I just looked at the dmd code and it seems that the compiler simply tries UFCS for a a.b expression if b is not a field/property of a. Since capacity is a template defined in druntime it just works. I find this very brittle. I assumed that since it worked, the compiler specifically recognizes capacity and then lowers to a call to druntime.

I see your point now, capacity is not an array property, but rather a convenience function that is implemented in druntime. Still, from a user perspective, capacity looks like an array property. What happens behind the scenes (a hack, if you ask me) is not relevant in this case.

What about assumeSafeAppend? Or core.thread and core.sync which are mentioned as "built-in" features elsewhere in the spec (see the betterC page for example)?

I don't see the relevance of this example. We are talking about the user perspective here, not how the compiler devs categorize the myriad of functions that are being called implicitly by the compiler.

.dup and .idup used to be built in, just like .ptr and .length. But they aren't anymore either. So really, why are there some druntime functions listed here and other ones not? It seems completely arbitrary.

Because you can directly call them as if they were properties of an array without any intervention whatsoever. In my opinion, if you can do array.whatever without needing to import anything, then whatever should be in the list of properties for arrays, regardless of how the compiler actually resolves the symbol.

RazvanN7 avatar Jul 29 '22 13:07 RazvanN7

(a hack, if you ask me)

.sort used to be a built-in property too. Then it got replaced by a UFCS function. This "hack" is the direction the language has been going for some time.

I don't see the relevance of this example.

assumeSafeAppend is one way how you explicitly modify capacity (another one being reserve). It is another function in object. From the user perspective, capacity, reserve, and assumeSafeAppend all work exactly the same way.

As you yourself said: "if you can do array.whatever without needing to import anything, then whatever should be in the list of properties for arrays"

(I disagree with that statement, but my point is that your position is inconsistent with itself.)

Because you can directly call them as if they were properties of an array without any intervention whatsoever.

They're exactly the same as any other library import, including the possibility of overrides and conflicts:

module cap2;
void idup(T)(T t) {}
import cap2;
void main() {
        int[] a;
        a.idup;
}

cap.d(4): Error: function cap2.idup!(int[]).idup at cap2.d(2) conflicts with unction object.idup!int.idup at /home/me/d/dmd2/linux/bin64/../../src/druntime/import/object.d(3555)

That does NOT happen with the fully built ins: try changing idup to length there and find no error. length is built into the compiler, idup isn't.

So, what's the actual logic here in terms of the spec?

adamdruppe avatar Jul 29 '22 14:07 adamdruppe

@adamdruppe sort, reserve and assumeSafeAppend modify the array. capacity does not, it's just a getter property similar to length. Fair point about built in properties vs runtime functions and overloading. (Unlike sort, I think capacity is something only the runtime can calculate. It couldn't be implemented outside the runtime without knowledge of the internal runtime array code not in a public API.)

ntrel avatar Jul 30 '22 08:07 ntrel

Thanks for your pull request and interest in making D better, @RazvanN7! 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

Auto-close Bugzilla Severity Description
22141 enhancement Property .capacity is not listed in the array properties section

dlang-bot avatar Aug 12 '22 08:08 dlang-bot

I think that a better litmus test for whether the language spec should specify that $fun is a built-in property of arrays (or any other type), as opposed to only listing $fun in the library docs, is to imagine a completely independent implementation of the language and to ask the question: what is the minimum amount of features that they must implement, for their implementation to be considered compliant with the D language spec? And specifically, whether $func is part of this minimal set of features.

Now, an argument can be made about Druntime and Phobos: are competing implementations of the language supposed to be able to reuse Druntime and Phobos, with minimum to no changes from the reference implementation? Perhaps only Phobos? But what about core.sys - surely that's so simple that any compliant implementation of language should be able to reuse it? And if you look at the other stuff in core.*, most (all?) of them look like regular D code that any user of the language should expect to be able to write for their project.

TL;DR Instead of arguing about is an isn't part of the language based on the current implementation, we should decide what is the minimum set of stuff that is supposed to be. Otherwise, there will be always push and pull about whether we should document the implementation or change it to match the docs.

PetarKirov avatar Oct 05 '22 06:10 PetarKirov

Honestly, I find the distinction irrelevant. The reader of the docs is 99.9% of times someone who doesn't care about the implementation. For that particular person, the fact that you can use .capacity without importing anything makes it look like it is a builtin property. The fact that .capacity is implemented as a template in druntime is an implementation detail that nobody actually cares about so it could be documented as a property because it looks like one.

If we want to highlight the distinction between builtin properties and druntime calls we can do that, but that brings exactly 0 value to anyone reading the docs.

RazvanN7 avatar Oct 05 '22 08:10 RazvanN7