https://github.com/cyndyishida updated https://github.com/llvm/llvm-project/pull/190203
>From 20939a4ee1b2eff6a143fd857b8034d86320d023 Mon Sep 17 00:00:00 2001 From: Cyndy Ishida <[email protected]> Date: Tue, 24 Mar 2026 08:14:56 -0700 Subject: [PATCH 1/2] [Modules] Enrich diags for out of date module deps Replace the opaque `ErrorStr` parameter in `ModuleManager::addModule` with a structured object that carries specific information for reporting. This allows the diagnostics to emit targeted notes instead of a single error string appended to the main diagnostic. Information that is relevant but is tied to implementation details of the compilation is reported as notes (e.g. signature mismatches) This patch additionally * Adds minor comments to places I found unintuitive * Adds a contrived test case for signature mismatches * Sets `InputFilesValidation` based on module kind at construction --- .../Basic/DiagnosticSerializationKinds.td | 8 +- .../include/clang/Serialization/ModuleFile.h | 22 ++++- .../clang/Serialization/ModuleManager.h | 87 +++++++++++----- clang/lib/Serialization/ASTReader.cpp | 51 +++++----- clang/lib/Serialization/ModuleManager.cpp | 99 +++++++++++-------- .../modules-pch-signature-mismatch.c | 63 ++++++++++++ clang/test/Modules/explicit-build.cpp | 6 +- clang/test/Modules/invalid-module-dep.c | 2 + 8 files changed, 239 insertions(+), 99 deletions(-) create mode 100644 clang/test/ClangScanDeps/modules-pch-signature-mismatch.c diff --git a/clang/include/clang/Basic/DiagnosticSerializationKinds.td b/clang/include/clang/Basic/DiagnosticSerializationKinds.td index 475c19957c4be..5c74caf010ad9 100644 --- a/clang/include/clang/Basic/DiagnosticSerializationKinds.td +++ b/clang/include/clang/Basic/DiagnosticSerializationKinds.td @@ -78,10 +78,14 @@ def err_ast_file_with_compiler_errors : Error< def err_module_file_conflict : Error< "module '%0' is defined in both '%1' and '%2'">, DefaultFatal; def err_ast_file_not_found : Error< - "%select{PCH|module|precompiled}0 file '%1' not found%select{|: %3}2">, DefaultFatal; + "%select{PCH|module|precompiled}0 file '%1' not found">, DefaultFatal; +def note_ast_file_buffer_failed : Note< + "unable to load buffer for precompiled file: %0">; +def note_ast_file_signature_failed : Note< + "unable to verify precompiled file signature: %0">; def err_ast_file_out_of_date : Error< "%select{PCH|module|precompiled}0 file '%1' is out of date and " - "needs to be rebuilt%select{|: %3}2">, DefaultFatal; + "needs to be rebuilt">, DefaultFatal; def err_ast_file_invalid : Error< "file '%1' is not a valid %select{PCH|module|precompiled}0 file: %2">, DefaultFatal; def note_module_file_imported_by : Note< diff --git a/clang/include/clang/Serialization/ModuleFile.h b/clang/include/clang/Serialization/ModuleFile.h index 303bd65a8aad0..ae5d9828b19de 100644 --- a/clang/include/clang/Serialization/ModuleFile.h +++ b/clang/include/clang/Serialization/ModuleFile.h @@ -32,6 +32,7 @@ #include <cassert> #include <cstdint> #include <memory> +#include <optional> #include <string> #include <vector> @@ -120,6 +121,18 @@ class InputFile { bool isNotFound() const { return Val.getInt() == NotFound; } }; +/// Describes a single change detected in a module file or input file. +struct Change { + enum ModificationKind { + Size, + ModTime, + Content, + None, + } Kind; + std::optional<int64_t> Old = std::nullopt; + std::optional<int64_t> New = std::nullopt; +}; + /// Specifies the high-level result of validating input files. enum class InputFilesValidation { /// Initial value, before the validation has been performed. @@ -145,7 +158,11 @@ enum class InputFilesValidation { class ModuleFile { public: ModuleFile(ModuleKind Kind, ModuleFileKey FileKey, unsigned Generation) - : Kind(Kind), FileKey(std::move(FileKey)), Generation(Generation) {} + : Kind(Kind), FileKey(std::move(FileKey)), Generation(Generation), + InputFilesValidationStatus(Kind == MK_ExplicitModule || + Kind == MK_PrebuiltModule + ? InputFilesValidation::Disabled + : InputFilesValidation::NotStarted) {} ~ModuleFile(); // === General information === @@ -302,8 +319,7 @@ class ModuleFile { /// Useful when encountering a changed input file. This way, we can check /// what kind of validation has been done already and can try to figure out /// why a changed file hasn't been discovered earlier. - InputFilesValidation InputFilesValidationStatus = - InputFilesValidation::NotStarted; + InputFilesValidation InputFilesValidationStatus; // === Source Locations === diff --git a/clang/include/clang/Serialization/ModuleManager.h b/clang/include/clang/Serialization/ModuleManager.h index 1ef9aeee7e1fd..de64d42d4ceb5 100644 --- a/clang/include/clang/Serialization/ModuleManager.h +++ b/clang/include/clang/Serialization/ModuleManager.h @@ -42,6 +42,58 @@ class PCHContainerReader; namespace serialization { +/// The result of attempting to add a new module. +class AddModuleResult { +public: + enum Kind { + /// The module file had already been loaded. + AlreadyLoaded, + /// The module file was just loaded in response to this call. + NewlyLoaded, + /// The module file is missing. + Missing, + /// The module file is out-of-date. + OutOfDate + }; + Kind K; + + ModuleFile *getModule() const { return Module; } + + StringRef getBufferError() const { + assert(K == Missing && !Module); + return BufferError; + } + + const SmallVector<Change, 2> &getChanges() const { + assert(K == OutOfDate && !Module); + return Changes; + } + + InputFilesValidation getValidationStatus() const { + assert(K == OutOfDate && !Module); + return ValidationStatus; + } + + StringRef getSignatureError() const { + assert(K == OutOfDate && !Module); + return SignatureError; + } + + void setOutOfDate(InputFilesValidation Status) { + K = OutOfDate; + ValidationStatus = Status; + } + +private: + friend class ModuleManager; + + ModuleFile *Module = nullptr; + SmallVector<Change, 2> Changes; + InputFilesValidation ValidationStatus = InputFilesValidation::NotStarted; + std::string BufferError; + std::string SignatureError; +}; + /// Manages the set of modules loaded by an AST reader. class ModuleManager { /// The chain of AST files, in the order in which we started to load @@ -96,6 +148,13 @@ class ModuleManager { /// just an non-owning pointer. GlobalModuleIndex *GlobalIndex = nullptr; + bool isModuleFileOutOfDate(off_t Size, time_t ModTime, off_t ExpectedSize, + time_t ExpectedModTime, AddModuleResult &Result); + + bool checkSignature(ASTFileSignature Signature, + ASTFileSignature ExpectedSignature, + AddModuleResult &Result); + /// State used by the "visit" operation to avoid malloc traffic in /// calls to visit(). struct VisitState { @@ -187,21 +246,6 @@ class ModuleManager { /// Number of modules loaded unsigned size() const { return Chain.size(); } - /// The result of attempting to add a new module. - enum AddModuleResult { - /// The module file had already been loaded. - AlreadyLoaded, - - /// The module file was just loaded in response to this call. - NewlyLoaded, - - /// The module file is missing. - Missing, - - /// The module file is out-of-date. - OutOfDate - }; - using ASTFileSignatureReader = ASTFileSignature (*)(StringRef); /// Attempts to create a new module and add it to the list of known @@ -230,21 +274,14 @@ class ModuleManager { /// \param ReadSignature Reads the signature from an AST file without actually /// loading it. /// - /// \param Module A pointer to the module file if the module was successfully - /// loaded. - /// - /// \param ErrorStr Will be set to a non-empty string if any errors occurred - /// while trying to load the module. - /// - /// \return A pointer to the module that corresponds to this file name, - /// and a value indicating whether the module was loaded. + /// \return The result of attempting to add the module, including a pointer + /// to the module file if successfully loaded. AddModuleResult addModule(ModuleFileName FileName, ModuleKind Type, SourceLocation ImportLoc, ModuleFile *ImportedBy, unsigned Generation, off_t ExpectedSize, time_t ExpectedModTime, ASTFileSignature ExpectedSignature, - ASTFileSignatureReader ReadSignature, - ModuleFile *&Module, std::string &ErrorStr); + ASTFileSignatureReader ReadSignature); /// Remove the modules starting from First (to the end). void removeModules(ModuleIterator First); diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index b211b0d32e1de..89dddf75cba40 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -2910,16 +2910,6 @@ InputFile ASTReader::getInputFile(ModuleFile &F, unsigned ID, bool Complain) { } } - struct Change { - enum ModificationKind { - Size, - ModTime, - Content, - None, - } Kind; - std::optional<int64_t> Old = std::nullopt; - std::optional<int64_t> New = std::nullopt; - }; auto HasInputContentChanged = [&](Change OriginalChange) { assert(ValidateASTInputFilesContent && "We should only check the content of the inputs with " @@ -5222,26 +5212,24 @@ ASTReader::ASTReadResult ASTReader::ReadASTCore( ModuleFile *ImportedBy, SmallVectorImpl<ImportedModule> &Loaded, off_t ExpectedSize, time_t ExpectedModTime, ASTFileSignature ExpectedSignature, unsigned ClientLoadCapabilities) { - ModuleFile *M; - std::string ErrorStr; - ModuleManager::AddModuleResult AddResult - = ModuleMgr.addModule(FileName, Type, ImportLoc, ImportedBy, - getGeneration(), ExpectedSize, ExpectedModTime, - ExpectedSignature, readASTFileSignature, - M, ErrorStr); - - switch (AddResult) { - case ModuleManager::AlreadyLoaded: + auto Result = ModuleMgr.addModule( + FileName, Type, ImportLoc, ImportedBy, getGeneration(), ExpectedSize, + ExpectedModTime, ExpectedSignature, readASTFileSignature); + ModuleFile *M = Result.getModule(); + + switch (Result.K) { + case AddModuleResult::AlreadyLoaded: { Diag(diag::remark_module_import) << M->ModuleName << M->FileName << (ImportedBy ? true : false) << (ImportedBy ? StringRef(ImportedBy->ModuleName) : StringRef()); return Success; + } - case ModuleManager::NewlyLoaded: + case AddModuleResult::NewlyLoaded: // Load module file below. break; - case ModuleManager::Missing: + case AddModuleResult::Missing: // The module file was missing; if the client can handle that, return // it. if (ClientLoadCapabilities & ARR_Missing) @@ -5249,11 +5237,12 @@ ASTReader::ASTReadResult ASTReader::ReadASTCore( // Otherwise, return an error. Diag(diag::err_ast_file_not_found) - << moduleKindForDiagnostic(Type) << FileName << !ErrorStr.empty() - << ErrorStr; + << moduleKindForDiagnostic(Type) << FileName; + if (!Result.getBufferError().empty()) + Diag(diag::note_ast_file_buffer_failed) << Result.getBufferError(); return Failure; - case ModuleManager::OutOfDate: + case AddModuleResult::OutOfDate: // We couldn't load the module file because it is out-of-date. If the // client can handle out-of-date, return it. if (ClientLoadCapabilities & ARR_OutOfDate) @@ -5261,8 +5250,16 @@ ASTReader::ASTReadResult ASTReader::ReadASTCore( // Otherwise, return an error. Diag(diag::err_ast_file_out_of_date) - << moduleKindForDiagnostic(Type) << FileName << !ErrorStr.empty() - << ErrorStr; + << moduleKindForDiagnostic(Type) << FileName; + for (const auto &C : Result.getChanges()) { + Diag(diag::note_fe_ast_file_modified) + << C.Kind << (C.Old && C.New) << llvm::itostr(C.Old.value_or(0)) + << llvm::itostr(C.New.value_or(0)); + } + Diag(diag::note_ast_file_input_files_validation_status) + << Result.getValidationStatus(); + if (!Result.getSignatureError().empty()) + Diag(diag::note_ast_file_signature_failed) << Result.getSignatureError(); return Failure; } diff --git a/clang/lib/Serialization/ModuleManager.cpp b/clang/lib/Serialization/ModuleManager.cpp index 022e2ef42f635..4d3815318257d 100644 --- a/clang/lib/Serialization/ModuleManager.cpp +++ b/clang/lib/Serialization/ModuleManager.cpp @@ -67,28 +67,30 @@ ModuleManager::lookupBuffer(StringRef Name) { return std::move(InMemoryBuffers[*Entry]); } -static bool checkModuleFile(off_t Size, time_t ModTime, off_t ExpectedSize, - time_t ExpectedModTime, std::string &ErrorStr) { +bool ModuleManager::isModuleFileOutOfDate(off_t Size, time_t ModTime, + off_t ExpectedSize, + time_t ExpectedModTime, + AddModuleResult &Result) { + bool OutOfDate = false; if (ExpectedSize && ExpectedSize != Size) { - ErrorStr = "module file has a different size than expected"; - return true; + Result.Changes.push_back({Change::Size, ExpectedSize, Size}); + OutOfDate = true; } if (ExpectedModTime && ExpectedModTime != ModTime) { - ErrorStr = "module file has a different modification time than expected"; - return true; + Result.Changes.push_back({Change::ModTime, ExpectedModTime, ModTime}); + OutOfDate = true; } - return false; + return OutOfDate; } -static bool checkSignature(ASTFileSignature Signature, - ASTFileSignature ExpectedSignature, - std::string &ErrorStr) { +bool ModuleManager::checkSignature(ASTFileSignature Signature, + ASTFileSignature ExpectedSignature, + AddModuleResult &Result) { if (!ExpectedSignature || Signature == ExpectedSignature) return false; - - ErrorStr = + Result.SignatureError = Signature ? "signature mismatch" : "could not read module signature"; return true; } @@ -106,13 +108,12 @@ static void updateModuleImports(ModuleFile &MF, ModuleFile *ImportedBy, } } -ModuleManager::AddModuleResult ModuleManager::addModule( +AddModuleResult ModuleManager::addModule( ModuleFileName FileName, ModuleKind Type, SourceLocation ImportLoc, ModuleFile *ImportedBy, unsigned Generation, off_t ExpectedSize, time_t ExpectedModTime, ASTFileSignature ExpectedSignature, - ASTFileSignatureReader ReadSignature, ModuleFile *&Module, - std::string &ErrorStr) { - Module = nullptr; + ASTFileSignatureReader ReadSignature) { + AddModuleResult Result; uint64_t InputFilesValidationTimestamp = 0; if (Type == MK_ImplicitModule) @@ -132,24 +133,31 @@ ModuleManager::AddModuleResult ModuleManager::addModule( std::optional<ModuleFileKey> FileKey = FileName.makeKey(FileMgr); if (!FileKey) { - ErrorStr = "module file not found"; - return Missing; + Result.K = AddModuleResult::Missing; + return Result; } - // Check whether we already loaded this module, before + // Check whether we already loaded this module before. + // Note: `isModuleFileOutOfDate` and `checkSignature` are mutally exclusive in + // practice. If a signature is stored, it means size/mtime values have been + // zeroed out. If size/mtime are non-NULL, the signature is empty. if (ModuleFile *ModuleEntry = lookup(*FileKey)) { - // Check file properties. - if (checkModuleFile(ModuleEntry->Size, ModuleEntry->ModTime, ExpectedSize, - ExpectedModTime, ErrorStr)) - return OutOfDate; + if (isModuleFileOutOfDate(ModuleEntry->Size, ModuleEntry->ModTime, + ExpectedSize, ExpectedModTime, Result)) { + Result.setOutOfDate(ModuleEntry->InputFilesValidationStatus); + return Result; + } // Check the stored signature. - if (checkSignature(ModuleEntry->Signature, ExpectedSignature, ErrorStr)) - return OutOfDate; + if (checkSignature(ModuleEntry->Signature, ExpectedSignature, Result)) { + Result.setOutOfDate(ModuleEntry->InputFilesValidationStatus); + return Result; + } - Module = ModuleEntry; + Result.Module = ModuleEntry; updateModuleImports(*ModuleEntry, ImportedBy, ImportLoc); - return AlreadyLoaded; + Result.K = AddModuleResult::AlreadyLoaded; + return Result; } // Load the contents of the module @@ -177,8 +185,8 @@ ModuleManager::AddModuleResult ModuleManager::addModule( FileMgr.getOptionalFileRef(FileName, /*OpenFile=*/true, /*CacheFailure=*/false); if (!Entry) { - ErrorStr = "module file not found"; - return Missing; + Result.K = AddModuleResult::Missing; + return Result; } ModTime = Entry->getModificationTime(); Size = Entry->getSize(); @@ -186,8 +194,12 @@ ModuleManager::AddModuleResult ModuleManager::addModule( } else if (getModuleCache().getInMemoryModuleCache().shouldBuildPCM( FileName)) { // Report that the module is out of date, since we tried (and failed) to - // import it earlier. - return OutOfDate; + // import it earlier. No ModuleFile exists yet, so derive the validation + // status from the module kind being loaded. + Result.setOutOfDate(Type == MK_ExplicitModule || Type == MK_PrebuiltModule + ? InputFilesValidation::Disabled + : InputFilesValidation::NotStarted); + return Result; } else { auto Buf = [&]() -> Expected<std::unique_ptr<llvm::MemoryBuffer>> { // Implicit modules live in the module cache. @@ -215,15 +227,16 @@ ModuleManager::AddModuleResult ModuleManager::addModule( }(); if (!Buf) { - ErrorStr = llvm::toString(Buf.takeError()); - return Missing; + Result.BufferError = llvm::toString(Buf.takeError()); + Result.K = AddModuleResult::Missing; + return Result; } NewFileBuffer = std::move(*Buf); ModuleBuffer = NewFileBuffer.get(); } - // Allocate a new module. + // Allocate bookkeeping for a module file not yet loaded into this reader. auto NewModule = std::make_unique<ModuleFile>(Type, *FileKey, Generation); NewModule->Index = Chain.size(); NewModule->FileName = FileName; @@ -236,21 +249,26 @@ ModuleManager::AddModuleResult ModuleManager::addModule( NewModule->Data = PCHContainerRdr.ExtractPCH(*NewModule->Buffer); // Check file properties. - if (checkModuleFile(Size, ModTime, ExpectedSize, ExpectedModTime, ErrorStr)) - return OutOfDate; + if (isModuleFileOutOfDate(Size, ModTime, ExpectedSize, ExpectedModTime, + Result)) { + Result.setOutOfDate(NewModule->InputFilesValidationStatus); + return Result; + } // Read the signature eagerly now so that we can check it. Avoid calling // ReadSignature unless there's something to check though. if (ExpectedSignature && checkSignature(ReadSignature(NewModule->Data), - ExpectedSignature, ErrorStr)) - return OutOfDate; + ExpectedSignature, Result)) { + Result.setOutOfDate(NewModule->InputFilesValidationStatus); + return Result; + } if (NewFileBuffer) getModuleCache().getInMemoryModuleCache().addPCM(FileName, std::move(NewFileBuffer)); // We're keeping this module. Store it in the map. - Module = Modules[*FileKey] = NewModule.get(); + Result.Module = Modules[*FileKey] = NewModule.get(); updateModuleImports(*NewModule, ImportedBy, ImportLoc); @@ -260,7 +278,8 @@ ModuleManager::AddModuleResult ModuleManager::addModule( Roots.push_back(NewModule.get()); Chain.push_back(std::move(NewModule)); - return NewlyLoaded; + Result.K = AddModuleResult::NewlyLoaded; + return Result; } void ModuleManager::removeModules(ModuleIterator First) { diff --git a/clang/test/ClangScanDeps/modules-pch-signature-mismatch.c b/clang/test/ClangScanDeps/modules-pch-signature-mismatch.c new file mode 100644 index 0000000000000..73803799e7539 --- /dev/null +++ b/clang/test/ClangScanDeps/modules-pch-signature-mismatch.c @@ -0,0 +1,63 @@ +// UNSUPPORTED: target={{.*}}-aix{{.*}} + +/// Test that a signature mismatch on a PCH's module dependency is diagnosed +/// during dependency scanning. +/// +/// 1. Scan PCH dependencies with modules hash content encoded. +/// 2. Explicitly build module A and the PCH. +/// 3. Modify A's header, resulting in the same byte length and rebuild A for a new signature. +/// 4. Scan a TU that uses the out of date PCH to detect signature mismatch. + +// RUN: rm -rf %t && mkdir %t +// RUN: split-file %s %t + +// RUN: sed "s|DIR|%/t|g" %t/cdb_pch.json.in > %t/cdb_pch.json +// RUN: clang-scan-deps -compilation-database %t/cdb_pch.json \ +// RUN: -format experimental-full -module-files-dir %t/build > %t/result_pch.json + +// RUN: %deps-to-rsp %t/result_pch.json --module-name=A > %t/A.rsp +// RUN: %deps-to-rsp %t/result_pch.json --tu-index=0 > %t/pch.rsp +// RUN: %clang @%t/A.rsp +// RUN: %clang @%t/pch.rsp + +// RUN: cp %t/a_alt.h %t/a.h +// RUN: %clang @%t/A.rsp + +// RUN: sed "s|DIR|%/t|g" %t/cdb_tu.json.in > %t/cdb_tu.json +// RUN: not clang-scan-deps -compilation-database %t/cdb_tu.json \ +// RUN: -format experimental-full -module-files-dir %t/build 2>&1 \ +// RUN: | FileCheck %s + +// CHECK: error:{{.*}}is out of date and needs to be rebuilt +// CHECK: note: earlier input file validation was disabled for this kind of precompiled file +// CHECK: note: unable to verify precompiled file signature: signature mismatch + +//--- module.modulemap +module A { + header "a.h" +} + +//--- a.h +typedef int A_t; + +//--- a_alt.h +typedef int A_u; + +//--- pch.h +#include "a.h" + +//--- tu.c + +//--- cdb_pch.json.in +[{ + "directory": "DIR", + "command": "clang -x c-header DIR/pch.h -fmodules -fmodules-cache-path=DIR/cache -Xclang -fmodules-hash-content -I DIR -o DIR/pch.h.pch", + "file": "DIR/pch.h" +}] + +//--- cdb_tu.json.in +[{ + "directory": "DIR", + "command": "clang -fsyntax-only DIR/tu.c -fmodules -fmodules-cache-path=DIR/cache -Xclang -fmodules-hash-content -I DIR -include DIR/pch.h -o DIR/tu.o", + "file": "DIR/tu.c" +}] diff --git a/clang/test/Modules/explicit-build.cpp b/clang/test/Modules/explicit-build.cpp index c2fe2024a3629..1cd3801a6c22a 100644 --- a/clang/test/Modules/explicit-build.cpp +++ b/clang/test/Modules/explicit-build.cpp @@ -170,7 +170,7 @@ // RUN: -fmodule-file=%t/nonexistent.pcm \ // RUN: %s 2>&1 | FileCheck --check-prefix=CHECK-NO-FILE %s // -// CHECK-NO-FILE: fatal error: module file '{{.*}}nonexistent.pcm' not found: module file not found +// CHECK-NO-FILE: fatal error: module file '{{.*}}nonexistent.pcm' not found // RUN: mv %t/a.pcm %t/a-tmp.pcm // RUN: not %clang_cc1 -triple=x86_64-linux-gnu -x c++ -std=c++11 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Rmodule-build -fno-modules-error-recovery \ @@ -202,6 +202,8 @@ // RUN: -fmodule-file=%t/c.pcm \ // RUN: %s -DHAVE_A -DHAVE_B -DHAVE_C 2>&1 | FileCheck --check-prefix=CHECK-MISMATCHED-B %s // -// CHECK-MISMATCHED-B: fatal error: module file '{{.*}}b.pcm' is out of date and needs to be rebuilt: module file has a different size than expected +// CHECK-MISMATCHED-B: fatal error: module file '{{.*}}b.pcm' is out of date and needs to be rebuilt +// CHECK-MISMATCHED-B-NEXT: note: size changed from expected {{[0-9]+}} to {{[0-9]+}} +// CHECK-MISMATCHED-B-NEXT: note: earlier input file validation was disabled for this kind of precompiled file // CHECK-MISMATCHED-B-NEXT: note: imported by module 'c' // CHECK-MISMATCHED-B-NOT: note: diff --git a/clang/test/Modules/invalid-module-dep.c b/clang/test/Modules/invalid-module-dep.c index 2bcda8be5f1b6..82aaa22743929 100644 --- a/clang/test/Modules/invalid-module-dep.c +++ b/clang/test/Modules/invalid-module-dep.c @@ -37,6 +37,8 @@ #include <A/A.h> #include <B/B.h> // expected-warning {{conflicts with imported file}} \ // expected-note {{imported by module 'B'}} \ + // expected-note {{size changed}} \ + // expected-note {{earlier input file validation was disabled for this kind of precompiled file}} \ // expected-error {{out of date and needs to be rebuilt}} //--- Sysroot/usr/include/A/module.modulemap >From 7de7f3f06887ca549aa7fbaa9c21d4c5e028a671 Mon Sep 17 00:00:00 2001 From: Cyndy Ishida <[email protected]> Date: Thu, 9 Apr 2026 14:56:53 -0700 Subject: [PATCH 2/2] Update based on feedback --- clang/include/clang/Serialization/ModuleFile.h | 2 +- clang/include/clang/Serialization/ModuleManager.h | 6 +++++- clang/lib/Serialization/ASTReader.cpp | 5 ++++- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/clang/include/clang/Serialization/ModuleFile.h b/clang/include/clang/Serialization/ModuleFile.h index ae5d9828b19de..58f2fcba01e67 100644 --- a/clang/include/clang/Serialization/ModuleFile.h +++ b/clang/include/clang/Serialization/ModuleFile.h @@ -128,7 +128,7 @@ struct Change { ModTime, Content, None, - } Kind; + } Kind = None; std::optional<int64_t> Old = std::nullopt; std::optional<int64_t> New = std::nullopt; }; diff --git a/clang/include/clang/Serialization/ModuleManager.h b/clang/include/clang/Serialization/ModuleManager.h index de64d42d4ceb5..beed21f34a0ea 100644 --- a/clang/include/clang/Serialization/ModuleManager.h +++ b/clang/include/clang/Serialization/ModuleManager.h @@ -46,6 +46,8 @@ namespace serialization { class AddModuleResult { public: enum Kind { + /// State at construction. + None, /// The module file had already been loaded. AlreadyLoaded, /// The module file was just loaded in response to this call. @@ -55,7 +57,8 @@ class AddModuleResult { /// The module file is out-of-date. OutOfDate }; - Kind K; + + Kind getKind() const { return K; }; ModuleFile *getModule() const { return Module; } @@ -87,6 +90,7 @@ class AddModuleResult { private: friend class ModuleManager; + Kind K = None; ModuleFile *Module = nullptr; SmallVector<Change, 2> Changes; InputFilesValidation ValidationStatus = InputFilesValidation::NotStarted; diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 89dddf75cba40..f222722339468 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -5217,7 +5217,7 @@ ASTReader::ASTReadResult ASTReader::ReadASTCore( ExpectedModTime, ExpectedSignature, readASTFileSignature); ModuleFile *M = Result.getModule(); - switch (Result.K) { + switch (Result.getKind()) { case AddModuleResult::AlreadyLoaded: { Diag(diag::remark_module_import) << M->ModuleName << M->FileName << (ImportedBy ? true : false) @@ -5261,6 +5261,9 @@ ASTReader::ASTReadResult ASTReader::ReadASTCore( if (!Result.getSignatureError().empty()) Diag(diag::note_ast_file_signature_failed) << Result.getSignatureError(); return Failure; + + case AddModuleResult::None: + llvm_unreachable("Unexpected value from adding module."); } assert(M && "Missing module file"); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
