[libcxx-commits] [PATCH] D103478: [libc++][compare] Implement three_way_comparable[_with] concepts

[Christopher Di Bella via Phabricator via libcxx-commits]

cjdb added inline comments.


================
Comment at: libcxx/test/std/language.support/cmp/cmp.concept/three_way_comparable_with.compile.pass.cpp:76-79
+static_assert(
+    !check_three_way_comparable_with<int, int (S::*)() const volatile>());
+static_assert(!check_three_way_comparable_with<
+              int, int (S::*)() const volatile noexcept>());
----------------
Quuxplusone wrote:
> cjdb wrote:
> > Quuxplusone wrote:
> > > Nit: Please avoid taking clang-format's advice in cases like this where it's clearly making the formatting worse.
> > > Instead, try treating a clang-format diagnostic as a helpful hint that your lines may be too long. E.g. in this case you don't //really// need to test a `const volatile noexcept` abominable function type, right? This test clearly could be rewritten as `static_assert(!check_three_way_comparable_with<int, int*>())` (or any similar pair of types; the only important thing here is that the expression `a == b` would be ill-formed).
> > > Renaming `check_three_way_comparable_with` to simply `check` is another good way to improve the readability of this test file while also helping to make clang-format happy.
> > I'm sure @rarutyun's employer doesn't pay them to spend time overriding clang-format.
> @rarutyun, you wrote:
> > Clang-format suggests incorrect modifications that fail to compile if applied.
> 
> For generating the diff to upload, I use `git show -U999 > x.diff` and then "Upload file" into the Phab review. I don't use `arc` at all in my personal workflow. I don't know if this will help you, but at least I personally spend zero time wrestling with `arc`; some amount of time wrestling with `git` instead; and zero time wrestling with `clang-format` because `clang-format` is completely unused in my `git` workflow.
> 
> Re Chris's snipe: Don't worry about clang-format. Just write the code the way you'd write it naturally, and then act on Phab feedback (if any) from human libc++ maintainers. clang-format isn't the boss here. :)
> 
> Also, note that a massive number of these 230+242 lines can disappear entirely. I tentatively recommend just taking my 101+147 lines of more targeted testing from D107036 (although let's see if @ldionne has a strong preference for these, or wants both, or what). Either way, please delete or fix the misformatting throughout //this// file.
> Also, note that a massive number of these 230+242 lines can disappear entirely. I tentatively recommend just taking my 101+147 lines of more targeted testing from D107036 (although let's see if @ldionne has a strong preference for these, or wants both, or what). Either way, please delete or fix the misformatting throughout this file.

See https://reviews.llvm.org/D103478#inline-980152.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103478