InplaceVector project discussions

I created this thread to discuss InplaceVector. I hope it can stay.
My first questions:

  • Some implementations use std::vector under the hood, should we follow this approach or rather build it up from scratch completely? See.
  • TBD
1 Like

Great work getting that started @Hels151!

I imagine using std::vector under the hood will not work due to the constexpr requirements of inplace_vector. I think using the code from here would probably be the best place to start. You could use it directly because it has the right license.

So there’s actually a lot of value in using the final text from the paper or the standard if possible. It’s basically a form of test that we got it right. Basically copy and paste the synopsis - replace any exposition-only concepts with real ones and then implement constraints and such.

That said, if the other one has tests those would be a nice place to reuse and extend as needed.

Also copy the constraints back up to the declaration if they’ve used the old form in the text putting them in the description. Or hidden one by making a constructor trivial in some conditions in Remarks.

I don’t entirely understand why the CUDA implementation differentiates between InputIterator and ForwardIterator and provides a different implementation.
Could anyone help me out with this?

I suspect it has to do with the requirements of the internal algorithm they’re forwarding to? Maybe it needs multiple reads for cuda reasons?

Some features mentioned in the paper - such as:

template <container-compatible-range<T> R>
constexpr inplace_vector(from_range_t, R&& rg);

are not supported by GCC, as GCC doesn’t implement std::from_range_t just yet. Should I just not have it for gcc or how should I go about it?

It makes sense to not implement that overload for GCC. More generally, you could use the __cpp_lib_containers_ranges feature flag described here.

Another option is to add a shim, but I don’t think that would make sense in this case.

I reckon thread safety is not too important in terms of implementation just yet. The paper doesn’t seem to mention anything related to how for example data sharing might work between the threads. Right?

I don’t see anything, so it means the default container rules for thread safety. Some of the member functions, like at and get the same guarantees as const members do, but no heroics should be necessary.

I’m quite sure that I messed up a few things and a lot of things are lacking - but I’d appreciate it if someone could give me an initial review here.

Should I just go ahead with the PR and keep going with development or wait for the review to make sure I’m on the right track? Also, are we planning to add anything that is not described in the paper?

It looks like @Jeff-Garland has a review underway. I’ll also make one today.

Should I just go ahead with the PR and keep going with development or wait for the review to make sure I’m on the right track?

I made a pass on the PR this morning. Overall it looks like a good start – there’s come questions there about the zero capacity case and what the std actually means.

Also, are we planning to add anything that is not described in the paper?

Probably not or is there something someone had in mind?

1 Like

Sorry for the late reply, what I meant is that probably if the interest in the project grows then we can add some examples that present how inplace vector works and a few insightful descriptions. These would be both good for experiencing with the specification as well as providing use-cases for the users, etc. This is not something I’m planning to take on now as I barely have enough time to keep going with the main implementation. Also, some parts need better documentation as it is highly inspired by what was implemented in CUDA so I took a few things for granted…

2 Likes

An LWG issue has been filed on the InplaceVector wording. LWG seems inclined to accept this change based on the reflector thread.

And another: Issue 4151: Precondition of inplace_vector::swap

I filed a couple PRs for this project to fix some build issues I encountered. I was also going to look into enhancing the test suite for this project with the Godbolt link that Jeff provided.

Do we want to keep using assert for the tests? Or can we move to a testing framework like doctest or GTest?

Thanks for those!

IMO assert-style testing isn’t going to be enough as they don’t provide a facility for death tests. I’d suggest going with GTest for now. It’s used in beman.optional26, beman.exemplar, and is the most popular framework out there. We can always change frameworks later if needed.

1 Like

Do we want to keep using assert for the tests? Or can we move to a testing framework like doctest or GTest?

For constexpr tests I’d suggest using straight static_asserts instead of a gtest macro – that’s being done over in optional26, mostly. Philosophically do we need a non-constexpr test to validate that case as well? Probably not is my thought. So that would only leave runtime only tests using gtest.

btw – I’m not a fan of gtest as I’ve probably expressed elsewhere. It’s got a ton of features/weight that won’t be needed in my view. But I don’t want to hold progress over that issue – we can certainly rework tests later if we choose to.