Skip to content

Commit 8615ead

Browse files
authored
[clang] NFCI: Make ModuleFile::File non-optional (#74892)
AFAICT, `ModuleFile::File` can be `std::nullopt` only for PCM files loaded from the standard input. This patch starts setting that variable to `FileManager::getSTDIN()` in that case, which makes it possible to remove the optionality, and also simplifies code that actually reads the file. This is part of an effort to get rid of `Optional{File,Directory}EntryRefDegradesTo{File,Directory}EntryPtr`.
1 parent 18f0da2 commit 8615ead

File tree

5 files changed

+29
-39
lines changed

5 files changed

+29
-39
lines changed

clang/include/clang/Serialization/ModuleFile.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,8 @@ class InputFile {
123123
/// other modules.
124124
class ModuleFile {
125125
public:
126-
ModuleFile(ModuleKind Kind, unsigned Generation)
127-
: Kind(Kind), Generation(Generation) {}
126+
ModuleFile(ModuleKind Kind, FileEntryRef File, unsigned Generation)
127+
: Kind(Kind), File(File), Generation(Generation) {}
128128
~ModuleFile();
129129

130130
// === General information ===
@@ -176,7 +176,7 @@ class ModuleFile {
176176
bool DidReadTopLevelSubmodule = false;
177177

178178
/// The file entry for the module file.
179-
OptionalFileEntryRefDegradesToFileEntryPtr File;
179+
FileEntryRef File;
180180

181181
/// The signature of the module file, which may be used instead of the size
182182
/// and modification time to identify this particular file.

clang/lib/Serialization/ASTReader.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5786,7 +5786,7 @@ llvm::Error ASTReader::ReadSubmoduleBlock(ModuleFile &F,
57865786
PartialDiagnostic(diag::err_module_file_conflict,
57875787
ContextObj->DiagAllocator)
57885788
<< CurrentModule->getTopLevelModuleName() << CurFile->getName()
5789-
<< F.File->getName();
5789+
<< F.File.getName();
57905790
return DiagnosticError::create(CurrentImportLoc, ConflictError);
57915791
}
57925792
}

clang/lib/Serialization/ASTWriter.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1413,7 +1413,7 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, ASTContext &Context,
14131413

14141414
// If we have calculated signature, there is no need to store
14151415
// the size or timestamp.
1416-
Record.push_back(M.Signature ? 0 : M.File->getSize());
1416+
Record.push_back(M.Signature ? 0 : M.File.getSize());
14171417
Record.push_back(M.Signature ? 0 : getTimestampForOutput(M.File));
14181418

14191419
llvm::append_range(Record, M.Signature);

clang/lib/Serialization/GlobalModuleIndex.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -342,8 +342,8 @@ bool GlobalModuleIndex::loadedModuleFile(ModuleFile *File) {
342342
// If the size and modification time match what we expected, record this
343343
// module file.
344344
bool Failed = true;
345-
if (File->File->getSize() == Info.Size &&
346-
File->File->getModificationTime() == Info.ModTime) {
345+
if (File->File.getSize() == Info.Size &&
346+
File->File.getModificationTime() == Info.ModTime) {
347347
Info.File = File;
348348
ModulesByFile[File] = Known->second;
349349

clang/lib/Serialization/ModuleManager.cpp

Lines changed: 22 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type,
108108

109109
// Look for the file entry. This only fails if the expected size or
110110
// modification time differ.
111-
OptionalFileEntryRefDegradesToFileEntryPtr Entry;
111+
OptionalFileEntryRef Entry;
112112
if (Type == MK_ExplicitModule || Type == MK_PrebuiltModule) {
113113
// If we're not expecting to pull this file out of the module cache, it
114114
// might have a different mtime due to being moved across filesystems in
@@ -123,7 +123,7 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type,
123123
return OutOfDate;
124124
}
125125

126-
if (!Entry && FileName != "-") {
126+
if (!Entry) {
127127
ErrorStr = "module file not found";
128128
return Missing;
129129
}
@@ -150,7 +150,7 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type,
150150
};
151151

152152
// Check whether we already loaded this module, before
153-
if (ModuleFile *ModuleEntry = Modules.lookup(Entry)) {
153+
if (ModuleFile *ModuleEntry = Modules.lookup(*Entry)) {
154154
if (implicitModuleNamesMatch(Type, ModuleEntry, *Entry)) {
155155
// Check the stored signature.
156156
if (checkSignature(ModuleEntry->Signature, ExpectedSignature, ErrorStr))
@@ -163,10 +163,9 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type,
163163
}
164164

165165
// Allocate a new module.
166-
auto NewModule = std::make_unique<ModuleFile>(Type, Generation);
166+
auto NewModule = std::make_unique<ModuleFile>(Type, *Entry, Generation);
167167
NewModule->Index = Chain.size();
168168
NewModule->FileName = FileName.str();
169-
NewModule->File = Entry;
170169
NewModule->ImportLoc = ImportLoc;
171170
NewModule->InputFilesValidationTimestamp = 0;
172171

@@ -198,21 +197,15 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type,
198197
Entry->closeFile();
199198
return OutOfDate;
200199
} else {
201-
// Open the AST file.
202-
llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Buf((std::error_code()));
203-
if (FileName == "-") {
204-
Buf = llvm::MemoryBuffer::getSTDIN();
205-
} else {
206-
// Get a buffer of the file and close the file descriptor when done.
207-
// The file is volatile because in a parallel build we expect multiple
208-
// compiler processes to use the same module file rebuilding it if needed.
209-
//
210-
// RequiresNullTerminator is false because module files don't need it, and
211-
// this allows the file to still be mmapped.
212-
Buf = FileMgr.getBufferForFile(*NewModule->File,
213-
/*IsVolatile=*/true,
214-
/*RequiresNullTerminator=*/false);
215-
}
200+
// Get a buffer of the file and close the file descriptor when done.
201+
// The file is volatile because in a parallel build we expect multiple
202+
// compiler processes to use the same module file rebuilding it if needed.
203+
//
204+
// RequiresNullTerminator is false because module files don't need it, and
205+
// this allows the file to still be mmapped.
206+
auto Buf = FileMgr.getBufferForFile(NewModule->File,
207+
/*IsVolatile=*/true,
208+
/*RequiresNullTerminator=*/false);
216209

217210
if (!Buf) {
218211
ErrorStr = Buf.getError().message();
@@ -232,7 +225,7 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type,
232225
return OutOfDate;
233226

234227
// We're keeping this module. Store it everywhere.
235-
Module = Modules[Entry] = NewModule.get();
228+
Module = Modules[*Entry] = NewModule.get();
236229

237230
updateModuleImports(*NewModule, ImportedBy, ImportLoc);
238231

@@ -441,22 +434,19 @@ void ModuleManager::visit(llvm::function_ref<bool(ModuleFile &M)> Visitor,
441434
bool ModuleManager::lookupModuleFile(StringRef FileName, off_t ExpectedSize,
442435
time_t ExpectedModTime,
443436
OptionalFileEntryRef &File) {
444-
File = std::nullopt;
445-
if (FileName == "-")
437+
if (FileName == "-") {
438+
File = expectedToOptional(FileMgr.getSTDIN());
446439
return false;
440+
}
447441

448442
// Open the file immediately to ensure there is no race between stat'ing and
449443
// opening the file.
450-
OptionalFileEntryRef FileOrErr =
451-
expectedToOptional(FileMgr.getFileRef(FileName, /*OpenFile=*/true,
452-
/*CacheFailure=*/false));
453-
if (!FileOrErr)
454-
return false;
455-
456-
File = *FileOrErr;
444+
File = FileMgr.getOptionalFileRef(FileName, /*OpenFile=*/true,
445+
/*CacheFailure=*/false);
457446

458-
if ((ExpectedSize && ExpectedSize != File->getSize()) ||
459-
(ExpectedModTime && ExpectedModTime != File->getModificationTime()))
447+
if (File &&
448+
((ExpectedSize && ExpectedSize != File->getSize()) ||
449+
(ExpectedModTime && ExpectedModTime != File->getModificationTime())))
460450
// Do not destroy File, as it may be referenced. If we need to rebuild it,
461451
// it will be destroyed by removeModules.
462452
return true;

0 commit comments

Comments
 (0)