[clang] [Serialization] Fix lazy template loading (PR #133057)

[Jonas Hahnfeld via cfe-commits]

hahnjo wrote:

> As I hinted above, that doesn't work. More specifically, I tried
> 
> ```diff
> diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp 
> b/clang/lib/Serialization/ASTReaderDecl.cpp
> index 882d54f31280..d63824c4ba18 100644
> --- a/clang/lib/Serialization/ASTReaderDecl.cpp
> +++ b/clang/lib/Serialization/ASTReaderDecl.cpp
> @@ -2107,8 +2107,9 @@ void ASTDeclMerger::MergeDefinitionData(
>      auto *Def = DD.Definition;
>      DD = std::move(MergeDD);
>      DD.Definition = Def;
> -    for (auto *D : Def->redecls())
> -      cast<CXXRecordDecl>(D)->DefinitionData = &DD;
> +    Def = Def->getMostRecentDecl();
> +    while ((Def = Def->getPreviousDecl()))
> +      cast<CXXRecordDecl>(Def)->DefinitionData = &DD;
>      return;
>    }
>  
> ```

I'm stupid, the condition during the first iteration of the `while` loop will 
skip over the most recent declaration :dizzy_face: the following works:
```diff
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -2107,8 +2107,10 @@ void ASTDeclMerger::MergeDefinitionData(
     auto *Def = DD.Definition;
     DD = std::move(MergeDD);
     DD.Definition = Def;
-    for (auto *D : Def->redecls())
-      cast<CXXRecordDecl>(D)->DefinitionData = &DD;
+    Def = Def->getMostRecentDecl();
+    do {
+      cast<CXXRecordDecl>(Def)->DefinitionData = &DD;
+    } while ((Def = Def->getPreviousDecl()));
     return;
   }
 
```

> You can't use `getMostRecentDecl `, this will trigger deserializations. Then 
> it is the same with before. You need to implement `noload_redecls()`, 
> possibly in Redeclarable.h

If I understand correctly, we could implement this locally in 
`ASTReaderDecl.cpp`, matching a number of patterns in 
`ASTReader::finishPendingActions()`:
```diff
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp 
b/clang/lib/Serialization/ASTReaderDecl.cpp
index 882d54f31280..ec7c2449d77e 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -2107,8 +2107,10 @@ void ASTDeclMerger::MergeDefinitionData(
     auto *Def = DD.Definition;
     DD = std::move(MergeDD);
     DD.Definition = Def;
-    for (auto *D : Def->redecls())
-      cast<CXXRecordDecl>(D)->DefinitionData = &DD;
+    // FIXME: this duplicates logic from ASTReader::finishPendingActions()...
+    for (auto *R = Reader.getMostRecentExistingDecl(Def); R;
+         R = R->getPreviousDecl())
+      cast<CXXRecordDecl>(R)->DefinitionData = &DD;
     return;
   }
 
```
... which then of course begs the question why the existing code in 
`ASTReader::finishPendingActions()` doesn't already take care of it?

Another possibility is to use `merged_decls()`:
```diff
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp 
b/clang/lib/Serialization/ASTReaderDecl.cpp
index 882d54f31280..1112c64eeb02 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -2107,8 +2107,8 @@ void ASTDeclMerger::MergeDefinitionData(
     auto *Def = DD.Definition;
     DD = std::move(MergeDD);
     DD.Definition = Def;
-    for (auto *D : Def->redecls())
-      cast<CXXRecordDecl>(D)->DefinitionData = &DD;
+    for (auto *D : merged_redecls(Def))
+      D->DefinitionData = &DD;
     return;
   }
 
```

Which of these alternatives is preferable (assuming one of them actually works 
for @alexfh)?

https://github.com/llvm/llvm-project/pull/133057
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits