On 09.12.24 18:20, Tom Lane wrote:
walsender.h should depend on xlog.h, not vice versa. (Actually, the inclusion was circular until a couple hours ago, which was even sillier; but Bruce broke it in the expedient rather than logically correct direction.) Because of that poor decision, plus blind application of pgrminclude, we had a situation where half the system was depending on xlog.h to include such unrelated stuff as array.h and guc.h. Clean up the header inclusion, and manually revert a lot of what pgrminclude had done so things build again.This episode reinforces my feeling that pgrminclude should not be runwithout adult supervision. Inclusion changes in header files in particular need to be reviewed with great care. More generally, it'd be good if we had a clearer notion of module layering to dictate which headers can sanely include which others ... but that's a big task for another day.In short, pgrminclude turned a small human error into a giant mess that required half a day's work to clean up, because it had no idea which of some redundant-looking includes were reasonable to get rid of and which weren't. I am worried that IWYU might be prone to similar mess-amplification. Perhaps it has better heuristics as a result of doing more thorough semantic analysis, but heuristics are still only heuristics. One thing that I would look favorably on, given the mistakes we made in 2011, is automatic detection of circular #include's. Do you happen to know whether IWYU complains about that?
IWYU is built on compiler infrastructure and tracks where things are declared and where they are used and then suggests you to include exactly the header files where the things you use are declared (rather than some other header file that happens to pull in the one you actually need) and suggests to remove the includes that don't provide any declarations that you use. So it is not really a heuristic, it is perfectly accurate, barring bugs or complicated edge cases (cough, cough, CppAsString2()), assuming one agrees with that goal.
If you have two headers that circularly include each other, and you have the normal multiple-inclusion-guards, then one of the includes won't contribute anything to the overall set of declared things, so it would most likely be suggested for removal. That's not exactly the same thing as actual circular include detection, but it will indirectly tell you about it.
