Skip to content

Commit 83dcb34

Browse files
committed
clang/Modules: Error if ReadASTBlock does not find the main module
If ReadASTBlock does not find its top-level submodule, there's something wrong the with the PCM. Error in that case, to avoid hitting problems further from the source. Note that the Swift compiler sometimes hits a case in CompilerInstance::loadModule where the top-level submodule mysteriously does not have Module::IsFromModuleFile set. That will emit a confusing warn_missing_submodule, which was never intended for the main module. The recent audit of error-handling in ReadAST may have rooted out the real problem. If not, this commit will help to clarify the real problem, and replace a confusing warning with an error pointing at the malformed PCM file. We're specifically sniffing out whether the top-level submodule was found/processed, in case there is a malformed module file that is missing it. If there is an error encountered during ReadSubmoduleBlock the return status should already propagate through. It would be nice to detect other missing submodules around here to catch other instances of warn_missing_submodule closer to the source, but that's left as a future exercise. https://reviews.llvm.org/D70063
1 parent 2d06375 commit 83dcb34

File tree

3 files changed

+12
-0
lines changed

3 files changed

+12
-0
lines changed

clang/include/clang/Basic/DiagnosticSerializationKinds.td

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,8 @@ def note_module_file_imported_by : Note<
7474
"imported by %select{|module '%2' in }1'%0'">;
7575
def err_module_file_not_module : Error<
7676
"AST file '%0' was not built as a module">, DefaultFatal;
77+
def err_module_file_missing_top_level_submodule : Error<
78+
"module file '%0' is missing its top-level submodule">, DefaultFatal;
7779

7880
def remark_module_import : Remark<
7981
"importing module '%0'%select{| into '%3'}2 from '%1'">,

clang/include/clang/Serialization/Module.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,9 @@ class ModuleFile {
159159
/// Whether the PCH has a corresponding object file.
160160
bool PCHHasObjectFile = false;
161161

162+
/// Whether the top-level module has been read from the AST file.
163+
bool DidReadTopLevelSubmodule = false;
164+
162165
/// The file entry for the module file.
163166
const FileEntry *File = nullptr;
164167

clang/lib/Serialization/ASTReader.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4217,6 +4217,12 @@ ASTReader::ASTReadResult ASTReader::ReadAST(StringRef FileName,
42174217
if (ASTReadResult Result = ReadASTBlock(F, ClientLoadCapabilities))
42184218
return removeModulesAndReturn(Result);
42194219

4220+
// The AST block should always have a definition for the main module.
4221+
if (F.isModule() && !F.DidReadTopLevelSubmodule) {
4222+
Error(diag::err_module_file_missing_top_level_submodule, F.FileName);
4223+
return removeModulesAndReturn(Failure);
4224+
}
4225+
42204226
// Read the extension blocks.
42214227
while (!SkipCursorToBlock(F.Stream, EXTENSION_BLOCK_ID)) {
42224228
if (ASTReadResult Result = ReadExtensionBlock(F))
@@ -5489,6 +5495,7 @@ ASTReader::ReadSubmoduleBlock(ModuleFile &F, unsigned ClientLoadCapabilities) {
54895495
}
54905496
}
54915497

5498+
F.DidReadTopLevelSubmodule = true;
54925499
CurrentModule->setASTFile(F.File);
54935500
CurrentModule->PresumedModuleMapFile = F.ModuleMapPath;
54945501
}

0 commit comments

Comments
 (0)