[cfe-commits] [patch] Better diagnostics in for-range expressions.
Sam Panzer
panzer at google.com
Thu Aug 16 14:56:26 PDT 2012
Here's the next update for this patch.
On Wed, Aug 15, 2012 at 4:29 PM, Richard Smith <richard at metafoo.co.uk>wrote:
> A few small things:
>
> > diff --git a/lib/Sema/SemaOverload.cpp b/lib/Sema/SemaOverload.cpp
> > index a874489..1323978 100644
> > --- a/lib/Sema/SemaOverload.cpp
> > +++ b/lib/Sema/SemaOverload.cpp
> > @@ -11199,6 +11241,103 @@ ExprResult
> Sema::BuildLiteralOperatorCall(LookupResult &R,
> > return MaybeBindToTemporary(UDL);
> > }
> >
> > +/// Build a call to 'begin' or 'end' for a C++0x for-range statement.
> If the
>
> You may as well change the 'C++0x' references to 'C++11' in all the changed
> lines. (Check the quoted paragraphs match the final C++11 standard first,
> though.)
>
>
Done; the paragraph in the standard is the same. The reference to
[decl.spec.auto] p6 has changed, so I updated it.
> > +/// 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) {
> > + 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_DiagnosticIssued;
> > + }
> > + *CallExpr = ActOnCallExpr(S, MemberRef.get(), Loc, MultiExprArg(),
> Loc, 0);
> > + if (CallExpr->isInvalid()) {
> > + *CallExpr = ExprError();
> > + return FRS_DiagnosticIssued;
> > + }
> > + } 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);
> > +
> > + bool CandidateSetError = buildOverloadedCallSet(S, Fn, Fn, &Range,
> 1, Loc,
> > + CandidateSet,
> CallExpr);
> > + if (CandidateSet->empty() || CandidateSetError) {
> > + *CallExpr = ExprError();
> > + return FRS_NoViableFunction;
> > + }
> > + OverloadCandidateSet::iterator Best;
> > + OverloadingResult OverloadResult =
> > + CandidateSet->BestViableFunction(*this, Fn->getLocStart(),
> Best);
> > +
> > + switch (OverloadResult) {
> > + case OR_Success:
> > + *CallExpr =
> > + FinishOverloadedCallExpr(*this, S, Fn, Fn, Loc, &Range, 1,
> Loc, 0,
> > + CandidateSet, &Best, OverloadResult,
> > + /*AllowTypoCorrection=*/false);
> > + if (CallExpr->isInvalid())
> > + return FRS_DiagnosticIssued;
> > + break;
> > +
> > + case OR_No_Viable_Function:
> > + *CallExpr = ExprError();
> > + return FRS_NoViableFunction;
> > +
> > + case OR_Ambiguous:
> > + Diag(Range->getLocStart(), diag::err_ovl_ambiguous_call)
> > + << Best->Function->getName() << Range->getSourceRange();
> > + CandidateSet->NoteCandidates(*this, OCD_ViableCandidates,
> > + llvm::makeArrayRef(&Range, 1));
> > + *CallExpr = ExprError();
> > + return FRS_DiagnosticIssued;
> > +
> > + case OR_Deleted:
> > + Diag(Range->getLocStart(), diag::err_ovl_deleted_call)
> > + << Range->getSourceRange()
> > + << Best->Function->isDeleted()
> > + << Best->Function->getName()
> > + << getDeletedOrUnavailableSuffix(Best->Function);
> > + CandidateSet->NoteCandidates(*this, OCD_AllCandidates,
> > + llvm::makeArrayRef(&Range, 1));
> > + *CallExpr = ExprError();
> > + return FRS_DiagnosticIssued;
> > + }
>
> It looks like this is equivalent to calling FinishOverloadedCallExpr in all
> cases other than OR_No_Viable_Function.
>
It's *nearly* the same, but not quite. Unfortunately,
FinishOverloadedCallExpr doesn't return an invalid ExprResult on deleted
functions, but I can detect that one case separately.
>
> > @@ -1746,7 +1687,127 @@ Sema::ActOnCXXForRangeStmt(SourceLocation
> ForLoc, SourceLocation LParenLoc,
> >
> > return BuildCXXForRangeStmt(ForLoc, ColonLoc, RangeDecl.get(),
> > /*BeginEndDecl=*/0, /*Cond=*/0,
> /*Inc=*/0, DS,
> > - RParenLoc);
> > + RParenLoc, ShouldTryDeref);
> > +}
> > +
> > +// Emit the diagnostic appropriate to the given ForRangeStatus. The
> other
> > +// parameters are used for supplementary information.
> > +static void ForRangeDiagnostics(Sema &SemaRef, Sema::ForRangeStatus
> Status,
> > + Expr *Range, SourceLocation RangeLoc,
> > + OverloadCandidateSet *CandidateSet,
> > + Sema::BeginEndFunction BEF) {
> > + switch (Status) {
> > + case Sema::FRS_Success:
> > + // It worked; nothing to do.
> > + break;
> > +
> > + case Sema::FRS_DiagnosticIssued:
> > + // Diagnostic alread issued; note the range's type.
> > + SemaRef.Diag(Range->getLocStart(), diag::note_in_for_range)
> > + << RangeLoc << BEF << Range->getType();
> > + break;
> > +
> > + case Sema::FRS_NoViableFunction:
> > + SemaRef.Diag(Range->getLocStart(), diag::err_for_range_invalid)
> > + << RangeLoc << Range->getType() << BEF;
> > + CandidateSet->NoteCandidates(SemaRef, OCD_AllCandidates,
> > + llvm::makeArrayRef(&Range,
> /*NumArgs=*/1));
> > + break;
> > +
> > + case Sema::FRS_BeginEndMismatch:
> > + SemaRef.Diag(RangeLoc,
> diag::err_for_range_member_begin_end_mismatch)
> > + << RangeLoc << Range->getType() << BEF;
> > + break;
> > + }
> > +}
> > +
> > +/// \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 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,
> > + 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);
> > +
>
The standard also hasn't changed the text regarding _RangeT and __range,
below.
> > + 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;
>
> Can you just issue the diagnostic directly here? I don't think we should
> attempt recovery in this case. (My "If there's a viable begin() but lookup
> for end() fails" comment was intended to apply to this case too.)
>
>
Yes, though this means that I have two "diagnostic emitted" error codes:
one which needs the type and which of begin/end failed to be noted, and a
second which needs no extra diagnostics.
> > + }
> > + } 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);
> > + 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);
> > + return RangeStatus;
> > +}
> > +
> > +/// Speculatively attempt to dereference an invalid range expression.
> > +/// This function will not emit diagnostics, but returns StmtError if
> > +/// an error occurs.
> > +static StmtResult RetryWithDereferencedRange(Sema &SemaRef, Scope *S,
>
> Please include something about ForRange in this function name.
>
How about RebuildForRangeWithDereference?
>
> On Thu, Aug 9, 2012 at 4:07 PM, Sam Panzer <panzer at google.com> wrote:
>
>> 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/20120816/df69967e/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: for-range-diagnostic-update.patch
Type: application/octet-stream
Size: 51183 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120816/df69967e/attachment.obj>
More information about the cfe-commits
mailing list