<div>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.</div>
<div><br></div><div>I will also adjust the "missing begin/end" notes as Matthieu suggested.</div><div><br></div><div>-Sam<br><br><div class="gmail_quote">On Fri, Jun 8, 2012 at 12:36 PM, Matthieu Monrocq <span dir="ltr"><<a href="mailto:matthieu.monrocq@gmail.com" target="_blank">matthieu.monrocq@gmail.com</a>></span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br><br><div class="gmail_quote"><div><div>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><div><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></div><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<span><font color="#888888"><br><br>-- Matthieu.<br></font></span></div></div>
</blockquote></div><br></div>