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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to