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