[cfe-commits] [patch] Better diagnostics in for-range expressions.

[Sam Panzer]

New patch is ~10% smaller!


On Thu, Aug 16, 2012 at 3:14 PM, Richard Smith <richard at metafoo.co.uk>wrote:

> On Thu, Aug 16, 2012 at 2:56 PM, Sam Panzer <panzer at google.com> wrote:
>
>> 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:
>>
> [...]
>
>> > +  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.
>>
>
> 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.
>
>

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.



> > +    }
>>> > +  } 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?
>>
>
> SGTM
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120820/487c1039/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: for-range-update-2.patch
Type: application/octet-stream
Size: 44331 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120820/487c1039/attachment.obj>