I am in favor of running a tool like IWYU to fix the codebase (although I have never used it), but I am not sure if adding this to the build system is a good idea: our current contribution frequency (~1k commits/year?) likely does not generate enough extra/missing includes to require the overhead added to every build. There is also the issue of adding another dependency VS making it optional and creating an inconvenience for others: users that do not use the tool will generate compilation warnings/errors for users that do.
IMO, unless the overhead is unnoticeable, it would be enough to run IWYU once a semester/year to try to find and fix any the period's mistakes. If doing this periodic approach, we should probably add an util script that installs IWYU, builds/fixes the includes, and uninstalls IWYU, in order to automate the process. Best,Daniel Em terça-feira, 12 de janeiro de 2021 22:41:58 BRT, Gabe Black via gem5-dev <[email protected]> escreveu: Hi folks. Daniel has submitted a big change which fixes up a bunch of missing includes in files which were coincidentally getting the definitions they needed indirectly from some other file. Way down the line in c++20 I think the "modules" mechanism would be a great tool avoiding these sorts of problems from the get go, but in the meantime it would be helpful if we had a way to scan for these sorts of issues, instead of finding them when they cause problems. These sorts of overly broad, leaky includes also likely slow down builds by making the compiler process more text than it really needs to, and also introduces more dependencies at the scons level than are necessary. I'm not really familiar with it, but after a little bit of Googling I found a tool called iwyu (include what you use) which looks at includes and finds places where includes are missing (transitive includes), and also includes which are not being used. It looks like the way this tool is intended to be used is to substitute it for the compiler, and then run it through, for instance, make. Rather than use it that way, I'm thinking we might be able to set up additional scons rules which would build iwyu error reports based on Source declarations in scons, alongside the normal build outputs. There may be false positives in there somewhere, particularly from code we don't control, and so we'd likely want to add in ways to flag exceptions. If this sort of scons integration works out, it would be nice to make a pass of this tool part of the presubmit checks, assuming it doesn't add tons of time to the build. What do folks think? I don't necessarily have a lot of time to work on this myself, although I might give it a shot if I find some time. It could also be something for someone wanting to cut their teeth on scons to help with, and I could help give pointers and help with the high level design if someone wanted to try it. If you do want to give it a try, at the very least keep me in the loop so we don't have to redo things in a major way when it comes time to check something in. Gabe_______________________________________________ gem5-dev mailing list -- [email protected] To unsubscribe send an email to [email protected] %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
_______________________________________________ gem5-dev mailing list -- [email protected] To unsubscribe send an email to [email protected] %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
