https://github.com/ian-twilightcoder updated https://github.com/llvm/llvm-project/pull/89005
>From f6bcc20d07248069dee1ff19c1aa334152b311a8 Mon Sep 17 00:00:00 2001 From: Ian Anderson <i...@apple.com> Date: Tue, 16 Apr 2024 17:08:28 -0700 Subject: [PATCH] [clang][modules] HeaderSearch::MarkFileModuleHeader sets textual headers' HeaderFileInfo non-external when it shouldn't HeaderSearch::MarkFileModuleHeader is no longer properly checking for no-changes, and so sets the HeaderFileInfo for every `textual header` to non-external. --- clang/include/clang/Lex/HeaderSearch.h | 4 +- clang/lib/Lex/HeaderSearch.cpp | 14 +++-- clang/unittests/Lex/HeaderSearchTest.cpp | 67 ++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 4 deletions(-) diff --git a/clang/include/clang/Lex/HeaderSearch.h b/clang/include/clang/Lex/HeaderSearch.h index 5ac63dddd4d4e..d8ca1c528de36 100644 --- a/clang/include/clang/Lex/HeaderSearch.h +++ b/clang/include/clang/Lex/HeaderSearch.h @@ -90,7 +90,9 @@ struct HeaderFileInfo { LLVM_PREFERRED_TYPE(bool) unsigned isModuleHeader : 1; - /// Whether this header is a `textual header` in a module. + /// Whether this header is a `textual header` in a module. If a header is + /// textual in one module and normal in another module, this bit will not be + /// set, only `isModuleHeader`. LLVM_PREFERRED_TYPE(bool) unsigned isTextualModuleHeader : 1; diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp index 574723b33866a..18c0bab4a81e4 100644 --- a/clang/lib/Lex/HeaderSearch.cpp +++ b/clang/lib/Lex/HeaderSearch.cpp @@ -1313,11 +1313,19 @@ OptionalFileEntryRef HeaderSearch::LookupSubframeworkHeader( // File Info Management. //===----------------------------------------------------------------------===// +static bool moduleMembershipNeedsMerge(const HeaderFileInfo *HFI, + ModuleMap::ModuleHeaderRole Role) { + if (ModuleMap::isModular(Role)) + return !HFI->isModuleHeader || HFI->isTextualModuleHeader; + else if (!HFI->isModuleHeader && (Role & ModuleMap::TextualHeader)) + return !HFI->isTextualModuleHeader; + else + return false; +} + static void mergeHeaderFileInfoModuleBits(HeaderFileInfo &HFI, bool isModuleHeader, bool isTextualModuleHeader) { - assert((!isModuleHeader || !isTextualModuleHeader) && - "A header can't build with a module and be textual at the same time"); HFI.isModuleHeader |= isModuleHeader; if (HFI.isModuleHeader) HFI.isTextualModuleHeader = false; @@ -1432,7 +1440,7 @@ void HeaderSearch::MarkFileModuleHeader(FileEntryRef FE, if ((Role & ModuleMap::ExcludedHeader)) return; auto *HFI = getExistingFileInfo(FE); - if (HFI && HFI->isModuleHeader) + if (HFI && !moduleMembershipNeedsMerge(HFI, Role)) return; } diff --git a/clang/unittests/Lex/HeaderSearchTest.cpp b/clang/unittests/Lex/HeaderSearchTest.cpp index c578fa72c859e..b55d52df14d5d 100644 --- a/clang/unittests/Lex/HeaderSearchTest.cpp +++ b/clang/unittests/Lex/HeaderSearchTest.cpp @@ -308,5 +308,72 @@ TEST_F(HeaderSearchTest, HeaderMapFrameworkLookup) { EXPECT_EQ(Search.getIncludeNameForHeader(FE), "Foo/Foo.h"); } +TEST_F(HeaderSearchTest, HeaderFileInfoMerge) { + auto AddHeader = [&](std::string HeaderPath) -> FileEntryRef { + VFS->addFile(HeaderPath, 0, + llvm::MemoryBuffer::getMemBufferCopy("", HeaderPath), + /*User=*/std::nullopt, /*Group=*/std::nullopt, + llvm::sys::fs::file_type::regular_file); + return *Search.LookupFile( + HeaderPath, SourceLocation(), /*isAngled=*/false, /*FromDir=*/nullptr, + /*CurDir=*/nullptr, /*Includers=*/{}, /*SearchPath=*/nullptr, + /*RelativePath=*/nullptr, /*RequestingModule=*/nullptr, + /*SuggestedModule=*/nullptr, /*IsMapped=*/nullptr, + /*IsFrameworkFound=*/nullptr); + }; + + class MockExternalHeaderFileInfoSource : public ExternalHeaderFileInfoSource { + HeaderFileInfo GetHeaderFileInfo(FileEntryRef FE) { + HeaderFileInfo HFI; + auto FileName = FE.getName(); + if (FileName == ModularPath) + HFI.mergeModuleMembership(ModuleMap::NormalHeader); + else if (FileName == TextualPath) + HFI.mergeModuleMembership(ModuleMap::TextualHeader); + HFI.External = true; + HFI.IsValid = true; + return HFI; + } + + public: + std::string ModularPath = "/modular.h"; + std::string TextualPath = "/textual.h"; + }; + + auto ExternalSource = new MockExternalHeaderFileInfoSource(); + Search.SetExternalSource(ExternalSource); + + // Everything should start out external. + auto ModularFE = AddHeader(ExternalSource->ModularPath); + auto TextualFE = AddHeader(ExternalSource->TextualPath); + EXPECT_TRUE(Search.getExistingFileInfo(ModularFE)->External); + EXPECT_TRUE(Search.getExistingFileInfo(TextualFE)->External); + + // Marking the same role should keep it external + Search.MarkFileModuleHeader(ModularFE, ModuleMap::NormalHeader, /*isCompilingModuleHeader=*/false); + Search.MarkFileModuleHeader(TextualFE, ModuleMap::TextualHeader, /*isCompilingModuleHeader=*/false); + EXPECT_TRUE(Search.getExistingFileInfo(ModularFE)->External); + EXPECT_TRUE(Search.getExistingFileInfo(TextualFE)->External); + + // textual -> modular should update the HFI, but modular -> textual should be + // a no-op. + Search.MarkFileModuleHeader(ModularFE, ModuleMap::TextualHeader, /*isCompilingModuleHeader=*/false); + Search.MarkFileModuleHeader(TextualFE, ModuleMap::NormalHeader, /*isCompilingModuleHeader=*/false); + auto ModularFI = Search.getExistingFileInfo(ModularFE); + auto TextualFI = Search.getExistingFileInfo(TextualFE); + EXPECT_TRUE(ModularFI->External); + EXPECT_TRUE(ModularFI->isModuleHeader); + EXPECT_FALSE(ModularFI->isTextualModuleHeader); + EXPECT_FALSE(TextualFI->External); + EXPECT_TRUE(ModularFI->isModuleHeader); + EXPECT_FALSE(ModularFI->isTextualModuleHeader); + + // Compiling the module should make the HFI local. + Search.MarkFileModuleHeader(ModularFE, ModuleMap::NormalHeader, /*isCompilingModuleHeader=*/true); + Search.MarkFileModuleHeader(TextualFE, ModuleMap::NormalHeader, /*isCompilingModuleHeader=*/true); + EXPECT_FALSE(Search.getExistingFileInfo(ModularFE)->External); + EXPECT_FALSE(Search.getExistingFileInfo(TextualFE)->External); +} + } // namespace } // namespace clang _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits