On 10/06/2020 07.43, Matthew Bentley via SG14 wrote:
> Hi Jens,
> new version of colony proposal is online here:
> https://htmlpreview.github.io/?https://github.com/WG21-SG14/SG14/blob/master/Docs/Proposals/D0447R11%20-%20Introduction%20of%20colony%20to%20the%20Standard%20Library.html

Thanks.

> ie. same location as last.
> If you have time, I'd be interested to hear your thoughts, either over email or via the SG14 zoom call on June 11th (this's an open meeting, Michael Wong's invite info is forwarded below, Colony is the first topic on the agenda).

As a long-time contributor to WG21, I'll gate-crash any SG meeting
that I feel like joining (in these "can't go to the movies" times),
regardless of whether it is designated as "open" or not. :-)

> I've implemented all your changes (I think),

A few quick remarks; I won't have time for details before tormorrow's call:

We still lack a specification for the "Skipfield" parameter in the
normative wording. (Appendix A is not part of the proposed normative
wording, I suspect.)


operator==, operator!= in the class synopsis

I think the new preferred style is to put these as hidden friends
inside the class. This presumably helps overload resolution
performance. Also, operator!= can go, because the new operator
rewrite rules will automatically obtain != from operator==.


"Insertion location is the back of the container when no erasures have occured, when erasures have occured it will re-use existing erased element memory spaces where possible, and insert to those locations."

This is splicing two sentences ("when erasures have occured"). At the very least,
use a semicolon. A period (full stop) appears to be the better choice here,
though.


The synopsis shows superfluous semicolons at the end
of the definition of the default/limit constructors.


The colony(limits) constructor should be explicit.

The colony(limits) constructor doesn't forward its argument.

explicit colony(size_type n, std::limits block_capacity_limits = ...)

should simply say "implementation-defined" after the "=" sign, and not
try to depict a constructor call to std::limits.

Similar for the subsequent constructors.


The whole "block" specification is still underwhelming.
We have two mentions each of "min_block_capacity" and
"max_block_capacity" in the entire document, but nowhere does it
say what that's supposed to mean. Please, also add a paragraph
preceding the class synopsis introducing the "block" concept and its
vital parameters (max size, min size; maybe more). (You appear to
have decided that you need to talk about blocks in the normative
wording.)

I thought you had agreed that "reshape" was a better name
than set_block_capacity_limits. The execution in the wording
is lacking.


"If container is not empty, T shall be MoveInsertable or CopyInsertable into *this."

I think it should be "If the container is empty" (add definite article, for
improved English happiness).
Also, we can't have compile-time constraints ("MoveInsertable") apply depending
on run-time conditions ("empty").


The "iterators have relational operators" specification needs to
materialize in normative wording. Just saying that they "have" those
operators is not enough; you also need to say what the semantics are.


Jens