Repository navigation
[vector.capacity] p14 The precondition use "and" but should use "or" #5776
Description
Activity
I think it's "and" because when
sz > capacity(), the firstsize()elements use Cpp17MoveInsertable and the rest Cpp17DefaultInsertable.I think it's "and" because when sz > capacity(), the first size() elements use Cpp17MoveInsertable and the rest Cpp17DefaultInsertable.
But, in other words, when
sz < capacity(), the invocation ofresize()indeed does not requires the requirementCpp17MoveInsertableforA, as shown in the example.Moreover, consider this example
std::vector<A> vec{}; vec.reserve(2); std::cout<< vec.capacity()<<std::endl; // #1 vec.resize(10); // #2#1prints2and#2still be ok.From the comment of the implementation of
vector<T>::resize, it says it will default construct the element whensz > the first size(), see https://github.com/gcc-mirror/gcc/blob/master/libstdc%2B%2B-v3/include/bits/stl_vector.h#L1004When you do run out of capacity, you need the former set of requirements. Maybe what you want is to make the precondition of that set conditional. But I don't think that's editorial.
See my last comment above. I'm not sure whether the implementation didn't obey the requirement, or whether there are some inconsistencies in the standard. At least, according to your opinion, the requirement
Cpp17MoveInsertableforvoid resize(size_type sz);should be conditional.From the comment of the implementation of
vector<T>::resize, it says it will default construct the element whensz > the first size(), see https://github.com/gcc-mirror/gcc/blob/master/libstdc%2B%2B-v3/include/bits/stl_vector.h#L1004The comment doesn't follow the standard terminology, but IIUC the actual implementation is right here. In that case, the libstdc++ implementation is actually default-inserting new elements (with
__uninitialized_default_n_a) and moving or copying old ones (with__uninitialized_move_if_noexcept_a).We can't simply make Cpp17MoveInsertable or Cpp17DefaultInsertable conditional, because "Mandates" is missing here, and the well-formedness requirements, in my opinion, inappropriately appear in "Preconditions".
I think there are several possible defects.
- [vector.capacity]/16 suggests that old elements are copy-inserted under some conditions, but Cpp17CopyInsertable (which has semantic requirements) is not mentioned in Preconditions.
- Many named requirements in [container.alloc.reqmts]/2 (e.g. Cpp17DefaultInsertable) are merely requirements for well-formedness, and it's suspicious for them to appear in Preconditions instead of Mandates.
- Cpp17MoveInsertable and Cpp17CopyInsertable have additional semantic requirements, should they be split?
- There is implementation divergence on
resize: libstdc++ and msvc stl accept the non-move-insertableA, while libc++ rejects it. Currently all of them are conforming IIUC because Cpp17MoveInsertable only appears in Preconditions. If diagnosable parts of Preconditions are moved to Mandates, then the implementation divergence matters. It's not clear to me whether such divergence is intentionally allowed.
Anyway, I don't think this is editorial either. LWG issues seem needed.
But, in other words, when
sz < capacity(), the invocation ofresize()indeed does not requires the requirementCpp17MoveInsertableforA, as shown in the example.Yes, but simply changing the preconditions from "and" to "or" would not help that problem and would be totally wrong. That would imply that resizing when capacity is exceeded has to work for an object that can only be default constructed, but not copied or moved. That's nonsense.
When shrinking the vector, neither Cpp17MoveInsertable nor Cpp17DefaultInsertable is required. When growing within the current capacity, only Cpp17DefaultInsertable is required. When exceeding the capacity, both are required. So the current wording is correct. In general, for an arbitrary call to
resize, both are required.So there is no editorial issue here, and any change to those requirements would be very non-editorial.
- Cpp17MoveInsertable and Cpp17CopyInsertable have additional semantic requirements, should they be split?
At the time, there was a deliberate decision to not alter those requirements in Mandating the Standard Library:
Clause 21 - Containers library. That could be revisited now, but I don't know how much support there would be for doing that. It seems out of scope for an LWG issue, a paper would be needed.So the current wording is correct.
The wording "and" requires both conditions are true, I think. However, the condition
Cpp17MoveInsertableforAis false. This is the issue here.However, the condition
Cpp17MoveInsertableforAis false. This is the issue here.This is not an issue of standard wording or implementations (at this moment). Using such non-Cpp17MoveInsertable
Awithresizeresults in UB, and thus implementations may support it as a conforming extension, or reject it.i.e. the wording is correct. There is no editorial issue here.
i.e. the wording is correct. There is no editorial issue here.
My original option is
I'm not sure whether the implementation didn't obey the requirement, or whether there are some inconsistencies in the standard.
AFAIK, the implementations of GCC and MSVC regarding
resizeall support the case whenTis notCpp17MoveInsertable, not sure whether Clang supports this behavior or not since I didn't dig further in Clang's standard library source code, Clang may be similar to GCC here. According to the wording, the behavior would be UB ifTis notCpp17MoveInsertable, however, all major implementations have consistent behavior. Agree that this issue touches the domain that is non-editorialI'm not sure whether the implementation didn't obey the requirement, or whether there are some inconsistencies in the standard.
Implementations are conforming here IIUC, as currently they are not required to reject non-Cpp17MoveInsertable types.
The standard wording seemly has defects - copy insertion is suggested by the standard and conditionally used by all mainstream implementations (when the move construction is potentially throwing and copy construction is available), but the "Preconditions:" is only mentioning Cpp17MoveInsertable.
This is not limited to
vector'sresize. Perhaps almost all element-relocating operations ofvectoranddequeare affected. I've tried to submit an LWG issue for this.Clang may be similar to GCC here
Yes. But libc++ rejects non-(formally-)Cpp17MoveInsertable types when conditional move-or-copy-insertion is used. (Note that Clang can be used together with libc++, libstdc++, and MSVC STL.)
Reacted by XMHAccording to the wording, the behavior would be UB if
Tis notCpp17MoveInsertable, however, all major implementations have consistent behavior.Which is one possible outcome of UB.
I'm closing this, there is no editorial issue here.
Implementations are conforming here IIUC
From certain perspectives, you're right to say implementations are conforming here. Since the violation of precondition results in UB, which means the document imposes no requirement. Implementations are free to do anything here. Since the behaviors of all mainstream implementations are similar, so I think we might expel the behavior from UB.
@jwakely @JohelEGP @frederick-vs-ja
Even, a copyable-only(i.e. non-moveable) type is wide use in practice as the element type of avectorstruct T{ T() = default; T(T&){} // suppress the implicitly-declared move constructor, and cannot take a rvalue as the argument }; int main(){ std::vector<T> vec; vec.resize(1024); // UB here since `T` is not Cpp17MoveInsertable };
Using
Tas the element type will incur UB?No, that type is Cpp17MoveInsertable into
vec. There's no requirement to use a move constructor, just that construction from an rvalue passed toallocator_traits::constructworks. You can pass an rvalue to that copy constructor.Edit: Oh wait, you've edited the code. I was replying to the original that said
T(const T&). With that change, your type is not copy insertable or move insertable. It's disgusting too.This issues list is not the right place to learn how vector works.
Reacted by XMHSince the behaviors of all mainstream implementations are similar, so I think we might expel the behavior from UB.
As stated above, libc++ has a static assert. So not the same.
The modified example was my original thought when wrote the comment. I'm not learning how to use vector, I just want to say the use of T that causes UB is widely used in actually code.
No, because you can't use that type at all with
vector. Your example doesn't even compile.T that causes UB is widely used in actually code
I don't think this is true at all.
Type with
T(T&)copy constructors andT(T&&) = delete;move constructors are unusual, and perverse, and generally not usable with the standard library containers.Your example doesn't even compile.
You're right, this is my oversight. However, the first example is indeed usable in actual code. Such as deep copy an object instead of using shallow copy.
Deleting a move constructor is perverse. If you don't want it to be efficiently movable, just don't define a move constructor. Then you get deep copies of rvalues.
Getting undefined behaviour for such types is fine, they're irregular and unnecessary.
Reacted by A. Jiang
[vector.capacity] p14 says
Cpp17MoveInsertable is defined in [containers.container.alloc.reqmts] p2.3
[containers.container.alloc.reqmts] p2 says
Consider this example:
However,
Ais not Cpp17MoveInsertable sinceallocator_traits<A>::construct(m, p, RV)is ill-formed forA.