gnzlbg / static_vector

A dynamically-resizable vector with fixed capacity and embedded storage
https://gnzlbg.github.io/static_vector
167 stars 21 forks source link

Casey's feedback round 2 #31

Closed gnzlbg closed 5 years ago

gnzlbg commented 6 years ago

@CaseyCarter I fixed most of the issues you mentioned in the last commit but these ones remain open:

I don't know how to fix them yet, but will try to do so before the next deadline.

CaseyCarter commented 6 years ago

implementation of range constructor, insert, and emplace so that it is constexpr for trivial Ts and only requires EmplaceConstructible (the prototype value initializes all elements and then assigns to the from the range)

I'm reasonably certain there is no way to implement these operations under these constraints - you'll need to add requirements on T for the is_trivial<T> case. Or add the requirements generally and slightly overconstrain the non-trivial case.

the first version of the proposal used is_literal_t but I cannot recall why I changed it

Probably because is_literal is deprecated in C++17. I'm suggest is_trivially_copyable_v<T> && is_default_constructible_v<T> as a slight improvement over is_trivial: it doesn't really matter if T's default constructor is trivial, since you're going to value-initialize the objects anyway.

gnzlbg commented 6 years ago

@CaseyCarter

About: "constexpr adds a run-time cost to the initialization of fixed_capacity_vectors of "trivial" types"

I've mentioned that very large fixed_capacity_vectors are not the target use case of the class (they have many performance problems when compared against std::vector, like O(N) moves). I've also mentioned that if constexpr is improved to allow proper usage of std::aligned_storage in constexpr contexts, this issue can be fixed in a backwards compatible way.

I'm suggest is_trivially_copyable_v<T> && is_default_constructible_v<T> as a slight improvement over is_trivial

done.

destructor cannot be implicitly generated (the prototype does this, but maybe it is wrong?)

So I've removed the implicitly generated bits. The only requirement is that the destructor must be trivial if is_trivially_copyable_v<T> && is_default_constructible_v<T>.

CaseyCarter commented 6 years ago

The only requirement is that the destructor must be trivial if is_trivially_copyable_v<T> && is_default_constructible_v<T>.

is_trivially_copyable_v<T> implies is_trivially_destructible_v<T>.

gnzlbg commented 6 years ago

@CaseyCarter my time is extremely limited to work on this (I don't know how you manage). I have tried to improve it a bit, but maybe you could suggest what to prioritize. If you would only have time to fix one thing, what would that be?

The deadline for the next maling is 2018-02-12 14:00 UTC and I'd like to submit the revised paper one week before so that I don't miss the deadline again (is this enough? or should I do that sooner?).

CaseyCarter commented 6 years ago

If you would only have time to fix one thing, what would that be?

Since the paper is going to LEWG and LWG likely won't see it in JAX, focus less on nitpicky specification details and more on design surface.

is this enough? or should I do that sooner?

I've never had a problem submitting the Friday before the mailing deadline. (FYI Herb mentioned to me yesterday that there is a self-serve automated paper submission and P-number assignment system in the works which will make this process 100x better.)

gnzlbg commented 5 years ago

I've opened a new issue to track updating parts of container requirements where array is mentioned: #38