If the range in a for range statement doesn't have a viable begin/end function,

but can be dereferenced to form an expression which does have viable begin/end
functions, then typo-correct the range, even if something else goes wrong with
the statement (such as inaccessible begin/end or the wrong type of loop
variable).

In order to ensure we recover correctly and produce any followup diagnostics in
this case, redo semantic analysis on the for-range statement outside of the
diagnostic trap, after issuing the typo-correction.

llvm-svn: 164323

5 files changed, 70 insertions, 36 deletions

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 391a6d33b29..25e8b45b3e9 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -2542,17 +2542,28 @@ public:
                                         SourceLocation RParenLoc);
   StmtResult FinishObjCForCollectionStmt(Stmt *ForCollection, Stmt *Body);
 
+  enum BuildForRangeKind {
+    /// Initial building of a for-range statement.
+    BFRK_Build,
+    /// Instantiation or recovery rebuild of a for-range statement. Don't
+    /// attempt any typo-correction.
+    BFRK_Rebuild,
+    /// Determining whether a for-range statement could be built. Avoid any
+    /// unnecessary or irreversible actions.
+    BFRK_Check
+  };
+
   StmtResult ActOnCXXForRangeStmt(SourceLocation ForLoc, Stmt *LoopVar,
                                   SourceLocation ColonLoc, Expr *Collection,
                                   SourceLocation RParenLoc,
-                                  bool ShouldTryDeref);
+                                  BuildForRangeKind Kind);
   StmtResult BuildCXXForRangeStmt(SourceLocation ForLoc,
                                   SourceLocation ColonLoc,
                                   Stmt *RangeDecl, Stmt *BeginEndDecl,
                                   Expr *Cond, Expr *Inc,
                                   Stmt *LoopVarDecl,
                                   SourceLocation RParenLoc,
-                                  bool ShouldTryDeref);
+                                  BuildForRangeKind Kind);
   StmtResult FinishCXXForRangeStmt(Stmt *ForRange, Stmt *Body);
 
   StmtResult ActOnGotoStmt(SourceLocation GotoLoc,
diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp
index f58f90d56c5..db6b3d48f3a 100644
--- a/clang/lib/Parse/ParseStmt.cpp
+++ b/clang/lib/Parse/ParseStmt.cpp
@@ -1441,7 +1441,8 @@ StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc) {
     ForRangeStmt = Actions.ActOnCXXForRangeStmt(ForLoc, FirstPart.take(),
                                                 ForRangeInit.ColonLoc,
                                                 ForRangeInit.RangeExpr.get(),
-                                                T.getCloseLocation(), true);
+                                                T.getCloseLocation(),
+                                                Sema::BFRK_Build);
 
 
   // Similarly, we need to do the semantic analysis for a for-range
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 17d1de81e75..70ece511d10 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -1656,7 +1656,7 @@ static bool ObjCEnumerationCollection(Expr *Collection) {
 StmtResult
 Sema::ActOnCXXForRangeStmt(SourceLocation ForLoc,
                            Stmt *First, SourceLocation ColonLoc, Expr *Range,
-                           SourceLocation RParenLoc, bool ShouldTryDeref) {
+                           SourceLocation RParenLoc, BuildForRangeKind Kind) {
   if (!First || !Range)
     return StmtError();
 
@@ -1694,7 +1694,7 @@ Sema::ActOnCXXForRangeStmt(SourceLocation ForLoc,
 
   return BuildCXXForRangeStmt(ForLoc, ColonLoc, RangeDecl.get(),
                               /*BeginEndDecl=*/0, /*Cond=*/0, /*Inc=*/0, DS,
-                              RParenLoc, ShouldTryDeref);
+                              RParenLoc, Kind);
 }
 
 /// \brief Create the initialization, compare, and increment steps for
@@ -1782,8 +1782,8 @@ static Sema::ForRangeStatus BuildNonArrayForRange(Sema &SemaRef, Scope *S,
 }
 
 /// Speculatively attempt to dereference an invalid range expression.
-/// This function will not emit diagnostics, but returns StmtError if
-/// an error occurs.
+/// If the attempt fails, this function will return a valid, null StmtResult
+/// and emit no diagnostics.
 static StmtResult RebuildForRangeWithDereference(Sema &SemaRef, Scope *S,
                                                  SourceLocation ForLoc,
                                                  Stmt *LoopVarDecl,
@@ -1791,22 +1791,40 @@ static StmtResult RebuildForRangeWithDereference(Sema &SemaRef, Scope *S,
                                                  Expr *Range,
                                                  SourceLocation RangeLoc,
                                                  SourceLocation RParenLoc) {
-  Sema::SFINAETrap Trap(SemaRef);
-  ExprResult AdjustedRange = SemaRef.BuildUnaryOp(S, RangeLoc, UO_Deref, Range);
-  StmtResult SR =
-    SemaRef.ActOnCXXForRangeStmt(ForLoc, LoopVarDecl, ColonLoc,
-                                 AdjustedRange.get(), RParenLoc, false);
-  if (Trap.hasErrorOccurred())
-    return StmtError();
-  return SR;
-}
-
-/// BuildCXXForRangeStmt - Build or instantiate a C++0x for-range statement.
+  // Determine whether we can rebuild the for-range statement with a
+  // dereferenced range expression.
+  ExprResult AdjustedRange;
+  {
+    Sema::SFINAETrap Trap(SemaRef);
+
+    AdjustedRange = SemaRef.BuildUnaryOp(S, RangeLoc, UO_Deref, Range);
+    if (AdjustedRange.isInvalid())
+      return StmtResult();
+
+    StmtResult SR =
+      SemaRef.ActOnCXXForRangeStmt(ForLoc, LoopVarDecl, ColonLoc,
+                                   AdjustedRange.get(), RParenLoc,
+                                   Sema::BFRK_Check);
+    if (SR.isInvalid())
+      return StmtResult();
+  }
+
+  // The attempt to dereference worked well enough that it could produce a valid
+  // loop. Produce a fixit, and rebuild the loop with diagnostics enabled, in
+  // case there are any other (non-fatal) problems with it.
+  SemaRef.Diag(RangeLoc, diag::err_for_range_dereference)
+    << Range->getType() << FixItHint::CreateInsertion(RangeLoc, "*");
+  return SemaRef.ActOnCXXForRangeStmt(ForLoc, LoopVarDecl, ColonLoc,
+                                      AdjustedRange.get(), RParenLoc,
+                                      Sema::BFRK_Rebuild);
+}
+
+/// BuildCXXForRangeStmt - Build or instantiate a C++11 for-range statement.
 StmtResult
 Sema::BuildCXXForRangeStmt(SourceLocation ForLoc, SourceLocation ColonLoc,
                            Stmt *RangeDecl, Stmt *BeginEnd, Expr *Cond,
                            Expr *Inc, Stmt *LoopVarDecl,
-                           SourceLocation RParenLoc, bool ShouldTryDeref) {
+                           SourceLocation RParenLoc, BuildForRangeKind Kind) {
   Scope *S = getCurScope();
 
   DeclStmt *RangeDS = cast<DeclStmt>(RangeDecl);
@@ -1902,25 +1920,19 @@ Sema::BuildCXXForRangeStmt(SourceLocation ForLoc, SourceLocation ColonLoc,
 
       // If building the range failed, try dereferencing the range expression
       // unless a diagnostic was issued or the end function is problematic.
-      if (ShouldTryDeref && RangeStatus == FRS_NoViableFunction &&
+      if (Kind == BFRK_Build && RangeStatus == FRS_NoViableFunction &&
           BEFFailure == BEF_begin) {
         StmtResult SR = RebuildForRangeWithDereference(*this, S, ForLoc,
                                                        LoopVarDecl, ColonLoc,
                                                        Range, RangeLoc,
                                                        RParenLoc);
-        if (!SR.isInvalid()) {
-          // The attempt to dereference would succeed; return the result of
-          // recovery.
-          Diag(RangeLoc, diag::err_for_range_dereference)
-              << RangeLoc << RangeType
-              << FixItHint::CreateInsertion(RangeLoc, "*");
+        if (SR.isInvalid() || SR.isUsable())
           return SR;
-        }
       }
 
       // Otherwise, emit diagnostics if we haven't already.
       if (RangeStatus == FRS_NoViableFunction) {
-        Expr *Range = BEFFailure ?  EndRangeRef.get() : BeginRangeRef.get();
+        Expr *Range = BEFFailure ? EndRangeRef.get() : BeginRangeRef.get();
         Diag(Range->getLocStart(), diag::err_for_range_invalid)
             << RangeLoc << Range->getType() << BEFFailure;
         CandidateSet.NoteCandidates(*this, OCD_AllCandidates,
@@ -2003,8 +2015,9 @@ Sema::BuildCXXForRangeStmt(SourceLocation ForLoc, SourceLocation ColonLoc,
       return StmtError();
     }
 
-    // Attach  *__begin  as initializer for VD.
-    if (!LoopVar->isInvalidDecl()) {
+    // Attach  *__begin  as initializer for VD. Don't touch it if we're just
+    // trying to determine whether this would be a valid range.
+    if (!LoopVar->isInvalidDecl() && Kind != BFRK_Check) {
       AddInitializerToDecl(LoopVar, DerefExpr.get(), /*DirectInit=*/false,
                            /*TypeMayContainAuto=*/true);
       if (LoopVar->isInvalidDecl())
@@ -2015,6 +2028,11 @@ Sema::BuildCXXForRangeStmt(SourceLocation ForLoc, SourceLocation ColonLoc,
     RangeVar->setUsed();
   }
 
+  // Don't bother to actually allocate the result if we're just trying to
+  // determine whether it would be valid.
+  if (Kind == BFRK_Check)
+    return StmtResult();
+
   return Owned(new (Context) CXXForRangeStmt(RangeDS,
                                      cast_or_null<DeclStmt>(BeginEndDecl.get()),
                                              NotEqExpr.take(), IncrExpr.take(),
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 71d95ef41ff..b8748c7c7a9 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -1330,7 +1330,8 @@ public:
                                     Stmt *LoopVar,
                                     SourceLocation RParenLoc) {
     return getSema().BuildCXXForRangeStmt(ForLoc, ColonLoc, Range, BeginEnd,
-                                          Cond, Inc, LoopVar, RParenLoc, false);
+                                          Cond, Inc, LoopVar, RParenLoc,
+                                          Sema::BFRK_Rebuild);
   }
 
   /// \brief Build a new C++0x range-based for statement.
diff --git a/clang/test/SemaCXX/for-range-dereference.cpp b/clang/test/SemaCXX/for-range-dereference.cpp
index 3bf50f33300..bf3187da30e 100644
--- a/clang/test/SemaCXX/for-range-dereference.cpp
+++ b/clang/test/SemaCXX/for-range-dereference.cpp
@@ -17,17 +17,17 @@ struct DeletedEnd : public T {
 struct DeletedADLBegin { };
 
 int* begin(DeletedADLBegin) = delete; //expected-note {{candidate function has been explicitly deleted}} \
- expected-note 6 {{candidate function not viable: no known conversion}}
+ expected-note 5 {{candidate function not viable: no known conversion}}
 
 struct PrivateEnd {
   Data *begin();
 
  private:
-  Data *end(); // expected-note 1 {{declared private here}}
+  Data *end(); // expected-note 2 {{declared private here}}
 };
 
 struct ADLNoEnd { };
-Data * begin(ADLNoEnd); // expected-note 7 {{candidate function not viable: no known conversion}}
+Data * begin(ADLNoEnd); // expected-note 6 {{candidate function not viable: no known conversion}}
 
 struct OverloadedStar {
   T operator*();
@@ -70,10 +70,9 @@ expected-note {{when looking up 'end' function for range expression of type 'Del
   // the range is invalid.
   for (auto i : PE) { } // expected-error{{'end' is a private member of 'PrivateEnd'}}
 
-  // FIXME: This diagnostic should be improved as well. It should not mention a
-  // deleted function, and we should not issue a FixIt suggesting a dereference.
   PrivateEnd *pPE;
   for (auto i : pPE) { }// expected-error {{invalid range expression of type 'PrivateEnd *'}}
+  // expected-error@-1 {{'end' is a private member of 'PrivateEnd'}}
 
   DeletedADLBegin DAB;
   for (auto i : DAB) { } // expected-error {{call to deleted function 'begin'}}\
@@ -83,4 +82,8 @@ expected-note {{when looking up 'end' function for range expression of type 'Del
   for (auto i : *OS) { }
 
   for (auto i : OS) { } // expected-error {{invalid range expression of type 'OverloadedStar'; did you mean to dereference it with '*'?}}
+
+  for (Data *p : pt) { } // expected-error {{invalid range expression of type 'T *'; did you mean to dereference it with '*'?}}
+  // expected-error@-1 {{no viable conversion from 'Data' to 'Data *'}}
+  // expected-note@4 {{selected 'begin' function with iterator type 'Data *'}}
 }