Subject: Re: [PATCH v2 1/4] contrib: add iwyu script

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Wed, 06 Apr 2022 09:40:17 +0200

On Tue, Apr 05 2022, Garrit Franke wrote:

> add include-what-you-use helper.
>
> Signed-off-by: Garrit Franke <[email-masked]>
> ---
>  contrib/iwyu/README  | 33 +++++++++++++++++++++++++++++++++
>  contrib/iwyu/iwyu.sh |  2 ++
>  2 files changed, 35 insertions(+)
>  create mode 100644 contrib/iwyu/README
>  create mode 100755 contrib/iwyu/iwyu.sh
>
> diff --git a/contrib/iwyu/README b/contrib/iwyu/README
> new file mode 100644
> index 0000000000..5e2d218602
> --- /dev/null
> +++ b/contrib/iwyu/README
> @@ -0,0 +1,33 @@
> +Include What You Use
> +====================
> +
> +Include what you use (iwyu) [1] is a tool that points out which headers a file
> +should include. Moreover, it can point out includes that are not used by a file,
> +which makes it especially handy for cleanup tasks.
> +
> +To run this script, you will need iwyu to be installed on your system.
> +
> +The "iwyu.sh" script runs iwyu on a given object and omits mandatory headers
> +defined in "Documentation/CodingGuidelines".
> +
> +Example usage:
> +
> +    ./contrib/iwyu/iwyu.sh diff.o
> +
> +This yields:
> +
> +    diff.c should remove these lines:
> +    - #include "attr.h"  // lines 13-13
> +    - #include "submodule-config.h"  // lines 18-18
> +
> +In its current form, this script should not be used to auto-generate patches,
> +since there are still some false-positives that only a human can resolve. It is
> +meant as a starting point for further cleanups. It could be nice to integrate
> +this as a step in our CI, but we're not quite there yet.
> +
> +The inspiration for this script came from this [2] email-thread.
> +
> +Garrit Franke <[email-masked]>
> +
> +[1]: https://github.com/include-what-you-use/include-what-you-use
> +[2]: https://lore.kernel.org/all/[email-masked]/#t
> \ No newline at end of file

Note the "No newline at end of file" warnings, new files should have \n
at the end.

> diff --git a/contrib/iwyu/iwyu.sh b/contrib/iwyu/iwyu.sh
> new file mode 100755
> index 0000000000..3ef8639eae
> --- /dev/null
> +++ b/contrib/iwyu/iwyu.sh
> @@ -0,0 +1,2 @@
> +make $1 CC=include-what-you-use CFLAGS="-Xiwyu --verbose=1" 2>&1 \
> +| grep -v -E -e '^#include <' -e '^#include "(cache|git-compat-util|gettext)\.h"' 
> \ No newline at end of file

So that's just my one-line hack as-is (well, bisect.co replaced with $1)
:)

I think if we integrate some IWYU spport support that this is a rather
dead end to pursue.

A much better way would be to just do ("REAL_CC" is already understood
by sparse):

	REAL_CC=gcc make CC=contrib/iwyu.sh 

I.e. we shouldn't run make and then parse its output, but to have make
run a CC which wraps our REAL_CC.

Then you could either have it "really" compile, as e.g. the "sparse"
wrapper can do (note: note our sparse target but cgcc), or run whatever
"make" target, and then only having to parse the output of iwyu, not
iwyu+make.

Also, it looks like iwyu is a nicely API'd python library, so trying to
parse its output is probably a dead end too, can't we just ship a
trivial API-using script for it that gets all this as nicely
machine-readable data?

Or, if you look at iwyu.git its own source tree has a fix_includes.py
which seems to do that, *and* be able to "fix" the includes for you.

All of that is just stuff I didn't have time to poke at when posting a
quick reply in the original thread, but hopefully someone turning this
into a series of patches will :)
	
	$ python3 fix_includes.py -h
	Usage: fix_includes.py [options] [filename] ... < <output from include-what-you-use script>
	    OR fix_includes.py -s [other options] <filename> ...
	
	fix_includes.py reads the output from the include-what-you-use
	script on stdin -- run with --v=1 (default) verbose or above -- and,
	unless --sort_only or --dry_run is specified,
	modifies the files mentioned in the output, removing their old
	#include lines and replacing them with the lines given by the
	include_what_you_use script.  It also sorts the #include and
	forward-declare lines.