On Thu, Jun 7, 2012 at 3:47 PM, Douglas Gregor <[email protected]> wrote:
> > On May 18, 2012, at 4:42 PM, Kaelyn Uhrain wrote: > > > Author: rikka > > Date: Fri May 18 18:42:49 2012 > > New Revision: 157085 > > > > URL: http://llvm.org/viewvc/llvm-project?rev=157085&view=rev > > Log: > > Suggest adding 'typename' when it would make the compiler > > accept the template argument expression as a type. > > > > Modified: > > cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td > > cfe/trunk/lib/Sema/SemaTemplate.cpp > > cfe/trunk/test/SemaTemplate/typename-specifier.cpp > > > > Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td > > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=157085&r1=157084&r2=157085&view=diff > > > ============================================================================== > > --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) > > +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri May 18 > 18:42:49 2012 > > @@ -2361,6 +2361,8 @@ > > def note_member_of_template_here : Note<"member is declared here">; > > def err_template_arg_must_be_type : Error< > > "template argument for template type parameter must be a type">; > > +def err_template_arg_must_be_type_suggest : Error< > > + "template argument for template type parameter must be a type; did > you forget 'typename'?">; > > def err_template_arg_must_be_expr : Error< > > "template argument for non-type template parameter must be an > expression">; > > def err_template_arg_nontype_ambig : Error< > > > > Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp > > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=157085&r1=157084&r2=157085&view=diff > > > ============================================================================== > > --- cfe/trunk/lib/Sema/SemaTemplate.cpp (original) > > +++ cfe/trunk/lib/Sema/SemaTemplate.cpp Fri May 18 18:42:49 2012 > > @@ -2438,6 +2438,45 @@ > > > > return true; > > } > > + case TemplateArgument::Expression: { > > + // We have a template type parameter but the template argument is an > > + // expression; see if maybe it is missing the "typename" keyword. > > + CXXScopeSpec SS; > > + DeclarationNameInfo NameInfo; > > + > > + if (DeclRefExpr *ArgExpr = dyn_cast<DeclRefExpr>(Arg.getAsExpr())) { > > + SS.Adopt(ArgExpr->getQualifierLoc()); > > + NameInfo = ArgExpr->getNameInfo(); > > Shouldn't we check that there is a nested-name-specifier here? > As far as I'm aware, there is no need. The code below is not dependent on the CXXScopeSpec being valid, and if there isn't a nested-name-specifier then the CXXScopeSpec will be invalid after adopting an empty NestedNameSpecifierLoc. Plus, from what I can tell, any checks for whether the expression needs to have a nested-name-specifier are handled elsewhere (IIRC they're triggered before this code is reached). > + } else if (DependentScopeDeclRefExpr *ArgExpr = > > + dyn_cast<DependentScopeDeclRefExpr>(Arg.getAsExpr())) { > > + SS.Adopt(ArgExpr->getQualifierLoc()); > > + NameInfo = ArgExpr->getNameInfo(); > > + } else if (CXXDependentScopeMemberExpr *ArgExpr = > > + dyn_cast<CXXDependentScopeMemberExpr>(Arg.getAsExpr())) { > > + SS.Adopt(ArgExpr->getQualifierLoc()); > > + NameInfo = ArgExpr->getMemberNameInfo(); > > This should check that the base of the dependent-scoped member access > expression is actually the implicit 'this'. We currently end up inserting > 'typename' where we shouldn't: > > test/SemaTemplate/typename-specifier.cpp:139:10: error: template argument > for > template type parameter must be a type; did you forget 'typename'? > pair<this->ExampleItemSet::iterator, int> i; // expected-error... > ^ > Good point; fixed. The "typename" stuff delves into an area of C++ templates I've never used, and adding the suggestion turned out to be more challenging than I'd expected--it certainly required a lot more knowledge of template syntax and of how Clang deals with them than I would have guessed from the relatively straight-forward and self-contained error and recovery handling I'd encountered working on typo correction (emphasis on "relatively"). > + } > > + > > + if (NameInfo.getName()) { > > This should be limited to identifiers, because we don't want to correct > here: > > test/SemaTemplate/typename-specifier.cpp:139:10: error: template argument > for > template type parameter must be a type; did you forget 'typename'? > pair<ExampleItemSet::operator[], int> i; > ^ > Also fixed. > > + LookupResult Result(*this, NameInfo, LookupOrdinaryName); > > + LookupParsedName(Result, CurScope, &SS); > > + > > + bool CouldBeType = Result.getResultKind() == > > + LookupResult::NotFoundInCurrentInstantiation; > > + > > + for (LookupResult::iterator I = Result.begin(), IEnd = > Result.end(); > > + !CouldBeType && I != IEnd; ++I) { > > + CouldBeType = isa<TypeDecl>(*I); > > + } > > I don't understand why we have this loop here. If the name lookup is able > to find a type, we wouldn't be here in this code. It seems like the only > interesting case is the NotFoundInCurrentInstantiation case. > D'oh. It was left over from when I was trying to figure out how to look up whether an identifier might be a type and even how to get the nested-name-specifier in a form that I could use for the lookups. IIRC at least one of the avenues I'd tried needed the loop to handle lookups that returned multiple decls. > > > + if (CouldBeType) { > > + SourceLocation Loc = AL.getSourceRange().getBegin(); > > + Diag(Loc, diag::err_template_arg_must_be_type_suggest); > > + Diag(Param->getLocation(), diag::note_template_param_here); > > + return true; > > + } > > Did you mean for this to be a Fix-It? If so, it needs to recover by > changing the template argument itself. If you're not planning on doing that > work, it's fine, but please leave a FIXME to say that we'd like to do that > work. > I've added a FIXME since a Fix-It was beyond what I had able to figure out while muddling through all of the template code... The changes mentioned above along with a couple more tests have been committed as r158185. Thanks! Kaelyn > - Doug > > > + } > > + // fallthrough > > + } > > default: { > > // We have a template type parameter but the template argument > > // is not a type. > > > > Modified: cfe/trunk/test/SemaTemplate/typename-specifier.cpp > > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/typename-specifier.cpp?rev=157085&r1=157084&r2=157085&view=diff > > > ============================================================================== > > --- cfe/trunk/test/SemaTemplate/typename-specifier.cpp (original) > > +++ cfe/trunk/test/SemaTemplate/typename-specifier.cpp Fri May 18 > 18:42:49 2012 > > @@ -115,3 +115,37 @@ > > using typename BasicGeometry<mydim, int>::operator[]; // > expected-error {{typename is allowed for identifiers only}} > > }; > > } > > + > > + > > +namespace missing_typename { > > +template <class T1, class T2> struct pair {}; // expected-note 5 > {{template parameter is declared here}} > > + > > +template <class T1, class T2> > > +struct map { > > + typedef T1* iterator; > > +}; > > + > > +template <class T> > > +class ExampleClass1 { > > + struct ExampleItem; > > + > > + > > + struct ExampleItemSet { > > + typedef ExampleItem* iterator; > > + }; > > + > > + void foo() { > > + pair<ExampleItemSet::iterator, int> i; // expected-error {{template > argument for template type parameter must be a type; did you forget > 'typename'?}} > > + } > > + pair<ExampleItemSet::iterator, int> elt; // expected-error {{template > argument for template type parameter must be a type; did you forget > 'typename'?}} > > + > > + > > + typedef map<int, ExampleItem*> ExampleItemMap; > > + > > + static void bar() { > > + pair<ExampleItemMap::iterator, int> i; // expected-error {{template > argument for template type parameter must be a type; did you forget > 'typename'?}} > > + } > > + pair<ExampleItemMap::iterator, int> entry; // expected-error > {{template argument for template type parameter must be a type; did you > forget 'typename'?}} > > + pair<bar, int> foobar; // expected-error {{template argument for > template type parameter must be a type}} > > +}; > > +} // namespace missing_typename > > > > > > _______________________________________________ > > 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
