[libcxx-commits] [PATCH] D128236: [libc++] Add a test to pin down the set of transitive public includes

Mark de Wever via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Tue Jun 21 09:57:44 PDT 2022


Mordante added a comment.

Thanks for working on this! I think this is really useful to have. The makes it easier to remove includes from subheaders and see whether that as a user visible impact.
I'm a bit curious whether it will cause a lot of build failures. (I know it shouldn't.)



================
Comment at: libcxx/test/libcxx/transitive_includes.sh.cpp:15
+// forever, however we do try to group removals for a couple of releases
+// to avoid breaking users at every release.
+
----------------
Would it be an idea to add a "schedule" here too? Basically to avoid people working on a cleanup patch and have it rejected just because it's not "the right time". For example only cleanup when the next version number is a multiple of 3. (3 just because 15 is a multiple of 3.)
(But maybe we should discuss the details on Discord.)


================
Comment at: libcxx/test/libcxx/transitive_includes/expected.algorithm:1
+c++/v1/algorithm
+c++/v1/atomic
----------------
Should the path include `c++/v1`? If we want that we probably need to make the test unsupported for the unstable ABI too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128236



More information about the libcxx-commits mailing list