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
