r302596 - Don't mark a member as a member specialization until we know we're keeping the specialization.

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Tue May 9 16:02:10 PDT 2017


Author: rsmith
Date: Tue May  9 18:02:10 2017
New Revision: 302596

URL: http://llvm.org/viewvc/llvm-project?rev=302596&view=rev
Log:
Don't mark a member as a member specialization until we know we're keeping the specialization.

This improves our behavior in a few ways:

 * We now guarantee that if a member is marked as being a member
   specialization, there will actually be a member specialization declaration
   somewhere on its redeclaration chain. This fixes a crash in modules builds
   where we would try to check that there was a visible declaration of the
   member specialization and be surprised to not find any declaration of it at
   all.

 * We don't set the source location of the in-class declaration of the member
   specialization to the out-of-line declaration's location until we have
   actually finished merging them. This fixes some very silly looking
   diagnostics, where we'd point a "previous declaration is here" note at the
   same declaration we're complaining about. Ideally we wouldn't mess with the
   prior declaration's location at all, but too much code assumes that the
   first declaration of an entity is a reasonable thing to use as an indication
   of where it was declared, and that's not really true for a member
   specialization unless we fake it like this.

Modified:
    cfe/trunk/include/clang/Sema/Sema.h
    cfe/trunk/lib/Sema/SemaDecl.cpp
    cfe/trunk/lib/Sema/SemaTemplate.cpp
    cfe/trunk/test/PCH/cxx-templates.cpp
    cfe/trunk/test/PCH/cxx-templates.h
    cfe/trunk/test/SemaTemplate/explicit-specialization-member.cpp

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=302596&r1=302595&r2=302596&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Tue May  9 18:02:10 2017
@@ -6085,6 +6085,7 @@ public:
                          TemplateArgumentListInfo *ExplicitTemplateArgs,
                                            LookupResult &Previous);
   bool CheckMemberSpecialization(NamedDecl *Member, LookupResult &Previous);
+  void CompleteMemberSpecialization(NamedDecl *Member, LookupResult &Previous);
 
   DeclResult
   ActOnExplicitInstantiation(Scope *S,

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=302596&r1=302595&r2=302596&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue May  9 18:02:10 2017
@@ -1488,6 +1488,11 @@ bool Sema::ShouldWarnIfUnusedFileScopedD
   if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
     if (FD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation)
       return false;
+    // A non-out-of-line declaration of a member specialization was implicitly
+    // instantiated; it's the out-of-line declaration that we're interested in.
+    if (FD->getTemplateSpecializationKind() == TSK_ExplicitSpecialization &&
+        FD->getMemberSpecializationInfo() && !FD->isOutOfLine())
+      return false;
 
     if (const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(FD)) {
       if (MD->isVirtual() || IsDisallowedCopyOrAssign(MD))
@@ -1514,6 +1519,10 @@ bool Sema::ShouldWarnIfUnusedFileScopedD
     if (VD->isStaticDataMember() &&
         VD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation)
       return false;
+    if (VD->isStaticDataMember() &&
+        VD->getTemplateSpecializationKind() == TSK_ExplicitSpecialization &&
+        VD->getMemberSpecializationInfo() && !VD->isOutOfLine())
+      return false;
 
     if (VD->isInline() && !isMainFileLoc(*this, VD->getLocation()))
       return false;
@@ -6706,6 +6715,9 @@ NamedDecl *Sema::ActOnVariableDeclarator
     return NewTemplate;
   }
 
+  if (IsMemberSpecialization && !NewVD->isInvalidDecl())
+    CompleteMemberSpecialization(NewVD, Previous);
+
   return NewVD;
 }
 
@@ -8919,13 +8931,17 @@ Sema::ActOnFunctionDeclarator(Scope *S,
     }
   }
 
+  MarkUnusedFileScopedDecl(NewFD);
+
   if (getLangOpts().CPlusPlus) {
     if (FunctionTemplate) {
       if (NewFD->isInvalidDecl())
         FunctionTemplate->setInvalidDecl();
-      MarkUnusedFileScopedDecl(NewFD);
       return FunctionTemplate;
     }
+
+    if (isMemberSpecialization && !NewFD->isInvalidDecl())
+      CompleteMemberSpecialization(NewFD, Previous);
   }
 
   if (NewFD->hasAttr<OpenCLKernelAttr>()) {
@@ -8965,8 +8981,6 @@ Sema::ActOnFunctionDeclarator(Scope *S,
     }
   }
 
