git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Garrit Franke <garrit@slashdev.space>
Cc: git@vger.kernel.org, gitster@pobox.com
Subject: Re: [PATCH v2 1/4] contrib: add iwyu script
Date: Wed, 06 Apr 2022 09:40:17 +0200	[thread overview]
Message-ID: <220406.86ee2afy6h.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <20220405114505.24389-2-garrit@slashdev.space>


On Tue, Apr 05 2022, Garrit Franke wrote:

> add include-what-you-use helper.
>
> Signed-off-by: Garrit Franke <garrit@slashdev.space>
> ---
>  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 <garrit@slashdev.space>
> +
> +[1]: https://github.com/include-what-you-use/include-what-you-use
> +[2]: https://lore.kernel.org/all/220401.8635ixp3f4.gmgdl@evledraar.gmail.com/#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.
	

  reply	other threads:[~2022-04-06 12:06 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-31 19:44 [PATCH] bisect.c: remove unused includes Garrit Franke
2022-03-31 21:29 ` Junio C Hamano
2022-04-01  8:07   ` using iwyu (include-what-you-use) to analyze includes (was: [PATCH] bisect.c: remove unused includes) Ævar Arnfjörð Bjarmason
2022-04-05 11:45 ` [PATCH v2 0/4] various: use iwyu (include-what-you-use) to analyze includes Garrit Franke
2022-04-06  7:54   ` Ævar Arnfjörð Bjarmason
2022-04-05 11:45 ` [PATCH v2 1/4] contrib: add iwyu script Garrit Franke
2022-04-06  7:40   ` Ævar Arnfjörð Bjarmason [this message]
2022-04-05 11:45 ` [PATCH v2 2/4] bisect.c: remove unnecessary include Garrit Franke
2022-04-06  7:50   ` Ævar Arnfjörð Bjarmason
2022-04-06 16:41     ` Junio C Hamano
2022-04-05 11:45 ` [PATCH v2 3/4] serve.c: " Garrit Franke
2022-04-05 11:45 ` [PATCH v2 4/4] apply.c: " Garrit Franke

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=220406.86ee2afy6h.gmgdl@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=garrit@slashdev.space \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).