aaron.ballman added inline comments.
================ Comment at: clang/lib/Sema/SemaOverload.cpp:14290 return MaybeBindToTemporary(call); } ---------------- aaron.ballman wrote: > erichkeane wrote: > > rsmith wrote: > > > aaron.ballman wrote: > > > > erichkeane wrote: > > > > > Was this one missed too? > > > > I couldn't devise a test case that was failing with member function > > > > call expressions, so I left this one alone. We have a bunch of existing > > > > test coverage for calling a consteval member function, so I'm assuming > > > > this is correct, but if someone finds a test case that fails here, it's > > > > easy enough to fix. > > > This code is only reachable for a call through a pointer-to-member. We > > > don't need to worry about `consteval` member function pointers because > > > they can't escape constant-evaluated contexts anyway. Eg, > > > `(p->*&Class::consteval_fn)()` is ill-formed outside of a > > > constant-evaluated context -- we should make sure we have a test for that. > > I can't come up with one either, I think we're fine for now. > ``` > struct test { > consteval int f() const { return 12; } > }; > > constexpr test t; > int main() { > constexpr int i = (t.*&test::f)(); > } > ``` > @rsmith -- would you expect us to accept or reject this? GCC accepts, MSVC > rejects, Clang currently rejects. This is different from your test case > (because this is in a constant evaluated context), which we do already reject > with a decent message: https://godbolt.org/z/3nv4bco9M Thinking about this a bit more, I think that code should be accepted. ``` struct test { consteval int f() const { return 12; } }; constexpr test t; int main() { constexpr int i = t.f(); // If this works constexpr int j = (t.*&test::f)(); // This should also work } ``` However, when I make the obvious changes in this patch to support it, we stop getting the diagnostic outside of a constant evaluated context. So my plan is to land the small fixes we know are correct and are happy with, and we can debate this case more later. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111817/new/ https://reviews.llvm.org/D111817 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits