jansvoboda11 created this revision. jansvoboda11 requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits.
Depends on D114968 <https://reviews.llvm.org/D114968>. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D114971 Files: clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp clang/test/ClangScanDeps/modules-symlink.c
Index: clang/test/ClangScanDeps/modules-symlink.c =================================================================== --- /dev/null +++ clang/test/ClangScanDeps/modules-symlink.c @@ -0,0 +1,54 @@ +// RUN: rm -rf %t +// RUN: split-file %s %t + +//--- cdb_pch.json +[ + { + "directory": "DIR", + "command": "clang -x c-header DIR/pch.h -fmodules -gmodules -fimplicit-module-maps -fmodules-cache-path=DIR/cache -o DIR/pch.h.gch", + "file": "DIR/pch.h" + } +] + +//--- cdb_tu.json +[ + { + "directory": "DIR", + "command": "clang -c DIR/tu.c -fmodules -gmodules -fimplicit-module-maps -fmodules-cache-path=DIR/cache -include DIR/pch.h -o DIR/tu.o", + "file": "DIR/tu.c" + } +] + +//--- module.modulemap +module mod { header "symlink.h" } + +//--- pch.h +#include "symlink.h" + +//--- original.h +// Comment that will be stripped by the minimizer. +#define MACRO 1 + +//--- tu.c +#include "original.h" +static int foo = MACRO; // Macro usage that will trigger + // input file consistency checks. + +// RUN: ln -s %t/original.h %t/symlink.h + +// RUN: sed -e "s|DIR|%/t|g" %t/cdb_pch.json > %t/cdb.json +// RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full \ +// RUN: -generate-modules-path-args -module-files-dir %t/build > %t/result_pch.json +// +// RUN: %python %S/../../utils/module-deps-to-rsp.py %t/result_pch.json \ +// RUN: --module-name=mod > %t/mod.cc1.rsp +// RUN: %python %S/../../utils/module-deps-to-rsp.py %t/result_pch.json \ +// RUN: --tu-index=0 > %t/pch.rsp +// +// RUN: %clang @%t/mod.cc1.rsp +// RUN: %clang -x c-header %t/pch.h -fmodules -gmodules -fimplicit-module-maps \ +// RUN: -fmodules-cache-path=%t/cache -o %t/pch.h.gch -I %t @%t/pch.rsp + +// RUN: sed -e "s|DIR|%/t|g" %t/cdb_tu.json > %t/cdb.json +// RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full \ +// RUN: -generate-modules-path-args -module-files-dir %t/build > %t/result_tu.json Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp =================================================================== --- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp +++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp @@ -190,19 +190,20 @@ } void DependencyScanningWorkerFilesystem::disableMinimization( - StringRef RawFilename) { - llvm::SmallString<256> Filename; - llvm::sys::path::native(RawFilename, Filename); - NotToBeMinimized.insert(Filename); + StringRef Filename) { + // Only requesting the status' \c UniqueID. + // This prevents \c getOrCreateFileSystemEntry to unnecessarily read/minimize + // the file and to call \c shouldMinimize that reads \c NotToBeMinimized we're + // not done setting up yet. + if (auto FileSystemEntry = getOrCreateFileSystemEntry(Filename, RI_UniqueID)) + NotToBeMinimized.insert(FileSystemEntry->Status->getUniqueID()); } -bool DependencyScanningWorkerFilesystem::shouldMinimize(StringRef RawFilename) { - if (!shouldMinimizeBasedOnExtension(RawFilename)) +bool DependencyScanningWorkerFilesystem::shouldMinimize( + StringRef Filename, llvm::sys::fs::UniqueID UID) { + if (!shouldMinimizeBasedOnExtension(Filename)) return false; - - llvm::SmallString<256> Filename; - llvm::sys::path::native(RawFilename, Filename); - return !NotToBeMinimized.contains(Filename); + return !NotToBeMinimized.contains(UID); } void DependencyScanningWorkerFilesystem::ensureLocked( @@ -302,8 +303,15 @@ return FileSystemEntryResult(*LocalStat, nullptr, nullptr); llvm::sys::fs::UniqueID UID = (*LocalStat)->getUniqueID(); + if (RI == RI_UniqueID) { + // We're returning the original stat of a file that might need minimization, + // meaning the reported size won't be consistent with what we return for + // RI_StatOrContents. That's fine though, since the caller promises to only + // look the UniqueID of the returned status. + return FileSystemEntryResult(*LocalStat, nullptr, nullptr); + } - if (!shouldMinimize(Filename)) { + if (!shouldMinimize(Filename, UID)) { if (RI == RI_Status) { // If the caller wants a status of a file that doesn't need minimization, // reading it is not necessary. Index: clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h =================================================================== --- clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h +++ clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h @@ -11,8 +11,8 @@ #include "clang/Basic/LLVM.h" #include "clang/Lex/PreprocessorExcludedConditionalDirectiveSkipMapping.h" +#include "llvm/ADT/DenseSet.h" #include "llvm/ADT/StringMap.h" -#include "llvm/ADT/StringSet.h" #include "llvm/Support/Allocator.h" #include "llvm/Support/ErrorOr.h" #include "llvm/Support/VirtualFileSystem.h" @@ -199,7 +199,7 @@ private: /// Check whether the file should be minimized. - bool shouldMinimize(StringRef Filename); + bool shouldMinimize(StringRef Filename, llvm::sys::fs::UniqueID UID); /// Ensure \p Lock is locked with the mutex for entry at \p Path. /// Also assigns to \p SS if null. @@ -225,12 +225,13 @@ /// The information requested from \c getOrCreateFileSystemEntry. enum RequestedInformation { + RI_UniqueID, RI_Status, RI_Contents, }; - /// Get the full filesystem entry for \p Path from local cache, populating it - /// if necessary. + /// Get the filesystem entry for \p Path from local cache that contains only + /// the requested information. Populates the local cache if necessary. llvm::ErrorOr<FileSystemEntryResult> getOrCreateFileSystemEntry(StringRef Path, RequestedInformation RI); @@ -244,7 +245,7 @@ /// currently active preprocessor. ExcludedPreprocessorDirectiveSkipMapping *PPSkipMappings; /// The set of files that should not be minimized. - llvm::StringSet<> NotToBeMinimized; + llvm::DenseSet<llvm::sys::fs::UniqueID> NotToBeMinimized; }; } // end namespace dependencies
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits