I am also in favor of a tool like iwyu and I have used it in the past. However, iirc iwyu suggestions were not perfect. For example, it suggested to remove of "debug/xxx.hh" flags and in many cases required manual intervention. So before dealing with the scons mechanics we might have to deal with these cases.
I am also a little worried about adding more dependencies for every user. We often compile/run gem5 in clusters where lib/tool versions might not be the latest. It would be great if this was part of our regressions and before committing we would check if something is missing. Nikos On 13/01/2021 15:39, Daniel Carvalho via gem5-dev wrote:
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 <gem5-dev@gem5.org> 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 -- gem5-dev@gem5.org <mailto:gem5-dev@gem5.org> To unsubscribe send an email to gem5-dev-le...@gem5.org <mailto:gem5-dev-le...@gem5.org> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s _______________________________________________ gem5-dev mailing list -- gem5-dev@gem5.org To unsubscribe send an email to gem5-dev-le...@gem5.org %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. _______________________________________________ gem5-dev mailing list -- gem5-dev@gem5.org To unsubscribe send an email to gem5-dev-le...@gem5.org %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
