On Mon, Aug 26, 2013 at 11:40 PM, Nick Lewycky <[email protected]> wrote:
> On 26 August 2013 23:22, Chandler Carruth <[email protected]> wrote: > >> >> On Mon, Aug 26, 2013 at 11:09 PM, Nick Lewycky <[email protected]>wrote: >> >>> On 26 August 2013 22:53, Chandler Carruth <[email protected]> wrote: >>> >>>> I think this case has more problems than just verbosity... >>>> >>>> On Mon, Aug 26, 2013 at 10:38 PM, Nick Lewycky <[email protected]>wrote: >>>> >>>>> a.cc:1:56: error: no template named 'Foox'; did you mean 'Foo'? >>>>> template <typename T> class Foo {}; class Bar : public Foox {}; >>>>> [point at 'Foox' suggest 'Foo'] >>>>> >>>> >>>> Why does it assume Foox is a template? >>>> >>> >>> It's already proven that it's not not-a-template. >>> >>> a.cc:1:29: note: 'Foo' declared here >>>>> template <typename T> class Foo {}; class Bar : public Foox {}; >>>>> [point at 'Foo'] >>>>> a.cc:1:56: error: expected template argument list after template-id >>>>> template <typename T> class Foo {}; class Bar : public Foox {}; >>>>> [point at 'Foox'] >>>>> >>>> >>>> And given that we then hit this error, why do we even consider the Foo >>>> typo correction? Do we prefer that over a "Fooxie" class due to shorter >>>> edit distance? That doesn't seem right. I would intuitively expect the lack >>>> of "<..." to be a stronger signal than any edit distance, and thus >>>> disqualify template-ids from the typo correction candidate set. >>>> >>> >>> No. We only go down this patch after we've done a lookup and typo >>> correction on non-templates, and found nothing. >>> >> >> I'm suggesting that a missing header or exceeding the maximum edit >> distance threshold seems just as plausible as using a template without >> template arguments. I'm not claiming that I have some strong reason to >> believe one interpretation or the other to be more likely, only that it >> doesn't seem clear-cut in either direction to me. >> > > ... if it can't find a template, then it doesn't mention templates: > > $ echo 'class Bar : public Foo {};' | llvm/Debug+Asserts/bin/clang -x c++ - > <stdin>:1:20: error: expected class name > class Bar : public Foo {}; > ^ > 1 error generated. > > $ echo 'template<typename T> class Fooa; class Foob {}; class Bar : public > Foo {};' | llvm/Debug+Asserts/bin/clang -x c++ - > <stdin>:1:68: error: unknown class name 'Foo'; did you mean 'Foob'? > template<typename T> class Fooa; class Foob {}; class Bar : public Foo {}; > ^~~ > Foob > <stdin>:1:40: note: 'Foob' declared here > template<typename T> class Fooa; class Foob {}; class Bar : public Foo {}; > ^ > 1 error generated. > > Today, clang emits the exact same diagnostic "expected class name" even > when you do have Foo declared as a template. That's the only thing I'm > trying to fix, but it has this weird side-effect in that even asking Sema > isTemplateName() causes does typo-correction, and when that typo-correction > succeeds it issues this "no template" diagnostic. > I really do understand that, and I don't think your patch is wrong. I just think the problem in typo correction it exposes is severe enough to warrant fixing. Specifically, I don't think we should be typo-correcting to a template-id when we have syntactic indicators that a template-id isn't valid. For example, given "foo->baz", we should only correct to a method name "bar" if we can also correct to "bar()" because it has a zero-argument overload (modulo weird cases where the expression "foo->bar" is valid). There are likely other places (maybe even with methods!) where we get this wrong, but I think all of them are pretty dubious. Here, I think the lack of "<" is a *really* strong signal as there is just no way to use a template-id in that context. If all template parameters had default arguments, perhaps 'Foo<>' would be a good correction, but that's a more clever step.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
