[PATCH] D22725: [clang-tidy] Add check 'modernize-use-algorithm'
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 8 05:29:21 PDT 2016
aaron.ballman added a comment.
In https://reviews.llvm.org/D22725#506058, @JDevlieghere wrote:
> - Added function pointer test case
> - Used placeholders for diagnostics
> I extended the matchers to include `::memcpy` and `::memset` as well because the check otherwise does not work on my mac because the `cstring` header that ships with Xcode is implemented as shown below.
> ``` _LIBCPP_BEGIN_NAMESPACE_STD using ::size_t; using ::memcpy; using ::memset; ... _LIBCPP_END_NAMESPACE_STD ```
You should add a similar approach as a test case to ensure we're covering this usage.
> This is annoying as I have no way to discriminate functions that have the same signature. Unless there's a better solution, this seems like a reasonable trade-off to me. Of course I'm open to suggestions!
Given that you also have to handle the case where C++ code includes string.h (rather than cstring) to call memcpy, I think this is the only approach you can really take. Either you are getting a global declaration or one in the std namespace, and both should be handled when the signatures are correct.
> In https://reviews.llvm.org/D22725#505739, @aaron.ballman wrote:
> > The tests added mostly cover them -- I elaborated on the function pointer case, which I don't *think* will go wrong with this check, but should have a pathological test case for just to be sure.
> I added the test case. The call is indeed replaced, which I guess is fine?
Yes, that's the behavior I would expect.
> In https://reviews.llvm.org/D22725#505476, @Prazek wrote:
> > Did you manage to see what was wrong in the transformation that you did on LLVM?
> Not yet, thought it seemed to be related to `std::copy` rather than `std::fill`. I'm still trying to pinpoint the culprit but it's extremely time consuming as I have to recompile LLVM completely in order to run the tests...
More information about the cfe-commits