[clang] [clang] NFCI: Make `ModuleFile::File` non-optional (PR #74892)
https://github.com/jansvoboda11 closed https://github.com/llvm/llvm-project/pull/74892 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] NFCI: Make `ModuleFile::File` non-optional (PR #74892)
https://github.com/jansvoboda11 updated
https://github.com/llvm/llvm-project/pull/74892
>From fb5c8c9fe856aaa2a314effa26486d5fbf019140 Mon Sep 17 00:00:00 2001
From: Jan Svoboda
Date: Thu, 7 Dec 2023 14:04:35 -0800
Subject: [PATCH 1/2] [clang] NFCI: Make `ModuleFile::File` non-optional
---
.../include/clang/Serialization/ModuleFile.h | 6 +--
clang/lib/Serialization/ASTReader.cpp | 2 +-
clang/lib/Serialization/ASTWriter.cpp | 2 +-
clang/lib/Serialization/GlobalModuleIndex.cpp | 4 +-
clang/lib/Serialization/ModuleManager.cpp | 54 ---
5 files changed, 29 insertions(+), 39 deletions(-)
diff --git a/clang/include/clang/Serialization/ModuleFile.h
b/clang/include/clang/Serialization/ModuleFile.h
index 48be8676cc26a4..70ab61dec8b6bf 100644
--- a/clang/include/clang/Serialization/ModuleFile.h
+++ b/clang/include/clang/Serialization/ModuleFile.h
@@ -123,8 +123,8 @@ class InputFile {
/// other modules.
class ModuleFile {
public:
- ModuleFile(ModuleKind Kind, unsigned Generation)
- : Kind(Kind), Generation(Generation) {}
+ ModuleFile(ModuleKind Kind, FileEntryRef File, unsigned Generation)
+ : Kind(Kind), File(File), Generation(Generation) {}
~ModuleFile();
// === General information ===
@@ -176,7 +176,7 @@ class ModuleFile {
bool DidReadTopLevelSubmodule = false;
/// The file entry for the module file.
- OptionalFileEntryRefDegradesToFileEntryPtr File;
+ FileEntryRef File;
/// The signature of the module file, which may be used instead of the size
/// and modification time to identify this particular file.
diff --git a/clang/lib/Serialization/ASTReader.cpp
b/clang/lib/Serialization/ASTReader.cpp
index f22da838424b41..49f25d6648c801 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -5786,7 +5786,7 @@ llvm::Error ASTReader::ReadSubmoduleBlock(ModuleFile ,
PartialDiagnostic(diag::err_module_file_conflict,
ContextObj->DiagAllocator)
<< CurrentModule->getTopLevelModuleName() << CurFile->getName()
-<< F.File->getName();
+<< F.File.getName();
return DiagnosticError::create(CurrentImportLoc, ConflictError);
}
}
diff --git a/clang/lib/Serialization/ASTWriter.cpp
b/clang/lib/Serialization/ASTWriter.cpp
index 6df815234e235f..38e8b8ccbe058d 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -1413,7 +1413,7 @@ void ASTWriter::WriteControlBlock(Preprocessor ,
ASTContext ,
// If we have calculated signature, there is no need to store
// the size or timestamp.
- Record.push_back(M.Signature ? 0 : M.File->getSize());
+ Record.push_back(M.Signature ? 0 : M.File.getSize());
Record.push_back(M.Signature ? 0 : getTimestampForOutput(M.File));
llvm::append_range(Record, M.Signature);
diff --git a/clang/lib/Serialization/GlobalModuleIndex.cpp
b/clang/lib/Serialization/GlobalModuleIndex.cpp
index fb80a1998d0efe..dd4fc3e009050f 100644
--- a/clang/lib/Serialization/GlobalModuleIndex.cpp
+++ b/clang/lib/Serialization/GlobalModuleIndex.cpp
@@ -342,8 +342,8 @@ bool GlobalModuleIndex::loadedModuleFile(ModuleFile *File) {
// If the size and modification time match what we expected, record this
// module file.
bool Failed = true;
- if (File->File->getSize() == Info.Size &&
- File->File->getModificationTime() == Info.ModTime) {
+ if (File->File.getSize() == Info.Size &&
+ File->File.getModificationTime() == Info.ModTime) {
Info.File = File;
ModulesByFile[File] = Known->second;
diff --git a/clang/lib/Serialization/ModuleManager.cpp
b/clang/lib/Serialization/ModuleManager.cpp
index de4cd3d05853ac..7cf97125b8ef03 100644
--- a/clang/lib/Serialization/ModuleManager.cpp
+++ b/clang/lib/Serialization/ModuleManager.cpp
@@ -108,7 +108,7 @@ ModuleManager::addModule(StringRef FileName, ModuleKind
Type,
// Look for the file entry. This only fails if the expected size or
// modification time differ.
- OptionalFileEntryRefDegradesToFileEntryPtr Entry;
+ OptionalFileEntryRef Entry;
if (Type == MK_ExplicitModule || Type == MK_PrebuiltModule) {
// If we're not expecting to pull this file out of the module cache, it
// might have a different mtime due to being moved across filesystems in
@@ -123,7 +123,7 @@ ModuleManager::addModule(StringRef FileName, ModuleKind
Type,
return OutOfDate;
}
- if (!Entry && FileName != "-") {
+ if (!Entry) {
ErrorStr = "module file not found";
return Missing;
}
@@ -150,7 +150,7 @@ ModuleManager::addModule(StringRef FileName, ModuleKind
Type,
};
// Check whether we already loaded this module, before
- if (ModuleFile *ModuleEntry = Modules.lookup(Entry)) {
+ if (ModuleFile *ModuleEntry = Modules.lookup(*Entry)) {
if (implicitModuleNamesMatch(Type, ModuleEntry, *Entry)) {
[clang] [clang] NFCI: Make `ModuleFile::File` non-optional (PR #74892)
https://github.com/benlangmuir approved this pull request. https://github.com/llvm/llvm-project/pull/74892 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] NFCI: Make `ModuleFile::File` non-optional (PR #74892)
@@ -441,22 +434,19 @@ void
ModuleManager::visit(llvm::function_ref Visitor,
bool ModuleManager::lookupModuleFile(StringRef FileName, off_t ExpectedSize,
time_t ExpectedModTime,
OptionalFileEntryRef ) {
- File = std::nullopt;
- if (FileName == "-")
+ if (FileName == "-") {
+File = expectedToOptional(FileMgr.getSTDIN());
return false;
+ }
// Open the file immediately to ensure there is no race between stat'ing and
// opening the file.
- OptionalFileEntryRef FileOrErr =
- expectedToOptional(FileMgr.getFileRef(FileName, /*OpenFile=*/true,
-/*CacheFailure=*/false));
- if (!FileOrErr)
-return false;
-
- File = *FileOrErr;
+ File = expectedToOptional(FileMgr.getFileRef(FileName, /*OpenFile=*/true,
benlangmuir wrote:
Yeah, I would recommend that.
https://github.com/llvm/llvm-project/pull/74892
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] NFCI: Make `ModuleFile::File` non-optional (PR #74892)
https://github.com/benlangmuir edited https://github.com/llvm/llvm-project/pull/74892 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] NFCI: Make `ModuleFile::File` non-optional (PR #74892)
https://github.com/jansvoboda11 edited https://github.com/llvm/llvm-project/pull/74892 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] NFCI: Make `ModuleFile::File` non-optional (PR #74892)
@@ -441,22 +434,19 @@ void
ModuleManager::visit(llvm::function_ref Visitor,
bool ModuleManager::lookupModuleFile(StringRef FileName, off_t ExpectedSize,
time_t ExpectedModTime,
OptionalFileEntryRef ) {
- File = std::nullopt;
- if (FileName == "-")
+ if (FileName == "-") {
+File = expectedToOptional(FileMgr.getSTDIN());
return false;
+ }
// Open the file immediately to ensure there is no race between stat'ing and
// opening the file.
- OptionalFileEntryRef FileOrErr =
- expectedToOptional(FileMgr.getFileRef(FileName, /*OpenFile=*/true,
-/*CacheFailure=*/false));
- if (!FileOrErr)
-return false;
-
- File = *FileOrErr;
+ File = expectedToOptional(FileMgr.getFileRef(FileName, /*OpenFile=*/true,
jansvoboda11 wrote:
I guess I could've replace this with `getOptionalFileRef()` while I'm touching
the line.
https://github.com/llvm/llvm-project/pull/74892
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] NFCI: Make `ModuleFile::File` non-optional (PR #74892)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Jan Svoboda (jansvoboda11)
Changes
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`.
---
Full diff: https://github.com/llvm/llvm-project/pull/74892.diff
5 Files Affected:
- (modified) clang/include/clang/Serialization/ModuleFile.h (+3-3)
- (modified) clang/lib/Serialization/ASTReader.cpp (+1-1)
- (modified) clang/lib/Serialization/ASTWriter.cpp (+1-1)
- (modified) clang/lib/Serialization/GlobalModuleIndex.cpp (+2-2)
- (modified) clang/lib/Serialization/ModuleManager.cpp (+22-32)
``diff
diff --git a/clang/include/clang/Serialization/ModuleFile.h
b/clang/include/clang/Serialization/ModuleFile.h
index 48be8676cc26a4..70ab61dec8b6bf 100644
--- a/clang/include/clang/Serialization/ModuleFile.h
+++ b/clang/include/clang/Serialization/ModuleFile.h
@@ -123,8 +123,8 @@ class InputFile {
/// other modules.
class ModuleFile {
public:
- ModuleFile(ModuleKind Kind, unsigned Generation)
- : Kind(Kind), Generation(Generation) {}
+ ModuleFile(ModuleKind Kind, FileEntryRef File, unsigned Generation)
+ : Kind(Kind), File(File), Generation(Generation) {}
~ModuleFile();
// === General information ===
@@ -176,7 +176,7 @@ class ModuleFile {
bool DidReadTopLevelSubmodule = false;
/// The file entry for the module file.
- OptionalFileEntryRefDegradesToFileEntryPtr File;
+ FileEntryRef File;
/// The signature of the module file, which may be used instead of the size
/// and modification time to identify this particular file.
diff --git a/clang/lib/Serialization/ASTReader.cpp
b/clang/lib/Serialization/ASTReader.cpp
index f22da838424b41..49f25d6648c801 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -5786,7 +5786,7 @@ llvm::Error ASTReader::ReadSubmoduleBlock(ModuleFile ,
PartialDiagnostic(diag::err_module_file_conflict,
ContextObj->DiagAllocator)
<< CurrentModule->getTopLevelModuleName() << CurFile->getName()
-<< F.File->getName();
+<< F.File.getName();
return DiagnosticError::create(CurrentImportLoc, ConflictError);
}
}
diff --git a/clang/lib/Serialization/ASTWriter.cpp
b/clang/lib/Serialization/ASTWriter.cpp
index 6df815234e235f..38e8b8ccbe058d 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -1413,7 +1413,7 @@ void ASTWriter::WriteControlBlock(Preprocessor ,
ASTContext ,
// If we have calculated signature, there is no need to store
// the size or timestamp.
- Record.push_back(M.Signature ? 0 : M.File->getSize());
+ Record.push_back(M.Signature ? 0 : M.File.getSize());
Record.push_back(M.Signature ? 0 : getTimestampForOutput(M.File));
llvm::append_range(Record, M.Signature);
diff --git a/clang/lib/Serialization/GlobalModuleIndex.cpp
b/clang/lib/Serialization/GlobalModuleIndex.cpp
index fb80a1998d0efe..dd4fc3e009050f 100644
--- a/clang/lib/Serialization/GlobalModuleIndex.cpp
+++ b/clang/lib/Serialization/GlobalModuleIndex.cpp
@@ -342,8 +342,8 @@ bool GlobalModuleIndex::loadedModuleFile(ModuleFile *File) {
// If the size and modification time match what we expected, record this
// module file.
bool Failed = true;
- if (File->File->getSize() == Info.Size &&
- File->File->getModificationTime() == Info.ModTime) {
+ if (File->File.getSize() == Info.Size &&
+ File->File.getModificationTime() == Info.ModTime) {
Info.File = File;
ModulesByFile[File] = Known->second;
diff --git a/clang/lib/Serialization/ModuleManager.cpp
b/clang/lib/Serialization/ModuleManager.cpp
index de4cd3d05853ac..7cf97125b8ef03 100644
--- a/clang/lib/Serialization/ModuleManager.cpp
+++ b/clang/lib/Serialization/ModuleManager.cpp
@@ -108,7 +108,7 @@ ModuleManager::addModule(StringRef FileName, ModuleKind
Type,
// Look for the file entry. This only fails if the expected size or
// modification time differ.
- OptionalFileEntryRefDegradesToFileEntryPtr Entry;
+ OptionalFileEntryRef Entry;
if (Type == MK_ExplicitModule || Type == MK_PrebuiltModule) {
// If we're not expecting to pull this file out of the module cache, it
// might have a different mtime due to being moved across filesystems in
@@ -123,7 +123,7 @@ ModuleManager::addModule(StringRef FileName, ModuleKind
Type,
return OutOfDate;
}
- if (!Entry && FileName != "-") {
+ if (!Entry) {
ErrorStr = "module file not found";
return Missing;
}
@@ -150,7 +150,7 @@ ModuleManager::addModule(StringRef
[clang] [clang] NFCI: Make `ModuleFile::File` non-optional (PR #74892)
https://github.com/jansvoboda11 created
https://github.com/llvm/llvm-project/pull/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`.
>From fb5c8c9fe856aaa2a314effa26486d5fbf019140 Mon Sep 17 00:00:00 2001
From: Jan Svoboda
Date: Thu, 7 Dec 2023 14:04:35 -0800
Subject: [PATCH] [clang] NFCI: Make `ModuleFile::File` non-optional
---
.../include/clang/Serialization/ModuleFile.h | 6 +--
clang/lib/Serialization/ASTReader.cpp | 2 +-
clang/lib/Serialization/ASTWriter.cpp | 2 +-
clang/lib/Serialization/GlobalModuleIndex.cpp | 4 +-
clang/lib/Serialization/ModuleManager.cpp | 54 ---
5 files changed, 29 insertions(+), 39 deletions(-)
diff --git a/clang/include/clang/Serialization/ModuleFile.h
b/clang/include/clang/Serialization/ModuleFile.h
index 48be8676cc26a4..70ab61dec8b6bf 100644
--- a/clang/include/clang/Serialization/ModuleFile.h
+++ b/clang/include/clang/Serialization/ModuleFile.h
@@ -123,8 +123,8 @@ class InputFile {
/// other modules.
class ModuleFile {
public:
- ModuleFile(ModuleKind Kind, unsigned Generation)
- : Kind(Kind), Generation(Generation) {}
+ ModuleFile(ModuleKind Kind, FileEntryRef File, unsigned Generation)
+ : Kind(Kind), File(File), Generation(Generation) {}
~ModuleFile();
// === General information ===
@@ -176,7 +176,7 @@ class ModuleFile {
bool DidReadTopLevelSubmodule = false;
/// The file entry for the module file.
- OptionalFileEntryRefDegradesToFileEntryPtr File;
+ FileEntryRef File;
/// The signature of the module file, which may be used instead of the size
/// and modification time to identify this particular file.
diff --git a/clang/lib/Serialization/ASTReader.cpp
b/clang/lib/Serialization/ASTReader.cpp
index f22da838424b41..49f25d6648c801 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -5786,7 +5786,7 @@ llvm::Error ASTReader::ReadSubmoduleBlock(ModuleFile ,
PartialDiagnostic(diag::err_module_file_conflict,
ContextObj->DiagAllocator)
<< CurrentModule->getTopLevelModuleName() << CurFile->getName()
-<< F.File->getName();
+<< F.File.getName();
return DiagnosticError::create(CurrentImportLoc, ConflictError);
}
}
diff --git a/clang/lib/Serialization/ASTWriter.cpp
b/clang/lib/Serialization/ASTWriter.cpp
index 6df815234e235f..38e8b8ccbe058d 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -1413,7 +1413,7 @@ void ASTWriter::WriteControlBlock(Preprocessor ,
ASTContext ,
// If we have calculated signature, there is no need to store
// the size or timestamp.
- Record.push_back(M.Signature ? 0 : M.File->getSize());
+ Record.push_back(M.Signature ? 0 : M.File.getSize());
Record.push_back(M.Signature ? 0 : getTimestampForOutput(M.File));
llvm::append_range(Record, M.Signature);
diff --git a/clang/lib/Serialization/GlobalModuleIndex.cpp
b/clang/lib/Serialization/GlobalModuleIndex.cpp
index fb80a1998d0efe..dd4fc3e009050f 100644
--- a/clang/lib/Serialization/GlobalModuleIndex.cpp
+++ b/clang/lib/Serialization/GlobalModuleIndex.cpp
@@ -342,8 +342,8 @@ bool GlobalModuleIndex::loadedModuleFile(ModuleFile *File) {
// If the size and modification time match what we expected, record this
// module file.
bool Failed = true;
- if (File->File->getSize() == Info.Size &&
- File->File->getModificationTime() == Info.ModTime) {
+ if (File->File.getSize() == Info.Size &&
+ File->File.getModificationTime() == Info.ModTime) {
Info.File = File;
ModulesByFile[File] = Known->second;
diff --git a/clang/lib/Serialization/ModuleManager.cpp
b/clang/lib/Serialization/ModuleManager.cpp
index de4cd3d05853ac..7cf97125b8ef03 100644
--- a/clang/lib/Serialization/ModuleManager.cpp
+++ b/clang/lib/Serialization/ModuleManager.cpp
@@ -108,7 +108,7 @@ ModuleManager::addModule(StringRef FileName, ModuleKind
Type,
// Look for the file entry. This only fails if the expected size or
// modification time differ.
- OptionalFileEntryRefDegradesToFileEntryPtr Entry;
+ OptionalFileEntryRef Entry;
if (Type == MK_ExplicitModule || Type == MK_PrebuiltModule) {
// If we're not expecting to pull this file out of the module cache, it
// might have a different mtime due to being moved across filesystems in
@@ -123,7 +123,7 @@ ModuleManager::addModule(StringRef FileName, ModuleKind
Type,
return OutOfDate;
}
- if (!Entry && FileName != "-") {
+ if (!Entry) {
