Fix #13872

[Panke]

std.container.make!(Array!T) now returns a reference
to an actual container, not a 'null reference' that
initializes itself upon use.

A more elegant solution would be to provide an overload for Array, but if I define it in std/container/array.d it will issue a conflict (even if the template is more specialized) and to do it in std/container/util.d would require to import std.container.array and I am not sure if we want that, because of the ongoing effort to decouple the imports in phobos. Regardless, setting length to 0 shouldn't be a problem for any struct based container constructed without arguments.

[JakobOvrum]

I think this is a terrible hack, and some containers don't even have length, like linked lists; this patch would make it so that make informally guarantees identity, but only when the container has settable length... sounds rather arbitrary. Array's behaviour is consistent with in-built AAs, for what it's worth.

make does not currently guarantee identity for its return value, so I don't consider this a bug, and while I agree there needs to be a good way to give identity to these lazily-initialized containers, I don't think this is the way to do it.

[Panke]

First about the hack. Yes it is a terrible hack, but I is no problem that not all container support length nor that's is kind of arbitrary. It only has to work with std.containers and it works with all containers currently supported.

Second make's description says, that make should eliminate construction differences between class-based and struct-based containers. As far as it stands now, it does not.
The different std.container currently differ in their semantics for the most fundamental primitive their is: construction.

Third: I only made this patch, because I wanted to provide a example on how to construct an Array!(Array!int). Every approach I used was too embarrassing to put into the documentation. "You know, because make returns something like a null reference you'll have to add an element and remove it again, to force the payload to be initialized, after that Array will have proper reference semantics.".

I don't like this either, but we really have to do something about this. This is the best way, IMO.

[yebblies]

Why can't we just add a way to make an empty container to the actual containers? I can understand why the default init would be a null container, but making an empty one is pretty fundamental.

First about the hack. Yes it is a terrible hack, but I is no problem that not all container support length nor that's is kind of arbitrary. It only has to work with std.containers and it works with all containers currently supported.

But it should work with other containers too, so we either have to document setting length as the official way to do this, or (my preference) add a less obscure primitive to do it.

[Panke]

But it should work with other containers too, so we either have to document setting length as the official way to do this, or (my preference) add a less obscure primitive to do it.

I'm not so sure, that std.container.make is meant to be used with user defined containers. It is there to hide implementation details after all. As far as I understand, the primitive to create an empty container is make.

I have no hard feelings about this, though we should prefer the hacky solution, that our users cannot see over an more complicated interface.

Changing the documentation to make arbitrary behaviour correct by definition, is a even worse solution.

[yebblies]

I'm not so sure, that std.container.make is meant to be used with user defined containers

I'm not so sure either, and to be honest I've never used std.container. Maybe @andralex can comment on this?

[andralex]

I don't think it's a hack - indeed it was the intent of make in the first place, I can't believe I missed that. Creating a null container is readily doable by means of Whatevs.init or just creating a default-initialized Whatevs. make returning a null container essentially adds zero functionality.

[JakobOvrum]

make creating non-null containers makes sense, that's not the problem, it's the way it's done in this patch.

[yebblies]

What about calling the constructor with typeof(T.init[].front)[].init when no args are passed? That should give an empty but initialized container reliably.

[andralex]

@JakobOvrum oh I see, thanks. Perhaps it would make sense to require a primitive at the container level.

[andralex]

(though forcing a copy construction is also interesting)

[Panke]

@yebblies Why should this be more reliable?

alias T = Array!int;
is(typeof(typeof(T.init[].fornt)[].init) == int[]) or am I wrong?

This will call the constructor taking Stuff, which will call reserve(0), which in turn initializes the payload, which is as arbitrary as setting length to zero directly.

Or does it call the constructor taking U[] values...? There is nothing in the docs of std.container that would forbid a user defined container to

if(values.length == 0) return;

at the top of the constructor, making it exactly as unreliable as any other method.

Besides: https://issues.dlang.org/show_bug.cgi?id=13919

Does make even have to work with anything outside of std.container?

[yebblies]

We could alter the spec so that calling the constructor with an empty array guarantees the container will be initialized, is what I mean. I'm not so sure that we can rely on setting length to zero to do that, it seems much more reasonable that setting length to zero could completely wipe the container (or it might be impossible for some containers.)

I think that expecting that calling the constructor with an array that may or may not be empty to always initialize is reasonable.

Does make even have to work with anything outside of std.container?

This is what I was hoping @andralex could answer.

[andralex]

Does make even have to work with anything outside of std.container?

Not for the time being.

[JakobOvrum]

Does make even have to work with anything outside of std.container?

The proposed solution doesn't work with all standard containers, it only works with containers that have length (and settable length, at that). That already precludes the standard SList and DList.

There's also no guarantee that setting length to 0 causes identity. I think it's clear that we need to either amend an existing primitive or create a new one to facilitate this, like @yebblies' idea.

[Panke]

The proposed solution doesn't work with all standard containers, it only works with containers that have length (and settable length, at that). That already precludes the standard SList and DList.

You're right. Actually I thought currently Array were the only struct based container. So, how to do it?

[yebblies]

Pass an empty array to the array constructor.

[Panke]

I've added two additional tests for SList and DList and now an empty container is constructed by passing an empty array. I'm using a static one though to avoid an allocation. I hope that flies.

[quickfur]

ping
Are we going ahead with this, or does something more need to be done first?

[dgiagio-druid]

Where is "result" used in this function?

[Panke]

Thanks, will fix.

[JakobOvrum]

The description of the construction primitive C(x) should be updated to note that construction with empty ranges guarantees identity for containers with reference semantics. Otherwise LGTM.

[Panke]

Ping. Dunno if everyone gets a message if I just rebase.

[yebblies]

Ping. Dunno if everyone gets a message if I just rebase.

Nobody gets a message.

[JakobOvrum]

LGTM. @yebblies?

[yebblies]

Auto-merge toggled on