[cfe-commits] [patch] Better diagnostics in for-range expressions.
Sam Panzer
panzer at google.com
Thu Aug 9 16:07:26 PDT 2012
Here's the next version!
On Wed, Aug 8, 2012 at 3:51 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> > --- 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?)
>
Only the original BuildOverloadCallExpr needed to be in Sema. The other one
has been renamed to FinishOverloadedCallExpr and moved to SemaOverload.h.
>
> > 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.
>
Done. There was another call to ActOnCXXForRangeStmt() in
Parser::ParseForStmt(), where I set ShouldTryDeref to true.
>
> > 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.
>
Done. The note was extended to include which of begin() and end() was
problematic.
>
> > + 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?
>
Done. It's not immediately obvious which functions issue
diagnostics...though seeing which ones you picked out, I can see a pattern.
>
> > + }
> > + *CallExpr = ActOnCallExpr(S, MemberRef.get(), Loc, MultiExprArg(),
> Loc, 0);
> > + if (CallExpr->isInvalid()) {
> > + *CallExpr = ExprError();
> > + return FRS_InvalidFunction;
>
> This too.
>
Done.
>
> > + }
> > + } 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?
>
I don't *think* so. buildOverloadedCallSet should only return true when it
the candidate set is empty...but for good measure, I added a check.
>
> > +
> > + if (CandidateSet->empty()) {
> > + *CallExpr = ExprError();
> > + return FRS_InvalidFunction;
>
> You can use FRS_NoViableFunction here, and remove FRS_InvalidFunction.
>
>
Done.
> > + } 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.
>
Done.
>
> > + 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'.
>
It's not so much stale as incorrect. The case can be reached by using a
deleted ADL begin or end function, which wasn't tested (test added).
Moved and clarified...and then completely fixed (see my next comment).
>
> 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.
>
>
Okay. I canceled the recovery if a diagnostic was already issued. This
actually lets me clean up some of the code that actually does issue
diagnostics, remove a few ForRangeStatus's, and delete an extra function
parameter. Additionally, deleted functions receive better diagnostics than
they used to.
> /// 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) {
>
[...]
>
> + *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.
>
Done.
> > @@ -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.
>
Done, as noted above.
>
> > + 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.
>
SFINAETrap definitely cleans up the diagnostics orderings; no more spurious
diagnostics on the dereferenced range! I also discovered
Sema::BuildUnaryOp, which makes it easy to support both builtin and
overloaded operator '*', and cleans up the code that does emit diagnostics.
>
> 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/20120809/ed0f81dc/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: for-range-diagnostic.patch
Type: application/octet-stream
Size: 51941 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120809/ed0f81dc/attachment.obj>
More information about the cfe-commits
mailing list