llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-modules
Author: None (Sterling-Augustine)
<details>
<summary>Changes</summary>
pr/171769 added a nice optimization for namespace lookups, but has a bug where
if a key decl does not also appear in the chain, it will be omitted from the
module. Fix that.
This may not be exactly the correct fix, but it does resolve the issue
described in pr/171769. Another possible fix is to ensure that the key decl
always appears in the chain. But I suspect what is happening is that it isn't
written out the first time the ast file is written.
I still don't have a good reduced test case for this--I actually know almost
nothing at all about Clang modules and KeyDecls. Perhaps someone more familiar
with how it all works can propose one.
The symptom this fixes is below. The description isn't enough to reproduce the
problem, unfortunately, and I haven't been able to reduce it beyond several
hundred files.
header_b.h:
```
#include "header_a.h" // Declares namespace_a, and various things inside it.
namespace namespace_b = namespace_a;
```
and the error is:
```
<header_b.h>:13:34: error: declaration of 'namespace_a' must be imported
from module 'module_c' before it is required
13 | namespace namespace_b = ::<namespace_a>;
| ^
1 error generated.
```
Module_c declares more things in namespace_a, as do many other modules.
---
Full diff: https://github.com/llvm/llvm-project/pull/172898.diff
1 Files Affected:
- (modified) clang/lib/Serialization/ASTWriter.cpp (+1-2)
``````````diff
diff --git a/clang/lib/Serialization/ASTWriter.cpp
b/clang/lib/Serialization/ASTWriter.cpp
index 234615522e773..4154584cc30e2 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -4444,6 +4444,7 @@ class ASTDeclContextNameLookupTrait
};
ASTReader *Chain = Writer.getChain();
for (NamedDecl *D : Decls) {
+ AddDecl(D);
if (Chain && isa<NamespaceDecl>(D) && D->isFromASTFile() &&
D == Chain->getKeyDeclaration(D)) {
// In ASTReader, we stored only the key declaration of a namespace decl
@@ -4456,8 +4457,6 @@ class ASTDeclContextNameLookupTrait
Chain->forEachImportedKeyDecl(D, [&AddDecl](const Decl *D) {
AddDecl(cast<NamedDecl>(const_cast<Decl *>(D)));
});
- } else {
- AddDecl(D);
}
}
return std::make_pair(Start, DeclIDs.size());
``````````
</details>
https://github.com/llvm/llvm-project/pull/172898
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
