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

[Sam Panzer]

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.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120608/a97e9f6c/attachment.html>