From 8b533d97e0683a0c87096b95927a2e9ce02243d4 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Thu, 20 Sep 2012 21:52:32 +0000 Subject: [PATCH] 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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@164323 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Sema/Sema.h | 15 +++++- lib/Parse/ParseStmt.cpp | 3 +- lib/Sema/SemaStmt.cpp | 72 ++++++++++++++++---------- lib/Sema/TreeTransform.h | 3 +- test/SemaCXX/for-range-dereference.cpp | 13 +++-- 5 files changed, 70 insertions(+), 36 deletions(-) diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index 391a6d33b2..25e8b45b3e 100644 --- a/include/clang/Sema/Sema.h +++ b/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/lib/Parse/ParseStmt.cpp b/lib/Parse/ParseStmt.cpp index f58f90d56c..db6b3d48f3 100644 --- a/lib/Parse/ParseStmt.cpp +++ b/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/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp index 17d1de81e7..70ece511d1 100644 --- a/lib/Sema/SemaStmt.cpp +++ b/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(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(BeginEndDecl.get()), NotEqExpr.take(), IncrExpr.take(), diff --git a/lib/Sema/TreeTransform.h b/lib/Sema/TreeTransform.h index 71d95ef41f..b8748c7c7a 100644 --- a/lib/Sema/TreeTransform.h +++ b/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/test/SemaCXX/for-range-dereference.cpp b/test/SemaCXX/for-range-dereference.cpp index 3bf50f3330..bf3187da30 100644 --- a/test/SemaCXX/for-range-dereference.cpp +++ b/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 *'}} } -- 2.40.0