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