[AST] Better recovery on an expression refers to an invalid decl.

Prior to the patch, we didn't build a DeclRefExpr if the Decl being
referred to is invalid, because many clang downstream AST consumers
assume it, violating it will cause many diagnostic regressions.

With this patch, we build a DeclRefExpr enven for an invalid decl (when the
AcceptInvalidDecl is true), and wrap it with a dependent-type
RecoveryExpr (to prevent follow-up semantic analysis, and diagnostic
regressions).

This is a revised version of https://reviews.llvm.org/D76831

Reviewed By: sammccall

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

Author: hokein

clang/lib/Sema/SemaDecl.cpp

@@ -1275,7 +1275,7 @@ ExprResult Sema::ActOnNameClassifiedAsNonType(Scope *S, const CXXScopeSpec &SS,
   Result.resolveKind();
 
   bool ADL = UseArgumentDependentLookup(SS, Result, NextToken.is(tok::l_paren));
-  return BuildDeclarationNameExpr(SS, Result, ADL);
+  return BuildDeclarationNameExpr(SS, Result, ADL, /*AcceptInvalidDecl=*/true);
 }
 
 ExprResult Sema::ActOnNameClassifiedAsOverloadSet(Scope *S, Expr *E) {

clang/lib/Sema/SemaExpr.cpp

@@ -3183,8 +3183,9 @@ bool Sema::UseArgumentDependentLookup(const CXXScopeSpec &SS,
 /// as an expression.  This is only actually called for lookups that
 /// were not overloaded, and it doesn't promise that the declaration
 /// will in fact be used.
-static bool CheckDeclInExpr(Sema &S, SourceLocation Loc, NamedDecl *D) {
-  if (D->isInvalidDecl())
+static bool CheckDeclInExpr(Sema &S, SourceLocation Loc, NamedDecl *D,
+                            bool AcceptInvalid) {
+  if (D->isInvalidDecl() && !AcceptInvalid)
     return true;
 
   if (isa<TypedefNameDecl>(D)) {
@@ -3230,7 +3231,8 @@ ExprResult Sema::BuildDeclarationNameExpr(const CXXScopeSpec &SS,
   // result, because in the overloaded case the results can only be
   // functions and function templates.
   if (R.isSingleResult() && !ShouldLookupResultBeMultiVersionOverload(R) &&
-      CheckDeclInExpr(*this, R.getNameLoc(), R.getFoundDecl()))
+      CheckDeclInExpr(*this, R.getNameLoc(), R.getFoundDecl(),
+                      AcceptInvalidDecl))
     return ExprError();
 
   // Otherwise, just build an unresolved lookup expression.  Suppress
@@ -3262,7 +3264,7 @@ ExprResult Sema::BuildDeclarationNameExpr(
          "Cannot refer unambiguously to a function template");
 
   SourceLocation Loc = NameInfo.getLoc();
-  if (CheckDeclInExpr(*this, Loc, D)) {
+  if (CheckDeclInExpr(*this, Loc, D, AcceptInvalidDecl)) {
     // Recovery from invalid cases (e.g. D is an invalid Decl).
     // We use the dependent type for the RecoveryExpr to prevent bogus follow-up
     // diagnostics, as invalid decls use int as a fallback type.
@@ -3494,9 +3496,16 @@ ExprResult Sema::BuildDeclarationNameExpr(
     break;
   }
 
-  return BuildDeclRefExpr(VD, type, valueKind, NameInfo, &SS, FoundD,
-                          /*FIXME: TemplateKWLoc*/ SourceLocation(),
-                          TemplateArgs);
+  auto *E =
+      BuildDeclRefExpr(VD, type, valueKind, NameInfo, &SS, FoundD,
+                       /*FIXME: TemplateKWLoc*/ SourceLocation(), TemplateArgs);
+  // Clang AST consumers assume a DeclRefExpr refers to a valid decl. We
+  // wrap a DeclRefExpr referring to an invalid decl with a dependent-type
+  // RecoveryExpr to avoid follow-up semantic analysis (thus prevent bogus
+  // diagnostics).
+  if (VD->isInvalidDecl() && E)
+    return CreateRecoveryExpr(E->getBeginLoc(), E->getEndLoc(), {E});
+  return E;
 }
 
 static void ConvertUTF8ToWideString(unsigned CharByteWidth, StringRef Source,

clang/test/AST/ast-dump-recovery.cpp

@@ -406,8 +406,17 @@ void RecoveryExprForInvalidDecls(Unknown InvalidDecl) {
   InvalidDecl + 1;
   // CHECK:      BinaryOperator {{.*}}
   // CHECK-NEXT: |-RecoveryExpr {{.*}} '<dependent type>'
+  // CHECK-NEXT: | | `-DeclRefExpr {{.*}} 'InvalidDecl' 'int'
   // CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
   InvalidDecl();
   // CHECK:      CallExpr {{.*}}
   // CHECK-NEXT: `-RecoveryExpr {{.*}} '<dependent type>'
 }
+
+void RecoverToAnInvalidDecl() {
+  Unknown* foo; // invalid decl
+  goo; // the typo was correct to the invalid foo.
+  // Verify that RecoveryExpr has an inner DeclRefExpr.
+  // CHECK:      RecoveryExpr {{.*}} '<dependent type>' contains-errors lvalue
+  // CHECK-NEXT: `-DeclRefExpr {{.*}} 'foo' 'int *'
+}

clang/test/SemaTemplate/constraints.cpp

@@ -15,12 +15,11 @@ namespace PR45589 {
 
   template<bool B, typename T> constexpr int test = 0;
   template<bool B, typename T> requires C<T> constexpr int test<B, T> = 1;
-  template<bool B, typename T> requires (B && C<T>) || (X<T>::value && C<T>) constexpr int test<B, T> = 2; // expected-error {{non-constant expression}} expected-note {{subexpression}} expected-note {{instantiation of}} expected-note {{while substituting}}
+  template<bool B, typename T> requires (B && C<T>) || (X<T>::value && C<T>) constexpr int test<B, T> = 2; // expected-note {{instantiation of}} expected-note {{while substituting}}
   static_assert(test<true, False> == 2);
   static_assert(test<true, True> == 2);
   static_assert(test<true, char> == 2); // satisfaction of second term of || not considered
   static_assert(test<false, False> == 1);
   static_assert(test<false, True> == 2); // constraints are partially ordered
-  // FIXME: These diagnostics are excessive.
-  static_assert(test<false, char> == 1); // expected-note 2{{while}} expected-note 2{{during}}
+  static_assert(test<false, char> == 1); // expected-note {{while}} expected-note {{during}}
 }

clang/test/SemaTemplate/cxx2a-constraint-exprs.cpp

@@ -26,28 +26,22 @@ namespace unevaluated {
 
 namespace constant_evaluated {
   template<typename T> requires f<int[0]> struct S {};
-  // expected-note@-1{{in instantiation of}} expected-note@-1{{while substituting}} \
-     expected-error@-1{{substitution into constraint expression resulted in a non-constant expression}} \
-     expected-note@-1{{subexpression not valid}}
+  // expected-note@-1{{in instantiation of}} expected-note@-1{{while substituting}}
   using s = S<int>;
-  // expected-note@-1 2{{while checking}}
+  // expected-note@-1 {{while checking}}
   template<typename T> void foo() requires f<int[1]> { };
   // expected-note@-1{{in instantiation}} expected-note@-1{{while substituting}} \
-     expected-note@-1{{candidate template ignored}} expected-note@-1{{subexpression not valid}} \
-     expected-error@-1{{substitution into constraint expression resulted in a non-constant expression}}
+     expected-note@-1{{candidate template ignored}}
   int a = (foo<int>(), 0);
-  // expected-note@-1 2{{while checking}} expected-error@-1{{no matching function}} \
-     expected-note@-1 2{{in instantiation}}
+  // expected-note@-1 {{while checking}} expected-error@-1{{no matching function}} \
+     expected-note@-1 {{in instantiation}}
   template<typename T> void bar() requires requires { requires f<int[2]>; } { };
-  // expected-note@-1{{in instantiation}} expected-note@-1{{subexpression not valid}} \
+  // expected-note@-1{{in instantiation}} \
      expected-note@-1{{while substituting}} \
-     expected-error@-1{{substitution into constraint expression resulted in a non-constant expression}} \
-     expected-note@-1 2{{while checking the satisfaction of nested requirement}}
+     expected-note@-1 {{while checking the satisfaction of nested requirement}}
   int b = (bar<int>(), 0);
   template<typename T> struct M { static void foo() requires f<int[3]> { }; };
-  // expected-note@-1{{in instantiation}} expected-note@-1{{subexpression not valid}} \
-     expected-note@-1{{while substituting}} \
-     expected-error@-1{{substitution into constraint expression resulted in a non-constant expression}}
+  // expected-note@-1{{in instantiation}} expected-note@-1{{while substituting}}
   int c = (M<int>::foo(), 0);
-  // expected-note@-1 2{{while checking}}
+  // expected-note@-1 {{while checking}}
 }

clang/test/SemaTemplate/instantiate-var-template.cpp

@@ -28,7 +28,7 @@ namespace InstantiationDependent {
   template<typename T> constexpr T b = a<sizeof(sizeof(f(T())))>; // expected-error {{invalid application of 'sizeof' to an incomplete type 'void'}}
 
   static_assert(b<int> == 1, "");
-  static_assert(b<char> == 1, ""); // expected-note {{in instantiation of}} expected-error {{not an integral constant}}
+  static_assert(b<char> == 1, ""); // expected-note {{in instantiation of}}
 
   template<typename T> void f() {
     static_assert(a<sizeof(sizeof(f(T())))> == 0, ""); // expected-error {{static assertion failed due to requirement 'a<sizeof (sizeof (f(type-parameter-0-0())))> == 0'}} \