-  MarkUnusedFileScopedDecl(NewFD);
-
   // Here we have an function template explicit specialization at class scope.
   // The actually specialization will be postponed to template instatiation
   // time via the ClassScopeFunctionSpecializationDecl node.
@@ -9183,7 +9197,9 @@ bool Sema::CheckFunctionDeclaration(Scop
         if (OldTemplateDecl->getTemplatedDecl()->isDeleted()) {
           FunctionDecl *const OldTemplatedDecl =
               OldTemplateDecl->getTemplatedDecl();
+          // FIXME: This assert will not hold in the presence of modules.
           assert(OldTemplatedDecl->getCanonicalDecl() == OldTemplatedDecl);
+          // FIXME: We need an update record for this AST mutation.
           OldTemplatedDecl->setDeletedAsWritten(false);
         }
       }
@@ -13752,6 +13768,9 @@ CreateNewDecl:
   // record.
   AddPushedVisibilityAttribute(New);
 
+  if (isMemberSpecialization && !New->isInvalidDecl())
+    CompleteMemberSpecialization(New, Previous);
+
   OwnedDecl = true;
   // In C++, don't return an invalid declaration. We can't recover well from
   // the cases where we make the type anonymous.

Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=302596&r1=302595&r2=302596&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaTemplate.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplate.cpp Tue May  9 18:02:10 2017
@@ -7825,6 +7825,9 @@ bool Sema::CheckFunctionTemplateSpeciali
     // C++11 [dcl.constexpr]p1: An explicit specialization of a constexpr
     // function can differ from the template declaration with respect to
     // the constexpr specifier.
+    // FIXME: We need an update record for this AST mutation.
+    // FIXME: What if there are multiple such prior declarations (for instance,
+    // from different modules)?
     Specialization->setConstexpr(FD->isConstexpr());
   }
 
@@ -7872,9 +7875,11 @@ bool Sema::CheckFunctionTemplateSpeciali
     // flag to not-deleted, so that we can inherit that information from 'FD'.
     if (Specialization->isDeleted() && !SpecInfo->isExplicitSpecialization() &&
         !Specialization->getCanonicalDecl()->isReferenced()) {
+      // FIXME: This assert will not hold in the presence of modules.
       assert(
           Specialization->getCanonicalDecl() == Specialization &&
           "This must be the only existing declaration of this specialization");
+      // FIXME: We need an update record for this AST mutation.
       Specialization->setDeletedAsWritten(false);
     }
     SpecInfo->setTemplateSpecializationKind(TSK_ExplicitSpecialization);
@@ -7987,8 +7992,11 @@ Sema::CheckMemberSpecialization(NamedDec
     return false;
   }
 
