[PATCH] D116659: [llvm][clang][vfs] NFC: Extract directory iteration boilerplate into macro
jansvoboda11 updated this revision to Diff 399275.
jansvoboda11 added a comment.
Herald added a subscriber: hiraditya.
Replace macro with range of new iterators
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D116659/new/
https://reviews.llvm.org/D116659
Files:
clang/lib/Driver/ToolChain.cpp
clang/lib/Driver/ToolChains/AMDGPU.cpp
clang/lib/Driver/ToolChains/BareMetal.cpp
clang/lib/Driver/ToolChains/Cuda.cpp
clang/lib/Driver/ToolChains/Gnu.cpp
clang/lib/Driver/ToolChains/MSVC.cpp
clang/lib/Frontend/CompilerInstance.cpp
clang/lib/Frontend/FrontendAction.cpp
clang/lib/Lex/HeaderSearch.cpp
clang/lib/Lex/ModuleMap.cpp
clang/lib/Sema/SemaCodeComplete.cpp
llvm/include/llvm/Support/VirtualFileSystem.h
llvm/lib/Support/VirtualFileSystem.cpp
Index: llvm/lib/Support/VirtualFileSystem.cpp
===
--- llvm/lib/Support/VirtualFileSystem.cpp
+++ llvm/lib/Support/VirtualFileSystem.cpp
@@ -128,6 +128,13 @@
return (*F)->getBuffer(Name, FileSize, RequiresNullTerminator, IsVolatile);
}
+iterator_range
+FileSystem::dir_range(const Twine ) {
+ std::error_code EC;
+ return {directory_iterator_error_wrapper(dir_begin(Dir, EC), EC),
+ directory_iterator_error_wrapper()};
+}
+
std::error_code FileSystem::makeAbsolute(SmallVectorImpl ) const {
if (llvm::sys::path::is_absolute(Path))
return {};
Index: llvm/include/llvm/Support/VirtualFileSystem.h
===
--- llvm/include/llvm/Support/VirtualFileSystem.h
+++ llvm/include/llvm/Support/VirtualFileSystem.h
@@ -202,6 +202,38 @@
}
};
+/// Wrapper for directory_iterator that automatically handles error codes.
+class directory_iterator_error_wrapper {
+ directory_iterator It;
+
+public:
+ directory_iterator_error_wrapper(directory_iterator It, std::error_code )
+ : It(std::move(It)) {
+if (EC)
+ It = directory_iterator();
+ }
+
+ directory_iterator_error_wrapper() = default;
+
+ directory_iterator_error_wrapper ++() {
+std::error_code EC;
+It.increment(EC);
+if (EC)
+ It = directory_iterator();
+return *this;
+ }
+
+ const directory_entry *() const { return *It; }
+ const directory_entry *operator->() const { return &*It; }
+
+ bool operator==(const directory_iterator_error_wrapper ) const {
+return It == RHS.It;
+ }
+ bool operator!=(const directory_iterator_error_wrapper ) const {
+return !(*this == RHS);
+ }
+};
+
class FileSystem;
namespace detail {
@@ -274,6 +306,9 @@
virtual directory_iterator dir_begin(const Twine ,
std::error_code ) = 0;
+ /// Get a directory_iterator range for \p Dir.
+ iterator_range dir_range(const Twine );
+
/// Set the working directory. This will affect all following operations on
/// this file system and may propagate down for nested file systems.
virtual std::error_code setCurrentWorkingDirectory(const Twine ) = 0;
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -9896,20 +9896,18 @@
const bool isQt = Dirname.startswith("Qt") || Dirname == "ActiveQt";
const bool ExtensionlessHeaders =
IsSystem || isQt || Dir.endswith(".framework/Headers");
-std::error_code EC;
unsigned Count = 0;
-for (auto It = FS.dir_begin(Dir, EC);
- !EC && It != llvm::vfs::directory_iterator(); It.increment(EC)) {
+for (const auto : FS.dir_range(Dir)) {
if (++Count == 2500) // If we happen to hit a huge directory,
break; // bail out early so we're not too slow.
- StringRef Filename = llvm::sys::path::filename(It->path());
+ StringRef Filename = llvm::sys::path::filename(Entry.path());
// To know whether a symlink should be treated as file or a directory, we
// have to stat it. This should be cheap enough as there shouldn't be many
// symlinks.
- llvm::sys::fs::file_type Type = It->type();
+ llvm::sys::fs::file_type Type = Entry.type();
if (Type == llvm::sys::fs::file_type::symlink_file) {
-if (auto FileStatus = FS.status(It->path()))
+if (auto FileStatus = FS.status(Entry.path()))
Type = FileStatus->getType();
}
switch (Type) {
Index: clang/lib/Lex/ModuleMap.cpp
===
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -1057,21 +1057,16 @@
Result->InferExportWildcard = true;
// Look for subframeworks.
- std::error_code EC;
SmallString<128> SubframeworksDirName
= StringRef(FrameworkDir->getName());
llvm::sys::path::append(SubframeworksDirName, "Frameworks");
llvm::sys::path::native(SubframeworksDirName);
llvm::vfs::FileSystem =
[PATCH] D116659: [llvm][clang][vfs] NFC: Extract directory iteration boilerplate into macro
dexonsmith added a comment.
I think th eAPI is designed to ensure users don't forget to check the error
code.
What about a new API/iterator type that wraps the existing one and has a
"normal" `operator++`?
- Take an error as an out-parameter on construction
- Make it an `llvm::Error` which guarantees a crash if not checked
- This would wrap the other type and store a reference to the `Error`. If
there's an error during iteration, advances to end and sets the error.
Here's what I'm thinking:
Optional DirError;
for (vfs::error_directory_iterator File = D.getVFS().dir_begin(Cand.Path,
DirError),
FileEnd;
File != FileEnd; ++File) {
// ...
}
if (DirError)
return errorToErrorCode(std::move(*IterationEC));
At that point, it'd be easy to add a `dir_range` wrapper that creates an
iterator range:
for (auto : dir_range(Path, DirError)) {
// ...
}
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D116659/new/
https://reviews.llvm.org/D116659
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D116659: [llvm][clang][vfs] NFC: Extract directory iteration boilerplate into macro
jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith, arphaman.
Herald added subscribers: kerbowa, nhaehnle, jvesely.
jansvoboda11 requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.
The iteration over directory entries of a VFS is a bit unweildy, since it
requires using a pair of `llvm::vfs::directory_iterator`, calling
`llvm::vfs::directory_iterator::increment(std::error_code &)` and checking
`std::error_code`. Currently, there are 13 instances of this pattern in the
Clang codebase.
This patch simplifies iteration over directory entries by extracting the
boilerplate into a macro.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D116659
Files:
clang/lib/Driver/ToolChains/AMDGPU.cpp
clang/lib/Driver/ToolChains/BareMetal.cpp
clang/lib/Driver/ToolChains/Cuda.cpp
clang/lib/Driver/ToolChains/Gnu.cpp
clang/lib/Driver/ToolChains/MSVC.cpp
clang/lib/Frontend/CompilerInstance.cpp
clang/lib/Frontend/FrontendAction.cpp
clang/lib/Lex/HeaderSearch.cpp
clang/lib/Lex/ModuleMap.cpp
clang/lib/Sema/SemaCodeComplete.cpp
llvm/include/llvm/Support/VirtualFileSystem.h
Index: llvm/include/llvm/Support/VirtualFileSystem.h
===
--- llvm/include/llvm/Support/VirtualFileSystem.h
+++ llvm/include/llvm/Support/VirtualFileSystem.h
@@ -319,6 +319,13 @@
/// that of the process.
std::unique_ptr createPhysicalFileSystem();
+/// Expands to a loop header that uses iterator named IT for iterating over
+/// entries of the directory DIR within the VFS.
+#define LLVM_VFS_DIR_FOREACH(VFS, DIR, IT) \
+ std::error_code EC; \
+ for (llvm::vfs::directory_iterator IT = VFS.dir_begin(DIR, EC), End; \
+ !EC && IT != End; IT = IT.increment(EC))
+
/// A file system that allows overlaying one \p AbstractFileSystem on top
/// of another.
///
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -9888,10 +9888,8 @@
const bool isQt = Dirname.startswith("Qt") || Dirname == "ActiveQt";
const bool ExtensionlessHeaders =
IsSystem || isQt || Dir.endswith(".framework/Headers");
-std::error_code EC;
unsigned Count = 0;
-for (auto It = FS.dir_begin(Dir, EC);
- !EC && It != llvm::vfs::directory_iterator(); It.increment(EC)) {
+LLVM_VFS_DIR_FOREACH(FS, Dir, It) {
if (++Count == 2500) // If we happen to hit a huge directory,
break; // bail out early so we're not too slow.
StringRef Filename = llvm::sys::path::filename(It->path());
Index: clang/lib/Lex/ModuleMap.cpp
===
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -1057,16 +1057,12 @@
Result->InferExportWildcard = true;
// Look for subframeworks.
- std::error_code EC;
SmallString<128> SubframeworksDirName
= StringRef(FrameworkDir->getName());
llvm::sys::path::append(SubframeworksDirName, "Frameworks");
llvm::sys::path::native(SubframeworksDirName);
llvm::vfs::FileSystem = FileMgr.getVirtualFileSystem();
- for (llvm::vfs::directory_iterator
- Dir = FS.dir_begin(SubframeworksDirName, EC),
- DirEnd;
- Dir != DirEnd && !EC; Dir.increment(EC)) {
+ LLVM_VFS_DIR_FOREACH(FS, SubframeworksDirName, Dir) {
if (!StringRef(Dir->path()).endswith(".framework"))
continue;
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1740,16 +1740,13 @@
for (unsigned Idx = 0, N = SearchDirs.size(); Idx != N; ++Idx) {
bool IsSystem = SearchDirs[Idx].isSystemHeaderDirectory();
if (SearchDirs[Idx].isFramework()) {
-std::error_code EC;
SmallString<128> DirNative;
llvm::sys::path::native(SearchDirs[Idx].getFrameworkDir()->getName(),
DirNative);
// Search each of the ".framework" directories to load them as modules.
llvm::vfs::FileSystem = FileMgr.getVirtualFileSystem();
-for (llvm::vfs::directory_iterator Dir = FS.dir_begin(DirNative, EC),
- DirEnd;
- Dir != DirEnd && !EC; Dir.increment(EC)) {
+LLVM_VFS_DIR_FOREACH(FS, DirNative, Dir) {
if (llvm::sys::path::extension(Dir->path()) != ".framework")
continue;
@@ -1809,14 +1806,12 @@
if (SearchDir.haveSearchedAllModuleMaps())
return;
- std::error_code EC;
SmallString<128> Dir = SearchDir.getDir()->getName();
FileMgr.makeAbsolutePath(Dir);
SmallString<128> DirNative;
