freestanding namespace #90
Conversation
|
btw, current single-header-ness is not intentional, feel free to break the current header into multiple header files. |
that would open up a whole new discussion on how we would want to split it :D and I would rather leave it for a different (refactoring) PR As for this PR, I'm also tempted to add some convenience functions for the freestanding implementation (e.g., |
Yeah, also there's no none-freestanding-delete insert operation... Edit: That will have to be throwing. I say we should get the first minimal implementation started, we can then gradually decide which function our implementation want to keep, we definitely want to at least have "no freestanding-delete functions", "some freestanding-delete functions", "all freestanding-delete functions" as I think current "no freestanding-delete functions" are non-ergonomic. |
|
The problem is if we want to do this gradually, we might might-as-well consider continuing with the approach in #77 . So I think it will be helpful for us to scratch out the approach. |
With the previous approach, having to globally choose either a freestanding or non-freestanding implementation is a dealbreaker for me. Of course, I could just fork and do whatever I want, but I would rather avoid it, especially with such a big changes that would be hard to sync.
I guess it depends on what the end goal of a freestanding implementation is. For me, it's about getting compile time warnings when trying to use any potentially unsafe functions, but in theory there's more to freestanding than just that... |
|
I mean if your intention is to have an inplace vector that does not have any actively throwing functions, you'll need to delete any functions that increase the size of the vector without checking the capacity in advance, as they're required to throw. Along with Which I think is the current list of freestanding-deleted functions. |
|
If no throwing function is the intention, then having the standalone struct is actually a better idea. But I think if we are trying to play the line of "ah but this function has great utility, we should leave it in", we will have throwing functions otherwise the implementation will be non-compliance. I think I will reverse my previous stance, I am for a separate struct that is in the freestanding namespace, and I am for the struct to delete all freestanding-delete functions without leaving in convenience functions. |
I'm also fine with deleting all of them. Convenience functions are definitely not mandatory. |
Let's do that, if someone wants non-throwing convenience function in a restricted context they can disable exception. |
|
Actually, I will just add testing here. |
JeffGarland
left a comment
JeffGarland
left a comment
There was a problem hiding this comment.
I'm going to approve this PR as it looks like it's basically ready. The one big gaping omission here is an update to readme regarding the freestanding option. Of course that can be done in a separate PR so I won't hold this up for that.
I'm going to continue to point out that calling the freestanding implementation freestanding-deleted is backwards. The implementation is a freestanding implementation, not freestanding-deleted implementation. Yes, the net effect of a freestanding-implementation is to delete functions but the only time the standard applies 'freestanding-deleted' is in the context of a single function.
This is a valid point, what would you recommend us naming this? |
There was a problem hiding this comment.
Thank you so much for you contribution.
This is fantastic.
Looking at the implementation, I don't think we need to test both structs.
The separation between freestanding hosted and freestanding deleted test suites would be helpful for us to make sure those extra function throws as expected, but not really useful within the context of this PR.
It will be great if you can add some docs work, but we can do that in another PR.
Let me know if you want me to just merge this @20162026 .
|
Actually, I have a question, do the inplace_vector and freestanding::inplace_vector work with each other? e.g. on assignment, copy constructor, etc... I am more than happy to leave this behavior undefined, I just wonder if we need to guard against this at all. |
technically we could even allow conversion from one to another as there should be no problem as long as T and N are the same. Or just add some tests to ensue none of the tested compilers do something unexpected, e.g: TYPED_TEST(Constructors, freestandingConversion) {
using T = TestFixture::T;
using IV = beman::inplace_vector<T, 5>;
using FS = beman::freestanding::inplace_vector<T, 5>;
static_assert(std::is_constructible_v<FS, FS>);
static_assert(std::is_constructible_v<IV, IV>);
static_assert(!std::is_constructible_v<IV, FS>);
static_assert(!std::is_constructible_v<FS, IV>);
} |
|
Thanks for the doc work. I don't know if we want this, the fact that someone is mixing the two should be a code smell. |
|
@wusatosi can we merge or should we wait for another review? |
Let's ship it |
3 participants
Alternative of #77 using separate namespace for freestanding implementation.
The idea is to have two publicly facing variants of
inplace_vector:regularandfreestanding-delete.All mandatory functions should be moved into an internal-use-only parent struct
inplace_vector_base, from which the public-facing structs are derived.The regular
inplace_vectorwill have all thefreestanding-deletefunctions implemented.The freestanding
inplace_vectorwill havefreestanding-deletefunctions marked as= delete.moved to inplace_vector_base
[inplace.vector.cons], construct/copy/destroy
iterators
[inplace.vector.capacity], size/capacity
element access
[inplace.vector.data], data access
[inplace.vector.modifiers], modifiers
freestanding-deleted
[inplace.vector.cons], construct/copy/destroy
[inplace.vector.capacity], size/capacity
element access
[inplace.vector.modifiers], modifiers