[libcxx-commits] [PATCH] D124516: [libc++] Implement `std::expected` P0323R12

Fri Oct 21 13:31:47 PDT 2022

curdeius added inline comments.

================
Comment at: libcxx/include/__expected/unexpected.h:104
+
+  _LIBCPP_HIDE_FROM_ABI friend constexpr void swap(unexpected& __x, unexpected& __y) //
+      noexcept(noexcept(__x.swap(__y)))
----------------
philnik wrote:
> ldionne wrote:
> > ldionne wrote:
> > > Consider defining this one before `operator==` to make it closer to the member `swap`. That's also how it is in the spec, so bonus points for that.
> > I am not sure how much I like this `//` pattern because it's becoming very prevalent. I _really_ like the way you've formatted this though, but I wish clang-format could get that result without so much help. @philnik any opinions on whether we can make `clang-format` do what we want here? Or maybe @curdeius, I think you've worked on `clang-format` a bunch lately?
> What exactly are you asking for?
> ```lang=c++
>   _LIBCPP_HIDE_FROM_ABI friend constexpr void swap(unexpected& __x, unexpected& __y) noexcept(noexcept(__x.swap(__y)))
>     requires is_swappable_v<_Err>
>   {
>     __x.swap(__y);
>   }
> ```
> seems just fine to me.
Do you mean having (non-trivial) noexcept specifier on it's own line?
There's no option like this right now, probably because it's not something you have a lot in non-generic code.
It should be fairly easy to add an option to break before the noexcept specifier with possible values like the following: never (current behaviour), always (probably not very useful), only before non-trivial specifier i.e. with parentheses.
Another option wind be to add a penalty for this, not sure if that suits the needs of libc++ though.
So yeah, it would be necessary to clarify the desired formatting.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124516