llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-modules Author: llvmbot <details> <summary>Changes</summary> Backport c97e08e331736ae8c7d17bf1f24954570f564ad0 Requested by: @<!-- -->ChuanqiXu9 --- Full diff: https://github.com/llvm/llvm-project/pull/192885.diff 7 Files Affected: - (modified) clang/include/clang/AST/DeclBase.h (+14-1) - (modified) clang/include/clang/AST/DeclContextInternals.h (+3-1) - (modified) clang/lib/AST/Decl.cpp (+1) - (modified) clang/lib/AST/DeclBase.cpp (+2-1) - (modified) clang/lib/Sema/SemaLookup.cpp (+1-1) - (modified) clang/lib/Serialization/ASTReaderDecl.cpp (+1) - (added) clang/test/Modules/include-between-imports-enums.cppm (+47) ``````````diff diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h index 5519787d71f88..3c5a093439ceb 100644 --- a/clang/include/clang/AST/DeclBase.h +++ b/clang/include/clang/AST/DeclBase.h @@ -228,6 +228,14 @@ class alignas(8) Decl { /// module is imported. VisibleWhenImported, + /// This declaration has an owning module, and is not visible to the + /// current TU but we promoted it to be visible for various reasons, + /// e.g., we have the same declaration in the current TU but we'd + /// like to avoid parsing it again. + /// + /// The vibility should be never be serialized. + VisiblePromoted, + /// This declaration has an owning module, and is visible to lookups /// that occurs within that module. And it is reachable in other module /// when the owning module is transitively imported. @@ -668,7 +676,7 @@ class alignas(8) Decl { bool isInExportDeclContext() const; bool isInvisibleOutsideTheOwningModule() const { - return getModuleOwnershipKind() > ModuleOwnershipKind::VisibleWhenImported; + return getModuleOwnershipKind() > ModuleOwnershipKind::VisiblePromoted; } /// Whether this declaration comes from another module unit. @@ -872,6 +880,11 @@ class alignas(8) Decl { setModuleOwnershipKind(ModuleOwnershipKind::Visible); } + void setVisiblePromoted() { + if (isInvisibleOutsideTheOwningModule() && isFromASTFile()) + setModuleOwnershipKind(ModuleOwnershipKind::VisiblePromoted); + } + /// Get the kind of module ownership for this declaration. ModuleOwnershipKind getModuleOwnershipKind() const { return NextInContextAndBits.getInt(); diff --git a/clang/include/clang/AST/DeclContextInternals.h b/clang/include/clang/AST/DeclContextInternals.h index a0f37886cc014..d87d8e8a663fa 100644 --- a/clang/include/clang/AST/DeclContextInternals.h +++ b/clang/include/clang/AST/DeclContextInternals.h @@ -174,7 +174,9 @@ class StoredDeclsList { // Remove all declarations that are either external or are replaced with // external declarations with higher visibilities. DeclListNode::Decls *Tail = erase_if([Decls](NamedDecl *ND) { - if (ND->isFromASTFile()) + // If the declaration is promoted intentionally, keep it. + if (ND->isFromASTFile() && ND->getModuleOwnershipKind() != + Decl::ModuleOwnershipKind::VisiblePromoted) return true; // FIXME: Can we get rid of this loop completely? return llvm::any_of(Decls, [ND](NamedDecl *D) { diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index 66c625f41158a..231fc381bda8e 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -1193,6 +1193,7 @@ static bool isExportedFromModuleInterfaceUnit(const NamedDecl *D) { case Decl::ModuleOwnershipKind::Unowned: case Decl::ModuleOwnershipKind::ReachableWhenImported: case Decl::ModuleOwnershipKind::ModulePrivate: + case Decl::ModuleOwnershipKind::VisiblePromoted: return false; case Decl::ModuleOwnershipKind::Visible: case Decl::ModuleOwnershipKind::VisibleWhenImported: diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp index 0a1e442656c35..18b6a5b06bab8 100644 --- a/clang/lib/AST/DeclBase.cpp +++ b/clang/lib/AST/DeclBase.cpp @@ -402,7 +402,8 @@ void Decl::setLexicalDeclContext(DeclContext *DC) { } assert( - (getModuleOwnershipKind() != ModuleOwnershipKind::VisibleWhenImported || + ((getModuleOwnershipKind() != ModuleOwnershipKind::VisibleWhenImported && + getModuleOwnershipKind() != ModuleOwnershipKind::VisiblePromoted) || getOwningModule()) && "hidden declaration has no owning module"); } diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp index b9fac5a4a1153..7c5124a38d811 100644 --- a/clang/lib/Sema/SemaLookup.cpp +++ b/clang/lib/Sema/SemaLookup.cpp @@ -1575,7 +1575,7 @@ void Sema::makeMergedDefinitionVisible(NamedDecl *ND) { if (auto *ED = dyn_cast<EnumDecl>(ND); ED && ED->isFromGlobalModule() && !ED->isScoped()) { for (auto *ECD : ED->enumerators()) { - ECD->setVisibleDespiteOwningModule(); + ECD->setVisiblePromoted(); DeclContext *RedeclCtx = ED->getDeclContext()->getRedeclContext(); if (RedeclCtx->lookup(ECD->getDeclName()).empty()) RedeclCtx->makeDeclVisibleInContext(ECD); diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index f8e9caa3f5d1d..45ba4231204c5 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -636,6 +636,7 @@ void ASTDeclReader::VisitDecl(Decl *D) { break; case Decl::ModuleOwnershipKind::Unowned: case Decl::ModuleOwnershipKind::VisibleWhenImported: + case Decl::ModuleOwnershipKind::VisiblePromoted: case Decl::ModuleOwnershipKind::ReachableWhenImported: case Decl::ModuleOwnershipKind::ModulePrivate: break; diff --git a/clang/test/Modules/include-between-imports-enums.cppm b/clang/test/Modules/include-between-imports-enums.cppm new file mode 100644 index 0000000000000..547848e4057a9 --- /dev/null +++ b/clang/test/Modules/include-between-imports-enums.cppm @@ -0,0 +1,47 @@ +// RUN: rm -rf %t +// RUN: mkdir -p %t +// RUN: split-file %s %t +// +// RUN: %clang_cc1 -std=c++20 %t/A.cppm -emit-module-interface -o %t/A.pcm +// RUN: %clang_cc1 -std=c++20 %t/B.cppm -emit-module-interface -o %t/B.pcm +// RUN: %clang_cc1 -std=c++20 %t/use.cpp -fprebuilt-module-path=%t -verify -fsyntax-only +// +// RUN: %clang_cc1 -std=c++20 %t/A.cppm -emit-reduced-module-interface -o %t/A.pcm +// RUN: %clang_cc1 -std=c++20 %t/B.cppm -emit-reduced-module-interface -o %t/B.pcm +// RUN: %clang_cc1 -std=c++20 %t/use.cpp -fprebuilt-module-path=%t -verify -fsyntax-only + +// Test that a textual #include sandwiched between two import declarations +// of modules that both include the same header in their GMFs does not lose +// enum declarations. See https://github.com/llvm/llvm-project/issues/188853 + +//--- enum.h +#ifndef ENUM_H +#define ENUM_H +namespace ns { +enum E { Value1, Value2, Value3 }; +} +#endif + +//--- A.cppm +module; +#include "enum.h" +export module A; +export auto a = ns::Value1; + +//--- B.cppm +module; +#include "enum.h" +export module B; +export auto b = ns::Value2; + +//--- use.cpp +// expected-no-diagnostics +import A; +#include "enum.h" +import B; + +auto x = ns::Value3; + +namespace ns { +auto y = Value1; +} `````````` </details> https://github.com/llvm/llvm-project/pull/192885 _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
