New patch is ~10% smaller!<br><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Aug 16, 2012 at 3:14 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 class="gmail_quote"><div class="im">On Thu, Aug 16, 2012 at 2:56 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 update for this patch.<br><div class="gmail_extra"><br><br><div class="gmail_quote"><div>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>
</div></div></div></blockquote></div><div>[...] </div><div class="im"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_extra"><div class="gmail_quote">
<div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">> + if (CXXRecordDecl *D = RangeType->getAsCXXRecordDecl()) {</blockquote></div><div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<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><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></div></blockquote><div><br></div></div><div>I think previously I suggested that you should emit note_for_range_type directly in BuildForRangeBeginEndCall; that still seems like the best way to address this. I would also suggest that you don't emit the note in the case where FinishForRangeVarDecl fails, since in that case we've already emitted a diagnostic explaining what we're doing.</div>
<div><div class="h5">
<div> </div></div></div></div></blockquote><div><br></div><div>This wasn't possible before introducing SFINAETrap, though it simplifies the code now as you suggest. After a little adjusting, I moved NoteForRangeBeginEndFunction and FinishForRangeVarDecl back out of Sema.</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 class="gmail_quote"><div><div class="h5"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="gmail_extra"><div class="gmail_quote"><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><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></div><div>How about RebuildForRangeWithDereference?</div></div></div></blockquote><div><br></div></div></div><div>SGTM</div></div>
</blockquote></div><br></div>