[libcxx-commits] [PATCH] D138189: [libcxx] adds an include-what-you-use (IWYU) mapping file

[Christopher Di Bella via Phabricator via libcxx-commits]

cjdb marked 2 inline comments as done.
cjdb added a comment.

> I'm not suggesting we do that for now, just mentioning it.

Ack. Some thoughts below for when you do entertain this more seriously.

In D138189#3933948 <https://reviews.llvm.org/D138189#3933948>, @ldionne wrote:

> Thanks! I think I really like this. We could also potentially use this as a tool to aid our removal of transitive includes -- arguably, if we provide a tool to automatically fix people's code, it may become more reasonable to break people's code through transitive include removals a bit more eagerly.

Before committing to this, it'd be good to survey the trust folks have in IWYU (my hunch is that it's very low); and to keep the transitive headers for multiple years behind a macro, for folks unable to use IWYU for whatever reason (e.g. perhaps a library doesn't have a mapping file and has lots of detail headers that make using IWYU unreasonable).



================
Comment at: libcxx/include/libcxx.imp:1
+[
+  { include: [ "<__bits>", "private", "<bits>", "public" ] },
----------------
ldionne wrote:
> This patch doesn't ship this file with the libc++ headers. Did you mean to do that?
Oops, nice catch!


================
Comment at: libcxx/utils/generate_iwyu_mapping.py:1
+#!/usr/bin/env python
+
----------------
ldionne wrote:
> Just to clarify, we will not need to change this script unless we add a new detail header at the top level (which should be rare), right?
Yeah, that and if a specific directory needs mapping (e.g. say you add `__detail/`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138189