[libcxx-commits] [PATCH] D104002: [libcxx][modularisation] splits `<utility>` into self-contained headers

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added a comment.

In D104002#2809564 <https://reviews.llvm.org/D104002#2809564>, @cjdb wrote:

> In D104002#2809545 <https://reviews.llvm.org/D104002#2809545>, @Quuxplusone wrote:
>
>> Moving `unary_function` without also moving `binary_function` is unusual.
>
> I suggest you carefully read the changes to learn why I didn't move `binary_function` out of `<utility>`. Seriously, I have a good reason.

I assume you're going to move `binary_function` out of `__functional_base` eventually, when we break up that header?

Did you make any changes to the code itself? Assuming you did not, this LGTM with the two indentation nitpicks fixed and the CI passing.



================
Comment at: libcxx/test/std/utilities/intseq/intseq.make/make_integer_seq.fail.cpp:35
+  MakeSeqT
+      i; // expected-error@*:* {{static_assert failed "std::make_integer_sequence must have a non-negative sequence length"}}
 #endif
----------------
Why this weird indentation change? Please leave as previously.


================
Comment at: libcxx/test/std/utilities/utility/pairs/pair.astuple/tuple_element.fail.cpp:20-21
     typedef std::pair<int, short> T;
-    std::tuple_element<2, T>::type foo; // expected-error at utility:* {{Index out of bounds in std::tuple_element<std::pair<T1, T2>>}}
+    std::tuple_element<2, T>::type foo;
+    // expected-error@*:* {{Index out of bounds in std::tuple_element<std::pair<T1, T2>>}}
 
----------------
Same, please don't change indentation here. It's idiomatic to put the `expected-error` on the same line as the thing that triggers it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104002