aganea created this revision. aganea added reviewers: mstorsjo, hans, kbobyrev, dexonsmith, zero9178. Herald added subscribers: kadircet, arphaman, hiraditya. Herald added a project: All. aganea requested review of this revision. Herald added projects: clang, LLVM, clang-tools-extra. Herald added subscribers: cfe-commits, llvm-commits.
Before this patch, `UniqueID` was thought to store stable file IDs. However a recently reported issue <https://github.com/llvm/llvm-project/issues/61401> shows that the file IDs are not always stable, at least on Windows. The documentation for the underlying Windows API <https://learn.microsoft.com/en-us/windows/win32/api/fileapi/ns-fileapi-by_handle_file_information> states: //(note the emphasis)// > To determine whether two **open handles** represent the same file, combine > the identifier and the volume serial number for each file and compare them. Currently, `UniqueID` does not keep the file handles open. The fact that file IDs were stable until now is only due to a NTFS implementation detail <https://github.com/llvm/llvm-project/issues/61401#issuecomment-1473072118>. In some cases, such as drives mounted on network locations, the underlying Windows drivers might or might not generate stable file IDs. To fix the issue, we keep the file handles open during the lifetime of their corresponding `UniqueID` instances. Since handles will live longer now, this requires particular attention when performing some file actions, such as file deletions. Should fix #61401 <https://github.com/llvm/llvm-project/issues/61401> and #22079 <https://github.com/llvm/llvm-project/issues/22079>. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D146490 Files: clang-tools-extra/clangd/unittests/FSTests.cpp clang/include/clang/Basic/FileEntry.h clang/lib/Basic/FileEntry.cpp clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp clang/tools/libclang/Indexing.cpp clang/unittests/Basic/FileManagerTest.cpp llvm/include/llvm/Support/FileSystem.h llvm/include/llvm/Support/FileSystem/UniqueID.h llvm/include/llvm/Support/Windows/ScopedHandle.h llvm/include/llvm/Support/Windows/WindowsSupport.h llvm/lib/Support/VirtualFileSystem.cpp llvm/lib/Support/Windows/Path.inc llvm/unittests/Support/FSUniqueIDTest.cpp llvm/unittests/Support/Path.cpp llvm/unittests/Support/VirtualFileSystemTest.cpp llvm/utils/split-file/split-file.cpp
Index: llvm/utils/split-file/split-file.cpp =================================================================== --- llvm/utils/split-file/split-file.cpp +++ llvm/utils/split-file/split-file.cpp @@ -162,14 +162,16 @@ // Delete output if it is a file or an empty directory, so that we can create // a directory. - sys::fs::file_status status; - if (std::error_code ec = sys::fs::status(output, status)) - if (ec.value() != static_cast<int>(std::errc::no_such_file_or_directory)) - fatal(output, ec.message()); - if (status.type() != sys::fs::file_type::file_not_found && - status.type() != sys::fs::file_type::directory_file && - status.type() != sys::fs::file_type::regular_file) - fatal(output, "output cannot be a special file"); + { + sys::fs::file_status status; + if (std::error_code ec = sys::fs::status(output, status)) + if (ec.value() != static_cast<int>(std::errc::no_such_file_or_directory)) + fatal(output, ec.message()); + if (status.type() != sys::fs::file_type::file_not_found && + status.type() != sys::fs::file_type::directory_file && + status.type() != sys::fs::file_type::regular_file) + fatal(output, "output cannot be a special file"); + } if (std::error_code ec = sys::fs::remove(output, /*IgnoreNonExisting=*/true)) if (ec.value() != static_cast<int>(std::errc::directory_not_empty) && ec.value() != static_cast<int>(std::errc::file_exists)) Index: llvm/unittests/Support/VirtualFileSystemTest.cpp =================================================================== --- llvm/unittests/Support/VirtualFileSystemTest.cpp +++ llvm/unittests/Support/VirtualFileSystemTest.cpp @@ -151,24 +151,23 @@ return FilesAndDirs.find(std::string(P.str())); } + UniqueID makeUniqueID() { return UniqueID::inMemory(FSID, FileID++); } + void addRegularFile(StringRef Path, sys::fs::perms Perms = sys::fs::all_all) { - vfs::Status S(Path, UniqueID(FSID, FileID++), - std::chrono::system_clock::now(), 0, 0, 1024, - sys::fs::file_type::regular_file, Perms); + vfs::Status S(Path, makeUniqueID(), std::chrono::system_clock::now(), 0, 0, + 1024, sys::fs::file_type::regular_file, Perms); addEntry(Path, S); } void addDirectory(StringRef Path, sys::fs::perms Perms = sys::fs::all_all) { - vfs::Status S(Path, UniqueID(FSID, FileID++), - std::chrono::system_clock::now(), 0, 0, 0, - sys::fs::file_type::directory_file, Perms); + vfs::Status S(Path, makeUniqueID(), std::chrono::system_clock::now(), 0, 0, + 0, sys::fs::file_type::directory_file, Perms); addEntry(Path, S); } void addSymlink(StringRef Path) { - vfs::Status S(Path, UniqueID(FSID, FileID++), - std::chrono::system_clock::now(), 0, 0, 0, - sys::fs::file_type::symlink_file, sys::fs::all_all); + vfs::Status S(Path, makeUniqueID(), std::chrono::system_clock::now(), 0, 0, + 0, sys::fs::file_type::symlink_file, sys::fs::all_all); addEntry(Path, S); } Index: llvm/unittests/Support/Path.cpp =================================================================== --- llvm/unittests/Support/Path.cpp +++ llvm/unittests/Support/Path.cpp @@ -705,6 +705,7 @@ ASSERT_NO_ERROR(fs::getUniqueID(Twine(TempPath2), D)); ASSERT_NE(D, F1); ::close(FileDescriptor2); + D = fs::UniqueID(); ASSERT_NO_ERROR(fs::remove(Twine(TempPath2))); @@ -715,6 +716,7 @@ ASSERT_NO_ERROR(fs::getUniqueID(Twine(TempPath2), D2)); ASSERT_EQ(D2, F1); + D2 = fs::UniqueID(); ::close(FileDescriptor); SmallString<128> Dir1; @@ -729,6 +731,10 @@ fs::createUniqueDirectory("dir2", Dir2)); ASSERT_NO_ERROR(fs::getUniqueID(Dir2.c_str(), F2)); ASSERT_NE(F1, F2); + + F1 = fs::UniqueID(); + F2 = fs::UniqueID(); + ASSERT_NO_ERROR(fs::remove(Dir1)); ASSERT_NO_ERROR(fs::remove(Dir2)); ASSERT_NO_ERROR(fs::remove(TempPath2)); @@ -887,6 +893,10 @@ EXPECT_FALSE(fs::equivalent(A, B)); ::close(FD2); + // Also close the file statuses which contain instances of UniqueID, which on + // Windows keep the file handle alive. See comment in UniqueID.h + A = fs::file_status(); + B = fs::file_status(); // Remove Temp2. ASSERT_NO_ERROR(fs::remove(Twine(TempPath2))); @@ -916,6 +926,11 @@ ASSERT_NO_ERROR(fs::status(Twine(TempPath2), B)); EXPECT_TRUE(fs::equivalent(A, B)); + // Also close the file statuses which contain instances of UniqueID, which on + // Windows keep the file handle alive. See comment in UniqueID.h + A = fs::file_status(); + B = fs::file_status(); + // Remove Temp1. ::close(FileDescriptor); ASSERT_NO_ERROR(fs::remove(Twine(TempPath))); Index: llvm/unittests/Support/FSUniqueIDTest.cpp =================================================================== --- llvm/unittests/Support/FSUniqueIDTest.cpp +++ llvm/unittests/Support/FSUniqueIDTest.cpp @@ -15,24 +15,24 @@ namespace { TEST(FSUniqueIDTest, construct) { - EXPECT_EQ(20u, UniqueID(20, 10).getDevice()); - EXPECT_EQ(10u, UniqueID(20, 10).getFile()); + EXPECT_EQ(20u, UniqueID::inMemory(20, 10).getDevice()); + EXPECT_EQ(10u, UniqueID::inMemory(20, 10).getFile()); } TEST(FSUniqueIDTest, equals) { - EXPECT_EQ(UniqueID(20, 10), UniqueID(20, 10)); - EXPECT_NE(UniqueID(20, 20), UniqueID(20, 10)); - EXPECT_NE(UniqueID(10, 10), UniqueID(20, 10)); + EXPECT_EQ(UniqueID::inMemory(20, 10), UniqueID::inMemory(20, 10)); + EXPECT_NE(UniqueID::inMemory(20, 20), UniqueID::inMemory(20, 10)); + EXPECT_NE(UniqueID::inMemory(10, 10), UniqueID::inMemory(20, 10)); } TEST(FSUniqueIDTest, less) { - EXPECT_FALSE(UniqueID(20, 2) < UniqueID(20, 2)); - EXPECT_FALSE(UniqueID(20, 3) < UniqueID(20, 2)); - EXPECT_FALSE(UniqueID(30, 2) < UniqueID(20, 2)); - EXPECT_FALSE(UniqueID(30, 2) < UniqueID(20, 40)); - EXPECT_TRUE(UniqueID(20, 2) < UniqueID(20, 3)); - EXPECT_TRUE(UniqueID(20, 2) < UniqueID(30, 2)); - EXPECT_TRUE(UniqueID(20, 40) < UniqueID(30, 2)); + EXPECT_FALSE(UniqueID::inMemory(20, 2) < UniqueID::inMemory(20, 2)); + EXPECT_FALSE(UniqueID::inMemory(20, 3) < UniqueID::inMemory(20, 2)); + EXPECT_FALSE(UniqueID::inMemory(30, 2) < UniqueID::inMemory(20, 2)); + EXPECT_FALSE(UniqueID::inMemory(30, 2) < UniqueID::inMemory(20, 40)); + EXPECT_TRUE(UniqueID::inMemory(20, 2) < UniqueID::inMemory(20, 3)); + EXPECT_TRUE(UniqueID::inMemory(20, 2) < UniqueID::inMemory(30, 2)); + EXPECT_TRUE(UniqueID::inMemory(20, 40) < UniqueID::inMemory(30, 2)); } } // anonymous namespace Index: llvm/lib/Support/Windows/Path.inc =================================================================== --- llvm/lib/Support/Windows/Path.inc +++ llvm/lib/Support/Windows/Path.inc @@ -58,6 +58,17 @@ } } +static HANDLE duplicateHandle(HANDLE Handle) { + if (Handle == INVALID_HANDLE_VALUE) + return INVALID_HANDLE_VALUE; + HANDLE NewHandle; + if (!::DuplicateHandle(::GetCurrentProcess(), Handle, ::GetCurrentProcess(), + &NewHandle, 0, 0, DUPLICATE_SAME_ACCESS)) { + return INVALID_HANDLE_VALUE; + } + return NewHandle; +} + namespace llvm { namespace sys { namespace windows { @@ -124,6 +135,8 @@ return UTF8ToUTF16(FullPath, Path16); } +void CommonHandleTraits::Close(handle_type h) { ::CloseHandle((HANDLE)h); } + } // end namespace windows namespace fs { @@ -157,14 +170,7 @@ return std::string(PathNameUTF8.data()); } -UniqueID file_status::getUniqueID() const { - // The file is uniquely identified by the volume serial number along - // with the 64-bit file identifier. - uint64_t FileID = (static_cast<uint64_t>(FileIndexHigh) << 32ULL) | - static_cast<uint64_t>(FileIndexLow); - - return UniqueID(VolumeSerialNumber, FileID); -} +UniqueID file_status::getUniqueID() const { return FileID; } ErrorOr<space_info> disk_space(const Twine &Path) { ULARGE_INTEGER Avail, Total, Free; @@ -647,12 +653,10 @@ bool equivalent(file_status A, file_status B) { assert(status_known(A) && status_known(B)); - return A.FileIndexHigh == B.FileIndexHigh && - A.FileIndexLow == B.FileIndexLow && A.FileSizeHigh == B.FileSizeHigh && + return A.FileID == B.FileID && A.FileSizeHigh == B.FileSizeHigh && A.FileSizeLow == B.FileSizeLow && A.LastWriteTimeHigh == B.LastWriteTimeHigh && - A.LastWriteTimeLow == B.LastWriteTimeLow && - A.VolumeSerialNumber == B.VolumeSerialNumber; + A.LastWriteTimeLow == B.LastWriteTimeLow; } std::error_code equivalent(const Twine &A, const Twine &B, bool &result) { @@ -730,8 +734,11 @@ perms_from_attrs(Info.dwFileAttributes), Info.nNumberOfLinks, Info.ftLastAccessTime.dwHighDateTime, Info.ftLastAccessTime.dwLowDateTime, Info.ftLastWriteTime.dwHighDateTime, Info.ftLastWriteTime.dwLowDateTime, - Info.dwVolumeSerialNumber, Info.nFileSizeHigh, Info.nFileSizeLow, - Info.nFileIndexHigh, Info.nFileIndexLow); + Info.nFileSizeHigh, Info.nFileSizeLow, + llvm::sys::fs::UniqueID(Info.dwVolumeSerialNumber, + ((uint64_t)Info.nFileIndexHigh << 32) | + Info.nFileIndexLow, + (uint64_t)duplicateHandle(FileHandle))); return std::error_code(); handle_status_error: Index: llvm/lib/Support/VirtualFileSystem.cpp =================================================================== --- llvm/lib/Support/VirtualFileSystem.cpp +++ llvm/lib/Support/VirtualFileSystem.cpp @@ -774,8 +774,8 @@ // This avoids difficulties in creating exactly equivalent in-memory FSes, // as often needed in multithreaded programs. static sys::fs::UniqueID getUniqueID(hash_code Hash) { - return sys::fs::UniqueID(std::numeric_limits<uint64_t>::max(), - uint64_t(size_t(Hash))); + return sys::fs::UniqueID::inMemory(std::numeric_limits<uint64_t>::max(), + uint64_t(size_t(Hash))); } static sys::fs::UniqueID getFileID(sys::fs::UniqueID Parent, llvm::StringRef Name, @@ -2619,7 +2619,7 @@ unsigned ID = ++UID; // The following assumes that uint64_t max will never collide with a real // dev_t value from the OS. - return UniqueID(std::numeric_limits<uint64_t>::max(), ID); + return UniqueID::inMemory(std::numeric_limits<uint64_t>::max(), ID); } void YAMLVFSWriter::addEntry(StringRef VirtualPath, StringRef RealPath, Index: llvm/include/llvm/Support/Windows/WindowsSupport.h =================================================================== --- llvm/include/llvm/Support/Windows/WindowsSupport.h +++ llvm/include/llvm/Support/Windows/WindowsSupport.h @@ -43,6 +43,7 @@ #include "llvm/Support/Compiler.h" #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/VersionTuple.h" +#include "llvm/Support/Windows/ScopedHandle.h" #include <cassert> #include <string> #include <system_error> @@ -51,6 +52,8 @@ // Must be included after windows.h #include <wincrypt.h> +using llvm::sys::windows; + namespace llvm { /// Determines if the program is running on Windows 8 or newer. This @@ -77,116 +80,48 @@ llvm::report_fatal_error(Twine(ErrMsg)); } -template <typename HandleTraits> -class ScopedHandle { - typedef typename HandleTraits::handle_type handle_type; - handle_type Handle; - - ScopedHandle(const ScopedHandle &other) = delete; - void operator=(const ScopedHandle &other) = delete; -public: - ScopedHandle() - : Handle(HandleTraits::GetInvalid()) {} - - explicit ScopedHandle(handle_type h) - : Handle(h) {} - - ~ScopedHandle() { - if (HandleTraits::IsValid(Handle)) - HandleTraits::Close(Handle); - } - - handle_type take() { - handle_type t = Handle; - Handle = HandleTraits::GetInvalid(); - return t; - } - - ScopedHandle &operator=(handle_type h) { - if (HandleTraits::IsValid(Handle)) - HandleTraits::Close(Handle); - Handle = h; - return *this; - } - - // True if Handle is valid. - explicit operator bool() const { - return HandleTraits::IsValid(Handle) ? true : false; - } - - operator handle_type() const { - return Handle; - } -}; +struct TypedHandleTraits { + using handle_type = HANDLE; -struct CommonHandleTraits { - typedef HANDLE handle_type; - - static handle_type GetInvalid() { - return INVALID_HANDLE_VALUE; - } + static handle_type GetInvalid() { return INVALID_HANDLE_VALUE; } static void Close(handle_type h) { - ::CloseHandle(h); - } - - static bool IsValid(handle_type h) { - return h != GetInvalid(); + CommonHandleTraits::Close((CommonHandleTraits::handle_type)h); } }; -struct JobHandleTraits : CommonHandleTraits { - static handle_type GetInvalid() { - return NULL; - } +struct JobHandleTraits : TypedHandleTraits { + static handle_type GetInvalid() { return NULL; } }; -struct CryptContextTraits : CommonHandleTraits { - typedef HCRYPTPROV handle_type; - - static handle_type GetInvalid() { - return 0; - } +struct CryptContextTraits : TypedHandleTraits { + using handle_type = HCRYPTPROV; - static void Close(handle_type h) { - ::CryptReleaseContext(h, 0); - } + static handle_type GetInvalid() { return 0; } - static bool IsValid(handle_type h) { - return h != GetInvalid(); - } + static void Close(handle_type h) { ::CryptReleaseContext(h, 0); } }; -struct RegTraits : CommonHandleTraits { - typedef HKEY handle_type; +struct RegTraits : TypedHandleTraits { + using handle_type = HKEY; - static handle_type GetInvalid() { - return NULL; - } - - static void Close(handle_type h) { - ::RegCloseKey(h); - } + static handle_type GetInvalid() { return NULL; } - static bool IsValid(handle_type h) { - return h != GetInvalid(); - } + static void Close(handle_type h) { ::RegCloseKey(h); } }; -struct FindHandleTraits : CommonHandleTraits { - static void Close(handle_type h) { - ::FindClose(h); - } +struct FindHandleTraits : TypedHandleTraits { + static void Close(handle_type h) { ::FindClose((HANDLE)h); } }; -struct FileHandleTraits : CommonHandleTraits {}; +struct FileHandleTraits : TypedHandleTraits {}; -typedef ScopedHandle<CommonHandleTraits> ScopedCommonHandle; -typedef ScopedHandle<FileHandleTraits> ScopedFileHandle; +typedef ScopedHandle<TypedHandleTraits> ScopedCommonHandle; +typedef ScopedHandle<FileHandleTraits> ScopedFileHandle; typedef ScopedHandle<CryptContextTraits> ScopedCryptContext; -typedef ScopedHandle<RegTraits> ScopedRegHandle; -typedef ScopedHandle<FindHandleTraits> ScopedFindHandle; -typedef ScopedHandle<JobHandleTraits> ScopedJobHandle; +typedef ScopedHandle<RegTraits> ScopedRegHandle; +typedef ScopedHandle<FindHandleTraits> ScopedFindHandle; +typedef ScopedHandle<JobHandleTraits> ScopedJobHandle; template <class T> class SmallVectorImpl; Index: llvm/include/llvm/Support/Windows/ScopedHandle.h =================================================================== --- /dev/null +++ llvm/include/llvm/Support/Windows/ScopedHandle.h @@ -0,0 +1,77 @@ +//===- ScopedHandle.h - Common Windows Include File -------------*- C++ -*-===// +// +// 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 +// +//===----------------------------------------------------------------------===// +// +// A helper that keeps a Windows HANDLE open. This helper is meant to be used +// in situations where it is not desirable to include windows.h. +// +//===----------------------------------------------------------------------===// + +//===----------------------------------------------------------------------===// +//=== WARNING: Implementation here must contain only generic Win32 code that +//=== is guaranteed to work on *all* Win32 variants. +//===----------------------------------------------------------------------===// + +#ifndef LLVM_SUPPORT_SCOPEDHANDLE_H +#define LLVM_SUPPORT_SCOPEDHANDLE_H + +namespace llvm { +namespace sys { +namespace windows { + +template <typename HandleTraits> class ScopedHandle { + typedef typename HandleTraits::handle_type handle_type; + handle_type Handle = HandleTraits::GetInvalid(); + + ScopedHandle(const ScopedHandle &other) = delete; + void operator=(const ScopedHandle &other) = delete; + +public: + ScopedHandle() = default; + + explicit ScopedHandle(handle_type h) : Handle(h) {} + + ~ScopedHandle() { + if (Handle != HandleTraits::GetInvalid()) + HandleTraits::Close(Handle); + } + + handle_type take() { + handle_type t = Handle; + Handle = HandleTraits::GetInvalid(); + return t; + } + + ScopedHandle &operator=(handle_type h) { + if (Handle != HandleTraits::GetInvalid()) + HandleTraits::Close(Handle); + Handle = h; + return *this; + } + + // True if Handle is valid. + explicit operator bool() const { + return Handle != HandleTraits::GetInvalid(); + } + + operator handle_type() const { return Handle; } +}; + +struct CommonHandleTraits { + // Don't use HANDLE here to avoid including windows.h. + using handle_type = uint64_t; + + static handle_type GetInvalid() { return (uint64_t)-1ULL; } + + static void Close(handle_type h); +}; + +} // end namespace windows. +} // end namespace sys +} // end namespace llvm. + +#endif Index: llvm/include/llvm/Support/FileSystem/UniqueID.h =================================================================== --- llvm/include/llvm/Support/FileSystem/UniqueID.h +++ llvm/include/llvm/Support/FileSystem/UniqueID.h @@ -19,33 +19,78 @@ #include <cstdint> #include <utility> +#if defined(_WIN32) +#include "llvm/Support/Windows/ScopedHandle.h" +#include <memory> +#undef max +#endif + namespace llvm { namespace sys { namespace fs { class UniqueID { - uint64_t Device; - uint64_t File; + uint64_t Device = 0; + uint64_t File = 0; + +#if defined(_WIN32) + // On Windows, the handle is kept here only to guarantee that the values of + // FileIndexHigh and FileIndexLow remain valid during the lifetime of the + // UniqueID instance. The underlying Windows API only guarantees valid IDs as + // long as the file is open, please see: + // https://learn.microsoft.com/en-us/windows/win32/api/winbase/ns-winbase-file_id_info + // and: + // https://learn.microsoft.com/en-us/windows/win32/api/fileapi/ns-fileapi-by_handle_file_information + + using H = + llvm::sys::windows::ScopedHandle<llvm::sys::windows::CommonHandleTraits>; + std::shared_ptr<H> Handle; +#endif public: UniqueID() = default; + +#if defined(LLVM_ON_UNIX) UniqueID(uint64_t Device, uint64_t File) : Device(Device), File(File) {} +#elif defined(_WIN32) + UniqueID(uint64_t VolumeSerialNumber, uint64_t FileIndex, uint64_t Handle) + : Device(VolumeSerialNumber), File(FileIndex), + Handle(std::make_shared<H>(Handle)) {} +#endif bool operator==(const UniqueID &Other) const { return Device == Other.Device && File == Other.File; } bool operator!=(const UniqueID &Other) const { return !(*this == Other); } bool operator<(const UniqueID &Other) const { - /// Don't use std::tie since it bloats the compile time of this header. - if (Device < Other.Device) - return true; - if (Other.Device < Device) - return false; - return File < Other.File; + return std::make_pair(Device, File) < + std::make_pair(Other.Device, Other.File); } uint64_t getDevice() const { return Device; } uint64_t getFile() const { return File; } + + // Create a UniqueID that is known to refer to in-memory files. The FileID + // argument must be a unique value representing a in-memory file. + static UniqueID inMemory(uint64_t DeviceID, uint64_t FileID) { +#if defined(LLVM_ON_UNIX) + return UniqueID(DeviceID, FileID); +#elif defined(_WIN32) + UniqueID ID; + ID.Device = DeviceID; + ID.File = FileID; + return ID; +#endif + } + + hash_code getHashValue() const { + return hash_value(std::make_pair(Device, File)); + } + +private: + UniqueID(uint64_t Key) : Device(Key), File(Key) {} + + friend struct DenseMapInfo<llvm::sys::fs::UniqueID>; }; } // end namespace fs @@ -54,18 +99,17 @@ // Support UniqueIDs as DenseMap keys. template <> struct DenseMapInfo<llvm::sys::fs::UniqueID> { static inline llvm::sys::fs::UniqueID getEmptyKey() { - auto EmptyKey = DenseMapInfo<std::pair<uint64_t, uint64_t>>::getEmptyKey(); - return {EmptyKey.first, EmptyKey.second}; + auto EmptyKey = DenseMapInfo<uint64_t>::getEmptyKey(); + return llvm::sys::fs::UniqueID(EmptyKey); } static inline llvm::sys::fs::UniqueID getTombstoneKey() { - auto TombstoneKey = - DenseMapInfo<std::pair<uint64_t, uint64_t>>::getTombstoneKey(); - return {TombstoneKey.first, TombstoneKey.second}; + auto TombstoneKey = DenseMapInfo<uint64_t>::getTombstoneKey(); + return llvm::sys::fs::UniqueID(TombstoneKey); } static hash_code getHashValue(const llvm::sys::fs::UniqueID &Tag) { - return hash_value(std::make_pair(Tag.getDevice(), Tag.getFile())); + return Tag.getHashValue(); } static bool isEqual(const llvm::sys::fs::UniqueID &LHS, Index: llvm/include/llvm/Support/FileSystem.h =================================================================== --- llvm/include/llvm/Support/FileSystem.h +++ llvm/include/llvm/Support/FileSystem.h @@ -232,10 +232,8 @@ ino_t fs_st_ino = 0; #elif defined (_WIN32) uint32_t NumLinks = 0; - uint32_t VolumeSerialNumber = 0; - uint32_t FileIndexHigh = 0; - uint32_t FileIndexLow = 0; - #endif + sys::fs::UniqueID FileID; +#endif public: file_status() = default; @@ -254,15 +252,13 @@ file_status(file_type Type, perms Perms, uint32_t LinkCount, uint32_t LastAccessTimeHigh, uint32_t LastAccessTimeLow, uint32_t LastWriteTimeHigh, uint32_t LastWriteTimeLow, - uint32_t VolumeSerialNumber, uint32_t FileSizeHigh, - uint32_t FileSizeLow, uint32_t FileIndexHigh, - uint32_t FileIndexLow) + uint32_t FileSizeHigh, uint32_t FileSizeLow, + sys::fs::UniqueID FileID) : basic_file_status(Type, Perms, LastAccessTimeHigh, LastAccessTimeLow, LastWriteTimeHigh, LastWriteTimeLow, FileSizeHigh, FileSizeLow), - NumLinks(LinkCount), VolumeSerialNumber(VolumeSerialNumber), - FileIndexHigh(FileIndexHigh), FileIndexLow(FileIndexLow) {} - #endif + NumLinks(LinkCount), FileID(FileID) {} +#endif UniqueID getUniqueID() const; uint32_t getLinkCount() const; Index: clang/unittests/Basic/FileManagerTest.cpp =================================================================== --- clang/unittests/Basic/FileManagerTest.cpp +++ clang/unittests/Basic/FileManagerTest.cpp @@ -47,11 +47,10 @@ auto fileType = IsFile ? llvm::sys::fs::file_type::regular_file : llvm::sys::fs::file_type::directory_file; - llvm::vfs::Status Status(StatPath ? StatPath : Path, - llvm::sys::fs::UniqueID(1, INode), - /*MTime*/{}, /*User*/0, /*Group*/0, - /*Size*/0, fileType, - llvm::sys::fs::perms::all_all); + llvm::vfs::Status Status( + StatPath ? StatPath : Path, llvm::sys::fs::UniqueID::inMemory(1, INode), + /*MTime*/ {}, /*User*/ 0, /*Group*/ 0, + /*Size*/ 0, fileType, llvm::sys::fs::perms::all_all); if (StatPath) Status.ExposesExternalVFSPath = true; StatCalls[Path] = Status; Index: clang/tools/libclang/Indexing.cpp =================================================================== --- clang/tools/libclang/Indexing.cpp +++ clang/tools/libclang/Indexing.cpp @@ -72,7 +72,7 @@ time_t ModTime; unsigned Offset; public: - PPRegion() : UniqueID(0, 0), ModTime(), Offset() {} + PPRegion() : ModTime(), Offset() {} PPRegion(llvm::sys::fs::UniqueID UniqueID, unsigned offset, time_t modTime) : UniqueID(UniqueID), ModTime(modTime), Offset(offset) {} @@ -95,10 +95,10 @@ template <> struct DenseMapInfo<PPRegion> { static inline PPRegion getEmptyKey() { - return PPRegion(llvm::sys::fs::UniqueID(0, 0), unsigned(-1), 0); + return PPRegion(llvm::sys::fs::UniqueID(), unsigned(-1), 0); } static inline PPRegion getTombstoneKey() { - return PPRegion(llvm::sys::fs::UniqueID(0, 0), unsigned(-2), 0); + return PPRegion(llvm::sys::fs::UniqueID(), unsigned(-2), 0); } static unsigned getHashValue(const PPRegion &S) { Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp =================================================================== --- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp +++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp @@ -101,8 +101,7 @@ DependencyScanningFilesystemSharedCache::CacheShard & DependencyScanningFilesystemSharedCache::getShardForUID( llvm::sys::fs::UniqueID UID) const { - auto Hash = llvm::hash_combine(UID.getDevice(), UID.getFile()); - return CacheShards[Hash % NumShards]; + return CacheShards[UID.getHashValue() % NumShards]; } const CachedFileSystemEntry * @@ -180,8 +179,8 @@ DependencyScanningWorkerFilesystem::getOrEmplaceSharedEntryForUID( TentativeEntry TEntry) { auto &Shard = SharedCache.getShardForUID(TEntry.Status.getUniqueID()); - return Shard.getOrEmplaceEntryForUID(TEntry.Status.getUniqueID(), - std::move(TEntry.Status), + llvm::sys::fs::UniqueID Id = TEntry.Status.getUniqueID(); + return Shard.getOrEmplaceEntryForUID(Id, std::move(TEntry.Status), std::move(TEntry.Contents)); } Index: clang/lib/Basic/FileEntry.cpp =================================================================== --- clang/lib/Basic/FileEntry.cpp +++ clang/lib/Basic/FileEntry.cpp @@ -17,8 +17,6 @@ using namespace clang; -FileEntry::FileEntry() : UniqueID(0, 0) {} - FileEntry::~FileEntry() = default; void FileEntry::closeFile() const { File.reset(); } Index: clang/include/clang/Basic/FileEntry.h =================================================================== --- clang/include/clang/Basic/FileEntry.h +++ clang/include/clang/Basic/FileEntry.h @@ -24,21 +24,12 @@ #include "llvm/ADT/StringRef.h" #include "llvm/Support/ErrorOr.h" #include "llvm/Support/FileSystem/UniqueID.h" +#include "llvm/Support/MemoryBuffer.h" +#include "llvm/Support/VirtualFileSystem.h" #include <optional> #include <utility> -namespace llvm { - -class MemoryBuffer; - -namespace vfs { - -class File; - -} // namespace vfs -} // namespace llvm - namespace clang { class FileEntryRef; @@ -353,7 +344,7 @@ class FileEntry { friend class FileManager; friend class FileEntryTestHelper; - FileEntry(); + FileEntry() = default; FileEntry(const FileEntry &) = delete; FileEntry &operator=(const FileEntry &) = delete; Index: clang-tools-extra/clangd/unittests/FSTests.cpp =================================================================== --- clang-tools-extra/clangd/unittests/FSTests.cpp +++ clang-tools-extra/clangd/unittests/FSTests.cpp @@ -33,7 +33,7 @@ // Main file is not cached. EXPECT_FALSE(StatCache.lookup(testPath("main")).has_value()); - llvm::vfs::Status S("fake", llvm::sys::fs::UniqueID(123, 456), + llvm::vfs::Status S("fake", llvm::sys::fs::UniqueID::inMemory(123, 456), std::chrono::system_clock::now(), 0, 0, 1024, llvm::sys::fs::file_type::regular_file, llvm::sys::fs::all_all);
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits