On Fri, Mar 27, 2020 at 5:16 PM Arthur O'Dwyer via cfe-commits < cfe-commits@lists.llvm.org> wrote:
> Richard: Okay, filed https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94376 ! > > David: You are correct, the bug in function_ref appeared only when > constructing from `const function_ref&&`. When I said that the constructor > template suppressed the copy constructor, I was wrong. The implicitly > generated copy constructor is still there, and it's the best match for > `const function_ref&` but not for `const function_ref&&`. The bug would > also appear with construction from `volatile function_ref&`, but, let's not > go there. :) > > IMHO (YMMV), it would be worth *additionally* adding a complex but > "realistic" test case that depends on the GCC bug, as opposed to your own > test's simple but "unrealistic" cast to `const function_ref&&`. Here's a > test case that I believe would have dereferenced null on GCC but would have > worked fine on Clang— > > TEST(FunctionRefTest, BadCopyGCCBug) { > > auto A = [] { return 1; }; > > function_ref<int()> W = A; > > auto LW = [=]() { // captures a data member 'const function_ref W' > > return [=]() { > > return W(); > > }; > > }(); > > auto LX = std::move(LW); // calls function_ref's 'const&&' constructor > > W = nullptr; > > ASSERT_EQ(LX(), 1); > > } > Yeah, I tend to err on the side of fairly isolated testing. I guess a fairly closer-to-practical, though smaller (in terms of feature interactions, not necessarily in lines of code) example might be to std::move on a const ref, rather than the raw cast - the next step beyond that would be to use a template to make it more realistic as to why you'd be moving a const ref, etc. But I think the narrower test is alright. - Dave > > –Arthur > > > On Fri, Mar 27, 2020 at 7:48 PM Richard Smith <rich...@metafoo.co.uk> > wrote: > >> On Thu, 26 Mar 2020 at 21:50, Arthur O'Dwyer via cfe-commits < >> cfe-commits@lists.llvm.org> wrote: >> >>> [...] >>> Argh! That is insanely sneaky, and it is quite probable IMHO that this >>> is a core-language bug in GCC, because GCC behaves differently from EDG and >>> Clang here. >>> https://godbolt.org/z/oCvLpv >>> On GCC, when you have a lambda nested within a lambda, and you >>> capture-by-copy a variable which was captured-by-copy using [=] or [i] (but >>> not C++14 init-capture [i=i]), then your data member for that capture will >>> have type `const I`, not just `I`. >>> On Clang and EDG, [=], [i], and [i=i] all behave equivalently, and >>> capture a data member with type `I`. >>> >>> I don't see anything about this on >>> https://gcc.gnu.org/bugzilla/buglist.cgi?quicksearch=lambda%20capture%20const >>> — Richard, you might be best able to describe the issue correctly ;) but if >>> you want me to file it, I will. (Might be a corollary of bug 86697 >>> <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86697>.) >>> >> >> Oh wow, so this would only fail with a combination of libstdc++ (which >> makes a copy of the lambda and destroy the original before copying the >> lambda) and GCC (which has a bug in how it determines the type of a nested >> capture)? Fascinating! :) >> >> [expr.prim.lambda.capture]p10 is the relevant rule: "The type of such a >> data member is the referenced type if the entity is a reference to an >> object, an lvalue reference to the referenced function type if the entity >> is a reference to a function, or the type of the corresponding captured >> entity otherwise." >> >> Regardless of whether you think the captured entity is the original >> variable or the member of the outer closure type, the type of that entity >> is not const-qualified. So the inner capture should not have a >> const-qualified type. >> >> Given that you identified the GCC capture bug, I think you should do the >> honors :) >> >> >>> Regardless, llvm::function_ref's SFINAE should still be fixed. This is a >>> bug in llvm::function_ref, exposed by a bug in GCC. (Or vice versa.) >>> >> >> Yup. >> > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits