[PATCH] D29951: Load lazily the template specialization in multi-module setups.

[Richard Smith via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: lib/Serialization/ASTReaderDecl.cpp:213-215
+      assert(isa<RedeclarableTemplateDecl>(D) ||
+             isa<TypeAliasTemplateDecl>(D) &&
+             "Decl doesn't have specializations.");
----------------
Can this ever fail at runtime? I'd expect the below code to not compile if `D` isn't one of these types.


================
Comment at: lib/Serialization/ASTReaderDecl.cpp:3911-3924
+    case UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION: {
+      // It will be added to the template's lazy specialization set when loaded.
+      SmallVector<serialization::DeclID, 1> SpecID;
+      SpecID.push_back(ReadDeclID());
+      if (auto *CTD = dyn_cast<ClassTemplateDecl>(D))
+        AddLazySpecializations(CTD, SpecID);
+      else if (auto *FTD = dyn_cast<FunctionTemplateDecl>(D))
----------------
This will end up being quadratic time and using quadratic storage if a module adds N specializations to a template; instead, please move the SmallVector out of the loop and call `AddLazySpecializations` once after reading all the update records for the declaration. (It's probably best to move it all the way out to `loadDeclUpdateRecords`, in case we have a large number of modules each adding some specializations -- eg, if module A imports std::vector and declares A, module B declares B and uses vector<A>, module C declares C and uses vector<B>, ..., we again should avoid quadratic behavior.)


https://reviews.llvm.org/D29951