1804160 - [Static-Analysis][Clang-Tidy] Static analysis produces unactionable/unavoidable warning-spam for DeMorgan's theorem simplifications, when run on NS_ASSERTION or MOZ_ASSERT with compound expression

Status: RESOLVED FIXED

(Developer Infrastructure :: Source Code Analysis, defect)

Description

[Daniel Holbert [:dholbert]]

STR:

1. ./mach static-analysis check layout/generic/nsPageContentFrame.cpp
2. Wait for output.

ACTUAL RESULTS:
Several warnings suggesting the application of DeMorgan theorem to "simplify" boolean expressions. Here's the first one:

Warning: readability-simplify-boolean-expr in layout/generic/nsPageContentFrame.cpp: boolean expression can be simplified by DeMorgan's theorem
layout/generic/nsPageContentFrame.cpp:101:9: warning: boolean expression can be simplified by DeMorgan's theorem [readability-simplify-boolean-expr]
         NS_ASSERTION(ratio >= 0.0 && ratio < 1.0,
         ^
 $OBJ/dist/include/nsDebug.h:101:11: note: expanded from macro 'NS_ASSERTION'
        if (!(expr)) {                                                       \
            ^~~~~~~

This is clang analyzing the post-processed C++ and rightly complaining that the boolean condition (if (!(a && b)) could be simplified. However, the code that we're authoring using the macro in this case is NS_ASSERTION(a && b, ...) which does not have room for simplification.

This makes for spammy reviewbot review-feedback (e.g. it threw me off in https://phabricator.services.mozilla.com/D163759#5385982 and made me scratch my head to see if there was anything worth addressing [there's not].)

Andi: maybe we should disable this warning, or somehow tune it so it doesn't spam for macro expansions? I wonder how often it actually helps folks vs. how often it misfires for cases like this.

Comment 1

[Andi [:andi]]

I think adding this option SimplifyDeMorgan: false to the checker should fix it.

Comment 2

[Andi [:andi]]

Attachment: Bug 1804160 for clang-tidy checker readability-simplify-boolean-expr do not activate SimplifyDeMorgan. r=dholbert

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6ad0553e3073
for clang-tidy checker readability-simplify-boolean-expr do not activate SimplifyDeMorgan. r=dholbert

Comment 5

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/6ad0553e3073