[cfe-commits] [patch] Better diagnostics in for-range expressions.
Richard Smith
richard at metafoo.co.uk
Wed Aug 8 15:51:36 PDT 2012
> --- a/include/clang/Sema/Sema.h
> +++ b/include/clang/Sema/Sema.h
> @@ -19,6 +19,7 @@
> #include "clang/Sema/AnalysisBasedWarnings.h"
> #include "clang/Sema/IdentifierResolver.h"
> #include "clang/Sema/ObjCMethodList.h"
> +#include "clang/Sema/Overload.h"
Can you make BuildOverloadedCallExpr local to SemaOverload.cpp to avoid
this include? (This is for OverloadCandidateSet::iterator, I think?)
> #include "clang/Sema/DeclSpec.h"
> #include "clang/Sema/ExternalSemaSource.h"
> #include "clang/Sema/LocInfoType.h"
> @@ -1924,6 +1925,46 @@ public:
> OverloadCandidateSet &CandidateSet,
> bool PartialOverloading = false);
>
> + // An enum used to represent the different possible results of
building a
> + // range-based for loop.
> + enum ForRangeStatus {
> + FRS_Success,
> + FRS_InvalidFunction,
> + FRS_NoViableFunction,
> + FRS_Ambiguous,
> + FRS_Deleted,
> + FRS_DiagnosticIssued,
> + FRS_BeginEndMismatch
> + };
> + // An enum to represent whether something is dealing with a call to
begin()
> + // or a call to end() in a range-based for loop.
> + enum BeginEndFunction {
> + BEF_begin,
> + BEF_end
> + };
> +
> + void NoteForRangeBeginEndFunction(Expr *E, BeginEndFunction BEF);
> + ForRangeStatus BuildForRangeBeginEndCall(Scope *S, SourceLocation Loc,
> + SourceLocation RangeLoc,
> + VarDecl *Decl,
> + BeginEndFunction BEF,
> + const DeclarationNameInfo
&NameInfo,
> + LookupResult &MemberLookup,
> + OverloadCandidateSet
*CandidateSet,
> + Expr *Range, ExprResult
*CallExpr,
> + const FunctionDecl
**FoundFunction);
> +
> + ExprResult BuildOverloadedCallExpr(Scope *S, Expr *Fn,
> + UnresolvedLookupExpr *ULE,
> + SourceLocation LParenLoc,
> + Expr **Args, unsigned NumArgs,
> + SourceLocation RParenLoc,
> + Expr *ExecConfig,
> + OverloadCandidateSet *CandidateSet,
> + OverloadCandidateSet::iterator
*Best,
> + OverloadingResult OverloadResult,
> + bool AllowTypoCorrection);
> +
> ExprResult BuildOverloadedCallExpr(Scope *S, Expr *Fn,
> UnresolvedLookupExpr *ULE,
> SourceLocation LParenLoc,
> @@ -1932,6 +1973,12 @@ public:
> Expr *ExecConfig,
> bool AllowTypoCorrection=true);
>
> + bool buildOverloadedCallSet(Scope *S, Expr *Fn, UnresolvedLookupExpr
*ULE,
> + Expr **Args, unsigned NumArgs,
> + SourceLocation RParenLoc,
> + OverloadCandidateSet *CandidateSet,
> + ExprResult *Result);
> +
> ExprResult CreateOverloadedUnaryOp(SourceLocation OpLoc,
> unsigned Opc,
> const UnresolvedSetImpl &Fns,
> @@ -2514,15 +2561,20 @@ public:
> StmtResult ActOnCXXForRangeStmt(SourceLocation ForLoc,
> SourceLocation LParenLoc, Stmt
*LoopVar,
> SourceLocation ColonLoc, Expr
*Collection,
> - SourceLocation RParenLoc);
> + SourceLocation RParenLoc,
> + bool ShouldTryDeref = true);
> StmtResult BuildCXXForRangeStmt(SourceLocation ForLoc,
> SourceLocation ColonLoc,
> Stmt *RangeDecl, Stmt *BeginEndDecl,
> Expr *Cond, Expr *Inc,
> Stmt *LoopVarDecl,
> - SourceLocation RParenLoc);
> + SourceLocation RParenLoc,
> + bool ShouldTryDeref = true);
We shouldn't try dereferencing the argument during template instantiation.
Please remove the default argument here and set it to false in
TreeTransform.h.
> StmtResult FinishCXXForRangeStmt(Stmt *ForRange, Stmt *Body);
>
> + bool FinishForRangeVarDecl(VarDecl *Decl, Expr *Init,
> + SourceLocation Loc, int diag);
> +
> StmtResult ActOnGotoStmt(SourceLocation GotoLoc,
> SourceLocation LabelLoc,
> LabelDecl *TheDecl);
> --- a/lib/Sema/SemaOverload.cpp
> +++ b/lib/Sema/SemaOverload.cpp
> @@ -11195,6 +11238,102 @@ ExprResult
Sema::BuildLiteralOperatorCall(LookupResult &R,
> return MaybeBindToTemporary(UDL);
> }
>
> +
> +/// Build a call to 'begin' or 'end' for a C++0x for-range statement. If
the
> +/// given LookupResult is non-empty, it is assumed to describe a member
which
> +/// will be invoked. Otherwise, the function will be found via argument
> +/// dependent lookup.
> +/// CallExpr is set to a valid expression and FRS_Success returned on
success,
> +/// otherwise CallExpr is set to ExprError() and some non-success value
> +/// is returned.
> +Sema::ForRangeStatus
> +Sema::BuildForRangeBeginEndCall(Scope *S, SourceLocation Loc,
> + SourceLocation RangeLoc, VarDecl *Decl,
> + BeginEndFunction BEF,
> + const DeclarationNameInfo &NameInfo,
> + LookupResult &MemberLookup,
> + OverloadCandidateSet *CandidateSet,
> + Expr *Range, ExprResult *CallExpr,
> + const FunctionDecl **FoundFunction) {
In the cases where this function returns FRS_DiagnosticIssued after
producing a diagnostic which isn't for-range specific, it'd be useful for
it to issue diag::note_for_range_type like it used to.
> + CandidateSet->clear();
> + if (!MemberLookup.empty()) {
> + ExprResult MemberRef =
> + BuildMemberReferenceExpr(Range, Range->getType(), Loc,
> + /*IsPtr=*/false, CXXScopeSpec(),
> + /*TemplateKWLoc=*/SourceLocation(),
> + /*FirstQualifierInScope=*/0,
> + MemberLookup,
> + /*TemplateArgs=*/0);
> + if (MemberRef.isInvalid()) {
> + *CallExpr = ExprError();
> + return FRS_InvalidFunction;
This should be FRS_DiagnosticIssued, I think?
> + }
> + *CallExpr = ActOnCallExpr(S, MemberRef.get(), Loc, MultiExprArg(),
Loc, 0);
> + if (CallExpr->isInvalid()) {
> + *CallExpr = ExprError();
> + return FRS_InvalidFunction;
This too.
> + }
> + } else {
> + UnresolvedSet<0> FoundNames;
> + // C++0x [stmt.ranged]p1: For the purposes of this name lookup,
namespace
> + // std is an associated namespace.
> + UnresolvedLookupExpr *Fn =
> + UnresolvedLookupExpr::Create(Context, /*NamingClass=*/0,
> + NestedNameSpecifierLoc(), NameInfo,
> + /*NeedsADL=*/true,
/*Overloaded=*/false,
> + FoundNames.begin(), FoundNames.end(),
> + /*LookInStdNamespace=*/true);
> +
> + buildOverloadedCallSet(S, Fn, Fn, &Range, 1, Loc, CandidateSet,
CallExpr);
Do you need to handle the case where this returns true?
> +
> + if (CandidateSet->empty()) {
> + *CallExpr = ExprError();
> + return FRS_InvalidFunction;
You can use FRS_NoViableFunction here, and remove FRS_InvalidFunction.
> + } else {
> + OverloadCandidateSet::iterator Best;
> + OverloadingResult OverloadResult =
> + CandidateSet->BestViableFunction(*this, Fn->getLocStart(),
Best);
> + switch (OverloadResult) {
> + case OR_Success:
> + *CallExpr =
> + BuildOverloadedCallExpr(S, Fn, Fn, Loc, &Range, 1, Loc, 0,
> + CandidateSet, &Best, OverloadResult,
> + /*AllowTypoCorrection=*/false);
> + if (CallExpr->isInvalid())
> + return FRS_InvalidFunction;
This should be FRS_DiagnosticIssued.
> + break;
> +
> + case OR_No_Viable_Function:
> + *CallExpr = ExprError();
> + return FRS_NoViableFunction;
> +
> + case OR_Ambiguous:
> + *FoundFunction = Best->Function;
> + *CallExpr = ExprError();
> + return FRS_Ambiguous;
> +
> + case OR_Deleted:
> + // FIXME: this case is never actually reached, since
ActOnCallExpr
> + // detects deleted functions, issues a diagnostic, and returns
> + // an invalid ExprResult.
> + // As a result, the diagnostics for pointers to types with
deleted begin
> + // or end functions are not as clear as they could be.
I think this comment is stale; the ActOnCallExpr call is in the other arm
of this 'if'.
I'm also not sure that it's a good idea to try to recover if we find an
ambiguous or deleted 'begin' or 'end' function. Those both indicate that
the range expression was probably fine, but that range-based for is not
supposed to work for this type.
> + *FoundFunction = Best->Function;
> + *CallExpr = ExprError();
> + return FRS_Deleted;
> + }
> + }
> +
> + }
> + if (FinishForRangeVarDecl(Decl, CallExpr->get(), Loc,
> + diag::err_for_range_iter_deduction_failure))
{
> + NoteForRangeBeginEndFunction(CallExpr->get(), BEF);
> + return FRS_DiagnosticIssued;
> + }
> + return FRS_Success;
> +}
> +
> +
> /// FixOverloadedFunctionReference - E is an expression that refers to
> /// a C++ overloaded function (possibly with some parentheses and
> /// perhaps a '&' around it). We have resolved the overloaded function
> diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp
> index 9affe98..acc5042 100644
> --- a/lib/Sema/SemaStmt.cpp
> +++ b/lib/Sema/SemaStmt.cpp
[...]
> +static Sema::ForRangeStatus BuildNonArrayForRange(Sema &SemaRef, Scope
*S,
> + Expr *BeginRange, Expr
*EndRange,
> + QualType RangeType,
> + VarDecl *BeginVar,
> + VarDecl *EndVar,
> + SourceLocation ColonLoc,
> + OverloadCandidateSet
*CandidateSet,
> + ExprResult *BeginExpr,
> + ExprResult *EndExpr,
> + const FunctionDecl
**FoundFunction,
> + Sema::BeginEndFunction *BEF)
{
> + DeclarationNameInfo BeginNameInfo(
> + &SemaRef.PP.getIdentifierTable().get("begin"), ColonLoc);
> + DeclarationNameInfo
EndNameInfo(&SemaRef.PP.getIdentifierTable().get("end"),
> + ColonLoc);
> +
> + LookupResult BeginMemberLookup(SemaRef, BeginNameInfo,
> + Sema::LookupMemberName);
> + LookupResult EndMemberLookup(SemaRef, EndNameInfo,
Sema::LookupMemberName);
> +
> + if (CXXRecordDecl *D = RangeType->getAsCXXRecordDecl()) {
> + // - if _RangeT is a class type, the unqualified-ids begin and end
are
> + // looked up in the scope of class _RangeT as if by class member
access
> + // lookup (3.4.5), and if either (or both) finds at least one
> + // declaration, begin-expr and end-expr are __range.begin() and
> + // __range.end(), respectively;
> + SemaRef.LookupQualifiedName(BeginMemberLookup, D);
> + SemaRef.LookupQualifiedName(EndMemberLookup, D);
> +
> + if (BeginMemberLookup.empty() != EndMemberLookup.empty()) {
> + *BEF = BeginMemberLookup.empty() ? Sema::BEF_end : Sema::BEF_begin;
> + return Sema::FRS_BeginEndMismatch;
> + }
> + } else {
> + // - otherwise, begin-expr and end-expr are begin(__range) and
> + // end(__range), respectively, where begin and end are looked up
with
> + // argument-dependent lookup (3.4.2). For the purposes of this name
> + // lookup, namespace std is an associated namespace.
> +
> + }
> +
> + *BEF = Sema::BEF_begin;
> + Sema::ForRangeStatus RangeStatus =
> + SemaRef.BuildForRangeBeginEndCall(S, ColonLoc, ColonLoc, BeginVar,
> + Sema::BEF_begin, BeginNameInfo,
> + BeginMemberLookup, CandidateSet,
> + BeginRange, BeginExpr,
FoundFunction);
> + if (RangeStatus != Sema::FRS_Success)
> + return RangeStatus;
> +
> + *BEF = Sema::BEF_end;
> + RangeStatus =
> + SemaRef.BuildForRangeBeginEndCall(S, ColonLoc, ColonLoc, EndVar,
> + Sema::BEF_end, EndNameInfo,
> + EndMemberLookup, CandidateSet,
> + EndRange, EndExpr,
FoundFunction);
> + return RangeStatus;
If there's a viable begin(...) but lookup for end(...) fails, it's not
likely that there's a problem with the range expression, and we shouldn't
try to recover in this case.
> }
>
> /// BuildCXXForRangeStmt - Build or instantiate a C++0x for-range
statement.
> @@ -1754,7 +1818,7 @@ StmtResult
> Sema::BuildCXXForRangeStmt(SourceLocation ForLoc, SourceLocation
ColonLoc,
> Stmt *RangeDecl, Stmt *BeginEnd, Expr *Cond,
> Expr *Inc, Stmt *LoopVarDecl,
> - SourceLocation RParenLoc) {
> + SourceLocation RParenLoc, bool
ShouldTryDeref) {
> Scope *S = getCurScope();
>
> DeclStmt *RangeDS = cast<DeclStmt>(RangeDecl);
> @@ -1808,9 +1872,9 @@ Sema::BuildCXXForRangeStmt(SourceLocation ForLoc,
SourceLocation ColonLoc,
>
> // begin-expr is __range.
> BeginExpr = BeginRangeRef;
> - if (FinishForRangeVarDecl(*this, BeginVar, BeginRangeRef.get(),
ColonLoc,
> + if (FinishForRangeVarDecl(BeginVar, BeginRangeRef.get(), ColonLoc,
>
diag::err_for_range_iter_deduction_failure)) {
> - NoteForRangeBeginEndFunction(*this, BeginExpr.get(), BEF_begin);
> + NoteForRangeBeginEndFunction(BeginExpr.get(), BEF_begin);
> return StmtError();
> }
>
> @@ -1834,62 +1898,72 @@ Sema::BuildCXXForRangeStmt(SourceLocation ForLoc,
SourceLocation ColonLoc,
> BoundExpr.get());
> if (EndExpr.isInvalid())
> return StmtError();
> - if (FinishForRangeVarDecl(*this, EndVar, EndExpr.get(), ColonLoc,
> + if (FinishForRangeVarDecl(EndVar, EndExpr.get(), ColonLoc,
>
diag::err_for_range_iter_deduction_failure)) {
> - NoteForRangeBeginEndFunction(*this, EndExpr.get(), BEF_end);
> + NoteForRangeBeginEndFunction(EndExpr.get(), BEF_end);
> return StmtError();
> }
> } else {
> - DeclarationNameInfo
BeginNameInfo(&PP.getIdentifierTable().get("begin"),
> - ColonLoc);
> - DeclarationNameInfo
EndNameInfo(&PP.getIdentifierTable().get("end"),
> - ColonLoc);
> -
> - LookupResult BeginMemberLookup(*this, BeginNameInfo,
LookupMemberName);
> - LookupResult EndMemberLookup(*this, EndNameInfo, LookupMemberName);
> -
> - if (CXXRecordDecl *D = RangeType->getAsCXXRecordDecl()) {
> - // - if _RangeT is a class type, the unqualified-ids begin and
end are
> - // looked up in the scope of class _RangeT as if by class
member access
> - // lookup (3.4.5), and if either (or both) finds at least one
> - // declaration, begin-expr and end-expr are __range.begin() and
> - // __range.end(), respectively;
> - LookupQualifiedName(BeginMemberLookup, D);
> - LookupQualifiedName(EndMemberLookup, D);
> -
> - if (BeginMemberLookup.empty() != EndMemberLookup.empty()) {
> - Diag(ColonLoc, diag::err_for_range_member_begin_end_mismatch)
> - << RangeType << BeginMemberLookup.empty();
> + Expr *BeginRange = BeginRangeRef.get();
> + Expr *EndRange = EndRangeRef.get();
> + const FunctionDecl *FoundFunction = NULL;
> + OverloadCandidateSet CandidateSet(RangeLoc);
> + Sema::BeginEndFunction BEFFailure;
> + ForRangeStatus RangeStatus =
> + BuildNonArrayForRange(*this, S, BeginRange, EndRange,
RangeType,
> + BeginVar, EndVar, ColonLoc,
&CandidateSet,
> + &BeginExpr, &EndExpr, &FoundFunction,
> + &BEFFailure);
> +
> + // Attempt to dereference pointers and try again.
> + if (RangeStatus != FRS_Success) {
We shouldn't attempt recovery if we've issued a diagnostic already.
> + if (!ShouldTryDeref)
> + return StmtError();
> + if (!RangeType->isPointerType()) {
> + const ExprResult & TheCall = BEFFailure ? EndExpr: BeginExpr;
> + ForRangeDiagnostics(*this, RangeStatus,
> + BEFFailure ? EndRange : BeginRange,
> + TheCall, FoundFunction,
> + RangeLoc, &CandidateSet, BEFFailure);
> return StmtError();
> }
> - } else {
> - // - otherwise, begin-expr and end-expr are begin(__range) and
> - // end(__range), respectively, where begin and end are looked
up with
> - // argument-dependent lookup (3.4.2). For the purposes of this
name
> - // lookup, namespace std is an associated namespace.
> - }
>
> - BeginExpr = BuildForRangeBeginEndCall(*this, S, ColonLoc, BeginVar,
> - BEF_begin, BeginNameInfo,
> - BeginMemberLookup,
> - BeginRangeRef.get());
> - if (BeginExpr.isInvalid())
> - return StmtError();
> -
> - EndExpr = BuildForRangeBeginEndCall(*this, S, ColonLoc, EndVar,
> - BEF_end, EndNameInfo,
> - EndMemberLookup,
EndRangeRef.get());
> - if (EndExpr.isInvalid())
> - return StmtError();
> + ExprResult DereferencedRange =
> + CreateBuiltinUnaryOp(RangeLoc, UO_Deref, Range);
> +
> + // FIXME: The second ForLoc should really be LParenLoc, though
> + // this is only used in one diagnostic in ActOnCXXForRangeStmt.
> + StmtResult SR = ActOnCXXForRangeStmt(ForLoc, ForLoc, LoopVarDecl,
> + ColonLoc,
DereferencedRange.get(),
> + RParenLoc, false);
It's not great that we produce diagnostics here, and it's doubly not great
that we do so before explaining why we're trying to use *expr as a range.
Have you considered trapping these diagnostics somehow?
DiagnoseInvalidRedeclaration in SemaDecl.cpp uses SFINAETrap for this. With
that change, you should be able to make all cases other than
FRS_NoViableFunction issue their diagnostics immediately, which should
simplify things somewhat. You could also support calling an overloaded
operator* by such a technique.
On Wed, Aug 8, 2012 at 11:37 AM, Sam Panzer <panzer at google.com> wrote:
> Ping
>
>
> On Fri, Jul 27, 2012 at 3:06 PM, Sam Panzer <panzer at google.com> wrote:
>
>> Here is the next version of the patch. The major change consists of
>> reworking the way that I attempt to dereference the range - now it's
>> actually the range that gets a green star.
>>
>> This patch correctly suppresses diagnostics on incorrect attempts to
>> dereference in most cases, but there are extra error messages issued in one
>> instance and a missing clarification in another.
>> Specifically, if a pointer to a type with a deleted begin() or end()
>> function is passed as the range, there will be a (somewhat mysterious)
>> error complaining about the use of a deleted function in addition to the
>> error explaining that the range was invalid; a similar extra diagnostic
>> appears if begin() or end() is inaccessible (i.e. private) on the
>> pointed-to type.
>>
>> The missing clarification comes from non-pointer-type ranges which have
>> inaccessible begin() or end() functions. We do not issue a diagnostic that
>> specifies that the range was invalid, because the checks that detect
>> incorrect access occur in the destructor of LookupResult. There is
>> currently no way to tell if the expression was invalid, and we happily
>> continue building the AST as if there were no error.
>> I included a few test cases that note this behavior and have appropriate
>> FIXME's for when it is corrected.
>>
>> On Thu, Jul 26, 2012 at 2:01 PM, Richard Smith <richard at metafoo.co.uk>wrote:
>>
>>> Hi Sam,
>>>
>>> Sorry for the massive delay reviewing this...
>>>
>>> > diff --git a/include/clang/Basic/DiagnosticSemaKinds.td
>>> b/include/clang/Basic/DiagnosticSemaKinds.td
>>> > index 52641b8..97b0469 100644
>>> > --- a/include/clang/Basic/DiagnosticSemaKinds.td
>>> > +++ b/include/clang/Basic/DiagnosticSemaKinds.td
>>> > @@ -1391,7 +1391,14 @@ def err_for_range_member_begin_end_mismatch :
>>> Error<
>>> > "range type %0 has '%select{begin|end}1' member but no
>>> '%select{end|begin}1' member">;
>>> > def err_for_range_begin_end_types_differ : Error<
>>> > "'begin' and 'end' must return the same type (got %0 and %1)">;
>>> > +def err_for_range_invalid: Error<
>>> > + "invalid range expression of type %0">;
>>> > +def note_for_range_adl_failure: Note<
>>> > + "no viable '%select{begin|end}0' function for range of type %1">;
>>>
>>> Please merge these into a single diagnostic (use the text of
>>> note_for_range_adl_failure as the error message).
>>>
>>
>> Done.
>>
>>
>>>
>>> > def note_for_range_type : Note<"range has type %0">;
>>> > +def err_for_range_dereference : Error<
>>> > + "invalid range expression of type %0; did you mean to dereference
>>> it "
>>> > + "with '*'?">;
>>> > def note_for_range_begin_end : Note<
>>> > "selected '%select{begin|end}0' %select{function|template }1%2 with
>>> iterator type %3">;
>>> >
>>> > diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
>>> > index a48cde0..29fbded 100644
>>> > --- a/include/clang/Sema/Sema.h
>>> > +++ b/include/clang/Sema/Sema.h
>>> > @@ -1923,7 +1923,24 @@ public:
>>> > Expr **Args, unsigned NumArgs,
>>> > SourceLocation RParenLoc,
>>> > Expr *ExecConfig,
>>> > - bool AllowTypoCorrection=true);
>>> > + OverloadCandidateSet
>>> *CandidateSet,
>>> > + bool AllowTypoCorrection,
>>> > + bool InRangeExpr);
>>> > +
>>> > + ExprResult BuildOverloadedCallExpr(Scope *S, Expr *Fn,
>>> > + UnresolvedLookupExpr *ULE,
>>> > + SourceLocation LParenLoc,
>>> > + Expr **Args, unsigned NumArgs,
>>> > + SourceLocation RParenLoc,
>>> > + Expr *ExecConfig,
>>> > + bool AllowTypoCorrection=true,
>>> > + bool InRangeExpr=false);
>>>
>>> I don't like passing InRangeExpr in here. It's too specific. It's also
>>> unnecessary (more on that later).
>>>
>>
>> > +
>>> > +
>>> > +// FIXME: Update this stale comment
>>>
>>> Please do :)
>>>
>>
>> Done. The next comment too :)
>>
>>
>>>
>>> > +/// ResolveOverloadedCallFn - Given the call expression that calls Fn
>>> > +/// (which eventually refers to the declaration Func) and the call
>>> > +/// arguments Args/NumArgs, attempt to resolve the function call down
>>> > +/// to a specific function. If overload resolution succeeds, returns
>>> > +/// the function declaration produced by overload
>>> > +/// resolution. Otherwise, emits diagnostics, deletes all of the
>>> > +/// arguments and Fn, and returns NULL.
>>> > +
>>> > +ExprResult Sema::BuildOverloadedCallExpr(Scope *S, Expr *Fn,
>>> > + UnresolvedLookupExpr *ULE,
>>> > + SourceLocation LParenLoc,
>>> > + Expr **Args, unsigned
>>> NumArgs,
>>> > + SourceLocation RParenLoc,
>>> > + Expr *ExecConfig,
>>> > + OverloadCandidateSet
>>> *CandidateSet,
>>> > + bool AllowTypoCorrection,
>>> > + bool InRangeExpr) {
>>> > + if (CandidateSet->empty())
>>> > return BuildRecoveryCallExpr(*this, S, Fn, ULE, LParenLoc,
>>> > llvm::MutableArrayRef<Expr *>(Args,
>>> NumArgs),
>>> > RParenLoc, /*EmptyLookup=*/true,
>>> > AllowTypoCorrection);
>>> > - }
>>> > -
>>> > - UnbridgedCasts.restore();
>>> >
>>> > OverloadCandidateSet::iterator Best;
>>> > - switch (CandidateSet.BestViableFunction(*this, Fn->getLocStart(),
>>> Best)) {
>>> > + switch (CandidateSet->BestViableFunction(*this, Fn->getLocStart(),
>>> Best)) {
>>> > case OR_Success: {
>>> > FunctionDecl *FDecl = Best->Function;
>>> > MarkFunctionReferenced(Fn->getExprLoc(), FDecl);
>>> > @@ -9770,6 +9773,10 @@ Sema::BuildOverloadedCallExpr(Scope *S, Expr
>>> *Fn, UnresolvedLookupExpr *ULE,
>>> > }
>>> >
>>> > case OR_No_Viable_Function: {
>>> > + // In case we're in a range expression, let the range handling
>>> code take
>>> > + // care of it.
>>> > + if (InRangeExpr)
>>> > + break;
>>>
>>> This is the only use of InRangeExpr in this function, and we don't call
>>> this function in the case where BestViableFunction returns
>>> OR_No_Viable_Function. Is this parameter really needed?
>>>
>>
>> Nope, it's gone now.
>>
>> > +/// This version of BuildOverloadedCallExpr creates an
>>> OverloadCandidateSet
>>> > +/// first.
>>> > +ExprResult Sema::BuildOverloadedCallExpr(Scope *S, Expr *Fn,
>>> > + UnresolvedLookupExpr *ULE,
>>> > + SourceLocation LParenLoc,
>>> > + Expr **Args, unsigned
>>> NumArgs,
>>> > + SourceLocation RParenLoc,
>>> > + Expr *ExecConfig,
>>> > + bool AllowTypoCorrection,
>>> > + bool InRangeExpr) {
>>> > +#ifndef NDEBUG
>>> > + if (ULE->requiresADL()) {
>>> > + // To do ADL, we must have found an unqualified name.
>>> > + assert(!ULE->getQualifier() && "qualified name with ADL");
>>> > +
>>> > + // We don't perform ADL for implicit declarations of builtins.
>>> > + // Verify that this was correctly set up.
>>> > + FunctionDecl *F;
>>> > + if (ULE->decls_begin() + 1 == ULE->decls_end() &&
>>> > + (F = dyn_cast<FunctionDecl>(*ULE->decls_begin())) &&
>>> > + F->getBuiltinID() && F->isImplicit())
>>> > + llvm_unreachable("performing ADL for builtin");
>>> > +
>>> > + // We don't perform ADL in C.
>>> > + assert(getLangOpts().CPlusPlus && "ADL enabled in C");
>>> > + } else
>>> > + assert(!ULE->isStdAssociatedNamespace() &&
>>> > + "std is associated namespace but not doing ADL");
>>> > +#endif
>>>
>>> This checking belongs in buildOverloadedCallSet. We want to perform
>>> these checks for range-based for calls too.
>>>
>>
>> Done.
>>
>>
>>>
>>> > +
>>> > + OverloadCandidateSet CandidateSet(Fn->getExprLoc());
>>> > + ExprResult result;
>>> > + if (buildOverloadedCallSet(S, Fn, ULE, Args, NumArgs, LParenLoc,
>>> > + &CandidateSet, &result))
>>> > + return result;
>>> > +
>>> > + return BuildOverloadedCallExpr(S, Fn, ULE, LParenLoc, Args, NumArgs,
>>> > + RParenLoc, ExecConfig, &CandidateSet,
>>> > + AllowTypoCorrection, InRangeExpr);
>>> > +
>>> > }
>>> >
>>> > static bool IsOverloaded(const UnresolvedSetImpl &Functions) {
>>> > diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp
>>> > index 9be1d34..18bcf89 100644
>>> > --- a/lib/Sema/SemaStmt.cpp
>>> > +++ b/lib/Sema/SemaStmt.cpp
>>> > @@ -1578,18 +1578,35 @@ void NoteForRangeBeginEndFunction(Sema
>>> &SemaRef, Expr *E,
>>> > << BEF << IsTemplate << Description << E->getType();
>>> > }
>>> >
>>> > +
>>> > +enum ForRangeStatus {
>>> > + FRS_Success,
>>> > + FRS_InvalidMemberFunction,
>>> > + FRS_NoOverload,
>>> > + FRS_DeletedOrAmbiguous,
>>> > + FRS_NoViableFunction,
>>> > + FRS_InvalidIteratorType,
>>> > + FRS_BeginEndMismatch
>>> > +};
>>> > +
>>> > /// Build a call to 'begin' or 'end' for a C++0x for-range statement.
>>> If the
>>> > /// given LookupResult is non-empty, it is assumed to describe a
>>> member which
>>> > /// will be invoked. Otherwise, the function will be found via
>>> argument
>>> > /// dependent lookup.
>>> > -static ExprResult BuildForRangeBeginEndCall(Sema &SemaRef, Scope *S,
>>> > - SourceLocation Loc,
>>> > - VarDecl *Decl,
>>> > - BeginEndFunction BEF,
>>> > - const DeclarationNameInfo
>>> &NameInfo,
>>> > - LookupResult
>>> &MemberLookup,
>>> > - Expr *Range) {
>>> > - ExprResult CallExpr;
>>> > +/// CallExpr is set and FRS_Success returned on success, otherwise
>>> some
>>> > +/// non-success value is returned.
>>> > +static ForRangeStatus
>>> > +BuildForRangeBeginEndCall(Sema &SemaRef, Scope *S,
>>> > + SourceLocation Loc,
>>> > + SourceLocation RangeLoc,
>>> > + VarDecl *Decl,
>>> > + BeginEndFunction BEF,
>>> > + const DeclarationNameInfo &NameInfo,
>>> > + LookupResult &MemberLookup,
>>> > + OverloadCandidateSet *CandidateSet,
>>> > + Expr *Range,
>>> > + ExprResult *CallExpr) {
>>>
>>> Now this is performing the call to BestViableFunction itself, this
>>> really belongs in SemaOverload.cpp.
>>>
>>>
>> Done. It took some rearranging :)
>> Side-effects of the move include shifting the ForRangeStatus and
>> BeginEndFunction enums into Sema, along with FinishForRangeVarDecl(). If
>> this isn't as nice of a result as you had hoped, I can work on moving it
>> back.
>>
>>
>>> > + CandidateSet->clear();
>>> > if (!MemberLookup.empty()) {
>>> > ExprResult MemberRef =
>>> > SemaRef.BuildMemberReferenceExpr(Range, Range->getType(), Loc,
>>> > @@ -1598,12 +1615,16 @@ static ExprResult
>>> BuildForRangeBeginEndCall(Sema &SemaRef, Scope *S,
>>> > /*FirstQualifierInScope=*/0,
>>> > MemberLookup,
>>> > /*TemplateArgs=*/0);
>>> > - if (MemberRef.isInvalid())
>>> > - return ExprError();
>>> > - CallExpr = SemaRef.ActOnCallExpr(S, MemberRef.get(), Loc,
>>> MultiExprArg(),
>>> > + if (MemberRef.isInvalid()) {
>>> > + *CallExpr = ExprError();
>>> > + return FRS_InvalidMemberFunction;
>>> > + }
>>> > + *CallExpr = SemaRef.ActOnCallExpr(S, MemberRef.get(), Loc,
>>> MultiExprArg(),
>>> > Loc, 0);
>>> > - if (CallExpr.isInvalid())
>>> > - return ExprError();
>>> > + if (CallExpr->isInvalid()) {
>>> > + *CallExpr = ExprError();
>>> > + return FRS_InvalidMemberFunction;
>>> > + }
>>> > } else {
>>> > UnresolvedSet<0> FoundNames;
>>> > // C++0x [stmt.ranged]p1: For the purposes of this name lookup,
>>> namespace
>>> > @@ -1614,20 +1635,49 @@ static ExprResult
>>> BuildForRangeBeginEndCall(Sema &SemaRef, Scope *S,
>>> > /*NeedsADL=*/true,
>>> /*Overloaded=*/false,
>>> > FoundNames.begin(),
>>> FoundNames.end(),
>>> > /*LookInStdNamespace=*/true);
>>> > - CallExpr = SemaRef.BuildOverloadedCallExpr(S, Fn, Fn, Loc,
>>> &Range, 1, Loc,
>>> > - 0,
>>> /*AllowTypoCorrection=*/false);
>>> > - if (CallExpr.isInvalid()) {
>>> > - SemaRef.Diag(Range->getLocStart(), diag::note_for_range_type)
>>> > - << Range->getType();
>>> > - return ExprError();
>>> > +
>>> > + SemaRef.buildOverloadedCallSet(S, Fn, Fn, &Range, 1, Loc,
>>> CandidateSet,
>>> > + CallExpr);
>>> > +
>>> > + OverloadCandidateSet::iterator Best;
>>> > + if (CandidateSet->empty()) {
>>> > + *CallExpr = ExprError();
>>> > + return FRS_NoOverload;
>>> > + } else {
>>> > + switch (CandidateSet->BestViableFunction(SemaRef,
>>> Fn->getLocStart(),
>>> > + Best)) {
>>> > + case OR_Success:
>>> > + *CallExpr =
>>> > + SemaRef.BuildOverloadedCallExpr(S, Fn, Fn, Loc, &Range,
>>> 1, Loc, 0,
>>> > + CandidateSet,
>>> > +
>>> /*AllowTypoCorrection=*/false,
>>> > + /*InRangeExpr=*/true);
>>>
>>> In the common case of a success, we're calling BestViableFunction twice;
>>> once here and once in BuildOverloadedCallExpr. Can you factor out the
>>> 'OR_Success' case from there and just call it directly?
>>>
>>
>> Done.
>>
>>
>>>
>>> > + if (CallExpr->isInvalid())
>>> > + return FRS_NoOverload;
>>> > + break;
>>> > +
>>> > + case OR_No_Viable_Function:
>>> > + *CallExpr = ExprError();
>>> > + return FRS_NoViableFunction;
>>> > +
>>> > + case OR_Deleted:
>>> > + case OR_Ambiguous:
>>> > + // Here we want to defer to the diagnostics in
>>> BuildOverloadedCallExpr.
>>> > + // The return value doesn't matter, since we're returning an
>>> ExprError()
>>> > + // anyway.
>>> > + SemaRef.BuildOverloadedCallExpr(S, Fn, Fn, Loc, &Range, 1,
>>> Loc, 0,
>>> > + /*AllowTypoCorrection=*/false,
>>> > + /*InRangeExpr=*/true);
>>> > + *CallExpr = ExprError();
>>> > + return FRS_DeletedOrAmbiguous;
>>>
>>> This case is a bit awkward: we're calling BuildOverloadedCallExpr in
>>> order to generate diagnostics which we don't actually want (the diagnostics
>>> we do want are produced later by ForRangeDiagnostics). How about producing
>>> the diagnostics directly here, instead of calling BuildOverloadedCallExpr?
>>>
>>
>> I was looking to avoid duplicating the code that emits the diagnostic,
>> but the cure was worse. Done.
>>
>>
>>>
>>> > + }
>>> > }
>>> > +
>>> > }
>>> > - if (FinishForRangeVarDecl(SemaRef, Decl, CallExpr.get(), Loc,
>>> > -
>>> diag::err_for_range_iter_deduction_failure)) {
>>> > - NoteForRangeBeginEndFunction(SemaRef, CallExpr.get(), BEF);
>>> > - return ExprError();
>>> > - }
>>> > - return CallExpr;
>>> > + if (FinishForRangeVarDecl(SemaRef, Decl, CallExpr->get(), Loc,
>>> > +
>>> diag::err_for_range_iter_deduction_failure))
>>> > + return FRS_InvalidIteratorType;
>>> > + return FRS_Success;
>>> > }
>>> >
>>> > }
>>> > @@ -1700,6 +1750,129 @@ Sema::ActOnCXXForRangeStmt(SourceLocation
>>> ForLoc, SourceLocation LParenLoc,
>>> > RParenLoc);
>>> > }
>>> >
>>> > +// Emit the diagnostic appropriate to the given ForRangeStatus. The
>>> other
>>> > +// parameters are used for supplementary information.
>>> > +static void ForRangeDiagnostics(Sema &SemaRef, ForRangeStatus st,
>>> Expr *Range,
>>> > + const ExprResult &CallExpr,
>>> > + SourceLocation RangeLoc,
>>> > + OverloadCandidateSet *CandidateSet,
>>> > + BeginEndFunction BEF) {
>>> > + switch (st) {
>>> > + case FRS_Success:
>>> > + // It worked; nothing to do.
>>> > + break;
>>> > +
>>> > + case FRS_InvalidMemberFunction:
>>> > + SemaRef.Diag(Range->getLocStart(), diag::err_for_range_invalid)
>>> > + << RangeLoc << Range->getType();
>>> > + SemaRef.Diag(Range->getLocStart(),
>>> diag::note_for_range_adl_failure)
>>> > + << BEF << Range->getType();
>>> > + break;
>>> > +
>>> > + case FRS_NoOverload:
>>> > + SemaRef.Diag(Range->getLocStart(), diag::err_for_range_invalid)
>>> > + << RangeLoc << Range->getType();
>>> > + SemaRef.Diag(Range->getLocStart(),
>>> diag::note_for_range_adl_failure)
>>> > + << BEF << Range->getType();
>>> > + break;
>>> > +
>>> > + case FRS_DeletedOrAmbiguous:
>>> > + // Some of the diagnostics are emitted in
>>> BuildOverloadedCallExpr, so we
>>> > + // just add these.
>>> > + SemaRef.Diag(Range->getLocStart(), diag::err_for_range_invalid)
>>> > + << RangeLoc << Range->getType();
>>> > + SemaRef.Diag(Range->getLocStart(),
>>> diag::note_for_range_adl_failure)
>>> > + << BEF << Range->getType();
>>> > + break;
>>> > +
>>> > + case FRS_NoViableFunction:
>>> > + SemaRef.Diag(Range->getLocStart(), diag::err_for_range_invalid)
>>> > + << RangeLoc << Range->getType();
>>> > + CandidateSet->NoteCandidates(SemaRef, OCD_AllCandidates,
>>> > + llvm::makeArrayRef(&Range,
>>> /*NumArgs=*/1));
>>> > + SemaRef.Diag(Range->getLocStart(),
>>> diag::note_for_range_adl_failure)
>>> > + << BEF << Range->getType();
>>> > + break;
>>> > +
>>> > + case FRS_InvalidIteratorType:
>>> > + NoteForRangeBeginEndFunction(SemaRef, CallExpr.get(), BEF);
>>> > + break;
>>> > +
>>> > + case FRS_BeginEndMismatch:
>>> > + SemaRef.Diag(RangeLoc,
>>> diag::err_for_range_member_begin_end_mismatch)
>>> > + << Range->getType() << BEF;
>>> > + break;
>>> > + }
>>> > +}
>>>
>>> I don't like that we sometimes produce notes here which attach to
>>> diagnostics produced in BuildForRangeBeginEndCall. I'd like to generally
>>> see this restructured as follows:
>>>
>>>
>> BuildForRangeBeginEndCall's return value indicates either 'everything was
>>> OK', 'this isn't a valid range because there's no viable begin/end
>>> function', or 'something else went wrong and I produced a diagnostic'.
>>> Then, produce a diagnostic here only in the second case.
>>>
>>
>> Done. This required that I move NoteForRangeBeginEndFunction() into Sema
>> so that BuildForRangeBeginEndCall() could call it.
>>
>>
>>>
>>> > +
>>> > +/// \brief Create the initialization, compare, and increment steps for
>>> > +/// the range-based for loop expression.
>>> > +/// This function does not handle array-based for loops,
>>> > +/// which are created in Sema::BuildCXXForRangeStmt.
>>> > +///
>>> > +/// \returns a ForRangeStatus indicating success or what kind of
>>> error occurred.
>>> > +/// BeginExpr and EndExpr are set and FRS_Success is returned on
>>> success;
>>> > +/// CandidateSet and BEF are set and some non-success value is
>>> returned on
>>> > +/// failure.
>>> > +static ForRangeStatus BuildNonArrayForRange(Sema &SemaRef, Scope *S,
>>> > + Expr *BeginRange, Expr
>>> *EndRange,
>>> > + QualType RangeType,
>>> > + VarDecl *BeginVar,
>>> > + VarDecl *EndVar,
>>> > + SourceLocation ColonLoc,
>>> > + OverloadCandidateSet
>>> *CandidateSet,
>>> > + ExprResult *BeginExpr,
>>> > + ExprResult *EndExpr,
>>> > + BeginEndFunction *BEF) {
>>> > + DeclarationNameInfo BeginNameInfo(
>>> > + &SemaRef.PP.getIdentifierTable().get("begin"), ColonLoc);
>>> > + DeclarationNameInfo
>>> EndNameInfo(&SemaRef.PP.getIdentifierTable().get("end"),
>>> > + ColonLoc);
>>> > +
>>> > + LookupResult BeginMemberLookup(SemaRef, BeginNameInfo,
>>> > + Sema::LookupMemberName);
>>> > + LookupResult EndMemberLookup(SemaRef, EndNameInfo,
>>> Sema::LookupMemberName);
>>> > +
>>> > + if (CXXRecordDecl *D = RangeType->getAsCXXRecordDecl()) {
>>> > + // - if _RangeT is a class type, the unqualified-ids begin and
>>> end are
>>> > + // looked up in the scope of class _RangeT as if by class
>>> member access
>>> > + // lookup (3.4.5), and if either (or both) finds at least one
>>> > + // declaration, begin-expr and end-expr are __range.begin() and
>>> > + // __range.end(), respectively;
>>> > + SemaRef.LookupQualifiedName(BeginMemberLookup, D);
>>> > + SemaRef.LookupQualifiedName(EndMemberLookup, D);
>>> > +
>>> > + if (BeginMemberLookup.empty() != EndMemberLookup.empty()) {
>>> > + *BEF = BeginMemberLookup.empty() ? BEF_end : BEF_begin;
>>> > + return FRS_BeginEndMismatch;
>>> > + }
>>> > + } else {
>>> > + // - otherwise, begin-expr and end-expr are begin(__range) and
>>> > + // end(__range), respectively, where begin and end are looked
>>> up with
>>> > + // argument-dependent lookup (3.4.2). For the purposes of this
>>> name
>>> > + // lookup, namespace std is an associated namespace.
>>> > +
>>> > + }
>>> > +
>>> > + *BEF = BEF_begin;
>>> > + ForRangeStatus
>>> > + RangeStatus = BuildForRangeBeginEndCall(SemaRef, S, ColonLoc,
>>> > + ColonLoc, BeginVar,
>>> > + BEF_begin,
>>> BeginNameInfo,
>>> > + BeginMemberLookup,
>>> > + CandidateSet,
>>> > + BeginRange, BeginExpr);
>>> > + if (RangeStatus != FRS_Success)
>>> > + return st;
>>> > +
>>> > + *BEF = BEF_end;
>>> > + RangeStatus = BuildForRangeBeginEndCall(SemaRef, S, ColonLoc,
>>> ColonLoc,
>>> > + EndVar, BEF_end,
>>> EndNameInfo,
>>> > + EndMemberLookup,
>>> CandidateSet,
>>> > + EndRange, EndExpr);
>>> > + return RangeStatus;
>>> > +}
>>> > +
>>> > /// BuildCXXForRangeStmt - Build or instantiate a C++0x for-range
>>> statement.
>>> > StmtResult
>>> > Sema::BuildCXXForRangeStmt(SourceLocation ForLoc, SourceLocation
>>> ColonLoc,
>>> > @@ -1791,49 +1964,58 @@ Sema::BuildCXXForRangeStmt(SourceLocation
>>> ForLoc, SourceLocation ColonLoc,
>>> > return StmtError();
>>> > }
>>> > } else {
>>> > - DeclarationNameInfo
>>> BeginNameInfo(&PP.getIdentifierTable().get("begin"),
>>> > - ColonLoc);
>>> > - DeclarationNameInfo
>>> EndNameInfo(&PP.getIdentifierTable().get("end"),
>>> > - ColonLoc);
>>> > -
>>> > - LookupResult BeginMemberLookup(*this, BeginNameInfo,
>>> LookupMemberName);
>>> > - LookupResult EndMemberLookup(*this, EndNameInfo,
>>> LookupMemberName);
>>> > -
>>> > - if (CXXRecordDecl *D = RangeType->getAsCXXRecordDecl()) {
>>> > - // - if _RangeT is a class type, the unqualified-ids begin
>>> and end are
>>> > - // looked up in the scope of class _RangeT as if by class
>>> member access
>>> > - // lookup (3.4.5), and if either (or both) finds at least
>>> one
>>> > - // declaration, begin-expr and end-expr are __range.begin()
>>> and
>>> > - // __range.end(), respectively;
>>> > - LookupQualifiedName(BeginMemberLookup, D);
>>> > - LookupQualifiedName(EndMemberLookup, D);
>>> > -
>>> > - if (BeginMemberLookup.empty() != EndMemberLookup.empty()) {
>>> > - Diag(ColonLoc,
>>> diag::err_for_range_member_begin_end_mismatch)
>>> > - << RangeType << BeginMemberLookup.empty();
>>> > + Expr *BeginRange = BeginRangeRef.get();
>>> > + Expr *EndRange = EndRangeRef.get();
>>> > + OverloadCandidateSet CandidateSet(RangeLoc);
>>> > + BeginEndFunction BEFFailure;
>>> > + ForRangeStatus RangeStatus =
>>> > + BuildNonArrayForRange(*this, S, BeginRange, EndRange,
>>> RangeType,
>>> > + BeginVar, EndVar, ColonLoc,
>>> &CandidateSet,
>>> > + &BeginExpr, &EndExpr, &BEFFailure);
>>> > +
>>> > + // Attempt to dereference pointers and try again.
>>> > + if (RangeStatus != FRS_Success) {
>>> > + if (!RangeType->isPointerType()) {
>>> > + const ExprResult & TheCall = BEFFailure ? EndExpr:
>>> BeginExpr;
>>> > + ForRangeDiagnostics(*this, RangeStatus,
>>> > + BEFFailure ? EndRange : BeginRange,
>>> > + TheCall, RangeLoc, &CandidateSet,
>>> BEFFailure);
>>> > return StmtError();
>>> > }
>>> > - } else {
>>> > - // - otherwise, begin-expr and end-expr are begin(__range) and
>>> > - // end(__range), respectively, where begin and end are
>>> looked up with
>>> > - // argument-dependent lookup (3.4.2). For the purposes of
>>> this name
>>> > - // lookup, namespace std is an associated namespace.
>>> > + BeginRange =
>>> > + new (Context) UnaryOperator(BeginRange, UO_Deref,
>>> > + RangeType->getPointeeType(),
>>> > + VK_LValue, OK_Ordinary,
>>> RangeLoc);
>>> > + EndRange =
>>> > + new (Context) UnaryOperator(EndRange, UO_Deref,
>>> > + RangeType->getPointeeType(),
>>> > + VK_LValue, OK_Ordinary,
>>> RangeLoc);
>>> > +
>>> > + BeginEndFunction DerefBEF;
>>> > + ForRangeStatus DerefStatus
>>> > + = BuildNonArrayForRange(*this, S, BeginRange, EndRange,
>>> > + BeginRange->getType(),
>>> > + BeginVar, EndVar,
>>> > + ColonLoc, &CandidateSet,
>>> > + &BeginExpr, &EndExpr, &DerefBEF);
>>> > + if (DerefStatus != FRS_Success) {
>>> > + const ExprResult & TheCall = BEFFailure ? EndExpr:
>>> BeginExpr;
>>> > + ForRangeDiagnostics(*this, RangeStatus,
>>> > + DerefBEF ? EndRangeRef.get():
>>> BeginRangeRef.get(),
>>> > + TheCall, RangeLoc, &CandidateSet,
>>> BEFFailure);
>>> > + return StmtError();
>>> > + } else {
>>> > + // The attempt to dereference would likely succeed.
>>> > + Diag(RangeLoc, diag::err_for_range_dereference)
>>> > + << RangeLoc << RangeType
>>> > + << FixItHint::CreateInsertion(RangeLoc, "*");
>>>
>>> Can you make this comment a bit more precise? The bar for producing a
>>> fixit on an error is that the fix would succeed and would produce the same
>>> AST. Here, we're taking some liberties, because we're applying the '*'
>>> within the __begin and __end expressions instead of to the __range
>>> initializer. That's equivalent, but doesn't provide an identical AST, so
>>> you should either justify it or rebuild __range with the right expression
>>> here.
>>>
>>
>> I changed it to rebuild the correct expression here by introducing mutual
>> recursion with ActOnCXXForRangeStmt, limiting the depth to one.
>>
>>
>>>
>>> > + }
>>> > }
>>> > -
>>> > - BeginExpr = BuildForRangeBeginEndCall(*this, S, ColonLoc,
>>> BeginVar,
>>> > - BEF_begin, BeginNameInfo,
>>> > - BeginMemberLookup,
>>> > - BeginRangeRef.get());
>>> > - if (BeginExpr.isInvalid())
>>> > - return StmtError();
>>> > -
>>> > - EndExpr = BuildForRangeBeginEndCall(*this, S, ColonLoc, EndVar,
>>> > - BEF_end, EndNameInfo,
>>> > - EndMemberLookup,
>>> EndRangeRef.get());
>>> > - if (EndExpr.isInvalid())
>>> > - return StmtError();
>>> > }
>>> >
>>> > + assert(!BeginExpr.isInvalid() && !EndExpr.isInvalid() &&
>>> > + "invalid range expression in for loop");
>>> > +
>>> > // C++0x [decl.spec.auto]p6: BeginType and EndType must be the
>>> same.
>>> > QualType BeginType = BeginVar->getType(), EndType =
>>> EndVar->getType();
>>> > if (!Context.hasSameType(BeginType, EndType)) {
>>> > diff --git a/test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp
>>> b/test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp
>>> > index 96bb472..e5809f0 100644
>>> > --- a/test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp
>>> > +++ b/test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp
>>> > @@ -3,20 +3,21 @@
>>> > struct pr12960 {
>>> > int begin;
>>> > void foo(int x) {
>>> > - for (int& it : x) { // expected-error {{use of undeclared
>>> identifier 'begin'}} expected-note {{range has type 'int'}}
>>> > + for (int& it : x) { //expected-error{{invalid range expression}}\
>>> > + expected-note{{no viable 'begin' function for range of type
>>> 'int'}}
>>> > }
>>> > }
>>> > };
>>> >
>>> > namespace std {
>>> > template<typename T>
>>> > - auto begin(T &&t) -> decltype(t.begin()) { return t.begin(); } //
>>> expected-note 4{{ignored: substitution failure}}
>>> > + auto begin(T &&t) -> decltype(t.begin()) { return t.begin(); } //
>>> expected-note 3{{ignored: substitution failure}}
>>> > template<typename T>
>>> > auto end(T &&t) -> decltype(t.end()) { return t.end(); } //
>>> expected-note {{candidate template ignored: substitution failure [with T =
>>> }}
>>> >
>>> > template<typename T>
>>> > auto begin(T &&t) -> decltype(t.alt_begin()) { return
>>> t.alt_begin(); } // expected-note {{selected 'begin' template [with T = }} \
>>> > -
>>> expected-note 4{{candidate template ignored: substitution failure
>>> [with T = }}
>>> > +
>>> expected-note 3{{candidate template ignored: substitution failure
>>> [with T = }}
>>> > template<typename T>
>>> > auto end(T &&t) -> decltype(t.alt_end()) { return t.alt_end(); }
>>> // expected-note {{candidate template ignored: substitution failure [with T
>>> = }}
>>> >
>>> > @@ -116,9 +117,11 @@ void g() {
>>> > struct NoEndADL {
>>> > null_t alt_begin();
>>> > };
>>> > - for (auto u : NoBeginADL()) { // expected-error {{no matching
>>> function for call to 'begin'}} expected-note {{range has type 'NoBeginADL'}}
>>> > + for (auto u : NoBeginADL()) {// expected-error{{invalid range
>>> expression of type 'NoBeginADL'}} \
>>> > + expected-note{{no viable 'begin' function for range of type
>>> 'NoBeginADL'}}
>>> > }
>>> > - for (auto u : NoEndADL()) { // expected-error {{no matching
>>> function for call to 'end'}} expected-note {{range has type 'NoEndADL'}}
>>> > + for (auto u : NoEndADL()) { // expected-error{{invalid range
>>> expression of type 'NoEndADL'}} \
>>> > + expected-note{{no viable 'end' function for range of type
>>> 'NoEndADL'}}
>>> > }
>>> >
>>> > struct NoBegin {
>>> > @@ -156,8 +159,8 @@ void g() {
>>> > for (int n : NoCopy()) { // ok
>>> > }
>>> >
>>> > - for (int n : 42) { // expected-error {{no matching function for
>>> call to 'begin'}} \
>>> > - expected-note {{range has type 'int'}}
>>> > + for (int n : 42) { // expected-error{{invalid range expression of
>>> type 'int'}} \
>>> > + expected-note{{no viable 'begin' function for range of type
>>> 'int'}}
>>> > }
>>> >
>>> > for (auto a : *also_incomplete) { // expected-error {{cannot use
>>> incomplete type 'struct Incomplete' as a range}}
>>> > @@ -179,9 +182,10 @@ template void h<A(&)[13], int>(A(&)[13]); //
>>> expected-note {{requested here}}
>>> >
>>> > template<typename T>
>>> > void i(T t) {
>>> > - for (auto u : t) { // expected-error {{no matching function for
>>> call to 'begin'}} \
>>> > - expected-error {{member function 'begin' not
>>> viable}} \
>>> > - expected-note {{range has type}}
>>> > + for (auto u : t) { // expected-error {{invalid range expression of
>>> type 'A *'; did you mean to dereference it with '*'?}} \
>>> > + expected-error {{member function 'begin' not
>>> viable}}\
>>> > + expected-error{{invalid range expression of
>>> type 'const A'}}\
>>> > + expected-note{{no viable 'begin' function for
>>> range of type 'const A'}}
>>> > }
>>> > }
>>> > template void i<A[13]>(A*); // expected-note {{requested here}}
>>> > @@ -204,7 +208,8 @@ void end(VoidBeginADL);
>>> > void j() {
>>> > for (auto u : NS::ADL()) {
>>> > }
>>> > - for (auto u : NS::NoADL()) { // expected-error {{no matching
>>> function for call to 'begin'}} expected-note {{range has type}}
>>> > + for (auto u : NS::NoADL()) { // expected-error{{invalid range
>>> expression of type 'NS::NoADL'}}\
>>> > + expected-note {{no viable 'begin' function for range of type
>>> 'NS::NoADL'}}
>>> > }
>>> > for (auto a : VoidBeginADL()) { // expected-error {{cannot use type
>>> 'void' as an iterator}}
>>> > }
>>> > @@ -215,4 +220,3 @@ void example() {
>>> > for (int &x : array)
>>> > x *= 2;
>>> > }
>>> > -
>>> > diff --git a/test/SemaCXX/for-range-no-std.cpp
>>> b/test/SemaCXX/for-range-no-std.cpp
>>> > index fa42ca4..8f3213b 100644
>>> > --- a/test/SemaCXX/for-range-no-std.cpp
>>> > +++ b/test/SemaCXX/for-range-no-std.cpp
>>> > @@ -31,10 +31,11 @@ NS::iter end(NS::NoADL);
>>> > void f() {
>>> > int a[] = {1, 2, 3};
>>> > for (auto b : S()) {} // ok
>>> > - for (auto b : T()) {} // expected-error {{no matching function for
>>> call to 'begin'}} expected-note {{range has type}}
>>> > + for (auto b : T()) {} // expected-error {{invalid range expression
>>> of type 'T'}} expected-note {{no viable 'begin' function for range of type
>>> 'T'}}
>>> > for (auto b : a) {} // ok
>>> > for (int b : NS::ADL()) {} // ok
>>> > - for (int b : NS::NoADL()) {} // expected-error {{no matching
>>> function for call to 'begin'}} expected-note {{range has type}}
>>> > + for (int b : NS::NoADL()) {} // expected-error {{invalid range
>>> expression of type 'NS::NoADL'}} \
>>> > + expected-note {{no viable 'begin' function for range of type
>>> 'NS::NoADL'}}
>>> > }
>>> >
>>> > void PR11601() {
>>> > diff --git a/test/SemaCXX/typo-correction.cpp
>>> b/test/SemaCXX/typo-correction.cpp
>>> > index 893f08a..f8bb8d0 100644
>>> > --- a/test/SemaCXX/typo-correction.cpp
>>> > +++ b/test/SemaCXX/typo-correction.cpp
>>> > @@ -155,7 +155,8 @@ void Test3() {
>>> > struct R {};
>>> > bool begun(R);
>>> > void RangeTest() {
>>> > - for (auto b : R()) {} // expected-error {{use of undeclared
>>> identifier 'begin'}} expected-note {{range has type}}
>>> > + for (auto b : R()) {} // expected-error {{invalid range expression
>>> of type 'R'}}\
>>> > + expected-note {{no viable 'begin' function for range of type 'R'}}
>>> > }
>>> >
>>> > // PR 12019 - Avoid infinite mutual recursion in
>>> DiagnoseInvalidRedeclaration
>>> > --
>>> > 1.7.7.3
>>> >
>>>
>>> On Mon, Jul 23, 2012 at 3:41 PM, Sam Panzer <panzer at google.com> wrote:
>>>
>>>> Ping.
>>>>
>>>>
>>>> On Wed, Jul 11, 2012 at 4:29 PM, Sam Panzer <panzer at google.com> wrote:
>>>>
>>>>> Ping, and a slightly updated version which changed a few comments to
>>>>> make doxygen happy and renamed a few variables to fit the conventions
>>>>> better.
>>>>>
>>>>>
>>>>> On Mon, Jun 18, 2012 at 9:40 AM, Sam Panzer <panzer at google.com> wrote:
>>>>>
>>>>>> *sigh*
>>>>>> I ran format-patch from the correct branch this time.
>>>>>>
>>>>>> On Sun, Jun 17, 2012 at 4:49 PM, Richard Smith <richard at metafoo.co.uk
>>>>>> > wrote:
>>>>>>
>>>>>>> On Thu, Jun 14, 2012 at 10:32 AM, Sam Panzer <panzer at google.com>
>>>>>>> wrote:
>>>>>>> > I will now be including the word 'attach' in these emails as
>>>>>>> Gmail's
>>>>>>> > equivalent of -Wmissing-patch.
>>>>>>>
>>>>>>> Sadly that won't save you from running 'diff' in the wrong checkout.
>>>>>>> This is the printf / c_str patch.
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120808/0f3b3893/attachment.html>
More information about the cfe-commits
mailing list