I will now be including the word 'attach' in these emails as Gmail's equivalent of -Wmissing-patch.
On Wed, Jun 13, 2012 at 8:12 PM, Sean Silva <[email protected]> wrote: > I think you forgot to attach the patch. > > On Wed, Jun 13, 2012 at 2:14 PM, Sam Panzer <[email protected]> wrote: > >> 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. >> >> Existing test cases have been updated to match again. >> >> >> On Fri, Jun 8, 2012 at 3:06 PM, Sam Panzer <[email protected]> wrote: >> >>> 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 < >>> [email protected]> wrote: >>> >>>> >>>> >>>> On Fri, Jun 8, 2012 at 8:18 PM, Sam Panzer <[email protected]> wrote: >>>> >>>>> Ah. Yes. It's actually attached now :) >>>>> >>>>> >>>>> On Fri, Jun 8, 2012 at 11:16 AM, Matt Beaumont-Gay < >>>>> [email protected]> wrote: >>>>> >>>>>> ENOPATCH >>>>>> >>>>>> On Fri, Jun 8, 2012 at 11:13 AM, Sam Panzer <[email protected]> >>>>>> 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. >>>> >>> >>> >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> >> >
range-diagnostic.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
