r370562 - Introduce a DirectoryEntryRef that stores both a reference and an
Alex Lorenz via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 30 18:26:04 PDT 2019
Author: arphaman
Date: Fri Aug 30 18:26:04 2019
New Revision: 370562
URL: http://llvm.org/viewvc/llvm-project?rev=370562&view=rev
Log:
Introduce a DirectoryEntryRef that stores both a reference and an
accessed name to the directory entry
This commit introduces a parallel API that returns a DirectoryEntryRef
to the FileManager, similar to the parallel FileEntryRef API. All
uses will have to be update in follow-up patches. The immediate use of the new API in this
patch fixes the issue where a file manager was reused in clang-scan-deps,
but reported an different file path whenever a framework lookup was done through a symlink.
Differential Revision: https://reviews.llvm.org/D67026
Modified:
cfe/trunk/include/clang/Basic/FileManager.h
cfe/trunk/include/clang/Lex/DirectoryLookup.h
cfe/trunk/lib/Basic/FileManager.cpp
cfe/trunk/lib/Frontend/InitHeaderSearch.cpp
cfe/trunk/lib/Lex/HeaderSearch.cpp
cfe/trunk/lib/Lex/PPDirectives.cpp
cfe/trunk/test/ClangScanDeps/Inputs/subframework_header_dir_symlink_cdb.json
cfe/trunk/test/ClangScanDeps/subframework_header_dir_symlink.m
cfe/trunk/unittests/Lex/HeaderSearchTest.cpp
cfe/trunk/unittests/Lex/PPCallbacksTest.cpp
Modified: cfe/trunk/include/clang/Basic/FileManager.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/FileManager.h?rev=370562&r1=370561&r2=370562&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/FileManager.h (original)
+++ cfe/trunk/include/clang/Basic/FileManager.h Fri Aug 30 18:26:04 2019
@@ -45,12 +45,31 @@ class FileSystemStatCache;
class DirectoryEntry {
friend class FileManager;
+ // FIXME: We should not be storing a directory entry name here.
StringRef Name; // Name of the directory.
public:
StringRef getName() const { return Name; }
};
+/// A reference to a \c DirectoryEntry that includes the name of the directory
+/// as it was accessed by the FileManager's client.
+class DirectoryEntryRef {
+public:
+ const DirectoryEntry &getDirEntry() const { return *Entry->getValue(); }
+
+ StringRef getName() const { return Entry->getKey(); }
+
+private:
+ friend class FileManager;
+
+ DirectoryEntryRef(
+ llvm::StringMapEntry<llvm::ErrorOr<DirectoryEntry &>> *Entry)
+ : Entry(Entry) {}
+
+ const llvm::StringMapEntry<llvm::ErrorOr<DirectoryEntry &>> *Entry;
+};
+
/// Cached information about one file (either on disk
/// or in the virtual file system).
///
@@ -259,6 +278,29 @@ public:
/// virtual).
///
/// This returns a \c std::error_code if there was an error reading the
+ /// directory. On success, returns the reference to the directory entry
+ /// together with the exact path that was used to access a file by a
+ /// particular call to getDirectoryRef.
+ ///
+ /// \param CacheFailure If true and the file does not exist, we'll cache
+ /// the failure to find this file.
+ llvm::Expected<DirectoryEntryRef> getDirectoryRef(StringRef DirName,
+ bool CacheFailure = true);
+
+ /// Get a \c DirectoryEntryRef if it exists, without doing anything on error.
+ llvm::Optional<DirectoryEntryRef>
+ getOptionalDirectoryRef(StringRef DirName, bool CacheFailure = true) {
+ return llvm::expectedToOptional(getDirectoryRef(DirName, CacheFailure));
+ }
+
+ /// Lookup, cache, and verify the specified directory (real or
+ /// virtual).
+ ///
+ /// This function is deprecated and will be removed at some point in the
+ /// future, new clients should use
+ /// \c getDirectoryRef.
+ ///
+ /// This returns a \c std::error_code if there was an error reading the
/// directory. If there is no error, the DirectoryEntry is guaranteed to be
/// non-NULL.
///
Modified: cfe/trunk/include/clang/Lex/DirectoryLookup.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/DirectoryLookup.h?rev=370562&r1=370561&r2=370562&view=diff
==============================================================================
--- cfe/trunk/include/clang/Lex/DirectoryLookup.h (original)
+++ cfe/trunk/include/clang/Lex/DirectoryLookup.h Fri Aug 30 18:26:04 2019
@@ -36,14 +36,17 @@ public:
LT_HeaderMap
};
private:
- union { // This union is discriminated by isHeaderMap.
+ union DLU { // This union is discriminated by isHeaderMap.
/// Dir - This is the actual directory that we're referring to for a normal
/// directory or a framework.
- const DirectoryEntry *Dir;
+ DirectoryEntryRef Dir;
/// Map - This is the HeaderMap if this is a headermap lookup.
///
const HeaderMap *Map;
+
+ DLU(DirectoryEntryRef Dir) : Dir(Dir) {}
+ DLU(const HeaderMap *Map) : Map(Map) {}
} u;
/// DirCharacteristic - The type of directory this is: this is an instance of
@@ -62,24 +65,18 @@ private:
unsigned SearchedAllModuleMaps : 1;
public:
- /// DirectoryLookup ctor - Note that this ctor *does not take ownership* of
- /// 'dir'.
- DirectoryLookup(const DirectoryEntry *dir, SrcMgr::CharacteristicKind DT,
+ /// This ctor *does not take ownership* of 'Dir'.
+ DirectoryLookup(DirectoryEntryRef Dir, SrcMgr::CharacteristicKind DT,
bool isFramework)
- : DirCharacteristic(DT),
- LookupType(isFramework ? LT_Framework : LT_NormalDir),
- IsIndexHeaderMap(false), SearchedAllModuleMaps(false) {
- u.Dir = dir;
- }
+ : u(Dir), DirCharacteristic(DT),
+ LookupType(isFramework ? LT_Framework : LT_NormalDir),
+ IsIndexHeaderMap(false), SearchedAllModuleMaps(false) {}
- /// DirectoryLookup ctor - Note that this ctor *does not take ownership* of
- /// 'map'.
- DirectoryLookup(const HeaderMap *map, SrcMgr::CharacteristicKind DT,
+ /// This ctor *does not take ownership* of 'Map'.
+ DirectoryLookup(const HeaderMap *Map, SrcMgr::CharacteristicKind DT,
bool isIndexHeaderMap)
- : DirCharacteristic(DT), LookupType(LT_HeaderMap),
- IsIndexHeaderMap(isIndexHeaderMap), SearchedAllModuleMaps(false) {
- u.Map = map;
- }
+ : u(Map), DirCharacteristic(DT), LookupType(LT_HeaderMap),
+ IsIndexHeaderMap(isIndexHeaderMap), SearchedAllModuleMaps(false) {}
/// getLookupType - Return the kind of directory lookup that this is: either a
/// normal directory, a framework path, or a HeaderMap.
@@ -92,13 +89,17 @@ public:
/// getDir - Return the directory that this entry refers to.
///
const DirectoryEntry *getDir() const {
- return isNormalDir() ? u.Dir : nullptr;
+ return isNormalDir() ? &u.Dir.getDirEntry() : nullptr;
}
/// getFrameworkDir - Return the directory that this framework refers to.
///
const DirectoryEntry *getFrameworkDir() const {
- return isFramework() ? u.Dir : nullptr;
+ return isFramework() ? &u.Dir.getDirEntry() : nullptr;
+ }
+
+ Optional<DirectoryEntryRef> getFrameworkDirRef() const {
+ return isFramework() ? Optional<DirectoryEntryRef>(u.Dir) : None;
}
/// getHeaderMap - Return the directory that this entry refers to.
Modified: cfe/trunk/lib/Basic/FileManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=370562&r1=370561&r2=370562&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/FileManager.cpp (original)
+++ cfe/trunk/lib/Basic/FileManager.cpp Fri Aug 30 18:26:04 2019
@@ -107,16 +107,8 @@ void FileManager::addAncestorsAsVirtualD
addAncestorsAsVirtualDirs(DirName);
}
-/// Converts a llvm::ErrorOr<T &> to an llvm::ErrorOr<T *> by promoting
-/// the address of the inner reference to a pointer or by propagating the error.
-template <typename T>
-static llvm::ErrorOr<T *> promoteInnerReference(llvm::ErrorOr<T &> value) {
- if (value) return &*value;
- return value.getError();
-}
-
-llvm::ErrorOr<const DirectoryEntry *>
-FileManager::getDirectory(StringRef DirName, bool CacheFailure) {
+llvm::Expected<DirectoryEntryRef>
+FileManager::getDirectoryRef(StringRef DirName, bool CacheFailure) {
// stat doesn't like trailing separators except for root directory.
// At least, on Win32 MSVCRT, stat() cannot strip trailing '/'.
// (though it can strip '\\')
@@ -141,8 +133,11 @@ FileManager::getDirectory(StringRef DirN
// contains both virtual and real directories.
auto SeenDirInsertResult =
SeenDirEntries.insert({DirName, std::errc::no_such_file_or_directory});
- if (!SeenDirInsertResult.second)
- return promoteInnerReference(SeenDirInsertResult.first->second);
+ if (!SeenDirInsertResult.second) {
+ if (SeenDirInsertResult.first->second)
+ return DirectoryEntryRef(&*SeenDirInsertResult.first);
+ return llvm::errorCodeToError(SeenDirInsertResult.first->second.getError());
+ }
// We've not seen this before. Fill it in.
++NumDirCacheMisses;
@@ -163,7 +158,7 @@ FileManager::getDirectory(StringRef DirN
NamedDirEnt.second = statError;
else
SeenDirEntries.erase(DirName);
- return statError;
+ return llvm::errorCodeToError(statError);
}
// It exists. See if we have already opened a directory with the
@@ -179,7 +174,15 @@ FileManager::getDirectory(StringRef DirN
UDE.Name = InterndDirName;
}
- return &UDE;
+ return DirectoryEntryRef(&NamedDirEnt);
+}
+
+llvm::ErrorOr<const DirectoryEntry *>
+FileManager::getDirectory(StringRef DirName, bool CacheFailure) {
+ auto Result = getDirectoryRef(DirName, CacheFailure);
+ if (Result)
+ return &Result->getDirEntry();
+ return llvm::errorToErrorCode(Result.takeError());
}
llvm::ErrorOr<const FileEntry *>
Modified: cfe/trunk/lib/Frontend/InitHeaderSearch.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/InitHeaderSearch.cpp?rev=370562&r1=370561&r2=370562&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/InitHeaderSearch.cpp (original)
+++ cfe/trunk/lib/Frontend/InitHeaderSearch.cpp Fri Aug 30 18:26:04 2019
@@ -148,7 +148,7 @@ bool InitHeaderSearch::AddUnmappedPath(c
}
// If the directory exists, add it.
- if (auto DE = FM.getDirectory(MappedPathStr)) {
+ if (auto DE = FM.getOptionalDirectoryRef(MappedPathStr)) {
IncludePath.push_back(
std::make_pair(Group, DirectoryLookup(*DE, Type, isFramework)));
return true;
Modified: cfe/trunk/lib/Lex/HeaderSearch.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/HeaderSearch.cpp?rev=370562&r1=370561&r2=370562&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/HeaderSearch.cpp (original)
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp Fri Aug 30 18:26:04 2019
@@ -296,6 +296,7 @@ Module *HeaderSearch::lookupModule(Strin
/// getName - Return the directory or filename corresponding to this lookup
/// object.
StringRef DirectoryLookup::getName() const {
+ // FIXME: Use the name from \c DirectoryEntryRef.
if (isNormalDir())
return getDir()->getName();
if (isFramework())
@@ -496,7 +497,7 @@ Optional<FileEntryRef> DirectoryLookup::
// FrameworkName = "/System/Library/Frameworks/"
SmallString<1024> FrameworkName;
- FrameworkName += getFrameworkDir()->getName();
+ FrameworkName += getFrameworkDirRef()->getName();
if (FrameworkName.empty() || FrameworkName.back() != '/')
FrameworkName.push_back('/');
Modified: cfe/trunk/lib/Lex/PPDirectives.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=370562&r1=370561&r2=370562&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/PPDirectives.cpp (original)
+++ cfe/trunk/lib/Lex/PPDirectives.cpp Fri Aug 30 18:26:04 2019
@@ -1695,7 +1695,7 @@ Optional<FileEntryRef> Preprocessor::Loo
// Give the clients a chance to recover.
SmallString<128> RecoveryPath;
if (Callbacks->FileNotFound(Filename, RecoveryPath)) {
- if (auto DE = FileMgr.getDirectory(RecoveryPath)) {
+ if (auto DE = FileMgr.getOptionalDirectoryRef(RecoveryPath)) {
// Add the recovery path to the list of search paths.
DirectoryLookup DL(*DE, SrcMgr::C_User, false);
HeaderInfo.AddSearchPath(DL, isAngled);
Modified: cfe/trunk/test/ClangScanDeps/Inputs/subframework_header_dir_symlink_cdb.json
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ClangScanDeps/Inputs/subframework_header_dir_symlink_cdb.json?rev=370562&r1=370561&r2=370562&view=diff
==============================================================================
--- cfe/trunk/test/ClangScanDeps/Inputs/subframework_header_dir_symlink_cdb.json (original)
+++ cfe/trunk/test/ClangScanDeps/Inputs/subframework_header_dir_symlink_cdb.json Fri Aug 30 18:26:04 2019
@@ -6,7 +6,7 @@
},
{
"directory": "DIR",
- "command": "clang -E DIR/subframework_header_dir_symlink2.m -FInputs/frameworks_symlink -iframework Inputs/frameworks",
+ "command": "clang -E DIR/subframework_header_dir_symlink2.m -FInputs/frameworks -iframework Inputs/frameworks_symlink",
"file": "DIR/subframework_header_dir_symlink2.m"
}
]
Modified: cfe/trunk/test/ClangScanDeps/subframework_header_dir_symlink.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ClangScanDeps/subframework_header_dir_symlink.m?rev=370562&r1=370561&r2=370562&view=diff
==============================================================================
--- cfe/trunk/test/ClangScanDeps/subframework_header_dir_symlink.m (original)
+++ cfe/trunk/test/ClangScanDeps/subframework_header_dir_symlink.m Fri Aug 30 18:26:04 2019
@@ -10,9 +10,8 @@
// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/subframework_header_dir_symlink_cdb.json > %t.cdb
// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -reuse-filemanager=0 | \
// RUN: FileCheck %s
-// FIXME: Make this work when the filemanager is reused:
// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -reuse-filemanager=1 | \
-// RUN: not FileCheck %s
+// RUN: FileCheck %s
#ifndef EMPTY
#include "Framework/Framework.h"
Modified: cfe/trunk/unittests/Lex/HeaderSearchTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Lex/HeaderSearchTest.cpp?rev=370562&r1=370561&r2=370562&view=diff
==============================================================================
--- cfe/trunk/unittests/Lex/HeaderSearchTest.cpp (original)
+++ cfe/trunk/unittests/Lex/HeaderSearchTest.cpp Fri Aug 30 18:26:04 2019
@@ -39,7 +39,7 @@ protected:
void addSearchDir(llvm::StringRef Dir) {
VFS->addFile(Dir, 0, llvm::MemoryBuffer::getMemBuffer(""), /*User=*/None,
/*Group=*/None, llvm::sys::fs::file_type::directory_file);
- auto DE = FileMgr.getDirectory(Dir);
+ auto DE = FileMgr.getOptionalDirectoryRef(Dir);
assert(DE);
auto DL = DirectoryLookup(*DE, SrcMgr::C_User, /*isFramework=*/false);
Search.AddSearchPath(DL, /*isAngled=*/false);
Modified: cfe/trunk/unittests/Lex/PPCallbacksTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Lex/PPCallbacksTest.cpp?rev=370562&r1=370561&r2=370562&view=diff
==============================================================================
--- cfe/trunk/unittests/Lex/PPCallbacksTest.cpp (original)
+++ cfe/trunk/unittests/Lex/PPCallbacksTest.cpp Fri Aug 30 18:26:04 2019
@@ -145,7 +145,7 @@ protected:
// Add header's parent path to search path.
StringRef SearchPath = llvm::sys::path::parent_path(HeaderPath);
- auto DE = FileMgr.getDirectory(SearchPath);
+ auto DE = FileMgr.getOptionalDirectoryRef(SearchPath);
DirectoryLookup DL(*DE, SrcMgr::C_User, false);
HeaderInfo.AddSearchPath(DL, IsSystemHeader);
}
More information about the cfe-commits
mailing list