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