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

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added inline comments.


================
Comment at: libcxx/include/system_error:227
 
+#if _LIBCPP_STD_VER <= 17
+
----------------
avogelsgesang wrote:
> Mordante wrote:
> > avogelsgesang wrote:
> > > 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...
> > I have a preference, but we don't a required style. Traditionally the `> xx` was more common.
> > 
> > I have a preference
> You didn't mention what preference that is? I assume it is `> xx`, i.e. in  line with the "traditional" style?
> 
> I updated the review with to use `>xx` now
I didn't mention it since code reviews shouldn't be about getting the code in the exact preferred style of the reviewer. Since we already use different styles there's no "project style".

But you guessed right and I personally prefer `> XX`, but a portion of that is just because I'm used to it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131363