-  // If this is a friend, just bail out here before we start turning
-  // things into explicit specializations.
+  // A member specialization in a friend declaration isn't really declaring
+  // an explicit specialization, just identifying a specific (possibly implicit)
+  // specialization. Don't change the template specialization kind.
+  //
+  // FIXME: Is this really valid? Other compilers reject.
   if (Member->getFriendObjectKind() != Decl::FOK_None) {
     // Preserve instantiation information.
     if (InstantiatedFrom && isa<CXXMethodDecl>(Member)) {
@@ -8038,66 +8046,36 @@ Sema::CheckMemberSpecialization(NamedDec
                                        false))
     return true;
 
-  // Note that this is an explicit instantiation of a member.
-  // the original declaration to note that it is an explicit specialization
-  // (if it was previously an implicit instantiation). This latter step
-  // makes bookkeeping easier.
-  if (isa<FunctionDecl>(Member)) {
+  // Note that this member specialization is an "instantiation of" the
+  // corresponding member of the original template.
+  if (auto *MemberFunction = dyn_cast<FunctionDecl>(Member)) {
     FunctionDecl *InstantiationFunction = cast<FunctionDecl>(Instantiation);
     if (InstantiationFunction->getTemplateSpecializationKind() ==
           TSK_ImplicitInstantiation) {
-      InstantiationFunction->setTemplateSpecializationKind(
-                                                  TSK_ExplicitSpecialization);
-      InstantiationFunction->setLocation(Member->getLocation());
       // Explicit specializations of member functions of class templates do not
       // inherit '=delete' from the member function they are specializing.
       if (InstantiationFunction->isDeleted()) {
+        // FIXME: This assert will not hold in the presence of modules.
         assert(InstantiationFunction->getCanonicalDecl() ==
                InstantiationFunction);
+        // FIXME: We need an update record for this AST mutation.
         InstantiationFunction->setDeletedAsWritten(false);
       }
     }
 
-    cast<FunctionDecl>(Member)->setInstantiationOfMemberFunction(
-                                        cast<CXXMethodDecl>(InstantiatedFrom),
-                                                  TSK_ExplicitSpecialization);
-    MarkUnusedFileScopedDecl(InstantiationFunction);
-  } else if (isa<VarDecl>(Member)) {
-    VarDecl *InstantiationVar = cast<VarDecl>(Instantiation);
-    if (InstantiationVar->getTemplateSpecializationKind() ==
-          TSK_ImplicitInstantiation) {
-      InstantiationVar->setTemplateSpecializationKind(
-                                                  TSK_ExplicitSpecialization);
-      InstantiationVar->setLocation(Member->getLocation());
-    }
-
-    cast<VarDecl>(Member)->setInstantiationOfStaticDataMember(
+    MemberFunction->setInstantiationOfMemberFunction(
+        cast<CXXMethodDecl>(InstantiatedFrom), TSK_ExplicitSpecialization);
+  } else if (auto *MemberVar = dyn_cast<VarDecl>(Member)) {
+    MemberVar->setInstantiationOfStaticDataMember(
         cast<VarDecl>(InstantiatedFrom), TSK_ExplicitSpecialization);
-    MarkUnusedFileScopedDecl(InstantiationVar);
-  } else if (isa<CXXRecordDecl>(Member)) {
-    CXXRecordDecl *InstantiationClass = cast<CXXRecordDecl>(Instantiation);
-    if (InstantiationClass->getTemplateSpecializationKind() ==
-          TSK_ImplicitInstantiation) {
-      InstantiationClass->setTemplateSpecializationKind(
-                                                   TSK_ExplicitSpecialization);
-      InstantiationClass->setLocation(Member->getLocation());
-    }
-
-    cast<CXXRecordDecl>(Member)->setInstantiationOfMemberClass(
-                                        cast<CXXRecordDecl>(InstantiatedFrom),
-                                                   TSK_ExplicitSpecialization);
-  } else {
-    assert(isa<EnumDecl>(Member) && "Only member enums remain");
-    EnumDecl *InstantiationEnum = cast<EnumDecl>(Instantiation);
-    if (InstantiationEnum->getTemplateSpecializationKind() ==
-          TSK_ImplicitInstantiation) {
-      InstantiationEnum->setTemplateSpecializationKind(
-                                                   TSK_ExplicitSpecialization);
-      InstantiationEnum->setLocation(Member->getLocation());
-    }
-
-    cast<EnumDecl>(Member)->setInstantiationOfMemberEnum(
+  } else if (auto *MemberClass = dyn_cast<CXXRecordDecl>(Member)) {
+    MemberClass->setInstantiationOfMemberClass(
+        cast<CXXRecordDecl>(InstantiatedFrom), TSK_ExplicitSpecialization);
+  } else if (auto *MemberEnum = dyn_cast<EnumDecl>(Member)) {
+    MemberEnum->setInstantiationOfMemberEnum(
         cast<EnumDecl>(InstantiatedFrom), TSK_ExplicitSpecialization);
+  } else {
+    llvm_unreachable("unknown member specialization kind");
   }
 
   // Save the caller the trouble of having to figure out which declaration
@@ -8107,6 +8085,43 @@ Sema::CheckMemberSpecialization(NamedDec
   return false;
 }
 
+/// Complete the explicit specialization of a member of a class template by
+/// updating the instantiated member to be marked as an explicit specialization.
+///
+/// \param OrigD The member declaration instantiated from the template.
+/// \param Loc The location of the explicit specialization of the member.
+template<typename DeclT>
+static void completeMemberSpecializationImpl(Sema &S, DeclT *OrigD,
+                                             SourceLocation Loc) {
+  if (OrigD->getTemplateSpecializationKind() != TSK_ImplicitInstantiation)
+    return;
+
+  // FIXME: Inform AST mutation listeners of this AST mutation.
+  // FIXME: If there are multiple in-class declarations of the member (from
+  // multiple modules, or a declaration and later definition of a member type),
+  // should we update all of them?
+  OrigD->setTemplateSpecializationKind(TSK_ExplicitSpecialization);
+  OrigD->setLocation(Loc);
+}
+
+void Sema::CompleteMemberSpecialization(NamedDecl *Member,
+                                        LookupResult &Previous) {
+  NamedDecl *Instantiation = cast<NamedDecl>(Member->getCanonicalDecl());
+  if (Instantiation == Member)
+    return;
+
+  if (auto *Function = dyn_cast<CXXMethodDecl>(Instantiation))
+    completeMemberSpecializationImpl(*this, Function, Member->getLocation());
+  else if (auto *Var = dyn_cast<VarDecl>(Instantiation))
+    completeMemberSpecializationImpl(*this, Var, Member->getLocation());
+  else if (auto *Record = dyn_cast<CXXRecordDecl>(Instantiation))
+    completeMemberSpecializationImpl(*this, Record, Member->getLocation());
+  else if (auto *Enum = dyn_cast<EnumDecl>(Instantiation))
+    completeMemberSpecializationImpl(*this, Enum, Member->getLocation());
+  else
+    llvm_unreachable("unknown member specialization kind");
+}
+
 /// \brief Check the scope of an explicit instantiation.
 ///
 /// \returns true if a serious error occurs, false otherwise.

Modified: cfe/trunk/test/PCH/cxx-templates.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/cxx-templates.cpp?rev=302596&r1=302595&r2=302596&view=diff
==============================================================================
--- cfe/trunk/test/PCH/cxx-templates.cpp (original)
+++ cfe/trunk/test/PCH/cxx-templates.cpp Tue May  9 18:02:10 2017
@@ -108,3 +108,11 @@ namespace cyclic_module_load {
 template int local_extern::f<int[]>(); // expected-note {{in instantiation of}}
 #endif
 template int local_extern::g<int[]>();
+
+namespace MemberSpecializationLocation {
+#ifndef NO_ERRORS
+  // expected-note at cxx-templates.h:* {{previous}}
+  template<> float A<int>::n; // expected-error {{redeclaration of 'n' with a different type}}
+#endif
+  int k = A<int>::n;
+}

Modified: cfe/trunk/test/PCH/cxx-templates.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/cxx-templates.h?rev=302596&r1=302595&r2=302596&view=diff
==============================================================================
--- cfe/trunk/test/PCH/cxx-templates.h (original)
+++ cfe/trunk/test/PCH/cxx-templates.h Tue May  9 18:02:10 2017
@@ -358,3 +358,6 @@ namespace rdar15468709c {
   }
 }
 
+namespace MemberSpecializationLocation {
+  template<typename T> struct A { static int n; };
+}

Modified: cfe/trunk/test/SemaTemplate/explicit-specialization-member.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/explicit-specialization-member.cpp?rev=302596&r1=302595&r2=302596&view=diff
==============================================================================
--- cfe/trunk/test/SemaTemplate/explicit-specialization-member.cpp (original)
+++ cfe/trunk/test/SemaTemplate/explicit-specialization-member.cpp Tue May  9 18:02:10 2017
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify %s -fcxx-exceptions
 template<typename T>
 struct X0 {
   typedef T* type;
@@ -57,3 +57,12 @@ template<typename T> struct Helper {
 template<typename T> void Helper<T>::func<2>() {} // expected-error {{cannot specialize a member}} \
                                                   // expected-error {{no function template matches}}
 }
+
+namespace SpecLoc {
+  template <typename T> struct A {
+    static int n; // expected-note {{previous}}
+    static void f(); // expected-note {{previous}}
+  };
+  template<> float A<int>::n; // expected-error {{different type}}
+  template<> void A<int>::f() throw(); // expected-error {{does not match}}
+}




More information about the cfe-commits mailing list