[libcxx-commits] [PATCH] D124123: [libc++] Remove <functional> includes

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const accepted this revision as: var-const.
var-const added a subscriber: ldionne.
var-const added a comment.

@philnik Can you add some more context to the patch and commit description? I think it would be useful to explain why it's safe to do this change now, at least.

LGTM, but because it can be a breaking change for users, please wait for @ldionne to review.



================
Comment at: libcxx/docs/ReleaseNotes.rst:70
 
-- Some libc++ headers no longer transitively include all of ``<algorithm>``, ``<chrono>`` and ``<utility>``.
-  If, after updating libc++, you see compiler errors related to missing declarations in
-  namespace ``std``, it might be because one of your source files now needs to
-  ``#include <algorithm>``, ``#include <chrono>`` and/or ``#include <utility>``.
+- Some libc++ headers no longer transitively include all of
+    - ``<algorithm>``,
----------------
Nit: I think it would make sense to add a colon after `of` to precede the list.


================
Comment at: libcxx/docs/ReleaseNotes.rst:71
+- Some libc++ headers no longer transitively include all of
+    - ``<algorithm>``,
+    - ``<chrono>``,
----------------
Optional: I think it's more common for list elements to end in `;` semicolons.


================
Comment at: libcxx/docs/ReleaseNotes.rst:73
+    - ``<chrono>``,
+    - ``<functional>``, and
+    - ``<utility>``.
----------------
I'd remove this `and` -- it seems largely like a leftover from the previous state when this was a sentence and not a list.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124123