[libcxx-commits] [PATCH] D135180: [NFC][libc++] Moves transitive includes location.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

In D135180#3835200 <https://reviews.llvm.org/D135180#3835200>, @philnik wrote:

> I think you misunderstood what I'm questioning. I'm not complaining about moving the includes to the bottom of the header - that's a good idea; I'm complaining about moving them outside the include guard. I don't see and benefit in doing that, but it might cause the compiler to load the files more often than necessary, since the include guard doesn't cover them anymore.

Oh. I must say I don't remember why and don't understand why I left the original comment that triggered this change: https://reviews.llvm.org/D133212#inline-1285216. I must have thought that keeping the includes inside the header guards would mean that the cyclic includes problem isn't fixed, which isn't true.

Regardless, I agree that this would probably inhibit the header guard optimization from triggering, which would be rather bad. I think we should keep the parts of this change that move transitive includes from the top to the bottom of the header, but drop the parts that move them outside of the include guards. Sorry for my misleading comment on D133212 <https://reviews.llvm.org/D133212>.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135180