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 > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
