[cfe-commits] r157085 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaTemplate.cpp test/SemaTemplate/typename-specifier.cpp

[Douglas Gregor]

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?

> +    } 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...
         ^


> +    }
> +
> +    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;
         ^


> +      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.

> +      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.

	- 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits