We could add it to scons as an extra flag which is defaulting to False, and properly set in our regression scripts (I am also in favour of adding IWYU)
Kind Regards Giacomo > -----Original Message----- > From: Nikos Nikoleris via gem5-dev <gem5-dev@gem5.org> > Sent: 13 January 2021 15:57 > To: gem5-dev@gem5.org > Cc: Nikos Nikoleris <nikos.nikole...@arm.com> > Subject: [gem5-dev] Re: IWYU tool and include checking from scons > > 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 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
