[Sema] Fix a crash-on-invalid when a template parameter list has a class definition or non-reference class type

[ahatanak]

Summary

Currently clang crashes with an assertion failure in SemaAccess.cpp ("Assertion failed: (Access == AS_private || Access == AS_protected)") when compiling the following invalid code:

class C0 {
public:
  template<typename T0, typename T1 = T0 // missing closing angle bracket
  struct S0 {};

  C0() : m(new S0<int>) {}
  S0<int> *m;

  template<class C1 *p>
  void foo1();
};

C1 *x;

This seems to happen because:

• The definition of struct S0 is parsed as part of the template parameters list because of the missing closing angle bracket.
• Sema adds struct S0 to class C0's scope. But if struct S0 is introduced in a template parameter list, it should add the struct to the enclosing scope (the file scope), just like it would add class C1 to the file scope.
• It looks up S0 in C0's scope and then checks its access specifier when LookupResult is destructed. The assertion fails because S0's access specifier is not set (it's AS_none).

To fix the crash, I made changes in Sema::ActOnTag to check whether it's parsing a template parameter list and, if it is, move S0 to the correct scope. Also, the TagDecl is marked as invalid if the class is defined in a template parameter list. Note that test/SemaCXX/PR16677.cpp loses one error diagnostic because of this change.

rdar://problem/31783961
rdar://problem/19570630

[ahatanak]

ping

[erik.pilkington]

LGTM!

include/clang/Parse/Parser.h

private:
   /// would be best implemented in the parser.
   enum DeclSpecContext {
     DSC_normal, // normal context
     DSC_class,  // class context, enables 'friend'
     DSC_type_specifier, // C++ type-specifier-seq or C specifier-qualifier-list
     DSC_trailing, // C++11 trailing-type-specifier in a trailing return type
     DSC_alias_declaration, // C++11 type-specifier-seq in an alias-declaration
     DSC_top_level, // top-level/namespace declaration context
+    DSC_template_param, // template parameter context
     DSC_template_type_arg, // template type argument context
     DSC_objc_method_result, // ObjC method result context, enables 'instancetype'
     DSC_condition // condition declaration context
   };
 
   /// Is this a context in which we are parsing just a type-specifier (or
   /// trailing-type-specifier)?
   static bool isTypeSpecifier(DeclSpecContext DSC) {
     switch (DSC) {
     case DSC_normal:
+    case DSC_template_param:
     case DSC_class:
     case DSC_top_level:
     case DSC_objc_method_result:
     case DSC_condition:
       return false;
 
     case DSC_template_type_arg:
     case DSC_type_specifier:
     case DSC_trailing:
     case DSC_alias_declaration:
       return true;
     }
     llvm_unreachable("Missing DeclSpecContext case");
   }
 
   /// Is this a context in which we can perform class template argument
   /// deduction?
   static bool isClassTemplateDeductionContext(DeclSpecContext DSC) {
     switch (DSC) {
     case DSC_normal:
+    case DSC_template_param:
     case DSC_class:
     case DSC_top_level:
     case DSC_condition:
     case DSC_type_specifier:
       return true;
 
     case DSC_objc_method_result:
     case DSC_template_type_arg:

include/clang/Sema/Sema.h

Decl *ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
                  SourceLocation KWLoc, CXXScopeSpec &SS,
                  IdentifierInfo *Name, SourceLocation NameLoc,
                  AttributeList *Attr, AccessSpecifier AS,
                  SourceLocation ModulePrivateLoc,
                  MultiTemplateParamsArg TemplateParameterLists,
                  bool &OwnedDecl, bool &IsDependent,
                  SourceLocation ScopedEnumKWLoc,
                  bool ScopedEnumUsesClassTag, TypeResult UnderlyingType,
-                 bool IsTypeSpecifier, SkipBodyInfo *SkipBody = nullptr);
+                 bool IsTypeSpecifier, bool IsTemplateParamOrArg,
+                 SkipBodyInfo *SkipBody = nullptr);
 
   Decl *ActOnTemplatedFriendTag(Scope *S, SourceLocation FriendLoc,
                                 unsigned TagSpec, SourceLocation TagLoc,
                                 CXXScopeSpec &SS,
                                 IdentifierInfo *Name, SourceLocation NameLoc,
                                 AttributeList *Attr,
                                 MultiTemplateParamsArg TempParamLists);
 

lib/Parse/ParseDecl.cpp

 /// \param Context the declarator context, which is one of the
 /// Declarator::TheContext enumerator values.
 Parser::DeclSpecContext
 Parser::getDeclSpecContextFromDeclaratorContext(unsigned Context) {
   if (Context == Declarator::MemberContext)
     return DSC_class;
   if (Context == Declarator::FileContext)
     return DSC_top_level;
+  if (Context == Declarator::TemplateParamContext)
+    return DSC_template_param;
   if (Context == Declarator::TemplateTypeArgContext)
     return DSC_template_type_arg;
   if (Context == Declarator::TrailingReturnContext)
     return DSC_trailing;
   if (Context == Declarator::AliasDeclContext ||
       Context == Declarator::AliasTemplateContext)
     return DSC_alias_declaration;
   return DSC_normal;
void Parser::ParseEnumSpecifier(SourceLocation StartLoc, DeclSpec &DS,
   bool IsDependent = false;
   const char *PrevSpec = nullptr;
   unsigned DiagID;
   Decl *TagDecl = Actions.ActOnTag(getCurScope(), DeclSpec::TST_enum, TUK,
                                    StartLoc, SS, Name, NameLoc, attrs.getList(),
                                    AS, DS.getModulePrivateSpecLoc(), TParams,
                                    Owned, IsDependent, ScopedEnumKWLoc,
                                    IsScopedUsingClassTag, BaseType,
-                                   DSC == DSC_type_specifier, &SkipBody);
+                                   DSC == DSC_type_specifier,
+                                   DSC == DSC_template_param ||
+                                   DSC == DSC_template_type_arg, &SkipBody);
 
   if (SkipBody.ShouldSkip) {
     assert(TUK == Sema::TUK_Definition && "can only skip a definition");
 
     BalancedDelimiterTracker T(*this, tok::l_brace);
     T.consumeOpen();
     T.skipToEnd();
 

lib/Parse/ParseDeclCXX.cpp

if (TemplateId) {
     // Declaration or definition of a class type
     TagOrTempResult = Actions.ActOnTag(getCurScope(), TagType, TUK, StartLoc,
                                        SS, Name, NameLoc, attrs.getList(), AS,
                                        DS.getModulePrivateSpecLoc(),
                                        TParams, Owned, IsDependent,
                                        SourceLocation(), false,
                                        clang::TypeResult(),
                                        DSC == DSC_type_specifier,
-                                       &SkipBody);
+                                       DSC == DSC_template_param ||
+                                       DSC == DSC_template_type_arg, &SkipBody);
 
     // If ActOnTag said the type was dependent, try again with the
     // less common call.
     if (IsDependent) {
       assert(TUK == Sema::TUK_Reference || TUK == Sema::TUK_Friend);
       TypeResult = Actions.ActOnDependentTag(getCurScope(), TagType, TUK,
                                              SS, Name, StartLoc, NameLoc);
     }

lib/Parse/ParseTemplate.cpp

 ///         ...
 ///         parameter-declaration
 Decl *
 Parser::ParseNonTypeTemplateParameter(unsigned Depth, unsigned Position) {
   // Parse the declaration-specifiers (i.e., the type).
   // FIXME: The type should probably be restricted in some way... Not all
   // declarators (parts of declarators?) are accepted for parameters.
   DeclSpec DS(AttrFactory);
-  ParseDeclarationSpecifiers(DS);
+  ParseDeclarationSpecifiers(DS, ParsedTemplateInfo(), AS_none,
+                             DSC_template_param);
 
   // Parse this as a typename.
   Declarator ParamDecl(DS, Declarator::TemplateParamContext);
   ParseDeclarator(ParamDecl);
   if (DS.getTypeSpecType() == DeclSpec::TST_unspecified) {
     Diag(Tok.getLocation(), diag::err_expected_template_parameter);
     return nullptr;
   }

lib/Sema/SemaDecl.cpp

Decl *Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
                      IdentifierInfo *Name, SourceLocation NameLoc,
                      AttributeList *Attr, AccessSpecifier AS,
                      SourceLocation ModulePrivateLoc,
                      MultiTemplateParamsArg TemplateParameterLists,
                      bool &OwnedDecl, bool &IsDependent,
                      SourceLocation ScopedEnumKWLoc,
                      bool ScopedEnumUsesClassTag,
                      TypeResult UnderlyingType,
-                     bool IsTypeSpecifier, SkipBodyInfo *SkipBody) {
+                     bool IsTypeSpecifier, bool IsTemplateParamOrArg,
+                     SkipBodyInfo *SkipBody) {
   // If this is not a definition, it must have a name.
   IdentifierInfo *OrigName = Name;
   assert((Name != nullptr || TUK == TUK_Definition) &&
          "Nameless record must be a definition!");
   assert(TemplateParameterLists.size() == 0 || TUK != TUK_Reference);
 
   OwnedDecl = false;
   TagTypeKind Kind = TypeWithKeyword::getTagTypeKindForTypeSpec(TagSpec);
if (getLangOpts().CPlusPlus && Name && DC && StdNamespace &&
     }
   }
 
   // If we didn't find a previous declaration, and this is a reference
   // (or friend reference), move to the correct scope.  In C++, we
   // also need to do a redeclaration lookup there, just in case
   // there's a shadow friend decl.
   if (Name && Previous.empty() &&
-      (TUK == TUK_Reference || TUK == TUK_Friend)) {
+      (TUK == TUK_Reference || TUK == TUK_Friend || IsTemplateParamOrArg)) {
     if (Invalid) goto CreateNewDecl;
     assert(SS.isEmpty());
 
-    if (TUK == TUK_Reference) {
+    if (TUK == TUK_Reference || IsTemplateParamOrArg) {
       // C++ [basic.scope.pdecl]p5:
       //   -- for an elaborated-type-specifier of the form
       //
       //          class-key identifier
       //
       //      if the elaborated-type-specifier is used in the
       //      decl-specifier-seq or parameter-declaration-clause of a
       //      function defined in namespace scope, the identifier is
if (getLangOpts().CPlusPlus) {
         StdBadAlloc = cast<CXXRecordDecl>(New);
     } else
       New = RecordDecl::Create(Context, Kind, SearchDC, KWLoc, Loc, Name,
                                cast_or_null<RecordDecl>(PrevDecl));
   }
 
   // C++11 [dcl.type]p3:
   //   A type-specifier-seq shall not define a class or enumeration [...].
-  if (getLangOpts().CPlusPlus && IsTypeSpecifier && TUK == TUK_Definition) {
+  if (getLangOpts().CPlusPlus && (IsTypeSpecifier || IsTemplateParamOrArg) &&
+      TUK == TUK_Definition) {
     Diag(New->getLocation(), diag::err_type_defined_in_type_specifier)
       << Context.getTagDeclType(New);
     Invalid = true;
   }
 
   // Maybe add qualifier info.
   if (SS.isNotEmpty()) {
     if (SS.isSet()) {

lib/Sema/SemaDeclCXX.cpp

if (SS.isEmpty()) {
       bool IsDependent = false;
       return ActOnTag(S, TagSpec, TUK_Friend, TagLoc, SS, Name, NameLoc,
                       Attr, AS_public,
                       /*ModulePrivateLoc=*/SourceLocation(),
                       MultiTemplateParamsArg(), Owned, IsDependent,
                       /*ScopedEnumKWLoc=*/SourceLocation(),
                       /*ScopedEnumUsesClassTag=*/false,
                       /*UnderlyingType=*/TypeResult(),
-                      /*IsTypeSpecifier=*/false);
+                      /*IsTypeSpecifier=*/false,
+                      /*IsTemplateParamOrArg=*/false);
     }
 
     NestedNameSpecifierLoc QualifierLoc = SS.getWithLocInContext(Context);
     ElaboratedTypeKeyword Keyword
       = TypeWithKeyword::getKeywordForTagTypeKind(Kind);
     QualType T = CheckTypenameType(Keyword, TagLoc, QualifierLoc,
                                    *Name, NameLoc);
     if (T.isNull())

lib/Sema/SemaTemplate.cpp

Sema::ActOnExplicitInstantiation(Scope *S,
 
   bool Owned = false;
   bool IsDependent = false;
   Decl *TagD = ActOnTag(S, TagSpec, Sema::TUK_Reference,
                         KWLoc, SS, Name, NameLoc, Attr, AS_none,
                         /*ModulePrivateLoc=*/SourceLocation(),
                         MultiTemplateParamsArg(), Owned, IsDependent,
                         SourceLocation(), false, TypeResult(),
-                        /*IsTypeSpecifier*/false);
+                        /*IsTypeSpecifier*/false,
+                        /*IsTemplateParamOrArg*/false);
   assert(!IsDependent && "explicit instantiation of dependent name not yet handled");
 
   if (!TagD)
     return true;
 
   TagDecl *Tag = cast<TagDecl>(TagD);
   assert(!Tag->isEnum() && "shouldn't see enumerations here");
 

test/SemaCXX/PR16677.cpp

 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify %s
 
 class Class_With_Destructor {
   ~Class_With_Destructor() { }
 };
 
 template <class T>
 class Base { };
 
 template<class T,  // Should be angle bracket instead of comma
 class Derived : public Base<T> { // expected-error{{'Derived' cannot be defined in a type specifier}}
   Class_With_Destructor member;
-}; // expected-error{{a non-type template parameter cannot have type 'class Derived'}}
-   // expected-error@-1{{expected ',' or '>' in template-parameter-list}}
-   // expected-warning@-2{{declaration does not declare anything}}
+}; // expected-error{{expected ',' or '>' in template-parameter-list}}
+   // expected-warning@-1{{declaration does not declare anything}}
 

test/SemaCXX/invalid-template-params.cpp

+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify %s
+
+template<class> class Foo {
+  template<class UBar // expected-error {{expected ';' after class}}
+                      // expected-note@-1 {{'UBar' declared here}}
+  void foo1(); // expected-error {{a non-type template parameter cannot have type 'class UBar'}}
+               // expected-error@-1 {{expected ',' or '>' in template-parameter-list}}
+               // expected-warning@-2 {{declaration does not declare anything}}
+};
+
+Foo<int>::UBar g1; // expected-error {{no type named 'UBar' in 'Foo<int>'}}
+
+class C0 {
+public:
+  template<typename T0, typename T1 = T0 // missing closing angle bracket
+  struct S0 {}; // expected-error {{'S0' cannot be defined in a type specifier}}
+                // expected-error@-1 {{cannot combine with previous 'type-name' declaration specifier}}
+                // expected-error@-2 {{expected ',' or '>' in template-parameter-list}}
+                // expected-warning@-3 {{declaration does not declare anything}}
+  C0() : m(new S0<int>) {} // expected-error {{expected '(' for function-style cast or type construction}}
+                           // expected-error@-1 {{expected expression}}
+  S0<int> *m; // expected-error {{expected member name or ';' after declaration specifiers}}
+};