[libcxx-commits] [PATCH] D150300: [libc++][WIP] Try disabling transitive includes in all configurations to see impact on CI times

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

In D150300#4343062 <https://reviews.llvm.org/D150300#4343062>, @Mordante wrote:

> I also wonder whether we should look at removing the transitive includes in general (obviously in a separate patch). I expected some overhead from allowing them, but not these numbers. I still don't like it might break some users, but the benefits seem quite large.

Yeah, that's what this patch got me thinking about, too. This is kind of scary but it would likely provide worthwhile benefits to users. I do think landing a variant of this patch (aka where we change the default in the CI) is tied to removing the transitive includes by default for users, since we need to keep testing exactly what we ship, not something slightly different.

So if we were to remove the transitive includes for users, we'd want to give some grace period for users to update their code base, and give them a way to opt-in gradually into that change. So for example, we could:

1. Stop removing transitive includes under `_LIBCPP_REMOVE_TRANSITIVE_INCLUDES`, now when we remove transitive includes we start guarding it behind `_LIBCPP_REMOVE_TRANSITIVE_INCLUDES_2`.
2. Release-note that transitive includes will be removed by default in LLVM 18 (for example), and they can define `_LIBCPP_REMOVE_TRANSITIVE_INCLUDES` in LLVM 17 to start preparing their code base.
3. In LLVM 18, we flip the default to remove transitive includes and optionally we provide a way to opt into the old behavior with a macro like `_LIBCPP_PROVIDE_TRANSITIVE_INCLUDES`. This is the grace period.
4. In LLVM 19, we remove the escape hatch and unconditionally remove all transitive includes marked under `_LIBCPP_REMOVE_TRANSITIVE_INCLUDES`.
5. Now we begin the same cycle again but with `_LIBCPP_REMOVE_TRANSITIVE_INCLUDES_2`.
6. Optionally, we add a `#warning` if `_LIBCPP_REMOVE_TRANSITIVE_INCLUDES` is defined just so that people who used that macro to opt-in their code bases earlier can clean up their build scripts.

In all cases, we can never reuse the `_LIBCPP_REMOVE_TRANSITIVE_INCLUDES` name unless we know nobody's defining the macro anymore because otherwise we could break them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150300