[clang-tools-extra] Add clang-tidy check readability-math-missing-parentheses (PR #84481)

Julian Schmidt via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 18 04:20:13 PDT 2024


5chmidti wrote:

I let it run for a few minutes and didn't observe any crashes (on a release build though). However, I did find an issue with macros:
```c++
int sink(int);
#define FUN(ARG) (sink(ARG))
#define FUN2(ARG) sink((ARG))
#define FUN3(ARG) sink(ARG)

void f() {
    ...

    //CHECK-MESSAGES: :[[@LINE+4]]:17: warning: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
    //CHECK-FIXES: int r = FUN(0 + (1 * 2));
    int r = FUN(0 + 1 * 2);

    //CHECK-MESSAGES: :[[@LINE+4]]:17: warning: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
    //CHECK-FIXES: int s = FUN2(0 + (1 * 2));
    int s = FUN2(0 + 1 * 2);

    //CHECK-MESSAGES: :[[@LINE+4]]:17: warning: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
    //CHECK-FIXES: int s = FUN3(0 + (1 * 2));
    int t = FUN3(0 + 1 * 2);
}
```
All three of these fail, because the closing parentheses is not added.

Files with issues:
- `polly/lib/External/isl/isl_map.c`
- `/home/julian/dev/llvm/llvm-tmp/llvm/unittests/DebugInfo/MSF/MSFBuilderTest.cpp`
- unittest files will generally have issues with this, because of the test macros
  - found by invoking this inside the build dir: `run-clang-tidy -clang-tidy-binary /path/to/clang-tidy -checks="-*,readability-math-missing*" -quiet unittests` 

Checking `EndLoc.isValid()` reveals that the location is invalid for all of these cases (`llvm::errs() << "EndLoc: " << EndLoc.isValid() << "\n";`).

The documentation for `getLocForEndOfToken` also explicitly mentions this case for macros:
```
/// token where it expected something different that it received. If
/// the returned source location would not be meaningful (e.g., if
/// it points into a macro), this routine returns an invalid
/// source location.
```

https://github.com/llvm/llvm-project/pull/84481


More information about the cfe-commits mailing list