[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