[libcxx-commits] [PATCH] D112092: [libc++] LWG3001: add `remove_extent_t` to `weak_ptr::element_type`.

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added a comment.

I see buildkite is green, but for the record, I'd like to see this PR updated with `>`-instead-of-`>=` //and wait for buildkite to pass on that version also//, before landing.



================
Comment at: libcxx/include/__memory/shared_ptr.h:378
 struct __compatible_with
-#if _LIBCPP_STD_VER > 14
+#if _LIBCPP_STD_VER >= 17
     : is_convertible<remove_extent_t<_Tp>*, remove_extent_t<_Up>*> {};
----------------
Quuxplusone wrote:
> Please keep this as `> 14`. Even though `_LIBCPP_STD_VER` will never actually be set to "15", the intuition is that C++2b behaves more like 23 than like 20, C++1z (e.g. "C++15") behaves more like 17 than like 14, and so on. So we pretty consistently use `_LIBCPP_STD_VER > x` and `TEST_STD_VER > x` instead of `>=`.
> Ditto line 422; ditto throughout (and also the tests, such as where Louis commented about it).
@ldionne wrote:
> We've definitely been using `VER > n` as a convention in libc++ and the test suite, that's one of the few places where we're pretty consistent. However, I've always found it a bit weird and IMO it would be more natural to use `>=`. @Quuxplusone Would you (or someone else lurking on this review) oppose to making such a change? If so, why?

First off, I agree with your proposal for //this// patch (which is "revert to `>` and take this discussion about conventions to a separate PR").
I also agree that it's often kind of inconvenient for the reader to mentally translate `> 14` into "ah, this feature is new in C++**17**." I bet we can even find one or two historical bugs due to people getting that math wrong.
However, I believe my "intuitive explanation" above is pretty teachable. And any proposal to change the convention needs to come with a proposal of what to do about C++2b: Do you propose that we should start landing patches with `>= 23` instead of `> 20`, and update the `__config` line that right now says
```
#    define _LIBCPP_STD_VER 21  // current year, or date of c++2b ratification
```
? Or do you propose that we do a massive mechanical updating of `> 20` to `>= 23` every three years? or what?
(Therefore I'm happy to stop talking about this here, and you can just ping me in the I-think-//unlikely// event that such a PR materializes.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112092