[llvm] [llvm][Support] Make sys::fs::file_t into a seperate type (PR #160588)
Steven Wu via llvm-commits
llvm-commits at lists.llvm.org
Wed Sep 24 12:32:36 PDT 2025
https://github.com/cachemeifyoucan updated https://github.com/llvm/llvm-project/pull/160588
>From 2f9ef512e606807c6081605853f80e3736baee6a Mon Sep 17 00:00:00 2001
From: Steven Wu <stevenwu at apple.com>
Date: Wed, 24 Sep 2025 12:29:23 -0700
Subject: [PATCH 1/2] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20in?=
=?UTF-8?q?itial=20version?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Created using spr 1.3.7
---
llvm/include/llvm/Support/File.h | 55 +++++++++++++++++++++
llvm/include/llvm/Support/FileSystem.h | 24 +++------
llvm/include/llvm/Support/MemoryBuffer.h | 12 +----
llvm/lib/CAS/MappedFileRegionArena.cpp | 2 +-
llvm/lib/Object/ArchiveWriter.cpp | 2 +-
llvm/lib/Support/FileUtilities.cpp | 3 +-
llvm/lib/Support/Unix/Path.inc | 28 ++++++-----
llvm/lib/Support/VirtualFileSystem.cpp | 9 ++--
llvm/lib/Support/Windows/Path.inc | 4 +-
llvm/unittests/Support/MemoryBufferTest.cpp | 12 +++--
10 files changed, 97 insertions(+), 54 deletions(-)
create mode 100644 llvm/include/llvm/Support/File.h
diff --git a/llvm/include/llvm/Support/File.h b/llvm/include/llvm/Support/File.h
new file mode 100644
index 0000000000000..d1ce246cdb554
--- /dev/null
+++ b/llvm/include/llvm/Support/File.h
@@ -0,0 +1,55 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+/// \file
+/// This file declares llvm::sys::fs::file_t type.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_SUPPORT_FILE_H
+#define LLVM_SUPPORT_FILE_H
+
+namespace llvm::sys::fs {
+
+/// This class wraps the platform specific file handle/descriptor type to
+/// provide an unified representation.
+struct file_t {
+#if defined(_WIN32)
+ // A Win32 HANDLE is a typedef of void*
+ using value_type = void *;
+ static const value_type Invalid;
+#else
+ // A file descriptor on UNIX.
+ using value_type = int;
+ static constexpr value_type Invalid = -1;
+#endif
+ value_type Value;
+
+ /// Default constructor to invalid file.
+ file_t() : Value(Invalid) {}
+
+ /// Implicit constructor from underlying value.
+ // TODO: Make this explicit to flush out type mismatches.
+ file_t(value_type Value) : Value(Value) {}
+
+ /// Is a valid file.
+ bool isValid() const { return Value != Invalid; }
+
+ /// Get the underlying value and return a platform specific value.
+ value_type get() const { return Value; }
+};
+
+inline bool operator==(file_t LHS, file_t RHS) {
+ return LHS.get() == RHS.get();
+}
+
+inline bool operator!=(file_t LHS, file_t RHS) { return !(LHS == RHS); }
+
+} // namespace llvm::sys::fs
+
+#endif
diff --git a/llvm/include/llvm/Support/FileSystem.h b/llvm/include/llvm/Support/FileSystem.h
index c203779307840..72afcc4051854 100644
--- a/llvm/include/llvm/Support/FileSystem.h
+++ b/llvm/include/llvm/Support/FileSystem.h
@@ -35,6 +35,7 @@
#include "llvm/Support/Error.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/ErrorOr.h"
+#include "llvm/Support/File.h"
#include "llvm/Support/FileSystem/UniqueID.h"
#include "llvm/Support/MD5.h"
#include <cassert>
@@ -49,15 +50,6 @@ namespace llvm {
namespace sys {
namespace fs {
-#if defined(_WIN32)
-// A Win32 HANDLE is a typedef of void*
-using file_t = void *;
-#else
-using file_t = int;
-#endif
-
-LLVM_ABI extern const file_t kInvalidFile;
-
/// An enumeration for the file system's view of the type.
enum class file_type {
status_error,
@@ -645,13 +637,11 @@ LLVM_ABI std::error_code is_other(const Twine &path, bool &result);
LLVM_ABI std::error_code status(const Twine &path, file_status &result,
bool follow = true);
-/// A version for when a file descriptor is already available.
-LLVM_ABI std::error_code status(int FD, file_status &Result);
+/// A version for when a file is already available.
+LLVM_ABI std::error_code status(file_t F, file_status &Result);
-#ifdef _WIN32
/// A version for when a file descriptor is already available.
-LLVM_ABI std::error_code status(file_t FD, file_status &Result);
-#endif
+LLVM_ABI std::error_code status(int FD, file_status &Result);
/// Get file creation mode mask of the process.
///
@@ -1000,15 +990,15 @@ LLVM_ABI Expected<file_t> openNativeFile(const Twine &Name,
LLVM_ABI file_t convertFDToNativeFile(int FD);
#ifndef _WIN32
-inline file_t convertFDToNativeFile(int FD) { return FD; }
+inline file_t convertFDToNativeFile(int FD) { return file_t(FD); }
#endif
/// Return an open handle to standard in. On Unix, this is typically FD 0.
-/// Returns kInvalidFile when the stream is closed.
+/// Returns Invalid file_t when the stream is closed.
LLVM_ABI file_t getStdinHandle();
/// Return an open handle to standard out. On Unix, this is typically FD 1.
-/// Returns kInvalidFile when the stream is closed.
+/// Returns Invalid file_t when the stream is closed.
LLVM_ABI file_t getStdoutHandle();
/// Return an open handle to standard error. On Unix, this is typically FD 2.
diff --git a/llvm/include/llvm/Support/MemoryBuffer.h b/llvm/include/llvm/Support/MemoryBuffer.h
index f092c67265a31..e7f2b2090f453 100644
--- a/llvm/include/llvm/Support/MemoryBuffer.h
+++ b/llvm/include/llvm/Support/MemoryBuffer.h
@@ -21,23 +21,13 @@
#include "llvm/Support/CBindingWrapping.h"
#include "llvm/Support/Compiler.h"
#include "llvm/Support/ErrorOr.h"
+#include "llvm/Support/File.h"
#include "llvm/Support/MemoryBufferRef.h"
#include <cstddef>
#include <cstdint>
#include <memory>
namespace llvm {
-namespace sys {
-namespace fs {
-// Duplicated from FileSystem.h to avoid a dependency.
-#if defined(_WIN32)
-// A Win32 HANDLE is a typedef of void*
-using file_t = void *;
-#else
-using file_t = int;
-#endif
-} // namespace fs
-} // namespace sys
/// This interface provides simple read-only access to a block of memory, and
/// provides simple methods for reading files and standard input into a memory
diff --git a/llvm/lib/CAS/MappedFileRegionArena.cpp b/llvm/lib/CAS/MappedFileRegionArena.cpp
index 472843d78af6e..f5e76056a0698 100644
--- a/llvm/lib/CAS/MappedFileRegionArena.cpp
+++ b/llvm/lib/CAS/MappedFileRegionArena.cpp
@@ -373,7 +373,7 @@ Expected<int64_t> MappedFileRegionArena::allocateOffset(uint64_t AllocSize) {
ErrorOr<FileSizeInfo> FileSizeInfo::get(sys::fs::file_t File) {
#if LLVM_ON_UNIX && defined(MAPPED_FILE_BSIZE)
struct stat Status;
- int StatRet = ::fstat(File, &Status);
+ int StatRet = ::fstat(File.get(), &Status);
if (StatRet)
return errnoAsErrorCode();
uint64_t AllocatedSize = uint64_t(Status.st_blksize) * MAPPED_FILE_BSIZE;
diff --git a/llvm/lib/Object/ArchiveWriter.cpp b/llvm/lib/Object/ArchiveWriter.cpp
index 6fc0889afc6a8..efaec82f9b745 100644
--- a/llvm/lib/Object/ArchiveWriter.cpp
+++ b/llvm/lib/Object/ArchiveWriter.cpp
@@ -132,7 +132,7 @@ Expected<NewArchiveMember> NewArchiveMember::getFile(StringRef FileName,
if (!FDOrErr)
return FDOrErr.takeError();
sys::fs::file_t FD = *FDOrErr;
- assert(FD != sys::fs::kInvalidFile);
+ assert(FD.isValid());
if (auto EC = sys::fs::status(FD, Status))
return errorCodeToError(EC);
diff --git a/llvm/lib/Support/FileUtilities.cpp b/llvm/lib/Support/FileUtilities.cpp
index dbd6c324cf4dc..b8f92ac763e75 100644
--- a/llvm/lib/Support/FileUtilities.cpp
+++ b/llvm/lib/Support/FileUtilities.cpp
@@ -306,7 +306,8 @@ Error FilePermissionsApplier::apply(
return createFileError(OutputFilename, EC);
sys::fs::file_status OStat;
- if (std::error_code EC = sys::fs::status(FD, OStat))
+ if (std::error_code EC =
+ sys::fs::status(sys::fs::convertFDToNativeFile(FD), OStat))
return createFileError(OutputFilename, EC);
if (OStat.type() == sys::fs::file_type::regular_file) {
#ifndef _WIN32
diff --git a/llvm/lib/Support/Unix/Path.inc b/llvm/lib/Support/Unix/Path.inc
index 0d991ead72416..56b998d0d2440 100644
--- a/llvm/lib/Support/Unix/Path.inc
+++ b/llvm/lib/Support/Unix/Path.inc
@@ -120,8 +120,6 @@ namespace llvm {
namespace sys {
namespace fs {
-const file_t kInvalidFile = -1;
-
#if defined(__FreeBSD__) || defined(__NetBSD__) || defined(__OpenBSD__) || \
defined(__FreeBSD_kernel__) || defined(__linux__) || \
defined(__CYGWIN__) || defined(__DragonFly__) || defined(_AIX) || \
@@ -768,6 +766,10 @@ std::error_code status(const Twine &Path, file_status &Result, bool Follow) {
return fillStatus(StatRet, Status, Result);
}
+std::error_code status(file_t F, file_status &Result) {
+ return status(F.get(), Result);
+}
+
std::error_code status(int FD, file_status &Result) {
struct stat Status;
int StatRet = ::fstat(FD, &Status);
@@ -832,7 +834,7 @@ std::error_code setLastAccessAndModificationTime(int FD, TimePoint<> AccessTime,
#endif
}
-std::error_code mapped_file_region::init(int FD, uint64_t Offset,
+std::error_code mapped_file_region::init(file_t FD, uint64_t Offset,
mapmode Mode) {
assert(Size != 0);
@@ -861,13 +863,13 @@ std::error_code mapped_file_region::init(int FD, uint64_t Offset,
}
#endif // #if defined (__APPLE__)
- Mapping = ::mmap(nullptr, Size, prot, flags, FD, Offset);
+ Mapping = ::mmap(nullptr, Size, prot, flags, FD.get(), Offset);
if (Mapping == MAP_FAILED)
return errnoAsErrorCode();
return std::error_code();
}
-mapped_file_region::mapped_file_region(int fd, mapmode mode, size_t length,
+mapped_file_region::mapped_file_region(file_t fd, mapmode mode, size_t length,
uint64_t offset, std::error_code &ec)
: Size(length), Mode(mode) {
(void)Mode;
@@ -1128,9 +1130,9 @@ std::error_code openFile(const Twine &Name, int &ResultFD,
return std::error_code();
}
-Expected<int> openNativeFile(const Twine &Name, CreationDisposition Disp,
- FileAccess Access, OpenFlags Flags,
- unsigned Mode) {
+Expected<file_t> openNativeFile(const Twine &Name, CreationDisposition Disp,
+ FileAccess Access, OpenFlags Flags,
+ unsigned Mode) {
int FD;
std::error_code EC = openFile(Name, FD, Disp, Access, Flags, Mode);
@@ -1183,7 +1185,7 @@ std::error_code openFileForRead(const Twine &Name, int &ResultFD,
Expected<file_t> openNativeFileForRead(const Twine &Name, OpenFlags Flags,
SmallVectorImpl<char> *RealPath) {
- file_t ResultFD;
+ int ResultFD;
std::error_code EC = openFileForRead(Name, ResultFD, Flags, RealPath);
if (EC)
return errorCodeToError(EC);
@@ -1200,7 +1202,7 @@ Expected<size_t> readNativeFile(file_t FD, MutableArrayRef<char> Buf) {
#else
size_t Size = Buf.size();
#endif
- ssize_t NumRead = sys::RetryAfterSignal(-1, ::read, FD, Buf.data(), Size);
+ ssize_t NumRead = sys::RetryAfterSignal(-1, ::read, FD.get(), Buf.data(), Size);
if (NumRead == -1)
return errorCodeToError(errnoAsErrorCode());
// The underlying operation on these platforms allow opening directories
@@ -1224,7 +1226,7 @@ Expected<size_t> readNativeFileSlice(file_t FD, MutableArrayRef<char> Buf,
#endif
#ifdef HAVE_PREAD
ssize_t NumRead =
- sys::RetryAfterSignal(-1, ::pread, FD, Buf.data(), Size, Offset);
+ sys::RetryAfterSignal(-1, ::pread, FD.get(), Buf.data(), Size, Offset);
#else
if (lseek(FD, Offset, SEEK_SET) == -1)
return errorCodeToError(errnoAsErrorCode());
@@ -1297,8 +1299,8 @@ std::error_code unlockFile(int FD) {
std::error_code closeFile(file_t &F) {
file_t TmpF = F;
- F = kInvalidFile;
- return Process::SafelyCloseFileDescriptor(TmpF);
+ F = file_t::Invalid;
+ return Process::SafelyCloseFileDescriptor(TmpF.get());
}
template <typename T>
diff --git a/llvm/lib/Support/VirtualFileSystem.cpp b/llvm/lib/Support/VirtualFileSystem.cpp
index cf784595c2f1c..e71a5b691f6c5 100644
--- a/llvm/lib/Support/VirtualFileSystem.cpp
+++ b/llvm/lib/Support/VirtualFileSystem.cpp
@@ -56,7 +56,6 @@ using namespace llvm::vfs;
using llvm::sys::fs::file_t;
using llvm::sys::fs::file_status;
using llvm::sys::fs::file_type;
-using llvm::sys::fs::kInvalidFile;
using llvm::sys::fs::perms;
using llvm::sys::fs::UniqueID;
@@ -198,7 +197,7 @@ class RealFile : public File {
: FD(RawFD), S(NewName, {}, {}, {}, {}, {},
llvm::sys::fs::file_type::status_error, {}),
RealName(NewRealPathName.str()) {
- assert(FD != kInvalidFile && "Invalid or inactive file descriptor");
+ assert(FD.isValid() && "Invalid or inactive file descriptor");
}
public:
@@ -219,7 +218,7 @@ class RealFile : public File {
RealFile::~RealFile() { close(); }
ErrorOr<Status> RealFile::status() {
- assert(FD != kInvalidFile && "cannot stat closed file");
+ assert(FD.isValid() && "cannot stat closed file");
if (!S.isStatusKnown()) {
file_status RealStatus;
if (std::error_code EC = sys::fs::status(FD, RealStatus))
@@ -236,14 +235,14 @@ ErrorOr<std::string> RealFile::getName() {
ErrorOr<std::unique_ptr<MemoryBuffer>>
RealFile::getBuffer(const Twine &Name, int64_t FileSize,
bool RequiresNullTerminator, bool IsVolatile) {
- assert(FD != kInvalidFile && "cannot get buffer for closed file");
+ assert(FD.isValid() && "cannot get buffer for closed file");
return MemoryBuffer::getOpenFile(FD, Name, FileSize, RequiresNullTerminator,
IsVolatile);
}
std::error_code RealFile::close() {
std::error_code EC = sys::fs::closeFile(FD);
- FD = kInvalidFile;
+ FD = file_t::Invalid;
return EC;
}
diff --git a/llvm/lib/Support/Windows/Path.inc b/llvm/lib/Support/Windows/Path.inc
index be007b7abdb51..860256a1f865b 100644
--- a/llvm/lib/Support/Windows/Path.inc
+++ b/llvm/lib/Support/Windows/Path.inc
@@ -130,7 +130,7 @@ std::error_code widenPath(const Twine &Path8, SmallVectorImpl<wchar_t> &Path16,
namespace fs {
-const file_t kInvalidFile = INVALID_HANDLE_VALUE;
+const file_t::value_type file_t::Invalid = INVALID_HANDLE_VALUE;
std::string getMainExecutable(const char *argv0, void *MainExecAddr) {
SmallVector<wchar_t, MAX_PATH> PathName;
@@ -1397,7 +1397,7 @@ std::error_code unlockFile(int FD) {
std::error_code closeFile(file_t &F) {
file_t TmpF = F;
- F = kInvalidFile;
+ F = file_t::Invalid;
if (!::CloseHandle(TmpF))
return mapWindowsError(::GetLastError());
return std::error_code();
diff --git a/llvm/unittests/Support/MemoryBufferTest.cpp b/llvm/unittests/Support/MemoryBufferTest.cpp
index 2cc20c0ab5f0c..b52b4cabe2488 100644
--- a/llvm/unittests/Support/MemoryBufferTest.cpp
+++ b/llvm/unittests/Support/MemoryBufferTest.cpp
@@ -170,16 +170,22 @@ TEST_F(MemoryBufferTest, copy) {
#if LLVM_ENABLE_THREADS
TEST_F(MemoryBufferTest, createFromPipe) {
- sys::fs::file_t pipes[2];
+ int pipes[2];
#if LLVM_ON_UNIX
ASSERT_EQ(::pipe(pipes), 0) << strerror(errno);
#else
ASSERT_TRUE(::CreatePipe(&pipes[0], &pipes[1], nullptr, 0))
<< ::GetLastError();
#endif
- auto ReadCloser = make_scope_exit([&] { sys::fs::closeFile(pipes[0]); });
+ auto ReadCloser = make_scope_exit([&] {
+ sys::fs::file_t F(pipes[0]);
+ sys::fs::closeFile(F);
+ });
std::thread Writer([&] {
- auto WriteCloser = make_scope_exit([&] { sys::fs::closeFile(pipes[1]); });
+ auto WriteCloser = make_scope_exit([&] {
+ sys::fs::file_t F(pipes[1]);
+ sys::fs::closeFile(F);
+ });
for (unsigned i = 0; i < 5; ++i) {
std::this_thread::sleep_for(std::chrono::milliseconds(10));
#if LLVM_ON_UNIX
>From 9690e48487c32219620d5e7a5896fc5ae34f7f29 Mon Sep 17 00:00:00 2001
From: Steven Wu <stevenwu at apple.com>
Date: Wed, 24 Sep 2025 12:32:26 -0700
Subject: [PATCH 2/2] clang-format
Created using spr 1.3.7
---
llvm/lib/Support/Unix/Path.inc | 3 ++-
llvm/lib/Support/VirtualFileSystem.cpp | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/Support/Unix/Path.inc b/llvm/lib/Support/Unix/Path.inc
index 56b998d0d2440..49eea46dab55d 100644
--- a/llvm/lib/Support/Unix/Path.inc
+++ b/llvm/lib/Support/Unix/Path.inc
@@ -1202,7 +1202,8 @@ Expected<size_t> readNativeFile(file_t FD, MutableArrayRef<char> Buf) {
#else
size_t Size = Buf.size();
#endif
- ssize_t NumRead = sys::RetryAfterSignal(-1, ::read, FD.get(), Buf.data(), Size);
+ ssize_t NumRead =
+ sys::RetryAfterSignal(-1, ::read, FD.get(), Buf.data(), Size);
if (NumRead == -1)
return errorCodeToError(errnoAsErrorCode());
// The underlying operation on these platforms allow opening directories
diff --git a/llvm/lib/Support/VirtualFileSystem.cpp b/llvm/lib/Support/VirtualFileSystem.cpp
index e71a5b691f6c5..20460213fb0dc 100644
--- a/llvm/lib/Support/VirtualFileSystem.cpp
+++ b/llvm/lib/Support/VirtualFileSystem.cpp
@@ -53,8 +53,8 @@
using namespace llvm;
using namespace llvm::vfs;
-using llvm::sys::fs::file_t;
using llvm::sys::fs::file_status;
+using llvm::sys::fs::file_t;
using llvm::sys::fs::file_type;
using llvm::sys::fs::perms;
using llvm::sys::fs::UniqueID;
More information about the llvm-commits
mailing list