-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[llvm][Support] Make sys::fs::file_t into a seperate type #160588
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 6 commits
2f9ef51
9690e48
a67cbb3
724d163
964978d
dc3d12c
27a3e43
6e9c978
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| //===----------------------------------------------------------------------===// | ||
| // | ||
| // 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 | ||
|
|
||
| #include "llvm/Support/Compiler.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 *; | ||
| #else | ||
| // A file descriptor on UNIX. | ||
| using value_type = int; | ||
| #endif | ||
| value_type Value; | ||
|
|
||
| LLVM_ABI static const value_type Invalid; | ||
|
||
|
|
||
| /// 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 | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would "_t" mean in this context? Previously it was a short for "typedef", but here should we keep that? Or make it something more explicit, like
file?file_desc?file_descriptor?file_handle?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is mostly keeping the source compatibility with the old name to avoid the need to updating the entire project.
I don't mind doing that but just seems unnecessary. Maybe I can still use a deprecated
typedeffromfile_tto the new type name to avoid a hard transition. I am open to new name suggestions.