aaron.ballman added a comment. In D90042#2360042 <https://reviews.llvm.org/D90042#2360042>, @flx wrote:
> In D90042#2357078 <https://reviews.llvm.org/D90042#2357078>, @aaron.ballman > wrote: > >> In D90042#2356265 <https://reviews.llvm.org/D90042#2356265>, @flx wrote: >> >>> In D90042#2356180 <https://reviews.llvm.org/D90042#2356180>, @aaron.ballman >>> wrote: >>> >>>> In D90042#2350035 <https://reviews.llvm.org/D90042#2350035>, @flx wrote: >>>> >>>>> I should note that I was only able to reproduce the false positive with >>>>> the actual implementation std::function and not our fake version here. >>>> >>>> Any reason not to lift enough of the actual definition to be able to >>>> reproduce the issue in your test cases? Does the change in definitions >>>> break other tests? >>> >>> I poured over the actual definition and couldn't find any difference wrt >>> the call operator that would explain it. I would also think that: >>> >>> template <typename T> >>> void foo(T&& t) { >>> std::forward<T>(t).modify(); >>> } >>> >>> would be a simpler case that should trigger replacement, but it doesn't. Do >>> you have any idea what I could be missing? >> >> Perhaps silly question, but are you instantiating `foo()`? > > I think I added a full implementation of foo now, reverted the change, but am > still not getting the negative case to fail. Can you spot an issue with the > code? I can't, but to be honest, I'm not certain I understand how that false positive could happen in the first place. That's why I was hoping to see the original case -- one thing you could try is with the original code, pass `-E` to preprocess to a file, and then try reducing the test case from that output (either by hand or by using a tool like creduce), or did you already give that a shot? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90042/new/ https://reviews.llvm.org/D90042 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits