[PATCH] D86514: Correctly parse LateParsedTemplates in case of multiple dependent modules

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: clang/include/clang/Serialization/ASTReader.h:903-905
   // A list of late parsed template function data.
   SmallVector<uint64_t, 1> LateParsedTemplates;
 
----------------
Given the changes below, we shouldn't need this any more, and can instead use this name for the container that carries the `ModuleFile*`.


================
Comment at: clang/include/clang/Serialization/ASTReader.h:907-908
+  // A list of LateParsedTemplates paired with their module files.
+  llvm::MapVector<ModuleFile *, SmallVector<uint64_t, 1>>
+      LateParsedTemplatesModulesMap;
+
----------------
We don't need a map here; we never need to look up the vector from the module file.


================
Comment at: clang/lib/Serialization/ASTReader.cpp:3723-3726
       LateParsedTemplates.append(Record.begin(), Record.end());
+      LateParsedTemplatesModulesMap.insert(
+          std::make_pair(&F, std::move(LateParsedTemplates)));
+      LateParsedTemplates.clear();
----------------
We can directly initialize the vector in the container rather than initializing `LateParsedTemplates` and moving it. I think this also removes the only remaining use of the old `LateParsedTemplates`.


================
Comment at: clang/lib/Serialization/ASTReader.cpp:8392
         &LPTMap) {
-  for (unsigned Idx = 0, N = LateParsedTemplates.size(); Idx < N;
-       /* In loop */) {
-    FunctionDecl *FD = cast<FunctionDecl>(GetDecl(LateParsedTemplates[Idx++]));
+  for (auto &LPT : LateParsedTemplatesModulesMap) {
+    ModuleFile *FMod = LPT.first;
----------------



================
Comment at: clang/lib/Serialization/ASTReader.cpp:8394
+    ModuleFile *FMod = LPT.first;
+    SmallVector<uint64_t, 1> LateParsed(LPT.second);
+    for (unsigned Idx = 0, N = LateParsed.size(); Idx < N;
----------------
No need to copy the vector here, and we can avoid hardcoding the preallocated `SmallVector` size.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86514/new/

https://reviews.llvm.org/D86514