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); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
