<br><br><div class="gmail_quote">On Fri, Jun 8, 2012 at 8:18 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">
Ah. Yes. It's actually attached now :)<div class="HOEnZb"><div class="h5"><br><br><div class="gmail_quote">On Fri, Jun 8, 2012 at 11:16 AM, Matt Beaumont-Gay <span dir="ltr"><<a href="mailto:matthewbg@google.com" target="_blank">matthewbg@google.com</a>></span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">ENOPATCH<br>
<div><div><br>
On Fri, Jun 8, 2012 at 11:13 AM, Sam Panzer <<a href="mailto:panzer@google.com" target="_blank">panzer@google.com</a>> wrote:<br>
> Hi,<br>
><br>
> Richard Smith suggested that I try to improve the diagnostic emitted when<br>
> Clang encounters certain kinds of invalid C+11 ranged-based for loops.<br>
><br>
>> When a pointer to a container is used as the range in a range-based for,<br>
>> Clang's diagnostic is not awesome:<br>
>><br>
>>> struct S { int *begin(); int *end(); };<br>
>>> void f(S *p) {<br>
>>>  for (auto i : p) {}<br>
>>> }<br>
>>> tmp.cpp:3:15: error: use of undeclared identifier 'begin'<br>
>>>  for (auto i : p) {} }<br>
>>>              ^<br>
>>> tmp.cpp:3:15: note: range has type 'S *'<br>
>><br>
>><br>
>> We should do better than that, and suggest inserting the missing '*'.<br>
><br>
><br>
> This patch replaces the errors complaining about undeclared identifiers with<br>
> an error specific to ranged-for loops, along with an explanatory note. I<br>
> also updated the existing test cases to reflect the change. For example, the<br>
> above code now generates this:<br>
><br>
>> test.cpp:3:15: error: invalid range expression of type 'S *'<br>
>>  for (auto i : p) {}<br>
>>              ^ ~<br>
>> test.cpp:3:16: note: range expression is of type 'S *'; did you<br>
>>       mean to dereference it with '*'?<br>
>>  for (auto i : p) {}<br>
>>                ^<br>
>>                *<br>
>> test.cpp:3:14: note: no viable 'end' function for range of type<br>
>>       'S *'<br>
>>  for (auto i : p) {}<br>
><br>
><br>
> I hope that this is helpful, and comments are always welcome.<br>
><br>
> -Sam<br></div></div></blockquote></div></div></div></blockquote><div><br>Just a couple remarks.<br><br>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 ?<br>
<br>Second, I am not sure about the "missing begin" and "missing end" notes. I would advise having them only conditionally:<br>- if dereferencing would make it okay, then let's remove them, they are probably bogus<br>
- if dereferencing does not help (nothing for S& either), then we probably need to keep them<br><br>-- Matthieu.<br></div></div>