[cfe-commits] r164323 - in /cfe/trunk: include/clang/Sema/Sema.h lib/Parse/ParseStmt.cpp lib/Sema/SemaStmt.cpp lib/Sema/TreeTransform.h test/SemaCXX/for-range-dereference.cpp
Richard Smith
richard-llvm at metafoo.co.uk
Thu Sep 20 14:52:32 PDT 2012
Author: rsmith
Date: Thu Sep 20 16:52:32 2012
New Revision: 164323
URL: http://llvm.org/viewvc/llvm-project?rev=164323&view=rev
Log:
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.
Modified:
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Parse/ParseStmt.cpp
cfe/trunk/lib/Sema/SemaStmt.cpp
cfe/trunk/lib/Sema/TreeTransform.h
cfe/trunk/test/SemaCXX/for-range-dereference.cpp
Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=164323&r1=164322&r2=164323&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Thu Sep 20 16:52:32 2012
@@ -2542,17 +2542,28 @@
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,
Modified: cfe/trunk/lib/Parse/ParseStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseStmt.cpp?rev=164323&r1=164322&r2=164323&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseStmt.cpp (original)
+++ cfe/trunk/lib/Parse/ParseStmt.cpp Thu Sep 20 16:52:32 2012
@@ -1441,7 +1441,8 @@
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
Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=164323&r1=164322&r2=164323&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmt.cpp Thu Sep 20 16:52:32 2012
@@ -1656,7 +1656,7 @@
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 @@
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 @@
}
/// 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 @@
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;
+ // 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++0x for-range statement.
+/// 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 @@
// 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 @@
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 @@
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(),
Modified: cfe/trunk/lib/Sema/TreeTransform.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/TreeTransform.h?rev=164323&r1=164322&r2=164323&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/TreeTransform.h (original)
+++ cfe/trunk/lib/Sema/TreeTransform.h Thu Sep 20 16:52:32 2012
@@ -1330,7 +1330,8 @@
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.
Modified: cfe/trunk/test/SemaCXX/for-range-dereference.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/for-range-dereference.cpp?rev=164323&r1=164322&r2=164323&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/for-range-dereference.cpp (original)
+++ cfe/trunk/test/SemaCXX/for-range-dereference.cpp Thu Sep 20 16:52:32 2012
@@ -17,17 +17,17 @@
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 @@
// 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 at -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 @@
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 at -1 {{no viable conversion from 'Data' to 'Data *'}}
+ // expected-note at 4 {{selected 'begin' function with iterator type 'Data *'}}
}
More information about the cfe-commits
mailing list