[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

Fri Aug 11 01:32:46 PDT 2023

cor3ntin marked 2 inline comments as done.
cor3ntin added inline comments.

================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:4330-4331
+    ExprResult Res = FixOverloadedFunctionReference(From, Found, Fn);
+    if (Res.isInvalid())
+      return ExprError();

----------------
cor3ntin wrote:
> aaron.ballman wrote:
> > Do you have test coverage for this change or is it an NFC change?
> Before this patch `FixOverloadedFunctionReference` would never fail, now it can, in some places. I think it is covered by tests but I'll double check. The change here is mechanical, the important changes are in `FixOverloadedFunctionReference`
I remember.
Before this patch, taking the address of an overloaded function was always valid because lookup would always find either a static function or an implicit function.
But now it can find an explicit function which _requires_ a nested-name-qualifier (otherwise it is ill-formed), but we can only emit that diagnostic once we resolve to a particular function.
https://eel.is/c++draft/expr.unary.op#3.2.sentence-2

This is tested in `test/SemaCXX/cxx2b-deducing-this.cpp` L237 
Because of this new error, `FixOverloadedFunctionReference` can fail - it could not before - and so it's modified to return an `ExprResult`, and all call sites are modified. 

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140828/new/

https://reviews.llvm.org/D140828