[clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

FĂ©lix-Antoine Constantin via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 16 21:17:36 PST 2024


felix642 wrote:

@11happy what are you trying to achieve exactly and what is not working ? 
Is this related to the example that you provided above? 

>``` 
>auto lhsVar1Str = Lexer::getSourceText( >CharSourceRange::getTokenRange(Source.getSpellingLoc(lhsVar1->getBeginLoc()),Source.getSpellingLoc(lhsVar1->getEn>dLoc())),
>      Context.getSourceManager(), Context.getLangOpts());
>```
>it outputs this :
>```
 >//
>-  if (MY_MIN(value1, value2) < value3)
>-    value3 = MY_MIN(value1, value2); //
>+  value3 = std::min((a) < (b) ? (a) : (b), value3); //
> }
>```

If that's the case I'm guessing that you simply forgot to update the begin and end location of the if/then stmt with the spelling loc ?  

Regarding the request for a test case with macros, my intention was to understand how your check handles them. I believe updating a macro is risk free, but there might be use cases that I'm overlooking.
 One potential issue could arise if someone is using the macro in another translation unit without directly importing it. In that case, updating the macro without the necessary include might cause compilation failures. However, I'm unsure if this is a use case that needs consideration.

It might be worth to look if other checks are currently updating macros. This might be a decent indicator of the risk of doing so (If a few checks are already doing it then I might be overthinking it). Also, @PiotrZSL might be able to give us some insight on this since he has more experience with clang-tidy.

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


More information about the cfe-commits mailing list