[libcxx-commits] [PATCH] D148405: [libc++] Remove the chrono include from algorithm

Tue Apr 18 08:41:31 PDT 2023

Mordante added a comment.

In D148405#4274364 <https://reviews.llvm.org/D148405#4274364>, @iana wrote:

> In D148405#4274218 <https://reviews.llvm.org/D148405#4274218>, @Mordante wrote:
>
>> In D148405#4271485 <https://reviews.llvm.org/D148405#4271485>, @iana wrote:
>>
>>> In D148405#4271084 <https://reviews.llvm.org/D148405#4271084>, @Mordante wrote:
>>>
>>>> Interesting that it fails with modules. In general we keep these transitive includes to avoid possibly breaking user code that depends on these transitive includes. (Note it's a bug in the user's code to depend on these includes, but this is not easily spotted.) However when it block us we are free to remove them. So no objection to the removal.
>>>>
>>>> I expect this to fail in the CI since the transitive includes are not updated. Let me know if you need help with this.
>>>>
>>>> This change needs to be mentioned in our release notes (docs/ReleaseNotes.rst) since it may affect users.
>>>
>>> I'm not sure what I'm doing wrong, but `check-cxx` doesn't complain about the removed include. If I take algorithm -> chrono out of the csv files it also doesn't complain.
>>
>> Check-cxx needs to be executed for every language version. If you intend to do more of these cleanups I can explain what to do. If it's only one it's easier for me to commandeer the revision and apply the fixes.
>
> Ah. This was the only pure include cycle I found, but it's possible there will be some module cycles that still need to be broken up. It'd be good for me to know how to run it properly anyway, but also feel free to commandeer if you prefer.

Actually the method is described in `libcxx/test/libcxx/transitive_includes.sh.cpp:45` re-generating the files is done by building the target `libcxx-generate-files`.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148405