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

[Matthieu Monrocq]

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/f4955608/attachment.html>