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
936ec89e91e2dda8b6110b1fd1f9920509d7a17b.

diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 025b09d..64e9396 100644 (file)
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/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;
   }

diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 8d32fc0..9d5672c 100644 (file)
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/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).

diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index bfbdfbf..ff9d4d6 100644 (file)
--- a/clang/lib/Sema/TreeTransform.h
+++ b/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());

diff --git a/clang/test/SemaCXX/cxx11-crashes.cpp b/clang/test/SemaCXX/cxx11-crashes.cpp
index 19205db..d60782d 100644 (file)
--- a/clang/test/SemaCXX/cxx11-crashes.cpp
+++ b/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);
     }
   }
 }

diff --git a/clang/test/SemaCXX/for-range-crash.cpp b/clang/test/SemaCXX/for-range-crash.cpp
index 2163726..766f34e 100644 (file)
--- a/clang/test/SemaCXX/for-range-crash.cpp
+++ b/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;
+  }
+}