On 27 August 2013 00:04, Chandler Carruth <[email protected]> wrote:
> > 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. > Ah, okay! Then I'm glad I included it in the email. I was worried that you saw my example of what I thought was a bad diagnostic, and interpreted it as the goal of my patch. 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. > Good catch. Foo could be a template function (or a template variable in C++14 I suppose) instead of a template class. That I know how to fix. Unfortunately, the template typo correction is wired into LookupTemplateName. Even if I bypass isTemplateName and create a LookupResult with diagnostics suppressed, we still get the typo corrections. I do not know the design of clang well enough to know how to change that. Nick
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
