More robust fix for crash on invalid range-based for statement.

Reliably mark the loop variable declaration in a range for as having an
invalid initializer if anything goes wrong building the initializer. We
previously based this determination on whether an error was emitted,
which is not a reliable signal due to error suppression (during error
recovery etc).

Also, properly mark the variable as having initializer errors rather
than simply marking it invalid. This is necessary to mark any structured
bindings as invalid too.

This generalizes the previous fix in
936ec89.

clang/lib/Sema/SemaDecl.cpp

@@ -12317,7 +12317,7 @@ void Sema::ActOnInitializerError(Decl *D) {
       BD->setInvalidDecl();
 
   // Auto types are meaningless if we can't make sense of the initializer.
-  if (ParsingInitForAutoVars.count(D)) {
+  if (VD->getType()->isUndeducedType()) {
     D->setInvalidDecl();
     return;
   }

clang/lib/Sema/SemaStmt.cpp

@@ -2127,18 +2127,22 @@ StmtResult Sema::ActOnCXXForRangeStmt(Scope *S, SourceLocation ForLoc,
     return StmtError();
   }
 
+  // This function is responsible for attaching an initializer to LoopVar. We
+  // must call ActOnInitializerError if we fail to do so.
   Decl *LoopVar = DS->getSingleDecl();
   if (LoopVar->isInvalidDecl() || !Range ||
       DiagnoseUnexpandedParameterPack(Range, UPPC_Expression)) {
-    LoopVar->setInvalidDecl();
+    ActOnInitializerError(LoopVar);
     return StmtError();
   }
 
   // Build the coroutine state immediately and not later during template
   // instantiation
   if (!CoawaitLoc.isInvalid()) {
-    if (!ActOnCoroutineBodyStart(S, CoawaitLoc, "co_await"))
+    if (!ActOnCoroutineBodyStart(S, CoawaitLoc, "co_await")) {
+      ActOnInitializerError(LoopVar);
       return StmtError();
+    }
   }
 
   // Build  auto && __range = range-init
@@ -2150,7 +2154,7 @@ StmtResult Sema::ActOnCXXForRangeStmt(Scope *S, SourceLocation ForLoc,
                                            std::string("__range") + DepthStr);
   if (FinishForRangeVarDecl(*this, RangeVar, Range, RangeLoc,
                             diag::err_for_range_deduction_failure)) {
-    LoopVar->setInvalidDecl();
+    ActOnInitializerError(LoopVar);
     return StmtError();
   }
 
@@ -2159,14 +2163,20 @@ StmtResult Sema::ActOnCXXForRangeStmt(Scope *S, SourceLocation ForLoc,
       BuildDeclaratorGroup(MutableArrayRef<Decl *>((Decl **)&RangeVar, 1));
   StmtResult RangeDecl = ActOnDeclStmt(RangeGroup, RangeLoc, RangeLoc);
   if (RangeDecl.isInvalid()) {
-    LoopVar->setInvalidDecl();
+    ActOnInitializerError(LoopVar);
     return StmtError();
   }
 
-  return BuildCXXForRangeStmt(
+  StmtResult R = BuildCXXForRangeStmt(
       ForLoc, CoawaitLoc, InitStmt, ColonLoc, RangeDecl.get(),
       /*BeginStmt=*/nullptr, /*EndStmt=*/nullptr,
       /*Cond=*/nullptr, /*Inc=*/nullptr, DS, RParenLoc, Kind);
+  if (R.isInvalid()) {
+    ActOnInitializerError(LoopVar);
+    return StmtError();
+  }
+
+  return R;
 }
 
 /// Create the initialization, compare, and increment steps for
@@ -2349,22 +2359,6 @@ static StmtResult RebuildForRangeWithDereference(Sema &SemaRef, Scope *S,
       AdjustedRange.get(), RParenLoc, Sema::BFRK_Rebuild);
 }
 
-namespace {
-/// RAII object to automatically invalidate a declaration if an error occurs.
-struct InvalidateOnErrorScope {
-  InvalidateOnErrorScope(Sema &SemaRef, Decl *D, bool Enabled)
-      : Trap(SemaRef.Diags), D(D), Enabled(Enabled) {}
-  ~InvalidateOnErrorScope() {
-    if (Enabled && Trap.hasErrorOccurred())
-      D->setInvalidDecl();
-  }
-
-  DiagnosticErrorTrap Trap;
-  Decl *D;
-  bool Enabled;
-};
-}
-
 /// BuildCXXForRangeStmt - Build or instantiate a C++11 for-range statement.
 StmtResult Sema::BuildCXXForRangeStmt(SourceLocation ForLoc,
                                       SourceLocation CoawaitLoc, Stmt *InitStmt,
@@ -2391,11 +2385,6 @@ StmtResult Sema::BuildCXXForRangeStmt(SourceLocation ForLoc,
   DeclStmt *LoopVarDS = cast<DeclStmt>(LoopVarDecl);
   VarDecl *LoopVar = cast<VarDecl>(LoopVarDS->getSingleDecl());
 
-  // If we hit any errors, mark the loop variable as invalid if its type
-  // contains 'auto'.
-  InvalidateOnErrorScope Invalidate(*this, LoopVar,
-                                    LoopVar->getType()->isUndeducedType());
-
   StmtResult BeginDeclStmt = Begin;
   StmtResult EndDeclStmt = End;
   ExprResult NotEqExpr = Cond, IncrExpr = Inc;
@@ -2434,11 +2423,8 @@ StmtResult Sema::BuildCXXForRangeStmt(SourceLocation ForLoc,
     QualType RangeType = Range->getType();
 
     if (RequireCompleteType(RangeLoc, RangeType,
-                            diag::err_for_range_incomplete_type)) {
-      if (LoopVar->getType()->isUndeducedType())
-        LoopVar->setInvalidDecl();
+                            diag::err_for_range_incomplete_type))
       return StmtError();
-    }
 
     // Build auto __begin = begin-expr, __end = end-expr.
     // Divide by 2, since the variables are in the inner scope (loop body).

clang/lib/Sema/TreeTransform.h

@@ -8086,8 +8086,12 @@ TreeTransform<Derived>::TransformCXXForRangeStmt(CXXForRangeStmt *S) {
                                                   Cond.get(),
                                                   Inc.get(), LoopVar.get(),
                                                   S->getRParenLoc());
-    if (NewStmt.isInvalid())
+    if (NewStmt.isInvalid() && LoopVar.get() != S->getLoopVarStmt()) {
+      // Might not have attached any initializer to the loop variable.
+      getSema().ActOnInitializerError(
+          cast<DeclStmt>(LoopVar.get())->getSingleDecl());
       return StmtError();
+    }
   }
 
   StmtResult Body = getDerived().TransformStmt(S->getBody());

clang/test/SemaCXX/cxx11-crashes.cpp

@@ -71,6 +71,7 @@ namespace b6981007 {
       // We used to attempt to evaluate the initializer of this variable,
       // and crash because it has an undeduced type.
       const int &n(x);
+      constexpr int k = sizeof(x);
     }
   }
 }

clang/test/SemaCXX/for-range-crash.cpp

@@ -1,5 +1,10 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 %s
 
+// Ensure that we don't crash if errors are suppressed by an error limit.
+// RUN: not %clang_cc1 -fsyntax-only -std=c++17 -ferror-limit=1 %s
+
+error e; // expected-error {{unknown type name}}
+
 template <typename>
 class Bar {
   Bar<int> *variables_to_modify;
@@ -8,3 +13,18 @@ class Bar {
       delete c;
   }
 };
+
+void foo() {
+  int a;
+  struct X; // expected-note {{forward declaration}}
+  for (X x // expected-error {{incomplete type}}
+      : a) { // expected-error {{range expression of type 'int'}}
+    constexpr int n = sizeof(x);
+  }
+
+  struct S { int x, y; };
+  for (S [x, y] // expected-error {{must be 'auto'}}
+      : a) { // expected-error {{range expression}}
+    typename decltype(x)::a b;
+  }
+}