On Jun 7, 2012, at 6:09 PM, Kaelyn Uhrain wrote:
> 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).
Okay, this seems to work because we can't get NotFoundInCurrentInstantiation
with unqualified lookup.
> > + 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.
Come to think of it, do you even need to do a lookup here? A DeclRefExpr won't
refer to a type, so it's actually not an interesting case.
DependentScopeDeclRefExpr and CXXDependentScopeMemberExpr (the latter only when
it has a qualifier) are the interesting cases, and we wouldn't have built those
nodes unless name lookup for the entity ended up with
NotFoundInCurrentInstantiation.
>
> > + 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.
Okay, thanks for the fixes!
- Doug
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
