Author: Aiden Grossman Date: 2026-03-13T02:45:13Z New Revision: 0ca8324184ba7bcc572edbb5c48f1361841692bc
URL: https://github.com/llvm/llvm-project/commit/0ca8324184ba7bcc572edbb5c48f1361841692bc DIFF: https://github.com/llvm/llvm-project/commit/0ca8324184ba7bcc572edbb5c48f1361841692bc.diff LOG: Revert "[Serialization] [ASTWriter] Try to not record namespace as much as possible (#179178)" This reverts commit 8eb0dfe5b6d658fa9991612060a1840927fd2eea. Breaks some clang header module targets. See the original PR for discussion/reproducers. Added: Modified: clang/include/clang/Serialization/ASTWriter.h clang/lib/Serialization/ASTWriter.cpp clang/lib/Serialization/ASTWriterDecl.cpp clang/test/Modules/no-transitive-decl-change-4.cppm Removed: clang/test/Modules/no-transitive-change-namespace.cppm clang/test/Modules/pr184957.cppm ################################################################################ diff --git a/clang/include/clang/Serialization/ASTWriter.h b/clang/include/clang/Serialization/ASTWriter.h index 88b62c7f4e6b4..0f3993ad01693 100644 --- a/clang/include/clang/Serialization/ASTWriter.h +++ b/clang/include/clang/Serialization/ASTWriter.h @@ -928,6 +928,8 @@ class ASTWriter : public ASTDeserializationListener, void handleVTable(CXXRecordDecl *RD); + void addTouchedModuleFile(serialization::ModuleFile *); + private: // ASTDeserializationListener implementation void ReaderInitialized(ASTReader *Reader) override; diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index c4815e956e53f..2744d70b89aac 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -4081,6 +4081,10 @@ void ASTWriter::handleVTable(CXXRecordDecl *RD) { PendingEmittingVTables.push_back(RD); } +void ASTWriter::addTouchedModuleFile(serialization::ModuleFile *MF) { + TouchedModuleFiles.insert(MF); +} + //===----------------------------------------------------------------------===// // DeclContext's Name Lookup Table Serialization //===----------------------------------------------------------------------===// @@ -4125,6 +4129,7 @@ class ASTDeclContextNameLookupTraitBase { "have reference to loaded module file but no chain?"); using namespace llvm::support; + Writer.addTouchedModuleFile(F); endian::write<uint32_t>(Out, Writer.getChain()->getModuleFileID(F), llvm::endianness::little); } @@ -4322,14 +4327,6 @@ static bool isTULocalInNamedModules(NamedDecl *D) { return D->getLinkageInternal() == Linkage::Internal; } -static NamedDecl *getLocalRedecl(NamedDecl *D) { - for (auto *RD : D->redecls()) - if (!RD->isFromASTFile()) - return cast<NamedDecl>(RD); - - return D; -} - class ASTDeclContextNameLookupTrait : public ASTDeclContextNameTrivialLookupTrait { public: @@ -4403,13 +4400,6 @@ class ASTDeclContextNameLookupTrait NamedDecl *DeclForLocalLookup = getDeclForLocalLookup(Writer.getLangOpts(), D); - // Namespace may have many redeclarations in many TU. - // Also, these namespaces are symmetric. So that we try to use - // the local redecl if any. - if (isa<NamespaceDecl>(DeclForLocalLookup) && - DeclForLocalLookup->isFromASTFile()) - DeclForLocalLookup = getLocalRedecl(DeclForLocalLookup); - if (Writer.getDoneWritingDeclsAndTypes() && !Writer.wasDeclEmitted(DeclForLocalLookup)) return; @@ -4531,6 +4521,7 @@ class LazySpecializationInfoLookupTrait { "have reference to loaded module file but no chain?"); using namespace llvm::support; + Writer.addTouchedModuleFile(F); endian::write<uint32_t>(Out, Writer.getChain()->getModuleFileID(F), llvm::endianness::little); } @@ -4629,16 +4620,7 @@ void ASTWriter::GenerateSpecializationInfoLookupTable( Generator.insert(HashValue, Trait.getData(Specs, ExisitingSpecs), Trait); } - // Reduced BMI may not emit everything in the lookup table, - // If Reduced BMI **partially** emits some decls, - // then the generator may not emit the corresponding entry for the - // corresponding name is already there. See - // MultiOnDiskHashTableGenerator::insert and - // MultiOnDiskHashTableGenerator::emit for details. - // So we won't emit the lookup table if we're generating reduced BMI. - auto *ToEmitMaybeMergedLookupTable = - (!isGeneratingReducedBMI() && Lookups) ? &Lookups->Table : nullptr; - Generator.emit(LookupTable, Trait, ToEmitMaybeMergedLookupTable); + Generator.emit(LookupTable, Trait, Lookups ? &Lookups->Table : nullptr); } uint64_t ASTWriter::WriteSpecializationInfoLookupTable( @@ -4835,16 +4817,7 @@ void ASTWriter::GenerateNameLookupTable( // Create the on-disk hash table. Also emit the existing imported and // merged table if there is one. auto *Lookups = Chain ? Chain->getLoadedLookupTables(DC) : nullptr; - // Reduced BMI may not emit everything in the lookup table, - // If Reduced BMI **partially** emits some decls, - // then the generator may not emit the corresponding entry for the - // corresponding name is already there. See - // MultiOnDiskHashTableGenerator::insert and - // MultiOnDiskHashTableGenerator::emit for details. - // So we won't emit the lookup table if we're generating reduced BMI. - auto *ToEmitMaybeMergedLookupTable = - (!isGeneratingReducedBMI() && Lookups) ? &Lookups->Table : nullptr; - Generator.emit(LookupTable, Trait, ToEmitMaybeMergedLookupTable); + Generator.emit(LookupTable, Trait, Lookups ? &Lookups->Table : nullptr); const auto &ModuleLocalDecls = Trait.getModuleLocalDecls(); if (!ModuleLocalDecls.empty()) { @@ -4860,15 +4833,11 @@ void ASTWriter::GenerateNameLookupTable( ModuleLocalTrait); } - // See the above comment. We won't emit the merged table if we're generating - // reduced BMI. auto *ModuleLocalLookups = - (isGeneratingReducedBMI() && Chain && - Chain->getModuleLocalLookupTables(DC)) - ? &Chain->getModuleLocalLookupTables(DC)->Table - : nullptr; - ModuleLocalLookupGenerator.emit(ModuleLocalLookupTable, ModuleLocalTrait, - ModuleLocalLookups); + Chain ? Chain->getModuleLocalLookupTables(DC) : nullptr; + ModuleLocalLookupGenerator.emit( + ModuleLocalLookupTable, ModuleLocalTrait, + ModuleLocalLookups ? &ModuleLocalLookups->Table : nullptr); } const auto &TULocalDecls = Trait.getTULocalDecls(); @@ -4884,13 +4853,9 @@ void ASTWriter::GenerateNameLookupTable( TULookupGenerator.insert(Key, TULocalTrait.getData(IDs), TULocalTrait); } - // See the above comment. We won't emit the merged table if we're generating - // reduced BMI. - auto *TULocalLookups = - (isGeneratingReducedBMI() && Chain && Chain->getTULocalLookupTables(DC)) - ? &Chain->getTULocalLookupTables(DC)->Table - : nullptr; - TULookupGenerator.emit(TULookupTable, TULocalTrait, TULocalLookups); + auto *TULocalLookups = Chain ? Chain->getTULocalLookupTables(DC) : nullptr; + TULookupGenerator.emit(TULookupTable, TULocalTrait, + TULocalLookups ? &TULocalLookups->Table : nullptr); } } diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp index d2fb6f9e883d0..7646d5d5efe00 100644 --- a/clang/lib/Serialization/ASTWriterDecl.cpp +++ b/clang/lib/Serialization/ASTWriterDecl.cpp @@ -2234,15 +2234,8 @@ void ASTDeclWriter::VisitRedeclarable(Redeclarable<T> *D) { // redecl chain. unsigned I = Record.size(); Record.push_back(0); - if (Writer.Chain) { - // Namespace can have many redeclaration in many TU. - // We try to avoid touching every redeclaration to try to - // reduce the dependencies as much as possible. - if (!isa<NamespaceDecl>(DAsT)) - AddFirstDeclFromEachModule(DAsT, /*IncludeLocal=*/ false); - else - Record.AddDeclRef(/*Decl=*/nullptr); - } + if (Writer.Chain) + AddFirstDeclFromEachModule(DAsT, /*IncludeLocal*/false); // This is the number of imported first declarations + 1. Record[I] = Record.size() - I; @@ -2272,10 +2265,8 @@ void ASTDeclWriter::VisitRedeclarable(Redeclarable<T> *D) { // // FIXME: This is not correct; when we reach an imported declaration we // won't emit its previous declaration. - if (D->getPreviousDecl() && !D->getPreviousDecl()->isFromASTFile()) - (void)Writer.GetDeclRef(D->getPreviousDecl()); - if (!MostRecent->isFromASTFile()) - (void)Writer.GetDeclRef(MostRecent); + (void)Writer.GetDeclRef(D->getPreviousDecl()); + (void)Writer.GetDeclRef(MostRecent); } else { // We use the sentinel value 0 to indicate an only declaration. Record.push_back(0); diff --git a/clang/test/Modules/no-transitive-change-namespace.cppm b/clang/test/Modules/no-transitive-change-namespace.cppm deleted file mode 100644 index f67dd8533a6df..0000000000000 --- a/clang/test/Modules/no-transitive-change-namespace.cppm +++ /dev/null @@ -1,44 +0,0 @@ -// Test that adding a new decl in a module which originally only contained a namespace -// won't break the dependency. -// -// RUN: rm -rf %t -// RUN: split-file %s %t -// -// RUN: %clang_cc1 -std=c++20 -emit-reduced-module-interface %t/Root.cppm -o %t/Root.pcm -// RUN: %clang_cc1 -std=c++20 -emit-reduced-module-interface %t/N.cppm -o %t/N.pcm \ -// RUN: -fmodule-file=Root=%t/Root.pcm -// RUN: %clang_cc1 -std=c++20 -emit-reduced-module-interface %t/N.v1.cppm -o %t/N.v1.pcm \ -// RUN: -fmodule-file=Root=%t/Root.pcm -// RUN: %clang_cc1 -std=c++20 -emit-reduced-module-interface %t/M.cppm -o %t/M.pcm \ -// RUN: -fmodule-file=N=%t/N.pcm -fmodule-file=Root=%t/Root.pcm -// RUN: %clang_cc1 -std=c++20 -emit-reduced-module-interface %t/M.cppm -o %t/M.v1.pcm \ -// RUN: -fmodule-file=N=%t/N.v1.pcm -fmodule-file=Root=%t/Root.pcm -// -// RUN: diff %t/M.pcm %t/M.v1.pcm &> /dev/null - -//--- Root.cppm -export module Root; -export namespace N { - -} - -//--- N.cppm -export module N; -import Root; -export namespace N { - -} - -//--- N.v1.cppm -export module N; -import Root; -export namespace N { -struct NN {}; -} - -//--- M.cppm -export module M; -import N; -export namespace N { -struct MM {}; -} diff --git a/clang/test/Modules/no-transitive-decl-change-4.cppm b/clang/test/Modules/no-transitive-decl-change-4.cppm index bb7e47f32669e..944878be8df63 100644 --- a/clang/test/Modules/no-transitive-decl-change-4.cppm +++ b/clang/test/Modules/no-transitive-decl-change-4.cppm @@ -27,7 +27,7 @@ // RUN: %clang_cc1 -std=c++20 -emit-reduced-module-interface %t/B.cppm -o %t/B.v1.pcm \ // RUN: -fprebuilt-module-path=%t -fmodule-file=AWrapper=%t/AWrapper.v1.pcm -fmodule-file=A=%t/A.v1.pcm // -// RUN: diff %t/B.pcm %t/B.v1.pcm &> /dev/null +// RUN: not diff %t/B.pcm %t/B.v1.pcm &> /dev/null //--- T.cppm export module T; diff --git a/clang/test/Modules/pr184957.cppm b/clang/test/Modules/pr184957.cppm deleted file mode 100644 index c08c10d978938..0000000000000 --- a/clang/test/Modules/pr184957.cppm +++ /dev/null @@ -1,216 +0,0 @@ -// RUN: rm -rf %t -// RUN: mkdir %t -// RUN: split-file %s %t -// RUN: mkdir %t/tmp -// -// RUN: %clang_cc1 -std=c++23 -nostdinc++ -I %t/std %t/wrap.std.tt.cppm -emit-reduced-module-interface -o %t/wrap.std.tt.pcm -// RUN: %clang_cc1 -std=c++23 -nostdinc++ -I %t/std %t/wrap.std.vec.cppm -emit-reduced-module-interface -o %t/wrap.std.vec.pcm -// RUN: %clang_cc1 -std=c++23 -nostdinc++ -I %t/std %t/not_std.cppm -emit-reduced-module-interface -o %t/not_std.pcm -// RUN: %clang_cc1 -std=c++23 -nostdinc++ -I %t/std %t/wrap.std.tt2.cppm -emit-reduced-module-interface -o %t/wrap.std.tt2.pcm -// RUN: %clang_cc1 -std=c++23 -nostdinc++ -I %t/std -fmodule-file=wrap.std.vec=%t/wrap.std.vec.pcm %t/wrap.std.vec.reexport.cppm -emit-reduced-module-interface -o %t/wrap.std.vec.reexport.pcm -// RUN: %clang_cc1 -std=c++23 -nostdinc++ -I %t/std -fmodule-file=wrap.std.tt=%t/wrap.std.tt.pcm -fmodule-file=wrap.std.vec=%t/wrap.std.vec.pcm -fmodule-file=wrap.std.vec.reexport=%t/wrap.std.vec.reexport.pcm -fmodule-file=wrap.std.tt2=%t/wrap.std.tt2.pcm -fmodule-file=not_std=%t/not_std.pcm %t/k.repro_dep.cppm -emit-reduced-module-interface -o %t/k.repro_dep.pcm -// RUN: %clang_cc1 -std=c++23 -nostdinc++ -I %t/std -fmodule-file=wrap.std.tt=%t/wrap.std.tt.pcm -fmodule-file=wrap.std.vec=%t/wrap.std.vec.pcm -fmodule-file=wrap.std.tt2=%t/wrap.std.tt2.pcm -fmodule-file=wrap.std.vec.reexport=%t/wrap.std.vec.reexport.pcm -fmodule-file=not_std=%t/not_std.pcm -fmodule-file=k.repro_dep=%t/k.repro_dep.pcm %t/k.repro.cxx -fsyntax-only -verify - -//--- std/allocator.h -#ifndef _LIBCPP___MEMORY_ALLOCATOR_H -#define _LIBCPP___MEMORY_ALLOCATOR_H - -#include <size_t.h> - -namespace std { - -enum align_val_t { __zero = 0, __max = (size_t)-1 }; - -template <class _Tp> -inline _Tp* -__libcpp_allocate(size_t __n, [[__maybe_unused__]] size_t __align = 8) { - size_t __size = static_cast<size_t>(__n) * sizeof(_Tp); - return static_cast<_Tp*>(__builtin_operator_new(__size, static_cast<align_val_t>(__align))); -} - -template <class _Tp> -class allocator -{ -public: - typedef _Tp value_type; - - [[__nodiscard__]] _Tp* allocate(size_t __n) { - if (__builtin_is_constant_evaluated()) { - return static_cast<_Tp*>(::operator new(__n * sizeof(_Tp))); - } else { - return std::__libcpp_allocate<_Tp>(size_t(__n)); - } - } -}; - -template <class _Tp, class _Up> -inline bool -operator==(const allocator<_Tp>&, const allocator<_Up>&) noexcept { - return true; -} - -} - -#endif // _LIBCPP___MEMORY_ALLOCATOR_H - -//--- std/sys_wchar.h -#ifndef _WCHAR_H -#define _WCHAR_H 1 - -#ifndef __FILE_defined - #define __FILE_defined 1 - - struct _IO_FILE; - - typedef struct _IO_FILE FILE; -#endif - -#endif /* wchar.h */ - -//--- std/char_traits.h -#ifndef _LIBCPP___STRING_CHAR_TRAITS_H -#define _LIBCPP___STRING_CHAR_TRAITS_H - -namespace std { - -template <class _Tp> -inline size_t constexpr __constexpr_strlen(const _Tp* __str) noexcept { - if (__builtin_is_constant_evaluated()) { - size_t __i = 0; - for (; __str[__i] != '\0'; ++__i) - ; - return __i; - } - return 0; -} - -template <class _CharT> -struct char_traits; - -template <> -struct char_traits<char> { - using char_type = char; - - [[__nodiscard__]] static inline size_t constexpr - length(const char_type* __s) noexcept { - return std::__constexpr_strlen(__s); - } -}; - -} - -#endif // _LIBCPP___STRING_CHAR_TRAITS_H - -//--- std/type_traits -#ifndef _LIBCPP_TYPE_TRAITS -#define _LIBCPP_TYPE_TRAITS - -namespace std -{} - -#endif // _LIBCPP_TYPE_TRAITS - -//--- std/ptr diff _t.h -#ifndef _LIBCPP___CSTDDEF_PTRDIFF_T_H -#define _LIBCPP___CSTDDEF_PTRDIFF_T_H - -namespace std { - -using ptr diff _t = decltype(static_cast<int*>(nullptr) - static_cast<int*>(nullptr)); - -} - -#endif // _LIBCPP___CSTDDEF_PTRDIFF_T_H - -//--- std/size_t.h -#ifndef _LIBCPP___CSTDDEF_SIZE_T_H -#define _LIBCPP___CSTDDEF_SIZE_T_H - -namespace std { - -using size_t = decltype(sizeof(int)); - -} - -#endif // _LIBCPP___CSTDDEF_SIZE_T_H - -//--- wrap.std.tt.cppm -module; - -#include <type_traits> - -export module wrap.std.tt; - -//--- wrap.std.vec.cppm -module; - -#include <allocator.h> -#include <sys_wchar.h> - -export module wrap.std.vec; - -//--- not_std.cppm -module; - -#include <allocator.h> - - -export module not_std; - -namespace std { - template <class _Tp, class _Allocator> - class __split_buffer { - public: - __split_buffer() { - _Allocator a; - (void)a.allocate(1); - } - }; -} - -export namespace std { - template <typename T> - using problem = std::__split_buffer<T, std::allocator<T>>; -} - -export using ::operator new; - -//--- wrap.std.tt2.cppm -module; - -#include <type_traits> - -export module wrap.std.tt2; - -//--- wrap.std.vec.reexport.cppm -export module wrap.std.vec.reexport; - -export import wrap.std.vec; - -//--- k.repro_dep.cppm -module; - -#include <allocator.h> -#include <sys_wchar.h> -#include <char_traits.h> - -export module k.repro_dep; - -import wrap.std.tt; -import wrap.std.vec.reexport; -import wrap.std.vec; -import wrap.std.tt2; - -import not_std; - -using XYZ = std::char_traits<char>; - -//--- k.repro.cxx -// expected-no-diagnostics -import not_std; -import k.repro_dep; - -auto f() -> void -{ - std::problem< int > x; -} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
