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

[Sean Silva]

I think you forgot to attach the patch.

On Wed, Jun 13, 2012 at 2:14 PM, Sam Panzer <panzer at google.com> wrote:

> Here is the long-awaited patch that addresses Matthieu's suggestions. This
> version runs the same for-range building code on the dereferenced pointer
> as it did on the failing original, picking (and only showing the error
> messages from) the most successful of the two. It does not handle pointers
> to arrays (or containers/smart pointers that overload operator*), though
> this could be added later.
>
> Existing test cases have been updated to match again.
>
>
> On Fri, Jun 8, 2012 at 3:06 PM, Sam Panzer <panzer at google.com> wrote:
>
>> Yep, the FixItHint is issued only when there are methods for both begin
>> and end. I do not try to match a dereferenced pointer to non-method (ADL)
>> functions, though I'm currently working on another change that does. The
>> patch submitted previously would just issue the FixItHint if begin and end
>> methods exist, without doing any further checking, e.g. on their types.
>>
>> I will also adjust the "missing begin/end" notes as Matthieu suggested.
>>
>> -Sam
>>
>>
>> On Fri, Jun 8, 2012 at 12:36 PM, Matthieu Monrocq <
>> matthieu.monrocq at gmail.com> wrote:
>>
>>>
>>>
>>> On Fri, Jun 8, 2012 at 8:18 PM, Sam Panzer <panzer at google.com> wrote:
>>>
>>>> Ah. Yes. It's actually attached now :)
>>>>
>>>>
>>>> On Fri, Jun 8, 2012 at 11:16 AM, Matt Beaumont-Gay <
>>>> matthewbg at google.com> wrote:
>>>>
>>>>> ENOPATCH
>>>>>
>>>>> On Fri, Jun 8, 2012 at 11:13 AM, Sam Panzer <panzer at google.com> wrote:
>>>>> > Hi,
>>>>> >
>>>>> > Richard Smith suggested that I try to improve the diagnostic emitted
>>>>> when
>>>>> > Clang encounters certain kinds of invalid C+11 ranged-based for
>>>>> loops.
>>>>> >
>>>>> >> When a pointer to a container is used as the range in a range-based
>>>>> for,
>>>>> >> Clang's diagnostic is not awesome:
>>>>> >>
>>>>> >>> struct S { int *begin(); int *end(); };
>>>>> >>> void f(S *p) {
>>>>> >>>  for (auto i : p) {}
>>>>> >>> }
>>>>> >>> tmp.cpp:3:15: error: use of undeclared identifier 'begin'
>>>>> >>>  for (auto i : p) {} }
>>>>> >>>              ^
>>>>> >>> tmp.cpp:3:15: note: range has type 'S *'
>>>>> >>
>>>>> >>
>>>>> >> We should do better than that, and suggest inserting the missing
>>>>> '*'.
>>>>> >
>>>>> >
>>>>> > This patch replaces the errors complaining about undeclared
>>>>> identifiers with
>>>>> > an error specific to ranged-for loops, along with an explanatory
>>>>> note. I
>>>>> > also updated the existing test cases to reflect the change. For
>>>>> example, the
>>>>> > above code now generates this:
>>>>> >
>>>>> >> test.cpp:3:15: error: invalid range expression of type 'S *'
>>>>> >>  for (auto i : p) {}
>>>>> >>              ^ ~
>>>>> >> test.cpp:3:16: note: range expression is of type 'S *'; did you
>>>>> >>       mean to dereference it with '*'?
>>>>> >>  for (auto i : p) {}
>>>>> >>                ^
>>>>> >>                *
>>>>> >> test.cpp:3:14: note: no viable 'end' function for range of type
>>>>> >>       'S *'
>>>>> >>  for (auto i : p) {}
>>>>> >
>>>>> >
>>>>> > I hope that this is helpful, and comments are always welcome.
>>>>> >
>>>>> > -Sam
>>>>>
>>>>
>>> Just a couple remarks.
>>>
>>> First, this sounds like a prime example for a Fix-It, at the very
>>> condition that there *is* a  begin  and  end  functions/methods available
>>> on the dereferenced  p. Do you check by the way ?
>>>
>>> Second, I am not sure about the "missing begin" and "missing end" notes.
>>> I would advise having them only conditionally:
>>> - if dereferencing would make it okay, then let's remove them, they are
>>> probably bogus
>>> - if dereferencing does not help (nothing for S& either), then we
>>> probably need to keep them
>>>
>>> -- Matthieu.
>>>
>>
>>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120613/1d8d7b4e/attachment.html>