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

Bhuminjay Soni via cfe-commits cfe-commits at lists.llvm.org
Fri Jan 19 06:45:25 PST 2024


11happy wrote:

> I found some things that should be addressed.
> 
> @PiotrZSL you had a comment about "improving readability and promoting the use of standard library functions." in `ReleaseNotes.rst`, I just want to mention that this sentence is also present in the header and check documentation. I don't know if your comment is meant to be scoped to `ReleaseNotes.rst` or this (partial) sentence itself.
> 
> As @felix642 noticed in a review comment, and @PiotrZSL wrote in the linked issue, the check should also detect
> 
> ```c++
> if (value1 < value2)
>   value = value2;
> else
>   value = value1;
> ```
> 
> and `value = (value1 < value2) ? value1 : value2;`.
> 
> I would want to see these cases be handled by this check, the question I have is if this functionality should be added in this pr or in a follow-up pr (the issue should not be closed in this case). Thoughts? IMO, a follow-up pr is fine.

It currently handles this case:
```
if (value1 < value2)
  value = value2;
else
  value = value1;
  ```
  where it is not flagging/warning this type of code as it is two way assignment changing both variables to max,basically current implementation follows along the line of sinlge check referenced by https://pylint.pycqa.org/en/latest/user_guide/messages/refactor/consider-using-max-builtin.html
  also @felix642 suggested yesterday to modify code so that it should not flag this type which earlier used to.
  
  further about below one its a matter of discussion:
  ```
  and `value = (value1 < value2) ? value1 : value2;`.
  ```


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


More information about the cfe-commits mailing list