[clang] Don't typo-fix an expression in a SFINAE context.

If this is a SFINAE context, then continuing to look up names
(in particular, to treat a non-function as a function, and then
do ADL) might too-eagerly complete a type that it's not safe to
complete right now. We should just say "okay, that's a substitution
failure" and not do any more work than absolutely required.

Fixes llvm#52970.

Differential Revision: https://reviews.llvm.org/D117603

Author: Quuxplusone

clang/lib/Sema/Sema.cpp

@@ -2556,32 +2556,36 @@ bool Sema::tryToRecoverWithCall(ExprResult &E, const PartialDiagnostic &PD,
                                 bool (*IsPlausibleResult)(QualType)) {
   SourceLocation Loc = E.get()->getExprLoc();
   SourceRange Range = E.get()->getSourceRange();
-
-  QualType ZeroArgCallTy;
   UnresolvedSet<4> Overloads;
-  if (tryExprAsCall(*E.get(), ZeroArgCallTy, Overloads) &&
-      !ZeroArgCallTy.isNull() &&
-      (!IsPlausibleResult || IsPlausibleResult(ZeroArgCallTy))) {
-    // At this point, we know E is potentially callable with 0
-    // arguments and that it returns something of a reasonable type,
-    // so we can emit a fixit and carry on pretending that E was
-    // actually a CallExpr.
-    SourceLocation ParenInsertionLoc = getLocForEndOfToken(Range.getEnd());
-    bool IsMV = IsCPUDispatchCPUSpecificMultiVersion(E.get());
-    Diag(Loc, PD) << /*zero-arg*/ 1 << IsMV << Range
-                  << (IsCallableWithAppend(E.get())
-                          ? FixItHint::CreateInsertion(ParenInsertionLoc, "()")
-                          : FixItHint());
-    if (!IsMV)
-      notePlausibleOverloads(*this, Loc, Overloads, IsPlausibleResult);
-
-    // FIXME: Try this before emitting the fixit, and suppress diagnostics
-    // while doing so.
-    E = BuildCallExpr(nullptr, E.get(), Range.getEnd(), None,
-                      Range.getEnd().getLocWithOffset(1));
-    return true;
-  }
 
+  // If this is a SFINAE context, don't try anything that might trigger ADL
+  // prematurely.
+  if (!isSFINAEContext()) {
+    QualType ZeroArgCallTy;
+    if (tryExprAsCall(*E.get(), ZeroArgCallTy, Overloads) &&
+        !ZeroArgCallTy.isNull() &&
+        (!IsPlausibleResult || IsPlausibleResult(ZeroArgCallTy))) {
+      // At this point, we know E is potentially callable with 0
+      // arguments and that it returns something of a reasonable type,
+      // so we can emit a fixit and carry on pretending that E was
+      // actually a CallExpr.
+      SourceLocation ParenInsertionLoc = getLocForEndOfToken(Range.getEnd());
+      bool IsMV = IsCPUDispatchCPUSpecificMultiVersion(E.get());
+      Diag(Loc, PD) << /*zero-arg*/ 1 << IsMV << Range
+                    << (IsCallableWithAppend(E.get())
+                            ? FixItHint::CreateInsertion(ParenInsertionLoc,
+                                                         "()")
+                            : FixItHint());
+      if (!IsMV)
+        notePlausibleOverloads(*this, Loc, Overloads, IsPlausibleResult);
+
+      // FIXME: Try this before emitting the fixit, and suppress diagnostics
+      // while doing so.
+      E = BuildCallExpr(nullptr, E.get(), Range.getEnd(), None,
+                        Range.getEnd().getLocWithOffset(1));
+      return true;
+    }
+  }
   if (!ForceComplain) return false;
 
   bool IsMV = IsCPUDispatchCPUSpecificMultiVersion(E.get());

clang/lib/Sema/SemaExprMember.cpp

@@ -1645,6 +1645,9 @@ static ExprResult LookupMemberExpr(Sema &S, LookupResult &R,
           << BaseType << int(IsArrow) << BaseExpr.get()->getSourceRange()
           << FixItHint::CreateReplacement(OpLoc, "->");
 
+      if (S.isSFINAEContext())
+        return ExprError();
+
       // Recurse as an -> access.
       IsArrow = true;
       return LookupMemberExpr(S, R, BaseExpr, IsArrow, OpLoc, SS,

clang/test/SemaTemplate/pr52970.cpp

@@ -0,0 +1,63 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++14 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++17 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify=cxx20 %s
+// expected-no-diagnostics
+
+struct Incomplete;
+template <class T> struct Holder { T t; };
+
+namespace DotFollowingFunctionName {
+struct Good {
+  struct Nested {
+    int b;
+  } a;
+};
+
+struct Bad {
+  Holder<Incomplete> a();
+};
+
+template <class T>
+constexpr auto f(T t) -> decltype((t.a.b, true)) { return true; }
+constexpr bool f(...) { return false; }
+
+static_assert(DotFollowingFunctionName::f(Good{}), "");
+static_assert(!DotFollowingFunctionName::f(Bad{}), "");
+
+#if __cplusplus >= 202002L
+template <class T>
+concept C = requires(T t) { t.a.b; };
+  // cxx20-note@-1 {{because 't.a.b' would be invalid: reference to non-static member function must be called}}
+
+static_assert(C<Good>);
+static_assert(!C<Bad>);
+static_assert(C<Bad>); // cxx20-error {{static_assert failed}}
+  // cxx20-note@-1 {{because 'DotFollowingFunctionName::Bad' does not satisfy 'C'}}
+#endif
+} // namespace DotFollowingFunctionName
+
+namespace DotFollowingPointer {
+struct Good {
+  int begin();
+};
+using Bad = Holder<Incomplete> *;
+
+template <class T>
+constexpr auto f(T t) -> decltype((t.begin(), true)) { return true; }
+constexpr bool f(...) { return false; }
+
+static_assert(DotFollowingPointer::f(Good{}), "");
+static_assert(!DotFollowingPointer::f(Bad{}), "");
+
+#if __cplusplus >= 202002L
+template <class T>
+concept C = requires(T t) { t.begin(); };
+  // cxx20-note@-1 {{because 't.begin()' would be invalid: member reference type 'Holder<Incomplete> *' is a pointer}}
+
+static_assert(C<Good>);
+static_assert(!C<Bad>);
+static_assert(C<Bad>); // cxx20-error {{static_assert failed}}
+  // cxx20-note@-1 {{because 'DotFollowingPointer::Bad' (aka 'Holder<Incomplete> *') does not satisfy 'C'}}
+#endif
+} // namespace DotFollowingPointer