Here's the next update for this patch.<br><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Aug 15, 2012 at 4:29 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>A few small things:</div><div><br></div><div>> diff --git a/lib/Sema/SemaOverload.cpp b/lib/Sema/SemaOverload.cpp</div>
<div>> index a874489..1323978 100644</div><div>> --- a/lib/Sema/SemaOverload.cpp</div><div>
> +++ b/lib/Sema/SemaOverload.cpp</div><div>> @@ -11199,6 +11241,103 @@ ExprResult Sema::BuildLiteralOperatorCall(LookupResult &R,</div><div>> return MaybeBindToTemporary(UDL);</div><div>> }</div><div>
> </div><div><div>> +/// Build a call to 'begin' or 'end' for a C++0x for-range statement. If the</div><div><br></div></div><div>You may as well change the 'C++0x' references to 'C++11' in all the changed</div>
<div>lines. (Check the quoted paragraphs match the final C++11 standard first,</div><div>though.)</div><div><div><br></div></div></blockquote><div><br></div><div>Done; the paragraph in the standard is the same. The reference to [decl.spec.auto] p6 has changed, so I updated it.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div></div><div>> +/// given LookupResult is non-empty, it is assumed to describe a member which</div>
<div>> +/// will be invoked. Otherwise, the function will be found via argument</div>
<div>> +/// dependent lookup.</div><div>> +/// CallExpr is set to a valid expression and FRS_Success returned on success,</div><div>> +/// otherwise CallExpr is set to ExprError() and some non-success value</div>
<div>> +/// is returned.</div><div>> +Sema::ForRangeStatus</div><div>> +Sema::BuildForRangeBeginEndCall(Scope *S, SourceLocation Loc,</div><div>> + SourceLocation RangeLoc, VarDecl *Decl,</div>
<div>> + BeginEndFunction BEF,</div><div>> + const DeclarationNameInfo &NameInfo,</div><div>> + LookupResult &MemberLookup,</div>
<div>> + OverloadCandidateSet *CandidateSet,</div></div><div>> + Expr *Range, ExprResult *CallExpr) {</div><div><div>> + CandidateSet->clear();</div>
<div>> + if (!MemberLookup.empty()) {</div>
<div>> + ExprResult MemberRef =</div><div>> + BuildMemberReferenceExpr(Range, Range->getType(), Loc,</div><div>> + /*IsPtr=*/false, CXXScopeSpec(),</div><div>> + /*TemplateKWLoc=*/SourceLocation(),</div>
<div>> + /*FirstQualifierInScope=*/0,</div><div>> + MemberLookup,</div><div>> + /*TemplateArgs=*/0);</div><div>> + if (MemberRef.isInvalid()) {</div>
<div>> + *CallExpr = ExprError();</div></div><div>> + return FRS_DiagnosticIssued;</div><div><div>> + }</div><div>> + *CallExpr = ActOnCallExpr(S, MemberRef.get(), Loc, MultiExprArg(), Loc, 0);</div>
<div>
> + if (CallExpr->isInvalid()) {</div><div>> + *CallExpr = ExprError();</div></div><div>> + return FRS_DiagnosticIssued;</div><div><div>> + }</div><div>> + } else {</div><div>
> + UnresolvedSet<0> FoundNames;</div>
<div>> + // C++0x [stmt.ranged]p1: For the purposes of this name lookup, namespace</div><div>> + // std is an associated namespace.</div><div>> + UnresolvedLookupExpr *Fn =</div><div>> + UnresolvedLookupExpr::Create(Context, /*NamingClass=*/0,</div>
<div>> + NestedNameSpecifierLoc(), NameInfo,</div><div>> + /*NeedsADL=*/true, /*Overloaded=*/false,</div><div>> + FoundNames.begin(), FoundNames.end(),</div>
<div>> + /*LookInStdNamespace=*/true);</div><div>> +</div></div><div>> + bool CandidateSetError = buildOverloadedCallSet(S, Fn, Fn, &Range, 1, Loc,</div><div>> + CandidateSet, CallExpr);</div>
<div>> + if (CandidateSet->empty() || CandidateSetError) {</div><div><div>> + *CallExpr = ExprError();</div><div>> + return FRS_NoViableFunction;</div><div>> + }</div></div><div>
<div>> + OverloadCandidateSet::iterator Best;</div>
<div>> + OverloadingResult OverloadResult =</div><div>> + CandidateSet->BestViableFunction(*this, Fn->getLocStart(), Best);</div><div>> +</div><div>> + switch (OverloadResult) {</div><div>> + case OR_Success:</div>
<div>> + *CallExpr =</div></div><div>> + FinishOverloadedCallExpr(*this, S, Fn, Fn, Loc, &Range, 1, Loc, 0,</div><div><div>> + CandidateSet, &Best, OverloadResult,</div>
<div>> + /*AllowTypoCorrection=*/false);</div><div>> + if (CallExpr->isInvalid())</div></div><div>> + return FRS_DiagnosticIssued;</div><div><div>> + break;</div>
<div>
> +</div><div>> + case OR_No_Viable_Function:</div><div>> + *CallExpr = ExprError();</div><div>> + return FRS_NoViableFunction;</div><div>> +</div><div>> + case OR_Ambiguous:</div></div>
<div>> + Diag(Range->getLocStart(), diag::err_ovl_ambiguous_call)</div>
<div>> + << Best->Function->getName() << Range->getSourceRange();</div><div>> + CandidateSet->NoteCandidates(*this, OCD_ViableCandidates,</div><div>> + llvm::makeArrayRef(&Range, 1));</div>
<div>> + *CallExpr = ExprError();</div><div>> + return FRS_DiagnosticIssued;</div><div>> +</div><div>> + case OR_Deleted:</div><div>> + Diag(Range->getLocStart(), diag::err_ovl_deleted_call)</div>
<div>> + << Range->getSourceRange()</div><div>> + << Best->Function->isDeleted()</div><div>> + << Best->Function->getName()</div><div>> + << getDeletedOrUnavailableSuffix(Best->Function);</div>
<div>> + CandidateSet->NoteCandidates(*this, OCD_AllCandidates,</div><div>> + llvm::makeArrayRef(&Range, 1));</div><div>> + *CallExpr = ExprError();</div><div>> + return FRS_DiagnosticIssued;</div>
<div>> + }</div><div><br></div><div>It looks like this is equivalent to calling FinishOverloadedCallExpr in all</div><div>cases other than OR_No_Viable_Function.</div></blockquote><div><br></div><div>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.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><br></div><div>> @@ -1746,7 +1687,127 @@ Sema::ActOnCXXForRangeStmt(SourceLocation ForLoc, SourceLocation LParenLoc,</div>
<div>> </div><div>> return BuildCXXForRangeStmt(ForLoc, ColonLoc, RangeDecl.get(),</div><div>> /*BeginEndDecl=*/0, /*Cond=*/0, /*Inc=*/0, DS,</div><div>> - RParenLoc);</div>
<div>> + RParenLoc, ShouldTryDeref);</div><div>> +}</div><div><div>> +</div><div>> +// Emit the diagnostic appropriate to the given ForRangeStatus. The other</div><div>
> +// parameters are used for supplementary information.</div>
</div><div>> +static void ForRangeDiagnostics(Sema &SemaRef, Sema::ForRangeStatus Status,</div><div>> + Expr *Range, SourceLocation RangeLoc,</div><div>> + OverloadCandidateSet *CandidateSet,</div>
<div>> + Sema::BeginEndFunction BEF) {</div><div>> + switch (Status) {</div><div>> + case Sema::FRS_Success:</div><div><div>> + // It worked; nothing to do.</div>
<div>> + break;</div>
<div>> +</div></div><div>> + case Sema::FRS_DiagnosticIssued:</div><div>> + // Diagnostic alread issued; note the range's type.</div><div>> + SemaRef.Diag(Range->getLocStart(), diag::note_in_for_range)</div>
<div>> + << RangeLoc << BEF << Range->getType();</div><div>> + break;</div><div>> +</div><div>> + case Sema::FRS_NoViableFunction:</div><div><div>> + SemaRef.Diag(Range->getLocStart(), diag::err_for_range_invalid)</div>
</div><div>> + << RangeLoc << Range->getType() << BEF;</div><div><div>> + CandidateSet->NoteCandidates(SemaRef, OCD_AllCandidates,</div><div>> + llvm::makeArrayRef(&Range, /*NumArgs=*/1));</div>
</div><div>> + break;</div><div>> +</div><div>> + case Sema::FRS_BeginEndMismatch:</div><div>> + SemaRef.Diag(RangeLoc, diag::err_for_range_member_begin_end_mismatch)</div><div>> + << RangeLoc << Range->getType() << BEF;</div>
<div>> + break;</div><div>> + }</div><div>> +}</div><div><div>> +</div><div>> +/// \brief Create the initialization, compare, and increment steps for</div><div>> +/// the range-based for loop expression.</div>
<div>> +/// This function does not handle array-based for loops,</div><div>> +/// which are created in Sema::BuildCXXForRangeStmt.</div><div>> +///</div><div>> +/// \returns a ForRangeStatus indicating success or what kind of error occurred.</div>
<div>> +/// BeginExpr and EndExpr are set and FRS_Success is returned on success;</div><div>> +/// CandidateSet and BEF are set and some non-success value is returned on</div><div>> +/// failure.</div></div><div>
<div>> +static Sema::ForRangeStatus BuildNonArrayForRange(Sema &SemaRef, Scope *S,</div>
<div>> + Expr *BeginRange, Expr *EndRange,</div><div>> + QualType RangeType,</div><div>> + VarDecl *BeginVar,</div>
<div>> + VarDecl *EndVar,</div><div>> + SourceLocation ColonLoc,</div><div>> + OverloadCandidateSet *CandidateSet,</div>
<div>> + ExprResult *BeginExpr,</div><div>> + ExprResult *EndExpr,</div></div><div>> + Sema::BeginEndFunction *BEF) {</div>
<div>
<div>> + DeclarationNameInfo BeginNameInfo(</div><div>> + &SemaRef.PP.getIdentifierTable().get("begin"), ColonLoc);</div><div>> + DeclarationNameInfo EndNameInfo(&SemaRef.PP.getIdentifierTable().get("end"),</div>
<div>> + ColonLoc);</div><div>> +</div><div>> + LookupResult BeginMemberLookup(SemaRef, BeginNameInfo,</div><div>> + Sema::LookupMemberName);</div>
<div>> + LookupResult EndMemberLookup(SemaRef, EndNameInfo, Sema::LookupMemberName);</div><div>> +</div></div></blockquote><div><br></div><div>The standard also hasn't changed the text regarding _RangeT and __range, below.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div>> + if (CXXRecordDecl *D = RangeType->getAsCXXRecordDecl()) {</div><div>> + // - if _RangeT is a class type, the unqualified-ids begin and end are</div>
<div>> + // looked up in the scope of class _RangeT as if by class member access</div><div>> + // lookup (3.4.5), and if either (or both) finds at least one</div><div>> + // declaration, begin-expr and end-expr are __range.begin() and</div>
<div>> + // __range.end(), respectively;</div><div>> + SemaRef.LookupQualifiedName(BeginMemberLookup, D);</div><div>> + SemaRef.LookupQualifiedName(EndMemberLookup, D);</div><div>> +</div><div>> + if (BeginMemberLookup.empty() != EndMemberLookup.empty()) {</div>
</div><div><div>> + *BEF = BeginMemberLookup.empty() ? Sema::BEF_end : Sema::BEF_begin;</div><div>> + return Sema::FRS_BeginEndMismatch;</div><div><br></div></div><div>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.)</div>
<div>
<div><br></div></div></blockquote><div><br></div><div>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.</div>
<div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div></div><div>> + }</div><div>> + } else {</div><div>> + // - otherwise, begin-expr and end-expr are begin(__range) and</div>
<div>> + // end(__range), respectively, where begin and end are looked up with</div>
<div>> + // argument-dependent lookup (3.4.2). For the purposes of this name</div><div>> + // lookup, namespace std is an associated namespace.</div><div>> +</div><div>> + }</div><div>> +</div></div>
<div><div>
> + *BEF = Sema::BEF_begin;</div><div>> + Sema::ForRangeStatus RangeStatus =</div><div>> + SemaRef.BuildForRangeBeginEndCall(S, ColonLoc, ColonLoc, BeginVar,</div><div>> + Sema::BEF_begin, BeginNameInfo,</div>
<div>> + BeginMemberLookup, CandidateSet,</div></div><div>> + BeginRange, BeginExpr);</div><div><div>> + if (RangeStatus != Sema::FRS_Success)</div>
<div>> + return RangeStatus;</div><div>> +</div><div>> + *BEF = Sema::BEF_end;</div><div>> + RangeStatus =</div><div>> + SemaRef.BuildForRangeBeginEndCall(S, ColonLoc, ColonLoc, EndVar,</div><div>
> + Sema::BEF_end, EndNameInfo,</div><div>> + EndMemberLookup, CandidateSet,</div></div><div><div>> + EndRange, EndExpr);</div>
<div>> + return RangeStatus;</div><div>> +}</div><div>> +</div></div><div>> +/// Speculatively attempt to dereference an invalid range expression.</div><div>> +/// This function will not emit diagnostics, but returns StmtError if</div>
<div>> +/// an error occurs.</div><div>> +static StmtResult RetryWithDereferencedRange(Sema &SemaRef, Scope *S,</div><div><br></div><div>Please include something about ForRange in this function name.</div></blockquote>
<div><br></div><div>How about RebuildForRangeWithDereference?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div><br>
<div class="gmail_quote">
On Thu, Aug 9, 2012 at 4:07 PM, Sam Panzer <span dir="ltr"><<a href="mailto:panzer@google.com" target="_blank">panzer@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Here's the next version!<br><div class="gmail_extra"><br><br><div class="gmail_quote"><div>On Wed, Aug 8, 2012 at 3:51 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>> --- a/include/clang/Sema/Sema.h</div><div>> +++ b/include/clang/Sema/Sema.h</div><div>> @@ -19,6 +19,7 @@</div>
<div>> #include "clang/Sema/AnalysisBasedWarnings.h"</div><div>> #include "clang/Sema/IdentifierResolver.h"</div>
<div>> #include "clang/Sema/ObjCMethodList.h"</div><div>> +#include "clang/Sema/Overload.h"</div><div><br></div><div>Can you make BuildOverloadedCallExpr local to SemaOverload.cpp to avoid this include? (This is for OverloadCandidateSet::iterator, I think?)</div>
</blockquote><div><br></div></div><div>Only the original BuildOverloadCallExpr needed to be in Sema. The other one has been renamed to FinishOverloadedCallExpr and moved to SemaOverload.h.</div><div><div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><br></div><div>> StmtResult ActOnCXXForRangeStmt(SourceLocation ForLoc,</div>
<div>> SourceLocation LParenLoc, Stmt *LoopVar,</div><div>> SourceLocation ColonLoc, Expr *Collection,</div><div>> - SourceLocation RParenLoc);</div>
<div>> + SourceLocation RParenLoc,</div><div>> + bool ShouldTryDeref = true);</div><div>> StmtResult BuildCXXForRangeStmt(SourceLocation ForLoc,</div>
<div>> SourceLocation ColonLoc,</div><div>> Stmt *RangeDecl, Stmt *BeginEndDecl,</div><div>> Expr *Cond, Expr *Inc,</div>
<div>> Stmt *LoopVarDecl,</div><div>> - SourceLocation RParenLoc);</div><div>> + SourceLocation RParenLoc,</div>
<div>> + bool ShouldTryDeref = true);</div><div><br></div><div>We shouldn't try dereferencing the argument during template instantiation. Please remove the default argument here and set it to false in TreeTransform.h.</div>
</blockquote><div><br></div></div><div>Done. There was another call to ActOnCXXForRangeStmt() in Parser::ParseForStmt(), where I set ShouldTryDeref to true.</div><div><div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><br></div><div>> StmtResult FinishCXXForRangeStmt(Stmt *ForRange, Stmt *Body);</div><div>> </div><div>> + bool FinishForRangeVarDecl(VarDecl *Decl, Expr *Init,</div><div>> + SourceLocation Loc, int diag);</div>
<div>> +</div><div>> StmtResult ActOnGotoStmt(SourceLocation GotoLoc,</div><div>> SourceLocation LabelLoc,</div><div>> LabelDecl *TheDecl);</div><div>
> --- a/lib/Sema/SemaOverload.cpp</div><div>> +++ b/lib/Sema/SemaOverload.cpp</div><div>> @@ -11195,6 +11238,102 @@ ExprResult Sema::BuildLiteralOperatorCall(LookupResult &R,</div><div>> return MaybeBindToTemporary(UDL);</div>
<div>> }</div><div>> </div><div>> +</div><div>> +/// Build a call to 'begin' or 'end' for a C++0x for-range statement. If the</div><div>> +/// given LookupResult is non-empty, it is assumed to describe a member which</div>
<div>> +/// will be invoked. Otherwise, the function will be found via argument</div><div>> +/// dependent lookup.</div><div>> +/// CallExpr is set to a valid expression and FRS_Success returned on success,</div>
<div>> +/// otherwise CallExpr is set to ExprError() and some non-success value</div><div>> +/// is returned.</div><div>> +Sema::ForRangeStatus</div><div>> +Sema::BuildForRangeBeginEndCall(Scope *S, SourceLocation Loc,</div>
<div>> + SourceLocation RangeLoc, VarDecl *Decl,</div><div><div>> + BeginEndFunction BEF,</div><div>> + const DeclarationNameInfo &NameInfo,</div>
<div>> + LookupResult &MemberLookup,</div><div>> + OverloadCandidateSet *CandidateSet,</div></div><div>> + Expr *Range, ExprResult *CallExpr,</div>
<div>> + const FunctionDecl **FoundFunction) {</div><div><br></div><div>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.</div>
</blockquote><div><br></div></div></div><div>Done. The note was extended to include which of begin() and end() was problematic.</div><div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><br></div><div>> + CandidateSet->clear();</div><div>> + if (!MemberLookup.empty()) {</div><div>> + ExprResult MemberRef =</div><div>> + BuildMemberReferenceExpr(Range, Range->getType(), Loc,</div>
<div>> + /*IsPtr=*/false, CXXScopeSpec(),</div><div>> + /*TemplateKWLoc=*/SourceLocation(),</div><div>> + /*FirstQualifierInScope=*/0,</div>
<div>> + MemberLookup,</div><div>> + /*TemplateArgs=*/0);</div><div><div>> + if (MemberRef.isInvalid()) {</div><div>> + *CallExpr = ExprError();</div>
</div><div>> + return FRS_InvalidFunction;</div><div><br></div><div>This should be FRS_DiagnosticIssued, I think?</div></blockquote><div><br></div></div><div>Done. It's not immediately obvious which functions issue diagnostics...though seeing which ones you picked out, I can see a pattern.</div>
<div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><br></div><div>> + }</div><div>> + *CallExpr = ActOnCallExpr(S, MemberRef.get(), Loc, MultiExprArg(), Loc, 0);</div>
<div>
<div>> + if (CallExpr->isInvalid()) {</div><div>> + *CallExpr = ExprError();</div></div><div>> + return FRS_InvalidFunction;</div><div><br></div><div>This too.</div></blockquote><div><br></div></div>
<div>
Done.</div><div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><br></div><div>> + }</div>
<div>> + } else {</div><div>> + UnresolvedSet<0> FoundNames;</div><div>> + // C++0x [stmt.ranged]p1: For the purposes of this name lookup, namespace</div><div>> + // std is an associated namespace.</div>
<div>> + UnresolvedLookupExpr *Fn =</div><div>> + UnresolvedLookupExpr::Create(Context, /*NamingClass=*/0,</div><div>> + NestedNameSpecifierLoc(), NameInfo,</div><div>> + /*NeedsADL=*/true, /*Overloaded=*/false,</div>
<div>> + FoundNames.begin(), FoundNames.end(),</div><div>> + /*LookInStdNamespace=*/true);</div><div>> +</div><div>> + buildOverloadedCallSet(S, Fn, Fn, &Range, 1, Loc, CandidateSet, CallExpr);</div>
<div><br></div><div>Do you need to handle the case where this returns true?</div></blockquote><div><br></div></div><div>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.</div>
<div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div><br></div><div>> +</div><div>> + if (CandidateSet->empty()) {</div><div>> + *CallExpr = ExprError();</div>
</div><div>> + return FRS_InvalidFunction;</div>
<div><br></div><div>You can use FRS_NoViableFunction here, and remove FRS_InvalidFunction.</div><div><br></div></blockquote><div><br></div></div><div>Done.</div><div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div></div><div>> + } else {</div><div>> + OverloadCandidateSet::iterator Best;</div><div>> + OverloadingResult OverloadResult =</div>
<div>> + CandidateSet->BestViableFunction(*this, Fn->getLocStart(), Best);</div><div>> + switch (OverloadResult) {</div><div><div>> + case OR_Success:</div><div>> + *CallExpr =</div>
</div><div>> + BuildOverloadedCallExpr(S, Fn, Fn, Loc, &Range, 1, Loc, 0,</div><div>> + CandidateSet, &Best, OverloadResult,</div><div>> + /*AllowTypoCorrection=*/false);</div>
<div>> + if (CallExpr->isInvalid())</div><div>> + return FRS_InvalidFunction;</div><div><br></div><div>This should be FRS_DiagnosticIssued.</div></blockquote><div><br></div></div><div>Done.</div>
<div><div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div><br></div><div>> + break;</div><div>
> +</div><div>> + case OR_No_Viable_Function:</div><div>> + *CallExpr = ExprError();</div><div>> + return FRS_NoViableFunction;</div><div>> +</div></div><div>> + case OR_Ambiguous:</div>
<div>> + *FoundFunction = Best->Function;</div><div>> + *CallExpr = ExprError();</div><div>> + return FRS_Ambiguous;</div><div>> +</div><div>> + case OR_Deleted:</div><div>> + // FIXME: this case is never actually reached, since ActOnCallExpr</div>
<div>> + // detects deleted functions, issues a diagnostic, and returns</div><div>> + // an invalid ExprResult.</div><div>> + // As a result, the diagnostics for pointers to types with deleted begin</div>
<div>> + // or end functions are not as clear as they could be.</div><div><br></div><div>I think this comment is stale; the ActOnCallExpr call is in the other arm of this 'if'.</div></blockquote><div><br>
</div></div><div>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).</div><div><br></div><div>Moved and clarified...and then completely fixed (see my next comment).</div>
<div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><br></div><div>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.</div>
<div><br></div></blockquote><div><br></div></div><div>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.</div>
<div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>> /// FixOverloadedFunctionReference - E is an expression that refers to</div>
<div>> /// a C++ overloaded function (possibly with some parentheses and</div><div>> /// perhaps a '&' around it). We have resolved the overloaded function</div><div>> diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp</div>
<div>> index 9affe98..acc5042 100644</div><div>> --- a/lib/Sema/SemaStmt.cpp</div><div>> +++ b/lib/Sema/SemaStmt.cpp</div><div>[...]</div><div>> +static Sema::ForRangeStatus BuildNonArrayForRange(Sema &SemaRef, Scope *S,</div>
<div>
<div>> + Expr *BeginRange, Expr *EndRange,</div><div>> + QualType RangeType,</div><div>> + VarDecl *BeginVar,</div>
<div>> + VarDecl *EndVar,</div><div>> + SourceLocation ColonLoc,</div><div>> + OverloadCandidateSet *CandidateSet,</div>
<div>> + ExprResult *BeginExpr,</div><div>> + ExprResult *EndExpr,</div></div><div>> + const FunctionDecl **FoundFunction,</div>
<div>> + Sema::BeginEndFunction *BEF) { </div></blockquote></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>
<div>[...] </div></div></blockquote><div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>
> + *BEF = Sema::BEF_begin;</div><div>> + Sema::ForRangeStatus RangeStatus =</div><div>> + SemaRef.BuildForRangeBeginEndCall(S, ColonLoc, ColonLoc, BeginVar,</div><div>> + Sema::BEF_begin, BeginNameInfo,</div>
<div>> + BeginMemberLookup, CandidateSet,</div><div>> + BeginRange, BeginExpr, FoundFunction);</div><div>> + if (RangeStatus != Sema::FRS_Success)</div>
<div>> + return RangeStatus;</div><div>> +</div><div>> + *BEF = Sema::BEF_end;</div><div>> + RangeStatus =</div><div>> + SemaRef.BuildForRangeBeginEndCall(S, ColonLoc, ColonLoc, EndVar,</div><div>
> + Sema::BEF_end, EndNameInfo,</div><div>> + EndMemberLookup, CandidateSet,</div><div>> + EndRange, EndExpr, FoundFunction);</div>
<div>> + return RangeStatus;</div><div><br></div><div>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.</div>
</blockquote><div><br></div></div><div>Done.</div><div><div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>> @@ -1834,62 +1898,72 @@ Sema::BuildCXXForRangeStmt(SourceLocation ForLoc, SourceLocation ColonLoc,</div>
<div>> BoundExpr.get());</div><div>> if (EndExpr.isInvalid())</div>
<div>> return StmtError();</div><div>> - if (FinishForRangeVarDecl(*this, EndVar, EndExpr.get(), ColonLoc,</div><div>> + if (FinishForRangeVarDecl(EndVar, EndExpr.get(), ColonLoc,</div><div>> diag::err_for_range_iter_deduction_failure)) {</div>
<div>> - NoteForRangeBeginEndFunction(*this, EndExpr.get(), BEF_end);</div><div>> + NoteForRangeBeginEndFunction(EndExpr.get(), BEF_end);</div><div><div>> return StmtError();</div>
<div>> }</div>
<div>> } else {</div><div>> - DeclarationNameInfo BeginNameInfo(&PP.getIdentifierTable().get("begin"),</div><div>> - ColonLoc);</div><div>> - DeclarationNameInfo EndNameInfo(&PP.getIdentifierTable().get("end"),</div>
<div>> - ColonLoc);</div><div>> -</div><div>> - LookupResult BeginMemberLookup(*this, BeginNameInfo, LookupMemberName);</div><div>> - LookupResult EndMemberLookup(*this, EndNameInfo, LookupMemberName);</div>
<div>> -</div><div>> - if (CXXRecordDecl *D = RangeType->getAsCXXRecordDecl()) {</div><div>> - // - if _RangeT is a class type, the unqualified-ids begin and end are</div><div>> - // looked up in the scope of class _RangeT as if by class member access</div>
<div>> - // lookup (3.4.5), and if either (or both) finds at least one</div><div>> - // declaration, begin-expr and end-expr are __range.begin() and</div><div>> - // __range.end(), respectively;</div>
<div>> - LookupQualifiedName(BeginMemberLookup, D);</div><div>> - LookupQualifiedName(EndMemberLookup, D);</div><div>> -</div><div>> - if (BeginMemberLookup.empty() != EndMemberLookup.empty()) {</div>
<div>> - Diag(ColonLoc, diag::err_for_range_member_begin_end_mismatch)</div><div>> - << RangeType << BeginMemberLookup.empty();</div><div>> + Expr *BeginRange = BeginRangeRef.get();</div>
<div>> + Expr *EndRange = EndRangeRef.get();</div></div><div>> + const FunctionDecl *FoundFunction = NULL;</div><div>> + OverloadCandidateSet CandidateSet(RangeLoc);</div><div>> + Sema::BeginEndFunction BEFFailure;</div>
<div>
<div>> + ForRangeStatus RangeStatus =</div><div>> + BuildNonArrayForRange(*this, S, BeginRange, EndRange, RangeType,</div><div>> + BeginVar, EndVar, ColonLoc, &CandidateSet,</div>
</div><div>> + &BeginExpr, &EndExpr, &FoundFunction,</div><div><div>> + &BEFFailure);</div><div>> +</div><div>> + // Attempt to dereference pointers and try again.</div>
<div>> + if (RangeStatus != FRS_Success) {</div><div><br></div></div><div>We shouldn't attempt recovery if we've issued a diagnostic already.</div></blockquote><div><br></div></div></div><div>Done, as noted above.</div>
<div><div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><br></div><div>> + if (!ShouldTryDeref)</div><div>
> + return StmtError();</div><div><div>> + if (!RangeType->isPointerType()) {</div><div>> + const ExprResult & TheCall = BEFFailure ? EndExpr: BeginExpr;</div><div>> + ForRangeDiagnostics(*this, RangeStatus,</div>
<div>> + BEFFailure ? EndRange : BeginRange,</div></div><div>> + TheCall, FoundFunction,</div><div><div>> + RangeLoc, &CandidateSet, BEFFailure);</div>
<div>> return StmtError();</div><div>> }</div><div>> - } else {</div><div>> - // - otherwise, begin-expr and end-expr are begin(__range) and</div><div>> - // end(__range), respectively, where begin and end are looked up with</div>
<div>> - // argument-dependent lookup (3.4.2). For the purposes of this name</div><div>> - // lookup, namespace std is an associated namespace.</div></div><div><div>> - }</div><div>
> </div><div>> - BeginExpr = BuildForRangeBeginEndCall(*this, S, ColonLoc, BeginVar,</div>
<div>> - BEF_begin, BeginNameInfo,</div><div>> - BeginMemberLookup,</div><div>> - BeginRangeRef.get());</div>
<div>> - if (BeginExpr.isInvalid())</div><div>> - return StmtError();</div><div>> -</div><div>> - EndExpr = BuildForRangeBeginEndCall(*this, S, ColonLoc, EndVar,</div><div>> - BEF_end, EndNameInfo,</div>
<div>> - EndMemberLookup, EndRangeRef.get());</div><div>> - if (EndExpr.isInvalid())</div><div>> - return StmtError();</div></div><div>> + ExprResult DereferencedRange =</div>
<div>> + CreateBuiltinUnaryOp(RangeLoc, UO_Deref, Range);</div><div>> +</div><div>> + // FIXME: The second ForLoc should really be LParenLoc, though</div><div>> + // this is only used in one diagnostic in ActOnCXXForRangeStmt.</div>
<div>> + StmtResult SR = ActOnCXXForRangeStmt(ForLoc, ForLoc, LoopVarDecl,</div><div>> + ColonLoc, DereferencedRange.get(),</div><div>> + RParenLoc, false);</div>
<div><br></div><div>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.</div>
</blockquote><div><br></div></div></div><div>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.</div>
<div><div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div>
<div><br></div><div class="gmail_quote">On Wed, Aug 8, 2012 at 11:37 AM, Sam Panzer <span dir="ltr"><<a href="mailto:panzer@google.com" target="_blank">panzer@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Ping<div><div><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Jul 27, 2012 at 3:06 PM, Sam Panzer <span dir="ltr"><<a href="mailto:panzer@google.com" target="_blank">panzer@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
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.<div><br></div><div>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.</div>
<div>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.</div>
<div><br></div><div>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.</div>
<div>I included a few test cases that note this behavior and have appropriate FIXME's for when it is corrected.<div class="gmail_extra"><br><div class="gmail_quote"><div>On Thu, Jul 26, 2012 at 2:01 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>Hi Sam,</div><div><br></div><div>Sorry for the massive delay reviewing this...</div><div><br></div><div>> diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td</div>
<div>
> index 52641b8..97b0469 100644</div><div>> --- a/include/clang/Basic/DiagnosticSemaKinds.td</div><div>> +++ b/include/clang/Basic/DiagnosticSemaKinds.td</div><div>> @@ -1391,7 +1391,14 @@ def err_for_range_member_begin_end_mismatch : Error<</div>
<div>> "range type %0 has '%select{begin|end}1' member but no '%select{end|begin}1' member">;</div><div>> def err_for_range_begin_end_types_differ : Error<</div><div>> "'begin' and 'end' must return the same type (got %0 and %1)">;</div>
<div>> +def err_for_range_invalid: Error<</div><div>> + "invalid range expression of type %0">;</div><div>> +def note_for_range_adl_failure: Note<</div><div>> + "no viable '%select{begin|end}0' function for range of type %1">;</div>
<div><br></div><div>Please merge these into a single diagnostic (use the text of note_for_range_adl_failure as the error message).</div></blockquote><div><br></div></div><div>Done.</div><div><div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><br></div><div>> def note_for_range_type : Note<"range has type %0">;</div>
<div>> +def err_for_range_dereference : Error<</div><div>> + "invalid range expression of type %0; did you mean to dereference it "</div><div>> + "with '*'?">;</div><div>> def note_for_range_begin_end : Note<</div>
<div>> "selected '%select{begin|end}0' %select{function|template }1%2 with iterator type %3">;</div><div>> </div><div>> diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h</div>
<div>> index a48cde0..29fbded 100644</div><div>> --- a/include/clang/Sema/Sema.h</div><div>> +++ b/include/clang/Sema/Sema.h</div><div>> @@ -1923,7 +1923,24 @@ public:</div><div>> Expr **Args, unsigned NumArgs,</div>
<div>> SourceLocation RParenLoc,</div><div>> Expr *ExecConfig,</div><div>> - bool AllowTypoCorrection=true);</div>
<div>> + OverloadCandidateSet *CandidateSet,</div><div>> + bool AllowTypoCorrection,</div><div>> + bool InRangeExpr);</div>
<div>> +</div><div>> + ExprResult BuildOverloadedCallExpr(Scope *S, Expr *Fn,</div><div>> + UnresolvedLookupExpr *ULE,</div><div>> + SourceLocation LParenLoc,</div>
<div>> + Expr **Args, unsigned NumArgs,</div><div>> + SourceLocation RParenLoc,</div><div>> + Expr *ExecConfig,</div>
<div>> + bool AllowTypoCorrection=true,</div><div>> + bool InRangeExpr=false);</div><div><br></div><div>I don't like passing InRangeExpr in here. It's too specific. It's also unnecessary (more on that later).</div>
</blockquote><div><br></div></div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>> +</div><div>> +</div><div><div>> +// FIXME: Update this stale comment</div>
<div>
<br></div><div>Please do :)</div></div></blockquote><div><br></div><div>Done. The next comment too :)</div><div><div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>
<br>
</div><div>> +/// ResolveOverloadedCallFn - Given the call expression that calls Fn</div>
<div>> +/// (which eventually refers to the declaration Func) and the call</div><div>> +/// arguments Args/NumArgs, attempt to resolve the function call down</div><div>> +/// to a specific function. If overload resolution succeeds, returns</div>
<div>> +/// the function declaration produced by overload</div><div>> +/// resolution. Otherwise, emits diagnostics, deletes all of the</div><div>> +/// arguments and Fn, and returns NULL.</div><div>> +</div>
<div>
> +ExprResult Sema::BuildOverloadedCallExpr(Scope *S, Expr *Fn,</div><div>> + UnresolvedLookupExpr *ULE,</div><div>> + SourceLocation LParenLoc,</div>
<div>> + Expr **Args, unsigned NumArgs,</div><div>> + SourceLocation RParenLoc,</div><div>> + Expr *ExecConfig,</div>
<div>> + OverloadCandidateSet *CandidateSet,</div><div>> + bool AllowTypoCorrection,</div><div>> + bool InRangeExpr) {</div>
<div>> + if (CandidateSet->empty())</div><div>> return BuildRecoveryCallExpr(*this, S, Fn, ULE, LParenLoc,</div><div>> llvm::MutableArrayRef<Expr *>(Args, NumArgs),</div>
<div>> RParenLoc, /*EmptyLookup=*/true,</div><div>> AllowTypoCorrection);</div><div>> - }</div><div>> -</div><div>> - UnbridgedCasts.restore();</div>
<div>> </div><div>> OverloadCandidateSet::iterator Best;</div><div>> - switch (CandidateSet.BestViableFunction(*this, Fn->getLocStart(), Best)) {</div><div>> + switch (CandidateSet->BestViableFunction(*this, Fn->getLocStart(), Best)) {</div>
<div>> case OR_Success: {</div><div>> FunctionDecl *FDecl = Best->Function;</div><div>> MarkFunctionReferenced(Fn->getExprLoc(), FDecl);</div><div>> @@ -9770,6 +9773,10 @@ Sema::BuildOverloadedCallExpr(Scope *S, Expr *Fn, UnresolvedLookupExpr *ULE,</div>
<div>> }</div><div>> </div><div>> case OR_No_Viable_Function: {</div><div>> + // In case we're in a range expression, let the range handling code take</div><div>> + // care of it.</div><div>
> + if (InRangeExpr)</div><div>> + break;</div><div><br></div><div>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?</div>
</blockquote><div><br></div></div></div><div>Nope, it's gone now.</div><div><div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>> +/// This version of BuildOverloadedCallExpr creates an OverloadCandidateSet</div>
<div>> +/// first.</div><div>> +ExprResult Sema::BuildOverloadedCallExpr(Scope *S, Expr *Fn,</div>
<div>> + UnresolvedLookupExpr *ULE,</div><div>> + SourceLocation LParenLoc,</div><div>> + Expr **Args, unsigned NumArgs,</div>
<div>> + SourceLocation RParenLoc,</div><div>> + Expr *ExecConfig,</div><div>> + bool AllowTypoCorrection,</div>
<div>> + bool InRangeExpr) {</div><div>> +#ifndef NDEBUG</div><div>> + if (ULE->requiresADL()) {</div><div>> + // To do ADL, we must have found an unqualified name.</div>
<div>> + assert(!ULE->getQualifier() && "qualified name with ADL");</div><div>> +</div><div>> + // We don't perform ADL for implicit declarations of builtins.</div><div>> + // Verify that this was correctly set up.</div>
<div>> + FunctionDecl *F;</div><div>> + if (ULE->decls_begin() + 1 == ULE->decls_end() &&</div><div>> + (F = dyn_cast<FunctionDecl>(*ULE->decls_begin())) &&</div><div>
> + F->getBuiltinID() && F->isImplicit())</div>
<div>> + llvm_unreachable("performing ADL for builtin");</div><div>> +</div><div>> + // We don't perform ADL in C.</div><div>> + assert(getLangOpts().CPlusPlus && "ADL enabled in C");</div>
<div>> + } else</div><div>> + assert(!ULE->isStdAssociatedNamespace() &&</div><div>> + "std is associated namespace but not doing ADL");</div><div>> +#endif</div><div><br></div>
<div>This checking belongs in buildOverloadedCallSet. We want to perform these checks for range-based for calls too.</div></blockquote><div><br></div></div></div><div>Done.</div><div><div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><br></div><div>> + </div><div>> + OverloadCandidateSet CandidateSet(Fn->getExprLoc());</div>
<div>> + ExprResult result;</div><div>> + if (buildOverloadedCallSet(S, Fn, ULE, Args, NumArgs, LParenLoc,</div><div>> + &CandidateSet, &result))</div><div>> + return result;</div>
<div>> +</div><div>> + return BuildOverloadedCallExpr(S, Fn, ULE, LParenLoc, Args, NumArgs,</div><div>> + RParenLoc, ExecConfig, &CandidateSet,</div><div>> + AllowTypoCorrection, InRangeExpr);</div>
<div>> +</div><div>> }</div><div>> </div><div>> static bool IsOverloaded(const UnresolvedSetImpl &Functions) {</div><div>> diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp</div><div>> index 9be1d34..18bcf89 100644</div>
<div>> --- a/lib/Sema/SemaStmt.cpp</div><div>> +++ b/lib/Sema/SemaStmt.cpp</div><div>> @@ -1578,18 +1578,35 @@ void NoteForRangeBeginEndFunction(Sema &SemaRef, Expr *E,</div><div>> << BEF << IsTemplate << Description << E->getType();</div>
<div>> }</div><div>> </div><div>> +</div><div>> +enum ForRangeStatus {</div><div>> + FRS_Success,</div><div>> + FRS_InvalidMemberFunction,</div><div>> + FRS_NoOverload,</div><div>> + FRS_DeletedOrAmbiguous,</div>
<div>> + FRS_NoViableFunction,</div><div>> + FRS_InvalidIteratorType,</div><div>> + FRS_BeginEndMismatch</div><div>> +};</div><div>> +</div><div>> /// Build a call to 'begin' or 'end' for a C++0x for-range statement. If the</div>
<div>> /// given LookupResult is non-empty, it is assumed to describe a member which</div><div>> /// will be invoked. Otherwise, the function will be found via argument</div><div>> /// dependent lookup.</div>
<div>
> -static ExprResult BuildForRangeBeginEndCall(Sema &SemaRef, Scope *S,</div><div>> - SourceLocation Loc,</div><div>> - VarDecl *Decl,</div>
<div>> - BeginEndFunction BEF,</div><div>> - const DeclarationNameInfo &NameInfo,</div><div>> - LookupResult &MemberLookup,</div>
<div>> - Expr *Range) {</div><div>> - ExprResult CallExpr;</div><div>> +/// CallExpr is set and FRS_Success returned on success, otherwise some</div><div>> +/// non-success value is returned.</div>
<div>> +static ForRangeStatus</div><div>> +BuildForRangeBeginEndCall(Sema &SemaRef, Scope *S,</div><div>> + SourceLocation Loc,</div><div>> + SourceLocation RangeLoc,</div>
<div>> + VarDecl *Decl,</div><div>> + BeginEndFunction BEF,</div><div>> + const DeclarationNameInfo &NameInfo,</div><div>> + LookupResult &MemberLookup,</div>
<div>> + OverloadCandidateSet *CandidateSet,</div><div>> + Expr *Range,</div><div>> + ExprResult *CallExpr) {</div><div><br></div><div>Now this is performing the call to BestViableFunction itself, this really belongs in SemaOverload.cpp.</div>
<div><br></div></blockquote><div><br></div></div></div><div>Done. It took some rearranging :)</div><div>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.</div>
<div><div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div></div><div>> + CandidateSet->clear();</div><div>> if (!MemberLookup.empty()) {</div><div>
> ExprResult MemberRef =</div><div>> SemaRef.BuildMemberReferenceExpr(Range, Range->getType(), Loc,</div>
<div>> @@ -1598,12 +1615,16 @@ static ExprResult BuildForRangeBeginEndCall(Sema &SemaRef, Scope *S,</div><div>> /*FirstQualifierInScope=*/0,</div><div>> MemberLookup,</div>
<div>> /*TemplateArgs=*/0);</div><div>> - if (MemberRef.isInvalid())</div><div>> - return ExprError();</div><div>> - CallExpr = SemaRef.ActOnCallExpr(S, MemberRef.get(), Loc, MultiExprArg(),</div>
<div>> + if (MemberRef.isInvalid()) {</div><div>> + *CallExpr = ExprError();</div><div>> + return FRS_InvalidMemberFunction;</div><div>> + }</div><div>> + *CallExpr = SemaRef.ActOnCallExpr(S, MemberRef.get(), Loc, MultiExprArg(),</div>
<div>> Loc, 0);</div><div>> - if (CallExpr.isInvalid())</div><div>> - return ExprError();</div><div>> + if (CallExpr->isInvalid()) {</div><div>> + *CallExpr = ExprError();</div>
<div>> + return FRS_InvalidMemberFunction;</div><div>> + }</div><div>> } else {</div><div>> UnresolvedSet<0> FoundNames;</div><div>> // C++0x [stmt.ranged]p1: For the purposes of this name lookup, namespace</div>
<div>> @@ -1614,20 +1635,49 @@ static ExprResult BuildForRangeBeginEndCall(Sema &SemaRef, Scope *S,</div><div>> /*NeedsADL=*/true, /*Overloaded=*/false,</div><div>> FoundNames.begin(), FoundNames.end(),</div>
<div>> /*LookInStdNamespace=*/true);</div><div>> - CallExpr = SemaRef.BuildOverloadedCallExpr(S, Fn, Fn, Loc, &Range, 1, Loc,</div><div>> - 0, /*AllowTypoCorrection=*/false);</div>
<div>> - if (CallExpr.isInvalid()) {</div><div>> - SemaRef.Diag(Range->getLocStart(), diag::note_for_range_type)</div><div>> - << Range->getType();</div><div>> - return ExprError();</div>
<div>> +</div><div>> + SemaRef.buildOverloadedCallSet(S, Fn, Fn, &Range, 1, Loc, CandidateSet,</div><div>> + CallExpr);</div><div>> +</div><div>> + OverloadCandidateSet::iterator Best;</div>
<div>> + if (CandidateSet->empty()) {</div><div>> + *CallExpr = ExprError();</div><div>> + return FRS_NoOverload;</div><div>> + } else {</div><div>> + switch (CandidateSet->BestViableFunction(SemaRef, Fn->getLocStart(),</div>
<div>> + Best)) {</div><div>> + case OR_Success:</div><div>> + *CallExpr =</div><div>> + SemaRef.BuildOverloadedCallExpr(S, Fn, Fn, Loc, &Range, 1, Loc, 0,</div>
<div>> + CandidateSet,</div><div>> + /*AllowTypoCorrection=*/false,</div><div>> + /*InRangeExpr=*/true);</div>
<div><br></div><div>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?</div>
</blockquote><div><br></div></div></div><div>Done.</div><div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><br></div><div>> + if (CallExpr->isInvalid())</div><div>> + return FRS_NoOverload;</div><div>> + break;</div><div>> +</div><div>> + case OR_No_Viable_Function:</div><div>
> + *CallExpr = ExprError();</div><div>> + return FRS_NoViableFunction;</div><div>> +</div><div>> + case OR_Deleted:</div><div>> + case OR_Ambiguous:</div><div>> + // Here we want to defer to the diagnostics in BuildOverloadedCallExpr.</div>
<div>> + // The return value doesn't matter, since we're returning an ExprError()</div><div>> + // anyway.</div><div>> + SemaRef.BuildOverloadedCallExpr(S, Fn, Fn, Loc, &Range, 1, Loc, 0,</div>
<div>> + /*AllowTypoCorrection=*/false,</div><div>> + /*InRangeExpr=*/true);</div><div>> + *CallExpr = ExprError();</div><div>
> + return FRS_DeletedOrAmbiguous;</div>
<div><br></div><div>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?</div>
</blockquote><div><br></div></div><div>I was looking to avoid duplicating the code that emits the diagnostic, but the cure was worse. Done.</div><div><div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><br></div><div>> + }</div><div>> }</div><div>> +</div><div>> }</div><div>> - if (FinishForRangeVarDecl(SemaRef, Decl, CallExpr.get(), Loc,</div><div>> - diag::err_for_range_iter_deduction_failure)) {</div>
<div>> - NoteForRangeBeginEndFunction(SemaRef, CallExpr.get(), BEF);</div><div>> - return ExprError();</div><div>> - }</div><div>> - return CallExpr;</div><div>> + if (FinishForRangeVarDecl(SemaRef, Decl, CallExpr->get(), Loc,</div>
<div>> + diag::err_for_range_iter_deduction_failure))</div><div>> + return FRS_InvalidIteratorType;</div><div>> + return FRS_Success;</div><div>> }</div><div>> </div><div>> }</div>
<div>> @@ -1700,6 +1750,129 @@ Sema::ActOnCXXForRangeStmt(SourceLocation ForLoc, SourceLocation LParenLoc,</div><div>> RParenLoc);</div><div>> }</div><div>> </div><div>> +// Emit the diagnostic appropriate to the given ForRangeStatus. The other</div>
<div>> +// parameters are used for supplementary information.</div><div>> +static void ForRangeDiagnostics(Sema &SemaRef, ForRangeStatus st, Expr *Range,</div><div>> + const ExprResult &CallExpr,</div>
<div>> + SourceLocation RangeLoc,</div><div>> + OverloadCandidateSet *CandidateSet,</div><div>> + BeginEndFunction BEF) {</div>
<div>> + switch (st) {</div><div>> + case FRS_Success:</div><div>> + // It worked; nothing to do.</div><div>> + break;</div><div>> +</div><div>> + case FRS_InvalidMemberFunction:</div><div>> + SemaRef.Diag(Range->getLocStart(), diag::err_for_range_invalid)</div>
<div>> + << RangeLoc << Range->getType();</div><div>> + SemaRef.Diag(Range->getLocStart(), diag::note_for_range_adl_failure)</div><div>> + << BEF << Range->getType();</div>
<div>> + break;</div><div>> +</div><div>> + case FRS_NoOverload:</div><div>> + SemaRef.Diag(Range->getLocStart(), diag::err_for_range_invalid)</div><div>> + << RangeLoc << Range->getType();</div>
<div>> + SemaRef.Diag(Range->getLocStart(), diag::note_for_range_adl_failure)</div><div>> + << BEF << Range->getType();</div><div>> + break;</div><div>> +</div><div>> + case FRS_DeletedOrAmbiguous:</div>
<div>> + // Some of the diagnostics are emitted in BuildOverloadedCallExpr, so we</div><div>> + // just add these.</div><div>> + SemaRef.Diag(Range->getLocStart(), diag::err_for_range_invalid)</div><div>
> + << RangeLoc << Range->getType();</div><div>> + SemaRef.Diag(Range->getLocStart(), diag::note_for_range_adl_failure)</div><div>> + << BEF << Range->getType();</div>
<div>> + break;</div><div>> +</div><div>> + case FRS_NoViableFunction:</div><div>> + SemaRef.Diag(Range->getLocStart(), diag::err_for_range_invalid)</div><div>> + << RangeLoc << Range->getType();</div>
<div>> + CandidateSet->NoteCandidates(SemaRef, OCD_AllCandidates,</div><div>> + llvm::makeArrayRef(&Range, /*NumArgs=*/1));</div><div>> + SemaRef.Diag(Range->getLocStart(), diag::note_for_range_adl_failure)</div>
<div>> + << BEF << Range->getType();</div><div>> + break;</div><div>> +</div><div>> + case FRS_InvalidIteratorType:</div><div>> + NoteForRangeBeginEndFunction(SemaRef, CallExpr.get(), BEF);</div>
<div>> + break;</div><div>> +</div><div>> + case FRS_BeginEndMismatch:</div><div>> + SemaRef.Diag(RangeLoc, diag::err_for_range_member_begin_end_mismatch)</div><div>> + << Range->getType() << BEF;</div>
<div>> + break;</div><div>> + }</div><div>> +}</div><div><br></div><div>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:</div>
<div> <br></div></blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div></div><div>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.</div>
</blockquote><div><br></div></div></div><div>Done. This required that I move NoteForRangeBeginEndFunction() into Sema so that BuildForRangeBeginEndCall() could call it.</div><div><div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><br></div><div>> +</div><div>> +/// \brief Create the initialization, compare, and increment steps for</div><div>> +/// the range-based for loop expression.</div><div>> +/// This function does not handle array-based for loops,</div>
<div>> +/// which are created in Sema::BuildCXXForRangeStmt.</div><div>> +///</div><div>> +/// \returns a ForRangeStatus indicating success or what kind of error occurred.</div><div>> +/// BeginExpr and EndExpr are set and FRS_Success is returned on success;</div>
<div>> +/// CandidateSet and BEF are set and some non-success value is returned on</div><div>> +/// failure.</div><div>> +static ForRangeStatus BuildNonArrayForRange(Sema &SemaRef, Scope *S,</div><div>> + Expr *BeginRange, Expr *EndRange,</div>
<div>> + QualType RangeType,</div><div>> + VarDecl *BeginVar,</div><div>> + VarDecl *EndVar,</div>
<div>> + SourceLocation ColonLoc,</div><div>> + OverloadCandidateSet *CandidateSet,</div><div>> + ExprResult *BeginExpr,</div>
<div>> + ExprResult *EndExpr,</div><div>> + BeginEndFunction *BEF) {</div><div>> + DeclarationNameInfo BeginNameInfo(</div><div>
> + &SemaRef.PP.getIdentifierTable().get("begin"), ColonLoc);</div><div>> + DeclarationNameInfo EndNameInfo(&SemaRef.PP.getIdentifierTable().get("end"),</div><div>> + ColonLoc);</div>
<div>> +</div><div>> + LookupResult BeginMemberLookup(SemaRef, BeginNameInfo,</div><div>> + Sema::LookupMemberName);</div><div>> + LookupResult EndMemberLookup(SemaRef, EndNameInfo, Sema::LookupMemberName);</div>
<div>> +</div><div>> + if (CXXRecordDecl *D = RangeType->getAsCXXRecordDecl()) {</div><div>> + // - if _RangeT is a class type, the unqualified-ids begin and end are</div><div>> + // looked up in the scope of class _RangeT as if by class member access</div>
<div>> + // lookup (3.4.5), and if either (or both) finds at least one</div><div>> + // declaration, begin-expr and end-expr are __range.begin() and</div><div>> + // __range.end(), respectively;</div>
<div>> + SemaRef.LookupQualifiedName(BeginMemberLookup, D);</div><div>> + SemaRef.LookupQualifiedName(EndMemberLookup, D);</div><div>> +</div><div>> + if (BeginMemberLookup.empty() != EndMemberLookup.empty()) {</div>
<div>> + *BEF = BeginMemberLookup.empty() ? BEF_end : BEF_begin;</div><div>> + return FRS_BeginEndMismatch;</div><div>> + }</div><div>> + } else {</div><div>> + // - otherwise, begin-expr and end-expr are begin(__range) and</div>
<div>> + // end(__range), respectively, where begin and end are looked up with</div><div>> + // argument-dependent lookup (3.4.2). For the purposes of this name</div><div>> + // lookup, namespace std is an associated namespace.</div>
<div>> +</div><div>> + }</div><div>> +</div><div>> + *BEF = BEF_begin;</div><div>> + ForRangeStatus</div><div>> + RangeStatus = BuildForRangeBeginEndCall(SemaRef, S, ColonLoc,</div><div>> + ColonLoc, BeginVar,</div>
<div>> + BEF_begin, BeginNameInfo,</div><div>> + BeginMemberLookup,</div><div>> + CandidateSet,</div>
<div>> + BeginRange, BeginExpr);</div><div>> + if (RangeStatus != FRS_Success)</div><div>> + return st;</div><div>> +</div><div>> + *BEF = BEF_end;</div><div>
> + RangeStatus = BuildForRangeBeginEndCall(SemaRef, S, ColonLoc, ColonLoc,</div><div>> + EndVar, BEF_end, EndNameInfo,</div><div>> + EndMemberLookup, CandidateSet,</div>
<div>> + EndRange, EndExpr);</div><div>> + return RangeStatus;</div><div>> +}</div><div>> +</div><div>> /// BuildCXXForRangeStmt - Build or instantiate a C++0x for-range statement.</div>
<div>> StmtResult</div><div>> Sema::BuildCXXForRangeStmt(SourceLocation ForLoc, SourceLocation ColonLoc,</div><div>> @@ -1791,49 +1964,58 @@ Sema::BuildCXXForRangeStmt(SourceLocation ForLoc, SourceLocation ColonLoc,</div>
<div>> return StmtError();</div><div>> }</div><div>> } else {</div><div>> - DeclarationNameInfo BeginNameInfo(&PP.getIdentifierTable().get("begin"),</div><div>> - ColonLoc);</div>
<div>> - DeclarationNameInfo EndNameInfo(&PP.getIdentifierTable().get("end"),</div><div>> - ColonLoc);</div><div>> -</div><div>> - LookupResult BeginMemberLookup(*this, BeginNameInfo, LookupMemberName);</div>
<div>> - LookupResult EndMemberLookup(*this, EndNameInfo, LookupMemberName);</div><div>> -</div><div>> - if (CXXRecordDecl *D = RangeType->getAsCXXRecordDecl()) {</div><div>> - // - if _RangeT is a class type, the unqualified-ids begin and end are</div>
<div>> - // looked up in the scope of class _RangeT as if by class member access</div><div>> - // lookup (3.4.5), and if either (or both) finds at least one</div><div>> - // declaration, begin-expr and end-expr are __range.begin() and</div>
<div>> - // __range.end(), respectively;</div><div>> - LookupQualifiedName(BeginMemberLookup, D);</div><div>> - LookupQualifiedName(EndMemberLookup, D);</div><div>> -</div><div>> - if (BeginMemberLookup.empty() != EndMemberLookup.empty()) {</div>
<div>> - Diag(ColonLoc, diag::err_for_range_member_begin_end_mismatch)</div><div>> - << RangeType << BeginMemberLookup.empty();</div><div>> + Expr *BeginRange = BeginRangeRef.get();</div>
<div>> + Expr *EndRange = EndRangeRef.get();</div><div>> + OverloadCandidateSet CandidateSet(RangeLoc);</div><div>> + BeginEndFunction BEFFailure;</div><div>> + ForRangeStatus RangeStatus =</div>
<div>> + BuildNonArrayForRange(*this, S, BeginRange, EndRange, RangeType,</div><div>> + BeginVar, EndVar, ColonLoc, &CandidateSet,</div><div>> + &BeginExpr, &EndExpr, &BEFFailure);</div>
<div>> +</div><div>> + // Attempt to dereference pointers and try again.</div><div>> + if (RangeStatus != FRS_Success) {</div><div>> + if (!RangeType->isPointerType()) {</div><div>> + const ExprResult & TheCall = BEFFailure ? EndExpr: BeginExpr;</div>
<div>> + ForRangeDiagnostics(*this, RangeStatus,</div><div>> + BEFFailure ? EndRange : BeginRange,</div><div>> + TheCall, RangeLoc, &CandidateSet, BEFFailure);</div>
<div>> return StmtError();</div><div>> }</div><div>> - } else {</div><div>> - // - otherwise, begin-expr and end-expr are begin(__range) and</div><div>> - // end(__range), respectively, where begin and end are looked up with</div>
<div>> - // argument-dependent lookup (3.4.2). For the purposes of this name</div><div>> - // lookup, namespace std is an associated namespace.</div><div>> + BeginRange =</div><div>> + new (Context) UnaryOperator(BeginRange, UO_Deref,</div>
<div>> + RangeType->getPointeeType(),</div><div>> + VK_LValue, OK_Ordinary, RangeLoc);</div><div>> + EndRange =</div><div>> + new (Context) UnaryOperator(EndRange, UO_Deref,</div>
<div>> + RangeType->getPointeeType(),</div><div>> + VK_LValue, OK_Ordinary, RangeLoc);</div><div>> +</div><div>> + BeginEndFunction DerefBEF;</div>
<div>> + ForRangeStatus DerefStatus</div><div>> + = BuildNonArrayForRange(*this, S, BeginRange, EndRange,</div><div>> + BeginRange->getType(),</div><div>> + BeginVar, EndVar,</div>
<div>> + ColonLoc, &CandidateSet,</div><div>> + &BeginExpr, &EndExpr, &DerefBEF);</div><div>> + if (DerefStatus != FRS_Success) {</div>
<div>> + const ExprResult & TheCall = BEFFailure ? EndExpr: BeginExpr;</div><div>> + ForRangeDiagnostics(*this, RangeStatus,</div><div>> + DerefBEF ? EndRangeRef.get(): BeginRangeRef.get(),</div>
<div>> + TheCall, RangeLoc, &CandidateSet, BEFFailure);</div><div>> + return StmtError();</div><div>> + } else {</div><div>> + // The attempt to dereference would likely succeed.</div>
<div>> + Diag(RangeLoc, diag::err_for_range_dereference)</div><div>> + << RangeLoc << RangeType</div><div>> + << FixItHint::CreateInsertion(RangeLoc, "*");</div>
<div><br></div><div>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.</div>
</blockquote><div><br></div></div></div><div>I changed it to rebuild the correct expression here by introducing mutual recursion with ActOnCXXForRangeStmt, limiting the depth to one.</div><div><div><div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><br></div><div>> + }</div><div>> }</div><div>> -</div><div>> - BeginExpr = BuildForRangeBeginEndCall(*this, S, ColonLoc, BeginVar,</div><div>> - BEF_begin, BeginNameInfo,</div>
<div>> - BeginMemberLookup,</div><div>> - BeginRangeRef.get());</div><div>> - if (BeginExpr.isInvalid())</div><div>> - return StmtError();</div>
<div>> -</div><div>> - EndExpr = BuildForRangeBeginEndCall(*this, S, ColonLoc, EndVar,</div><div>> - BEF_end, EndNameInfo,</div><div>> - EndMemberLookup, EndRangeRef.get());</div>
<div>> - if (EndExpr.isInvalid())</div><div>> - return StmtError();</div><div>> }</div><div>> </div><div>> + assert(!BeginExpr.isInvalid() && !EndExpr.isInvalid() &&</div>
<div>> + "invalid range expression in for loop");</div><div>> +</div><div>> // C++0x [decl.spec.auto]p6: BeginType and EndType must be the same.</div><div>> QualType BeginType = BeginVar->getType(), EndType = EndVar->getType();</div>
<div>> if (!Context.hasSameType(BeginType, EndType)) {</div><div>> diff --git a/test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp b/test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp</div><div>> index 96bb472..e5809f0 100644</div>
<div>> --- a/test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp</div><div>> +++ b/test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp</div><div>> @@ -3,20 +3,21 @@</div><div>> struct pr12960 {</div><div>> int begin;</div>
<div>> void foo(int x) {</div><div>> - for (int& it : x) { // expected-error {{use of undeclared identifier 'begin'}} expected-note {{range has type 'int'}}</div><div>> + for (int& it : x) { //expected-error{{invalid range expression}}\</div>
<div>> + expected-note{{no viable 'begin' function for range of type 'int'}}</div><div>> }</div><div>> }</div><div>> };</div><div>> </div><div>> namespace std {</div><div>
> template<typename T></div><div>> - auto begin(T &&t) -> decltype(t.begin()) { return t.begin(); } // expected-note 4{{ignored: substitution failure}}</div><div>> + auto begin(T &&t) -> decltype(t.begin()) { return t.begin(); } // expected-note 3{{ignored: substitution failure}}</div>
<div>> template<typename T></div><div>> auto end(T &&t) -> decltype(t.end()) { return t.end(); } // expected-note {{candidate template ignored: substitution failure [with T = }}</div><div>> </div>
<div>> template<typename T></div><div>> auto begin(T &&t) -> decltype(t.alt_begin()) { return t.alt_begin(); } // expected-note {{selected 'begin' template [with T = }} \</div><div>
> - expected-note 4{{candidate template ignored: substitution failure [with T = }}</div>
<div>> + expected-note 3{{candidate template ignored: substitution failure [with T = }}</div><div>> template<typename T></div><div>
> auto end(T &&t) -> decltype(t.alt_end()) { return t.alt_end(); } // expected-note {{candidate template ignored: substitution failure [with T = }}</div><div>> </div><div>> @@ -116,9 +117,11 @@ void g() {</div>
<div>> struct NoEndADL {</div><div>> null_t alt_begin();</div><div>> };</div><div>> - for (auto u : NoBeginADL()) { // expected-error {{no matching function for call to 'begin'}} expected-note {{range has type 'NoBeginADL'}}</div>
<div>> + for (auto u : NoBeginADL()) {// expected-error{{invalid range expression of type 'NoBeginADL'}} \</div><div>> + expected-note{{no viable 'begin' function for range of type 'NoBeginADL'}}</div>
<div>> }</div><div>> - for (auto u : NoEndADL()) { // expected-error {{no matching function for call to 'end'}} expected-note {{range has type 'NoEndADL'}}</div><div>> + for (auto u : NoEndADL()) { // expected-error{{invalid range expression of type 'NoEndADL'}} \</div>
<div>> + expected-note{{no viable 'end' function for range of type 'NoEndADL'}}</div><div>> }</div><div>> </div><div>> struct NoBegin {</div><div>> @@ -156,8 +159,8 @@ void g() {</div>
<div>> for (int n : NoCopy()) { // ok</div><div>> }</div><div>> </div><div>> - for (int n : 42) { // expected-error {{no matching function for call to 'begin'}} \</div><div>> - expected-note {{range has type 'int'}}</div>
<div>> + for (int n : 42) { // expected-error{{invalid range expression of type 'int'}} \</div><div>> + expected-note{{no viable 'begin' function for range of type 'int'}}</div><div>> }</div>
<div>> </div><div>> for (auto a : *also_incomplete) { // expected-error {{cannot use incomplete type 'struct Incomplete' as a range}}</div><div>> @@ -179,9 +182,10 @@ template void h<A(&)[13], int>(A(&)[13]); // expected-note {{requested here}}</div>
<div>> </div><div>> template<typename T></div><div>> void i(T t) {</div><div>> - for (auto u : t) { // expected-error {{no matching function for call to 'begin'}} \</div><div>> - expected-error {{member function 'begin' not viable}} \</div>
<div>> - expected-note {{range has type}}</div><div>> + for (auto u : t) { // expected-error {{invalid range expression of type 'A *'; did you mean to dereference it with '*'?}} \</div>
<div>> + expected-error {{member function 'begin' not viable}}\</div><div>> + expected-error{{invalid range expression of type 'const A'}}\</div><div>> + expected-note{{no viable 'begin' function for range of type 'const A'}}</div>
<div>> }</div><div>> }</div><div>> template void i<A[13]>(A*); // expected-note {{requested here}}</div><div>> @@ -204,7 +208,8 @@ void end(VoidBeginADL);</div><div>> void j() {</div><div>> for (auto u : NS::ADL()) {</div>
<div>> }</div><div>> - for (auto u : NS::NoADL()) { // expected-error {{no matching function for call to 'begin'}} expected-note {{range has type}}</div><div>> + for (auto u : NS::NoADL()) { // expected-error{{invalid range expression of type 'NS::NoADL'}}\</div>
<div>> + expected-note {{no viable 'begin' function for range of type 'NS::NoADL'}}</div><div>> }</div><div>> for (auto a : VoidBeginADL()) { // expected-error {{cannot use type 'void' as an iterator}}</div>
<div>> }</div><div>> @@ -215,4 +220,3 @@ void example() {</div><div>> for (int &x : array)</div><div>> x *= 2;</div><div>> }</div><div>> -</div><div>> diff --git a/test/SemaCXX/for-range-no-std.cpp b/test/SemaCXX/for-range-no-std.cpp</div>
<div>> index fa42ca4..8f3213b 100644</div><div>> --- a/test/SemaCXX/for-range-no-std.cpp</div><div>> +++ b/test/SemaCXX/for-range-no-std.cpp</div><div>> @@ -31,10 +31,11 @@ NS::iter end(NS::NoADL);</div><div>
> void f() {</div>
<div>> int a[] = {1, 2, 3};</div><div>> for (auto b : S()) {} // ok</div><div>> - for (auto b : T()) {} // expected-error {{no matching function for call to 'begin'}} expected-note {{range has type}}</div>
<div>> + for (auto b : T()) {} // expected-error {{invalid range expression of type 'T'}} expected-note {{no viable 'begin' function for range of type 'T'}}</div><div>> for (auto b : a) {} // ok</div>
<div>> for (int b : NS::ADL()) {} // ok</div><div>> - for (int b : NS::NoADL()) {} // expected-error {{no matching function for call to 'begin'}} expected-note {{range has type}}</div><div>> + for (int b : NS::NoADL()) {} // expected-error {{invalid range expression of type 'NS::NoADL'}} \</div>
<div>> + expected-note {{no viable 'begin' function for range of type 'NS::NoADL'}}</div><div>> }</div><div>> </div><div>> void PR11601() {</div><div>> diff --git a/test/SemaCXX/typo-correction.cpp b/test/SemaCXX/typo-correction.cpp</div>
<div>> index 893f08a..f8bb8d0 100644</div><div>> --- a/test/SemaCXX/typo-correction.cpp</div><div>> +++ b/test/SemaCXX/typo-correction.cpp</div><div>> @@ -155,7 +155,8 @@ void Test3() {</div><div>> struct R {};</div>
<div>> bool begun(R);</div><div>> void RangeTest() {</div><div>> - for (auto b : R()) {} // expected-error {{use of undeclared identifier 'begin'}} expected-note {{range has type}}</div><div>> + for (auto b : R()) {} // expected-error {{invalid range expression of type 'R'}}\</div>
<div>> + expected-note {{no viable 'begin' function for range of type 'R'}}</div><div>> }</div><div>> </div><div>> // PR 12019 - Avoid infinite mutual recursion in DiagnoseInvalidRedeclaration</div>
<span><font color="#888888">
<div>> -- </div><div>> 1.7.7.3</div></font></span><div><div><div>> </div><br><div class="gmail_quote">On Mon, Jul 23, 2012 at 3:41 PM, Sam Panzer <span dir="ltr"><<a href="mailto:panzer@google.com" target="_blank">panzer@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Ping.<div><div><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Jul 11, 2012 at 4:29 PM, Sam Panzer <span dir="ltr"><<a href="mailto:panzer@google.com" target="_blank">panzer@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
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.<div><div><div class="gmail_extra"><br><br><div class="gmail_quote">
On Mon, Jun 18, 2012 at 9:40 AM, Sam Panzer <span dir="ltr"><<a href="mailto:panzer@google.com" target="_blank">panzer@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="font-family:arial,helvetica,sans-serif"><font>*sigh*<div>I ran format-patch from the correct branch this time.</div>
<div><div><div><div><br><div class="gmail_quote">On Sun, Jun 17, 2012 at 4:49 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>On Thu, Jun 14, 2012 at 10:32 AM, Sam Panzer <<a href="mailto:panzer@google.com" target="_blank">panzer@google.com</a>> wrote:<br>
> I will now be including the word 'attach' in these emails as Gmail's<br>
> equivalent of -Wmissing-patch.<br>
<br>
</div>Sadly that won't save you from running 'diff' in the wrong checkout.<br>
This is the printf / c_str patch.<br>
</blockquote></div><br></div></div></div></div></font></div>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br>
</div></div></blockquote></div></div></div><br></div>
</div>
</blockquote></div><br></div>
</div></div></blockquote></div><br>
</div></div></blockquote></div></div></div><br></div>
</blockquote></div><br>
</div></div></blockquote></div><br></div>