https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/187653
>From 2989b6afaa95ccba4b39b3849a0f9956ef7564f4 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu <[email protected]> Date: Fri, 20 Mar 2026 13:53:23 +0800 Subject: [PATCH 1/2] [clangd] [C++ Modules] Enable content validation for module input files The IsModuleFileUpToDate function was not properly validating input files for C++20 modules. By default, ASTReader skips input file validation for StandardCXXModule files unless ForceCheckCXX20ModulesInputFiles and ValidateASTInputFilesContent are both set. This change: - Passes ValidateASTInputFilesContent=true to ASTReader constructor - Uses ARR_OutOfDate flag for cleaner error handling - Simplifies the validation logic (ReadAST already validates internally) - Adds a test to verify header changes in module units are detected Assised with AI. --- clang-tools-extra/clangd/ModulesBuilder.cpp | 33 +++++---- .../unittests/PrerequisiteModulesTest.cpp | 70 +++++++++++++++++++ 2 files changed, 86 insertions(+), 17 deletions(-) diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp index 950392b91ac7c..387fe7ce0895a 100644 --- a/clang-tools-extra/clangd/ModulesBuilder.cpp +++ b/clang-tools-extra/clangd/ModulesBuilder.cpp @@ -269,28 +269,27 @@ bool IsModuleFileUpToDate(PathRef ModuleFilePath, PCHContainerOperations PCHOperations; CodeGenOptions CodeGenOpts; ASTReader Reader(PP, *ModCache, /*ASTContext=*/nullptr, - PCHOperations.getRawReader(), CodeGenOpts, {}); + PCHOperations.getRawReader(), CodeGenOpts, {}, + /*isysroot=*/"", + /*DisableValidationKind=*/DisableValidationForModuleKind::None, + /*AllowASTWithCompilerErrors=*/false, + /*AllowConfigurationMismatch=*/false, + /*ValidateSystemInputs=*/false, + /*ForceValidateUserInputs=*/true, + /*ValidateASTInputFilesContent=*/true); // We don't need any listener here. By default it will use a validator // listener. Reader.setListener(nullptr); - if (Reader.ReadAST(ModuleFileName::makeExplicit(ModuleFilePath), - serialization::MK_MainFile, SourceLocation(), - ASTReader::ARR_None) != ASTReader::Success) - return false; - - bool UpToDate = true; - Reader.getModuleManager().visit([&](serialization::ModuleFile &MF) -> bool { - Reader.visitInputFiles( - MF, /*IncludeSystem=*/false, /*Complain=*/false, - [&](const serialization::InputFile &IF, bool isSystem) { - if (!IF.getFile() || IF.isOutOfDate()) - UpToDate = false; - }); - return !UpToDate; - }); - return UpToDate; + // Use ARR_OutOfDate so that ReadAST returns OutOfDate instead of Failure + // when input files are modified. This allows us to detect staleness + // without treating it as a hard error. + // ReadAST will validate all input files internally and return OutOfDate + // if any file is modified. + return Reader.ReadAST(ModuleFileName::makeExplicit(ModuleFilePath), + serialization::MK_MainFile, SourceLocation(), + ASTReader::ARR_OutOfDate) == ASTReader::Success; } bool IsModuleFilesUpToDate( diff --git a/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp b/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp index 3132959a5967c..7cda291d3d3b7 100644 --- a/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp +++ b/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp @@ -644,6 +644,76 @@ import M; EXPECT_EQ(CDB.getGlobalScanningCount(), 1u); } +// Test that canReuse detects changes to headers included in module units. +// This verifies that the ASTReader correctly tracks header file dependencies +// in BMI files and that IsModuleFileUpToDate correctly validates them. +TEST_F(PrerequisiteModulesTests, CanReuseWithHeadersInModuleUnit) { + MockDirectoryCompilationDatabase CDB(TestDir, FS); + + // Create a header file that will be included in a module unit + CDB.addFile("header1.h", R"cpp( +inline int getValue() { return 42; } + )cpp"); + + // Module M includes header1.h in the global module fragment + CDB.addFile("M.cppm", R"cpp( +module; +#include "header1.h" +export module M; +export int m_value = getValue(); + )cpp"); + + // Module N imports M (similar structure to ReusabilityTest) + CDB.addFile("N.cppm", R"cpp( +export module N; +import :Part; +import M; + )cpp"); + + // Add a module partition (similar to ReusabilityTest) + CDB.addFile("N-part.cppm", R"cpp( +export module N:Part; + )cpp"); + + ModulesBuilder Builder(CDB); + + // Build prerequisite modules for N (which depends on M) + auto NInfo = Builder.buildPrerequisiteModulesFor(getFullPath("N.cppm"), FS); + EXPECT_TRUE(NInfo); + + ParseInputs NInput = getInputs("N.cppm", CDB); + std::unique_ptr<CompilerInvocation> Invocation = + buildCompilerInvocation(NInput, DiagConsumer); + + // Initially, canReuse should return true + EXPECT_TRUE(NInfo->canReuse(*Invocation, FS.view(TestDir))); + + // Test 1: Modify header1.h (included by M) + // canReuse should detect this change since M's BMI records header1.h as input + CDB.addFile("header1.h", R"cpp( +inline int getValue() { return 43; } + )cpp"); + EXPECT_FALSE(NInfo->canReuse(*Invocation, FS.view(TestDir))); + + // Rebuild and verify canReuse returns true again + NInfo = Builder.buildPrerequisiteModulesFor(getFullPath("N.cppm"), FS); + EXPECT_TRUE(NInfo->canReuse(*Invocation, FS.view(TestDir))); + + // Test 2: Modify the module source file itself + CDB.addFile("M.cppm", R"cpp( +module; +#include "header1.h" +export module M; +export int m_value = getValue(); +export int m_new_value = 10; + )cpp"); + EXPECT_FALSE(NInfo->canReuse(*Invocation, FS.view(TestDir))); + + // Rebuild after module source change + NInfo = Builder.buildPrerequisiteModulesFor(getFullPath("N.cppm"), FS); + EXPECT_TRUE(NInfo->canReuse(*Invocation, FS.view(TestDir))); +} + TEST_F(PrerequisiteModulesTests, PrebuiltModuleFileTest) { MockDirectoryCompilationDatabase CDB(TestDir, FS); >From 4ce58116ae8f345adc51ec5b845e1e7e810499e4 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu <[email protected]> Date: Mon, 23 Mar 2026 11:21:40 +0800 Subject: [PATCH 2/2] Update --- .../clangd/unittests/PrerequisiteModulesTest.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp b/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp index 7cda291d3d3b7..f072c299cad22 100644 --- a/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp +++ b/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp @@ -679,7 +679,7 @@ export module N:Part; // Build prerequisite modules for N (which depends on M) auto NInfo = Builder.buildPrerequisiteModulesFor(getFullPath("N.cppm"), FS); - EXPECT_TRUE(NInfo); + ASSERT_TRUE(NInfo); ParseInputs NInput = getInputs("N.cppm", CDB); std::unique_ptr<CompilerInvocation> Invocation = @@ -697,6 +697,7 @@ inline int getValue() { return 43; } // Rebuild and verify canReuse returns true again NInfo = Builder.buildPrerequisiteModulesFor(getFullPath("N.cppm"), FS); + ASSERT_TRUE(NInfo); EXPECT_TRUE(NInfo->canReuse(*Invocation, FS.view(TestDir))); // Test 2: Modify the module source file itself @@ -711,6 +712,7 @@ export int m_new_value = 10; // Rebuild after module source change NInfo = Builder.buildPrerequisiteModulesFor(getFullPath("N.cppm"), FS); + ASSERT_TRUE(NInfo); EXPECT_TRUE(NInfo->canReuse(*Invocation, FS.view(TestDir))); } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
