[libcxx-commits] [PATCH] D62228: Make `vector` unconditionally move elements when exceptions are disabled.

[Eric Fiselier via Phabricator via libcxx-commits]

EricWF marked an inline comment as done.
EricWF added a comment.

In D62228#1511999 <https://reviews.llvm.org/D62228#1511999>, @mclow.lists wrote:

> I'm not particularly in favor of this patch.
>  There was a discussion on the mailing list that (to my mind) petered out with no particular conclusion as to whether or not this was a good idea.


Louis and I talked more offline and there was some consensus that this minimal change was more acceptable.
But if there are additional concerns they can be raised here.

> I not that fond of  `-fno-exceptions` in general, **and** I think that special-casing `vector` here is the wrong approach.
>  If we really want to embrace this (and I don't), I think we should change `move_if_noexcept` (and the other noexcept type traits) to return true when exceptions are disabled.

You're proposing a non-standard and non-conforming extension. This change is not an extension. It is a conforming implementation of vector that is better optimized.
This change is orthogonal to the suggested non-conforming extension, and discussion about the extension is better had elsewhere.

> Also, these functions `__construct_backward` and `__construct_forwards` should be free functions, not non-standard extensions to `allocator_traits` (but that's existing code)

They have an interesting relationship with the allocator, but I agree they're mis-placed.



================
Comment at: include/utility:284
+#else
+        false,
 #endif
----------------
mclow.lists wrote:
> Why an enum?
> How about:
> ```
> #ifdef _LIBCPP_NO_EXCEPTIONS
> _LIBCPP_BOOL_CONSTANT(true) __exceptions_disabled;
> #else
> _LIBCPP_BOOL_CONSTANT(false) __exceptions_disabled;
> #endif
> ```
> 
It's just a quick way to stash a value without having to worry about introducing a name with linkage.


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62228/new/

https://reviews.llvm.org/D62228