[libcxx-commits] [PATCH] D131363: [libc++] Implement `operator<=>` for `error_category`

Mon Aug 8 14:18:13 PDT 2022

avogelsgesang added a subscriber: var-const.
avogelsgesang added inline comments.

================
Comment at: libcxx/include/system_error:227

+#if _LIBCPP_STD_VER <= 17
+
----------------
mumbleskates wrote:
> avogelsgesang wrote:
> > mumbleskates wrote:
> > > nit: Most checks of `_LIBCPP_STD_VER` are in terms of `>`, such that these blocks would be reversed, with C++20-onwards behavior gated behind `_LIBCPP_STD_VER > 17`. For consistency's sake I think it would be good to match that.
> > I usually prefer it this way around, because I value "keeping the order between synopsis and implementation consistent" over "consistently use `_LIBCPP_STD_VER > 17`".
> > 
> > Do you think swapping the `#if` here is worth it, although that would mean that the order in the synopsis no longer matches the order in the implementation?
> > 
> > (related: https://reviews.llvm.org/D131372#inline-1263658)
> okay, i can see that rationale. in my previous differentials (`<=>` for `pair` and `tuple`) i implemented it in the order you've seen me do, preferring a `> 17` check and putting modern operators (`==`, `<=>`) first and outdated ones in the `#else` branch in cases where `<=>` obsoletes the other operators.
> 
> understanding your rationale i have a weaker preference now, still leaning towards whatever our conventional momentum is.
@philnik @Mordante @ldionne @var-const does anyone of you have a preference here? Not sure which (purely stylistic) tradeoff we want to take here...

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131363