r283489 - [modules] Be sure to emit local specializations of imported templates, even if

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 6 13:30:52 PDT 2016


Author: rsmith
Date: Thu Oct  6 15:30:51 2016
New Revision: 283489

URL: http://llvm.org/viewvc/llvm-project?rev=283489&view=rev
Log:
[modules] Be sure to emit local specializations of imported templates, even if
the resulting specialization is not referenced by the rest of the AST. This
both avoids performing unnecessary reinstantiations in downstream users of the
AST file and fixes a bug (breaking modules self-host right now) where we would
sometimes fail to emit a definition of a class template specialization if we
imported just a declaration of it from elsewhere (see new testcase for reduced
example).

Added:
    cfe/trunk/test/Modules/Inputs/merge-template-specializations/
    cfe/trunk/test/Modules/Inputs/merge-template-specializations/a.h
    cfe/trunk/test/Modules/Inputs/merge-template-specializations/b.h
    cfe/trunk/test/Modules/Inputs/merge-template-specializations/c.h
    cfe/trunk/test/Modules/Inputs/merge-template-specializations/module.modulemap
    cfe/trunk/test/Modules/merge-template-specializations.cpp
Modified:
    cfe/trunk/include/clang/Serialization/ASTWriter.h
    cfe/trunk/lib/Serialization/ASTWriter.cpp
    cfe/trunk/test/Modules/cxx-templates.cpp

Modified: cfe/trunk/include/clang/Serialization/ASTWriter.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTWriter.h?rev=283489&r1=283488&r2=283489&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/ASTWriter.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTWriter.h Thu Oct  6 15:30:51 2016
@@ -373,9 +373,10 @@ private:
   /// it.
   llvm::SmallSetVector<const DeclContext *, 16> UpdatedDeclContexts;
 
-  /// \brief Keeps track of visible decls that were added in DeclContexts
-  /// coming from another AST file.
-  SmallVector<const Decl *, 16> UpdatingVisibleDecls;
+  /// \brief Keeps track of declarations that we must emit, even though we're
+  /// not guaranteed to be able to find them by walking the AST starting at the
+  /// translation unit.
+  SmallVector<const Decl *, 16> DeclsToEmitEvenIfUnreferenced;
 
   /// \brief The set of Objective-C class that have categories we
   /// should serialize.
@@ -667,6 +668,14 @@ private:
   void CompletedTagDefinition(const TagDecl *D) override;
   void AddedVisibleDecl(const DeclContext *DC, const Decl *D) override;
   void AddedCXXImplicitMember(const CXXRecordDecl *RD, const Decl *D) override;
+  void AddedCXXTemplateSpecialization(
+      const ClassTemplateDecl *TD,
+      const ClassTemplateSpecializationDecl *D) override;
+  void AddedCXXTemplateSpecialization(
+      const VarTemplateDecl *TD,
+      const VarTemplateSpecializationDecl *D) override;
+  void AddedCXXTemplateSpecialization(const FunctionTemplateDecl *TD,
+                                      const FunctionDecl *D) override;
   void ResolvedExceptionSpec(const FunctionDecl *FD) override;
   void DeducedReturnType(const FunctionDecl *FD, QualType ReturnType) override;
   void ResolvedOperatorDelete(const CXXDestructorDecl *DD,

Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=283489&r1=283488&r2=283489&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp Thu Oct  6 15:30:51 2016
@@ -4472,8 +4472,9 @@ uint64_t ASTWriter::WriteASTCore(Sema &S
                                                      Number.second));
 
   // Make sure visible decls, added to DeclContexts previously loaded from
