Author: kousikk Date: Thu Oct 10 08:29:01 2019 New Revision: 374366 URL: http://llvm.org/viewvc/llvm-project?rev=374366&view=rev Log: In openFileForRead don't cache erroneous entries if the error relates to them being directories. Add tests.
Summary: It seems that when the CachingFileSystem is first given a file to open that is actually a directory, it incorrectly caches that path to be errenous and throws an error when subsequently a directory open call is made for the same path. This change makes it so that we do NOT cache a path if it turns out we asked for a file when its a directory. Reviewers: arphaman Subscribers: dexonsmith, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D68193 Added: cfe/trunk/test/ClangScanDeps/Inputs/headerwithdirnamefollowedbyinclude.json cfe/trunk/test/ClangScanDeps/headerwithdirnamefollowedbyinclude.cpp Modified: cfe/trunk/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp Modified: cfe/trunk/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h?rev=374366&r1=374365&r2=374366&view=diff ============================================================================== --- cfe/trunk/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h (original) +++ cfe/trunk/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h Thu Oct 10 08:29:01 2019 @@ -168,6 +168,9 @@ private: return It == Cache.end() ? nullptr : It->getValue(); } + llvm::ErrorOr<const CachedFileSystemEntry *> + getOrCreateFileSystemEntry(const StringRef Filename); + DependencyScanningFilesystemSharedCache &SharedCache; /// The local cache is used by the worker thread to cache file system queries /// locally instead of querying the global cache every time. Modified: cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp?rev=374366&r1=374365&r2=374366&view=diff ============================================================================== --- cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp (original) +++ cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp Thu Oct 10 08:29:01 2019 @@ -122,14 +122,11 @@ DependencyScanningFilesystemSharedCache: return It.first->getValue(); } -llvm::ErrorOr<llvm::vfs::Status> -DependencyScanningWorkerFilesystem::status(const Twine &Path) { - SmallString<256> OwnedFilename; - StringRef Filename = Path.toStringRef(OwnedFilename); - - // Check the local cache first. - if (const CachedFileSystemEntry *Entry = getCachedEntry(Filename)) - return Entry->getStatus(); +llvm::ErrorOr<const CachedFileSystemEntry *> +DependencyScanningWorkerFilesystem::getOrCreateFileSystemEntry(const StringRef Filename) { + if (const CachedFileSystemEntry* Entry = getCachedEntry(Filename)) { + return Entry; + } // FIXME: Handle PCM/PCH files. // FIXME: Handle module map files. @@ -160,7 +157,17 @@ DependencyScanningWorkerFilesystem::stat // Store the result in the local cache. setCachedEntry(Filename, Result); - return Result->getStatus(); + return Result; +} + +llvm::ErrorOr<llvm::vfs::Status> +DependencyScanningWorkerFilesystem::status(const Twine &Path) { + SmallString<256> OwnedFilename; + StringRef Filename = Path.toStringRef(OwnedFilename); + const llvm::ErrorOr<const CachedFileSystemEntry *> Result = getOrCreateFileSystemEntry(Filename); + if (!Result) + return Result.getError(); + return (*Result)->getStatus(); } namespace { @@ -217,30 +224,8 @@ DependencyScanningWorkerFilesystem::open SmallString<256> OwnedFilename; StringRef Filename = Path.toStringRef(OwnedFilename); - // Check the local cache first. - if (const CachedFileSystemEntry *Entry = getCachedEntry(Filename)) - return createFile(Entry, PPSkipMappings); - - // FIXME: Handle PCM/PCH files. - // FIXME: Handle module map files. - - bool KeepOriginalSource = IgnoredFiles.count(Filename); - DependencyScanningFilesystemSharedCache::SharedFileSystemEntry - &SharedCacheEntry = SharedCache.get(Filename); - const CachedFileSystemEntry *Result; - { - std::unique_lock<std::mutex> LockGuard(SharedCacheEntry.ValueLock); - CachedFileSystemEntry &CacheEntry = SharedCacheEntry.Value; - - if (!CacheEntry.isValid()) { - CacheEntry = CachedFileSystemEntry::createFileEntry( - Filename, getUnderlyingFS(), !KeepOriginalSource); - } - - Result = &CacheEntry; - } - - // Store the result in the local cache. - setCachedEntry(Filename, Result); - return createFile(Result, PPSkipMappings); + const llvm::ErrorOr<const CachedFileSystemEntry *> Result = getOrCreateFileSystemEntry(Filename); + if (!Result) + return Result.getError(); + return createFile(Result.get(), PPSkipMappings); } Added: cfe/trunk/test/ClangScanDeps/Inputs/headerwithdirnamefollowedbyinclude.json URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ClangScanDeps/Inputs/headerwithdirnamefollowedbyinclude.json?rev=374366&view=auto ============================================================================== --- cfe/trunk/test/ClangScanDeps/Inputs/headerwithdirnamefollowedbyinclude.json (added) +++ cfe/trunk/test/ClangScanDeps/Inputs/headerwithdirnamefollowedbyinclude.json Thu Oct 10 08:29:01 2019 @@ -0,0 +1,7 @@ +[ + { + "directory": "DIR", + "command": "clang -c -IDIR -IInputs DIR/headerwithdirname_input.cpp", + "file": "DIR/headerwithdirname_input.cpp" + } +] Added: cfe/trunk/test/ClangScanDeps/headerwithdirnamefollowedbyinclude.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ClangScanDeps/headerwithdirnamefollowedbyinclude.cpp?rev=374366&view=auto ============================================================================== --- cfe/trunk/test/ClangScanDeps/headerwithdirnamefollowedbyinclude.cpp (added) +++ cfe/trunk/test/ClangScanDeps/headerwithdirnamefollowedbyinclude.cpp Thu Oct 10 08:29:01 2019 @@ -0,0 +1,21 @@ +// RUN: rm -rf %t.dir +// RUN: rm -rf %t.dir/foodir +// RUN: rm -rf %t.cdb + +// RUN: mkdir -p %t.dir +// RUN: mkdir -p %t.dir/foodir + +// RUN: cp %S/Inputs/header.h %t.dir/foodir/foodirheader.h +// RUN: cp %s %t.dir/headerwithdirname_input.cpp +// RUN: mkdir %t.dir/Inputs +// RUN: cp %S/Inputs/foodir %t.dir/Inputs/foodir +// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/headerwithdirnamefollowedbyinclude.json > %t.cdb +// +// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 | FileCheck %s + +#include <foodir> +#include "foodir/foodirheader.h" + +// CHECK: headerwithdirname_input.o +// CHECK-NEXT: headerwithdirname_input.cpp +// CHECK-NEXT: Inputs{{/|\\}}foodir _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
