[PATCH] checkpatch.pl: Reword IS_ENABLED() warning

[Rasmus Villemoes]

On 02/08/2022 14.33, Tom Rini wrote:
> While there are good reasons to use 'if (IS_ENABLED(CONFIG_...))' for
> runtime tests, there's equally good reasons to use '#ifdef CONFIG_...'
> for build time tests.  Reword this message to hopefully avoid confusion.
> 
> Cc: Simon Glass <sjg at chromium.org>
> Signed-off-by: Tom Rini <trini at konsulko.com>
> ---
>  scripts/checkpatch.pl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index fe13e265a3fe..2d737bdb991b 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2621,7 +2621,7 @@ sub u_boot_line {
>  	# use if instead of #if
>  	if ($realfile =~ /\.c$/ && $line =~ /^\+#if.*CONFIG.*/) {
>  		WARN("PREFER_IF",
> -		     "Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible\n" . $herecurr);
> +		     "Possible runtime #ifdef, see if using 'if (IS_ENABLED(CONFIG...))' instead is possible\n" . $herecurr);
>  	}

Eh, there obviously is no such thing as a "runtime #ifdef", so I don't
think this is an improvement.

Nor is the form "if (IS_ENABLED(...))" really a run-time thing anyway,
we certainly rely on the compiler to fold away any occurrence of
build-time constants inside if() and while() and similar. So also the
commit message is somewhat misleading.

The reason the if() form is preferred is to have the compiler check the
contained code for syntactic correctness, and there are two main ways
(that I can think of off hand) that the #if form can be necessary:

(1) The code references fields of structs that only exist when that
CONFIG is enabled, probably to save some memory when they are not needed.

(2) The code references functions which are only declared when that
CONFIG is enabled.

Now, there's usually not much to do about (1), except one could ask if
the memory saving is worth it (obviously if its some 4K debug array, not
so obviously if it's just two extra ints that go unused when
!CONFIG_FOO). For (2), I believe all guards of declarations in headers
by #ifdefs are bugs that need to be fixed, so that the if() form can be
used to elide emitting calls to such functions. The same goes for
guarding whole struct definitions or enums and similar.

Rasmus