[PATCH] D73498: [NFC] Remove extra headers included in Loop Unroll and LoopUnrollAndJam files

Reid Kleckner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 5 17:23:43 PST 2020


rnk added a comment.

In D73498#1851997 <https://reviews.llvm.org/D73498#1851997>, @anhtuyen wrote:

> Thanks, Reid @rnk for your planing out a detailed course of action. Below is my plan, and please let me know if you have objection.
>
> 1. Dave has done a very diligent review and I will further undelete (i.e. keep) all header files raised by him before committing the patch. I do not know the names of the 2/3 headers you think we better keep, but they will likely be kept based on Dave's comments. Since the chance of someone removes an include from a header file without checking if we can build is quite small,  the chances of having build errors is negligible too. This patch, though it is small, does remove many redundant headers which Dave has pointed out and thus it's worth it.


I don't think I agree with that. The way I see things, the most valuable includes to remove are the ones in headers. Includes in .cpp files only affect one .cpp file: the file itself. Removing includes that are needed but happen to be satisfied transitively from other includes makes it more difficult to remove includes from headers, because other files depend on them. So, it is a low-value change that makes it more difficult to perform a high value change. This is the basis of "include what you use" (IWYU) style, which is what we aim to practice.

The three headers I mentioned were the lines I commented on inline: None.h, SmallPtrSet.h, and DataLayout.h.


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

https://reviews.llvm.org/D73498





More information about the llvm-commits mailing list