-  // an AST file, are registered for serialization.
-  for (const auto *I : UpdatingVisibleDecls) {
+  // an AST file, are registered for serialization. Likewise for template
+  // specializations added to imported templates.
+  for (const auto *I : DeclsToEmitEvenIfUnreferenced) {
     GetDeclRef(I);
   }
 
@@ -5818,9 +5819,9 @@ void ASTWriter::AddedVisibleDecl(const D
     // that we write out all of its lookup results so we don't get a nasty
     // surprise when we try to emit its lookup table.
     for (auto *Child : DC->decls())
-      UpdatingVisibleDecls.push_back(Child);
+      DeclsToEmitEvenIfUnreferenced.push_back(Child);
   }
-  UpdatingVisibleDecls.push_back(D);
+  DeclsToEmitEvenIfUnreferenced.push_back(D);
 }
 
 void ASTWriter::AddedCXXImplicitMember(const CXXRecordDecl *RD, const Decl *D) {
@@ -5989,3 +5990,39 @@ void ASTWriter::AddedAttributeToRecord(c
     return;
   DeclUpdates[Record].push_back(DeclUpdate(UPD_ADDED_ATTR_TO_RECORD, Attr));
 }
+
+void ASTWriter::AddedCXXTemplateSpecialization(
+    const ClassTemplateDecl *TD, const ClassTemplateSpecializationDecl *D) {
+  assert(!WritingAST && "Already writing the AST!");
+
+  if (!TD->getFirstDecl()->isFromASTFile())
+    return;
+  if (Chain && Chain->isProcessingUpdateRecords())
+    return;
+
+  DeclsToEmitEvenIfUnreferenced.push_back(D);
+}
+
+void ASTWriter::AddedCXXTemplateSpecialization(
+    const VarTemplateDecl *TD, const VarTemplateSpecializationDecl *D) {
+  assert(!WritingAST && "Already writing the AST!");
+
+  if (!TD->getFirstDecl()->isFromASTFile())
+    return;
+  if (Chain && Chain->isProcessingUpdateRecords())
+    return;
+
+  DeclsToEmitEvenIfUnreferenced.push_back(D);
+}
+
+void ASTWriter::AddedCXXTemplateSpecialization(const FunctionTemplateDecl *TD,
+                                               const FunctionDecl *D) {
+  assert(!WritingAST && "Already writing the AST!");
+
+  if (!TD->getFirstDecl()->isFromASTFile())
+    return;
+  if (Chain && Chain->isProcessingUpdateRecords())
+    return;
+
+  DeclsToEmitEvenIfUnreferenced.push_back(D);
+}

Added: cfe/trunk/test/Modules/Inputs/merge-template-specializations/a.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-template-specializations/a.h?rev=283489&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/merge-template-specializations/a.h (added)
+++ cfe/trunk/test/Modules/Inputs/merge-template-specializations/a.h Thu Oct  6 15:30:51 2016
@@ -0,0 +1 @@
+template<unsigned> class SmallString {};

Added: cfe/trunk/test/Modules/Inputs/merge-template-specializations/b.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-template-specializations/b.h?rev=283489&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/merge-template-specializations/b.h (added)
+++ cfe/trunk/test/Modules/Inputs/merge-template-specializations/b.h Thu Oct  6 15:30:51 2016
@@ -0,0 +1,2 @@
+#include "a.h"
+void f(SmallString<256>&);

Added: cfe/trunk/test/Modules/Inputs/merge-template-specializations/c.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-template-specializations/c.h?rev=283489&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/merge-template-specializations/c.h (added)
+++ cfe/trunk/test/Modules/Inputs/merge-template-specializations/c.h Thu Oct  6 15:30:51 2016
@@ -0,0 +1,3 @@
+#include "a.h"
+struct X { SmallString<256> ss; };
+#include "b.h"

Added: cfe/trunk/test/Modules/Inputs/merge-template-specializations/module.modulemap
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-template-specializations/module.modulemap?rev=283489&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/merge-template-specializations/module.modulemap (added)
+++ cfe/trunk/test/Modules/Inputs/merge-template-specializations/module.modulemap Thu Oct  6 15:30:51 2016
@@ -0,0 +1,3 @@
+module a { header "a.h" export * }
+module b { header "b.h" export * }
+module c { header "c.h" export * }

Modified: cfe/trunk/test/Modules/cxx-templates.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/cxx-templates.cpp?rev=283489&r1=283488&r2=283489&view=diff
==============================================================================
--- cfe/trunk/test/Modules/cxx-templates.cpp (original)
+++ cfe/trunk/test/Modules/cxx-templates.cpp Thu Oct  6 15:30:51 2016
@@ -249,10 +249,10 @@ namespace Std {
 
 // CHECK-DUMP:      ClassTemplateDecl {{.*}} <{{.*[/\\]}}cxx-templates-common.h:1:1, {{.*}}>  col:{{.*}} in cxx_templates_common SomeTemplate
 // CHECK-DUMP:        ClassTemplateSpecializationDecl {{.*}} prev {{.*}} SomeTemplate
-// CHECK-DUMP-NEXT:     TemplateArgument type 'char [1]'
-// CHECK-DUMP:        ClassTemplateSpecializationDecl {{.*}} SomeTemplate definition
-// CHECK-DUMP-NEXT:     TemplateArgument type 'char [1]'
-// CHECK-DUMP:        ClassTemplateSpecializationDecl {{.*}} prev {{.*}} SomeTemplate
 // CHECK-DUMP-NEXT:     TemplateArgument type 'char [2]'
 // CHECK-DUMP:        ClassTemplateSpecializationDecl {{.*}} SomeTemplate definition
 // CHECK-DUMP-NEXT:     TemplateArgument type 'char [2]'
+// CHECK-DUMP:        ClassTemplateSpecializationDecl {{.*}} prev {{.*}} SomeTemplate
+// CHECK-DUMP-NEXT:     TemplateArgument type 'char [1]'
+// CHECK-DUMP:        ClassTemplateSpecializationDecl {{.*}} SomeTemplate definition
+// CHECK-DUMP-NEXT:     TemplateArgument type 'char [1]'

Added: cfe/trunk/test/Modules/merge-template-specializations.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/merge-template-specializations.cpp?rev=283489&view=auto
==============================================================================
--- cfe/trunk/test/Modules/merge-template-specializations.cpp (added)
+++ cfe/trunk/test/Modules/merge-template-specializations.cpp Thu Oct  6 15:30:51 2016
@@ -0,0 +1,5 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fimplicit-module-maps -fmodules-local-submodule-visibility -I%S/Inputs/merge-template-specializations -std=c++11 -verify %s
+// expected-no-diagnostics
+#include "c.h"
+X x;




More information about the cfe-commits mailing list