Lower lowest supported version to C++17. #22
Conversation
| auto final = std::construct_at(end(), std::forward<Args>(args)...); | ||
| // TODO: what is the feature flag for std::construct_at ?? | ||
| // auto final = std::construct_at(end(), std::forward<Args>(args)...); | ||
| // This is just construct_at expanded out. | ||
| auto final = ::new (end()) T(std::forward<Args>(args)...); |
There was a problem hiding this comment.
How do I selectively use construct_at instead of placement new?
construct_at doesn't have a feature-test macro.
There was a problem hiding this comment.
@DeveloperPaul123 do you know how to approach this?
There was a problem hiding this comment.
Looks like it could be __cpp_lib_constexpr_dynamic_alloc or __cpp_constexpr_dynamic_alloc as construct_at was added in P0784R7
There was a problem hiding this comment.
👌 thanks!
I will add a patch later.
I have a question for this, is there a constexpr construct_at for C++17? Placement new doesn't seem to be constexpr friendly.
There was a problem hiding this comment.
No problem!
And no... at least not that I'm aware of off the top of my head. I would have to do some digging to find out.
There was a problem hiding this comment.
Alright I guess constexpr stays at C++20 then.
wusatosi
requested review from
camio,
Hels15 and
DeveloperPaul123
as code owners
|
Let me know if you want me to merge |
| @@ -259,15 +272,22 @@ class inplace_vector : public inplace_vector_base<T, Capacity> { | |||
| base::uninitialized_copy(first, last, this->begin()); | |||
| } | |||
| } | |||
| #ifdef __cpp_lib_containers_ranges | |||
| #else | |||
| template <class Itr> constexpr inplace_vector(Itr first, Itr last) : base() { | |||
There was a problem hiding this comment.
Do you think it's worth constraining Itr here using old school SFINAE?
There was a problem hiding this comment.
That's a good question...
There was a problem hiding this comment.
Egh! I do hate writing enable_if and template magics.
I think we should leave this as is, we can come back and write constraints as cherry-on-top.
There's other things (tests and constexpr) I want to focus on.
| auto final = std::construct_at(end(), std::forward<Args>(args)...); | ||
| #else | ||
| // Note placement new may not be not constexpr friendly |
There was a problem hiding this comment.
| // Note placement new may not be not constexpr friendly | |
| // Note: placement new may not be non-constexpr friendly |
I think this is what was intended?
There was a problem hiding this comment.
Ops typo here, placement new is not constexpr friendly. Thanks for double checking 😅
This PR disables ranges and concept based on feature testing flags.
Swaps out C++ 20 specific functions with alternatives.