Improve error message when referencing a non-tag type with a tag

[rnk]

Summary

Other compilers accept invalid code here that we reject, and we need a
better error message to try to convince users that the code is really
incorrect. Consider:

class Foo {
  typedef MyIterHelper<Foo> iterator;
  friend class iterator;
};

Previously our wording was "elaborated type refers to a typedef".
"elaborated type" isn't widely known terminology, so the new diagnostic
tries to talk about tag types.

[rnk]

Ping, found this in my tree. Maybe you weren't that excited about the wording change.

[rnk]

• rebase

[inglorion]

I like it. I'll give others a little time to respond, but if no objections are raised and nobody else accepts first, I'll accept it tomorrow.

[rsmith]

include/clang/Basic/DiagnosticSemaKinds.td

4556–4558 ↗

(On Diff #79805)

"tag" is C terminology that C++ doesn't share; "specifier" would be correct in both languages.

Also, this wording suggests that *some* templates / template template arguments / ... can be referenced by, say, a struct specifier. How about:

%select{non-struct type|non-class type|non-union type|non-enum type|typedef|type alias|template|[...]}1 %0 cannot be referenced with %select{struct|interface|union|class|enum}2 specifier

(where we use "non-enum type" when the tag is enum, "non-union type" when the tag is union, and otherwise use "non-struct type" in C and "non-class type" in C++)?

[rnk]

include/clang/Basic/DiagnosticSemaKinds.td

4556–4558 ↗

(On Diff #79805)

OK, sounds good.

[rnk]

• new wording

[rsmith]

Thanks!

cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td

def note_previous_namespace_alias : Note<
   "previously defined as an alias for %0">;
 def warn_forward_class_redefinition : Warning<
   "redefinition of forward class %0 of a typedef name of an object type is ignored">,
   InGroup<DiagGroup<"objc-forward-class-redefinition">>;
 def err_redefinition_different_typedef : Error<
   "%select{typedef|type alias|type alias template}0 "
   "redefinition with different types%diff{ ($ vs $)|}1,2">;
 def err_tag_reference_non_tag : Error<
-  "elaborated type refers to %select{a non-tag type|a typedef|a type alias|a template|a type alias template|a template template argument}0">;
+  "%select{non-struct type|non-class type|non-union type|non-enum "
+  "type|typedef|type alias|template|type alias template|template "
+  "template argument}1 %0 cannot be referenced with a "
+  "%select{struct|interface|union|class|enum}2 specifier">;
 def err_tag_reference_conflict : Error<
-  "implicit declaration introduced by elaborated type conflicts with "
-  "%select{a declaration|a typedef|a type alias|a template}0 of the same name">;
+  "implicit declaration introduced by elaborated type conflicts with a "
+  "%select{non-struct type|non-class type|non-union type|non-enum "
+  "type|typedef|type alias|template|type alias template|template "
+  "template argument}0 of the same name">;
 def err_dependent_tag_decl : Error<
   "%select{declaration|definition}0 of "
   "%select{struct|interface|union|class|enum}1 in a dependent scope">;
 def err_tag_definition_of_typedef : Error<
   "definition of type %0 conflicts with %select{typedef|type alias}1 of the same name">;
 def err_conflicting_types : Error<"conflicting types for %0">;
 def err_different_pass_object_size_params : Error<
   "conflicting pass_object_size attributes on parameters">;

cfe/trunk/include/clang/Sema/Sema.h

Decl *BuildAnonymousStructOrUnion(Scope *S, DeclSpec &DS,
                                     const PrintingPolicy &Policy);
 
   Decl *BuildMicrosoftCAnonymousStruct(Scope *S, DeclSpec &DS,
                                        RecordDecl *Record);
 
   /// Common ways to introduce type names without a tag for use in diagnostics.
   /// Keep in sync with err_tag_reference_non_tag.
   enum NonTagKind {
-    NTK_Unknown,
+    NTK_NonStruct,
+    NTK_NonClass,
+    NTK_NonUnion,
+    NTK_NonEnum,
     NTK_Typedef,
     NTK_TypeAlias,
     NTK_Template,
     NTK_TypeAliasTemplate,
     NTK_TemplateTemplateArgument,
   };
 
   /// Given a non-tag type declaration, returns an enum useful for indicating
   /// what kind of non-tag type this is.
-  NonTagKind getNonTagTypeDeclKind(const Decl *D);
+  NonTagKind getNonTagTypeDeclKind(const Decl *D, TagTypeKind TTK);
 
   bool isAcceptableTagRedeclaration(const TagDecl *Previous,
                                     TagTypeKind NewTag, bool isDefinition,
                                     SourceLocation NewTagLoc,
                                     const IdentifierInfo *Name);
 
   enum TagUseKind {
     TUK_Reference,   // Reference to a tag:  'struct foo *X;'

cfe/trunk/lib/Sema/SemaDecl.cpp

 /// class for redeclaration (class, struct, or __interface).
 ///
 /// \returns true iff the tag kind is compatible.
 static bool isClassCompatTagKind(TagTypeKind Tag)
 {
   return Tag == TTK_Struct || Tag == TTK_Class || Tag == TTK_Interface;
 }
 
-Sema::NonTagKind Sema::getNonTagTypeDeclKind(const Decl *PrevDecl) {
+Sema::NonTagKind Sema::getNonTagTypeDeclKind(const Decl *PrevDecl,
+                                             TagTypeKind TTK) {
   if (isa<TypedefDecl>(PrevDecl))
     return NTK_Typedef;
   else if (isa<TypeAliasDecl>(PrevDecl))
     return NTK_TypeAlias;
   else if (isa<ClassTemplateDecl>(PrevDecl))
     return NTK_Template;
   else if (isa<TypeAliasTemplateDecl>(PrevDecl))
     return NTK_TypeAliasTemplate;
   else if (isa<TemplateTemplateParmDecl>(PrevDecl))
     return NTK_TemplateTemplateArgument;
-  return NTK_Unknown;
+  switch (TTK) {
+  case TTK_Struct:
+  case TTK_Interface:
+  case TTK_Class:
+    return getLangOpts().CPlusPlus ? NTK_NonClass : NTK_NonStruct;
+  case TTK_Union:
+    return NTK_NonUnion;
+  case TTK_Enum:
+    return NTK_NonEnum;
+  }
+  llvm_unreachable("invalid TTK");
 }
 
 /// \brief Determine whether a tag with a given kind is acceptable
 /// as a redeclaration of the given tag declaration.
 ///
 /// \returns true if the new tag kind is acceptable, false otherwise.
 bool Sema::isAcceptableTagRedeclaration(const TagDecl *Previous,
                                         TagTypeKind NewTag, bool isDefinition,
if (!Previous.empty()) {
     // lookup.  This is only actually possible in C++, where a few
     // things like templates still live in the tag namespace.
     } else {
       // Use a better diagnostic if an elaborated-type-specifier
       // found the wrong kind of type on the first
       // (non-redeclaration) lookup.
       if ((TUK == TUK_Reference || TUK == TUK_Friend) &&
           !Previous.isForRedeclaration()) {
-        NonTagKind NTK = getNonTagTypeDeclKind(PrevDecl);
-        Diag(NameLoc, diag::err_tag_reference_non_tag) << NTK;
+        NonTagKind NTK = getNonTagTypeDeclKind(PrevDecl, Kind);
+        Diag(NameLoc, diag::err_tag_reference_non_tag) << PrevDecl << NTK
+                                                       << Kind;
         Diag(PrevDecl->getLocation(), diag::note_declared_at);
         Invalid = true;
 
       // Otherwise, only diagnose if the declaration is in scope.
       } else if (!isDeclInScope(DirectPrevDecl, SearchDC, S,
                                 SS.isNotEmpty() || isExplicitSpecialization)) {
         // do nothing
 
       // Diagnose implicit declarations introduced by elaborated types.
       } else if (TUK == TUK_Reference || TUK == TUK_Friend) {
-        NonTagKind NTK = getNonTagTypeDeclKind(PrevDecl);
+        NonTagKind NTK = getNonTagTypeDeclKind(PrevDecl, Kind);
         Diag(NameLoc, diag::err_tag_reference_conflict) << NTK;
         Diag(PrevDecl->getLocation(), diag::note_previous_decl) << PrevDecl;
         Invalid = true;
 
       // Otherwise it's a declaration.  Call out a particularly common
       // case here.
       } else if (TypedefNameDecl *TND = dyn_cast<TypedefNameDecl>(PrevDecl)) {
         unsigned Kind = 0;

cfe/trunk/lib/Sema/SemaTemplate.cpp

TypeResult Sema::ActOnTagTemplateIdType(TagUseKind TUK,
   }
 
   if (TypeAliasTemplateDecl *TAT =
         dyn_cast_or_null<TypeAliasTemplateDecl>(Template.getAsTemplateDecl())) {
     // C++0x [dcl.type.elab]p2:
     //   If the identifier resolves to a typedef-name or the simple-template-id
     //   resolves to an alias template specialization, the
     //   elaborated-type-specifier is ill-formed.
-    Diag(TemplateLoc, diag::err_tag_reference_non_tag) << NTK_TypeAliasTemplate;
+    Diag(TemplateLoc, diag::err_tag_reference_non_tag)
+        << TAT << NTK_TypeAliasTemplate << TagKind;
     Diag(TAT->getLocation(), diag::note_declared_at);
   }
   
   QualType Result = CheckTemplateIdType(Template, TemplateLoc, TemplateArgs);
   if (Result.isNull())
     return TypeResult(true);
   
   // Check the tag kind
Sema::ActOnExplicitInstantiation(Scope *S,
   // original template.
   TagTypeKind Kind = TypeWithKeyword::getTagTypeKindForTypeSpec(TagSpec);
   assert(Kind != TTK_Enum &&
          "Invalid enum tag in class template explicit instantiation!");
 
   ClassTemplateDecl *ClassTemplate = dyn_cast<ClassTemplateDecl>(TD);
 
   if (!ClassTemplate) {
-    NonTagKind NTK = getNonTagTypeDeclKind(TD);
-    Diag(TemplateNameLoc, diag::err_tag_reference_non_tag) << NTK;
+    NonTagKind NTK = getNonTagTypeDeclKind(TD, Kind);
+    Diag(TemplateNameLoc, diag::err_tag_reference_non_tag) << TD << NTK << Kind;
     Diag(TD->getLocation(), diag::note_previous_use);
     return true;
   }
 
   if (!isAcceptableTagRedeclaration(ClassTemplate->getTemplatedDecl(),
                                     Kind, /*isDefinition*/false, KWLoc,
                                     ClassTemplate->getIdentifier())) {
     Diag(KWLoc, diag::err_use_with_wrong_tag)

cfe/trunk/lib/Sema/TreeTransform.h

if (!Tag) {
       // better diagnostics.
       LookupResult Result(SemaRef, Id, IdLoc, Sema::LookupTagName);
       SemaRef.LookupQualifiedName(Result, DC);
       switch (Result.getResultKind()) {
         case LookupResult::Found:
         case LookupResult::FoundOverloaded:
         case LookupResult::FoundUnresolvedValue: {
           NamedDecl *SomeDecl = Result.getRepresentativeDecl();
-          Sema::NonTagKind NTK = SemaRef.getNonTagTypeDeclKind(SomeDecl);
-          SemaRef.Diag(IdLoc, diag::err_tag_reference_non_tag) << NTK;
+          Sema::NonTagKind NTK = SemaRef.getNonTagTypeDeclKind(SomeDecl, Kind);
+          SemaRef.Diag(IdLoc, diag::err_tag_reference_non_tag) << SomeDecl
+                                                               << NTK << Kind;
           SemaRef.Diag(SomeDecl->getLocation(), diag::note_declared_at);
           break;
         }
         default:
           SemaRef.Diag(IdLoc, diag::err_not_tag_in_scope)
               << Kind << Id << DC << QualifierLoc.getSourceRange();
           break;
       }
TreeTransform<Derived>::TransformElaboratedType(TypeLocBuilder &TLB,
   if (T->getKeyword() != ETK_None && T->getKeyword() != ETK_Typename) {
     if (const TemplateSpecializationType *TST =
           NamedT->getAs<TemplateSpecializationType>()) {
       TemplateName Template = TST->getTemplateName();
       if (TypeAliasTemplateDecl *TAT = dyn_cast_or_null<TypeAliasTemplateDecl>(
               Template.getAsTemplateDecl())) {
         SemaRef.Diag(TL.getNamedTypeLoc().getBeginLoc(),
                      diag::err_tag_reference_non_tag)
-            << Sema::NTK_TypeAliasTemplate;
+            << TAT << Sema::NTK_TypeAliasTemplate
+            << ElaboratedType::getTagTypeKindForKeyword(T->getKeyword());
         SemaRef.Diag(TAT->getLocation(), diag::note_declared_at);
       }
     }
   }
 
   QualType Result = TL.getType();
   if (getDerived().AlwaysRebuild() ||
       QualifierLoc != TL.getQualifierLoc() ||

cfe/trunk/test/CXX/basic/basic.lookup/basic.lookup.elab/p2.cpp

 // RUN: %clang_cc1 -fsyntax-only -verify %s
 
 namespace test0 {
   struct A {
     static int foo;
   };
   
   namespace i0 {
     typedef int A; // expected-note {{declared here}}
 
     int test() {
-      struct A a; // expected-error {{elaborated type refers to a typedef}}
+      struct A a; // expected-error {{typedef 'A' cannot be referenced with a struct specifier}}
       return a.foo;
     }
   }
 
   namespace i1 {
     template <class> class A; // expected-note {{declared here}}
 
     int test() {
-      struct A a; // expected-error {{elaborated type refers to a template}}
+      struct A a; // expected-error {{template 'A' cannot be referenced with a struct specifier}}
       return a.foo;
     }
   }
 
   namespace i2 {
     int A;
 
     int test() {

cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.type.elab/p2-0x.cpp

 // RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
 
 struct A { typedef int type; };
 template<typename T> using X = A; // expected-note {{declared here}}
-struct X<int>* p2; // expected-error {{elaborated type refers to a type alias template}}
+struct X<int>* p2; // expected-error {{type alias template 'X' cannot be referenced with a struct specifier}}
 
 
 template<typename T> using Id = T; // expected-note {{declared here}}
 template<template<typename> class F>
 struct Y {
-  struct F<int> i; // expected-error {{elaborated type refers to a type alias template}}
+  struct F<int> i; // expected-error {{type alias template 'Id' cannot be referenced with a struct specifier}}
   typename F<A>::type j; // ok
 
   // FIXME: don't produce the diagnostic both for the definition and the instantiation.
   template<typename T> using U = F<char>; // expected-note 2{{declared here}}
-  struct Y<F>::template U<char> k; // expected-error 2{{elaborated type refers to a type alias template}}
+  struct Y<F>::template U<char> k; // expected-error 2{{type alias template 'U' cannot be referenced with a struct specifier}}
   typename Y<F>::template U<char> l; // ok
 };
 template struct Y<Id>; // expected-note {{requested here}}

cfe/trunk/test/CXX/drs/dr2xx.cpp

namespace dr252 { // dr252: yes
   };
   G::~G() {}
 }
 
 namespace dr254 { // dr254: yes
   template<typename T> struct A {
     typedef typename T::type type; // ok even if this is a typedef-name, because
                                    // it's not an elaborated-type-specifier
-    typedef struct T::type foo; // expected-error {{elaborated type refers to a typedef}}
+    typedef struct T::type foo; // expected-error {{typedef 'type' cannot be referenced with a struct specifier}}
   };
   struct B { struct type {}; };
   struct C { typedef struct {} type; }; // expected-note {{here}}
   A<B>::type n;
   A<C>::type n; // expected-note {{instantiation of}}
 }
 
 // dr256: dup 624
namespace dr298 { // dr298: yes
   typedef A B; // expected-note {{here}}
   typedef const A C; // expected-note {{here}}
 
   A::type i1;
   B::type i2;
   C::type i3;
 
   struct A a;
-  struct B b; // expected-error {{refers to a typedef}}
-  struct C c; // expected-error {{refers to a typedef}}
+  struct B b; // expected-error {{typedef 'B' cannot be referenced with a struct specifier}}
+  struct C c; // expected-error {{typedef 'C' cannot be referenced with a struct specifier}}
 
   B::B() {} // expected-error {{requires a type specifier}}
   B::A() {} // ok
   C::~C() {} // expected-error {{destructor cannot be declared using a typedef 'C' (aka 'const dr298::A') of the class name}}
 
   typedef struct D E; // expected-note {{here}}
   struct E {}; // expected-error {{conflicts with typedef}}
 

cfe/trunk/test/CXX/drs/dr4xx.cpp

 
 namespace dr407 { // dr407: 3.8
   struct S;
   typedef struct S S;
   void f() {
     struct S *p;
     {
       typedef struct S S; // expected-note {{here}}
-      struct S *p; // expected-error {{refers to a typedef}}
+      struct S *p; // expected-error {{typedef 'S' cannot be referenced with a struct specifier}}
     }
   }
   struct S {};
 
   namespace UsingDir {
     namespace A {
       struct S {}; // expected-note {{found}}
     }

cfe/trunk/test/CXX/temp/temp.decls/temp.friend/p1.cpp

namespace test7 {
   class C {
     int foo(A<int> &a) {
       return a.x; // expected-error {{'x' is a private member of 'test7::A<int>'}}
     }
   };
 
   // This shouldn't crash.
   template <class T> class D {
-    friend class A; // expected-error {{elaborated type refers to a template}}
+    friend class A; // expected-error {{template 'A' cannot be referenced with a class specifier}}
   };
   template class D<int>;
 }
 
 namespace test8 {
   template <class N> class A {
     static int x;
     template <class T> friend void foo();

cfe/trunk/test/CXX/temp/temp.spec/no-body.cpp

 #if __cplusplus >= 201103L
   // expected-error@-2 {{explicit instantiation of 'x' must occur at global scope}}
 #endif
 #endif
 }
 
 namespace unsupported {
 #ifndef FIXING
- template struct y;     // expected-error {{elaborated type refers to a template}}
+ template struct y;     // expected-error {{template 'y' cannot be referenced with a struct specifier}}
 #endif
 }
 
 template<typename T> void f0(T) { }
 template<typename T> void g0(T) { }
 template<typename T> struct x0 { }; // expected-note {{explicitly specialized declaration is here}}
 template<typename T> struct y0 { };
 

cfe/trunk/test/SemaCXX/PR8755.cpp

 // RUN: %clang_cc1 -fsyntax-only -verify %s
 
 template <typename T>
 struct A {
   typedef int iterator;  // expected-note{{declared here}}
 };
 
 template <typename T>
 void f() {
-  class A <T> ::iterator foo;  // expected-error{{elaborated type refers to a typedef}}
+  class A <T> ::iterator foo;  // expected-error{{typedef 'iterator' cannot be referenced with a class specifier}}
 }
 
 void g() {
   f<int>();  // expected-note{{in instantiation of function template}}
 }
 

cfe/trunk/test/SemaCXX/using-decl-templates.cpp

template<typename T> class UsingTypenameNNS {
   using typename T::X;
   typename X::X x;
 };
 
 namespace aliastemplateinst {
   template<typename T> struct A { };
   template<typename T> using APtr = A<T*>; // expected-note{{previous use is here}}
 
-  template struct APtr<int>; // expected-error{{elaborated type refers to a type alias template}}
+  template struct APtr<int>; // expected-error{{type alias template 'APtr' cannot be referenced with a struct specifier}}
 }
 
 namespace DontDiagnoseInvalidTest {
 template <bool Value> struct Base {
   static_assert(Value, ""); // expected-error {{static_assert failed}}
 };
 struct Derived : Base<false> { // expected-note {{requested here}}
   using Base<false>::Base; // OK. Don't diagnose that 'Base' isn't a base class of Derived.
 };
 } // namespace DontDiagnoseInvalidTest

cfe/trunk/test/SemaTemplate/template-id-expr.cpp

void f5() {
   A::template B<I>::template b1(); // expected-error {{'b1' following the 'template' keyword does not refer to a template}}
 }
 
 template void f5<0>(); // expected-note {{in instantiation of function template specialization 'f5<0>' requested here}}
 
 class C {};
 template <template <typename> class D>  // expected-note{{previous use is here}}
 class E {
-  template class D<C>;  // expected-error {{elaborated type refers to a template template argument}}
+  template class D<C>;  // expected-error {{template template argument 'D' cannot be referenced with a class specifier}}
 };