[PATCH] D22725: [clang-tidy] Add check 'modernize-use-algorithm'

[Aaron Ballman via cfe-commits]

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...





Repository:
  rL LLVM

https://reviews.llvm.org/D22725