[clang-tools-extra] [clang-tidy] add modernize-use-std-numbers (PR #66583)

Julian Schmidt via cfe-commits cfe-commits at lists.llvm.org
Fri Oct 27 09:25:46 PDT 2023


5chmidti wrote:

> I think you can't, instead of handling macro definitions, you just should handle places where macro is used. It's fine if someone define a macro MY_PI, check should flag usage of such macro, not necessarily a macro.
> 
> Instead of using MacroDefined and try to find those #define MY_PI, you should just use MacroExpands, and find usage of those system macros like M_PI, and suggest replacement usage of them without checking an value.
> 
> As for #define MY_PI 3.14, this should be handled like it is now, simply isMacro shouldn't be used, and warning should be produced inside macro.

Sounds good to do it like this for macros. My original thought about this was to be the as least intrusive as possible in terms of the number of matches and fixes this check does, but I'm all for replacing these macros.
Should variables be treated the same way? I.e. should the fix for
```c++
static constexpr double Phi = 1.6180339;
sink(Phi);
```
be changed from
```c++
static constexpr double Phi = std::numbers::phi;
sink(Phi)
```
to 
```c++
static constexpr double Phi = std::numbers::phi;
sink(std::numbers::phi)
```
?

I could add an option `MatchVariableUses` for this:
- Never (the current way)
- (LocalVariables (matches only uses of variables that are (function-)local declared))?
- Always (like the above proposed last change of the fixits)

defaulting to...

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


More information about the cfe-commits mailing list