<div style="font-family: arial, helvetica, sans-serif"><font size="2">I will now be including the word 'attach' in these emails as Gmail's equivalent of -Wmissing-patch.<br><br><div class="gmail_quote">On Wed, Jun 13, 2012 at 8:12 PM, Sean Silva <span dir="ltr"><<a href="mailto:silvas@purdue.edu" target="_blank">silvas@purdue.edu</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I think you forgot to attach the patch.<br><br><div class="gmail_quote"><div><div class="h5">On Wed, Jun 13, 2012 at 2:14 PM, Sam Panzer <span dir="ltr"><<a href="mailto:panzer@google.com" target="_blank">panzer@google.com</a>></span> wrote:<br>
</div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5"><div style="font-family:arial,helvetica,sans-serif"><font>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.<div>
<br></div><div>Existing test cases have been updated to match again.<div><div><br>
<br><div class="gmail_quote">On Fri, Jun 8, 2012 at 3:06 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">
<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><span><font color="#888888"><div><br></div></font></span><div><span><font color="#888888">-Sam</font></span><div>
<div><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></div></div>
</blockquote></div><br>
</div></div></div></font></div>
<br></div></div><div class="im">_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br></div></blockquote></div><br>
</blockquote></div><br></font></div>