Re: [PATCH 4/8] Makefile: Disable readability-math-missing-parentheses clang-tidy check

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 5582 bytes --]

On Fri, Oct 25, 2024 at 01:04:34AM +0200, Stefano Brivio wrote:
> With clang-tidy and LLVM 19:

Oh.. LLVM *19*, not *16*.  That would explain why I'm not seeing the errors.

> 
> /home/sbrivio/passt/conf.c:1218:29: error: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses,-warnings-as-errors]
>  1218 |                 const char *octet = str + 3 * i;
>       |                                           ^~~~~~
>       |                                           (    )
> /home/sbrivio/passt/ndp.c:285:18: error: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses,-warnings-as-errors]
>   285 |                                         .len            = 1 + 2 * n,
>       |                                                               ^~~~~~
>       |                                                               (    )
> /home/sbrivio/passt/ndp.c:329:23: error: '%' has higher precedence than '-'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses,-warnings-as-errors]
>   329 |                         memset(ptr, 0, 8 - dns_s_len % 8);      /* padding */
>       |                                            ^~~~~~~~~~~~~~
>       |                                            (            )
> /home/sbrivio/passt/pcap.c:131:20: error: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses,-warnings-as-errors]
>   131 |                 pcap_frame(iov + i * frame_parts, frame_parts, offset, &now);
>       |                                  ^~~~~~~~~~~~~~~~
>       |                                  (              )
> /home/sbrivio/passt/util.c:216:10: error: '/' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses,-warnings-as-errors]
>   216 |                 return (a->tv_nsec + 1000000000 - b->tv_nsec) / 1000 +
>       |                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>       |                        (                                            )
> /home/sbrivio/passt/util.c:217:10: error: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses,-warnings-as-errors]
>   217 |                        (a->tv_sec - b->tv_sec - 1) * 1000000;
>       |                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>       |                        (                                    )
> /home/sbrivio/passt/util.c:220:9: error: '/' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses,-warnings-as-errors]
>   220 |         return (a->tv_nsec - b->tv_nsec) / 1000 +
>       |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>       |                (                               )
> /home/sbrivio/passt/util.c:221:9: error: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses,-warnings-as-errors]
>   221 |                (a->tv_sec - b->tv_sec) * 1000000;
>       |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>       |                (                                )
> /home/sbrivio/passt/util.c:545:32: error: '/' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses,-warnings-as-errors]
>   545 |         return clone(fn, stack_area + stack_size / 2, flags, arg);
>       |                                       ^~~~~~~~~~~~~~~
>       |                                       (             )
> 
> Just... no.

I quite agree.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

> 
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> ---
>  Makefile | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 2ebc81e..9afbb74 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -270,6 +270,12 @@ docs: README.md
>  #	makes sense when those defines form an enum-like set, but
>  #	weird for cases like standalone constants, and causes other
>  #	awkwardness for a bunch of cases we use
> +#
> +# - readability-math-missing-parentheses
> +#	It's been a couple of centuries since multiplication has been granted
> +#	precedence over addition in modern mathematical notation. Adding
> +#	parentheses to reinforce that certainly won't improve readability.
> +
>  
>  clang-tidy: $(filter-out qrap.c,$(SRCS)) $(HEADERS)
>  	clang-tidy -checks=*,-modernize-*,\
> @@ -296,7 +302,8 @@ clang-tidy: $(filter-out qrap.c,$(SRCS)) $(HEADERS)
>  	-concurrency-mt-unsafe,\
>  	-readability-identifier-length,\
>  	-misc-include-cleaner,\
> -	-cppcoreguidelines-macro-to-enum \
> +	-cppcoreguidelines-macro-to-enum,\
> +	-readability-math-missing-parentheses \
>  	-config='{CheckOptions: [{key: bugprone-suspicious-string-compare.WarnOnImplicitComparison, value: "false"}]}' \
>  	--warnings-as-errors=* $(filter-out qrap.c,$(SRCS)) -- $(filter-out -pie,$(FLAGS) $(CFLAGS) $(CPPFLAGS)) -DCLANG_TIDY_58992
>  

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]