jansvoboda11 updated this revision to Diff 395399.
jansvoboda11 added a comment.
Prevent eager minimization of not-to-be-minimized files.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114971/new/
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
@@ -42,9 +42,9 @@
}
EntryRef DependencyScanningWorkerFilesystem::minimizeIfNecessary(
- const CachedFileSystemEntry &Entry) {
- if (Entry.isError() || Entry.isDirectory() ||
- !shouldMinimize(Entry.getName()))
+ const CachedFileSystemEntry &Entry, bool DisableMinimization) {
+ if (Entry.isError() || Entry.isDirectory() || DisableMinimization ||
+ !shouldMinimize(Entry))
return EntryRef(false, Entry);
CachedFileContents *Contents = Entry.getContents();
@@ -210,19 +210,18 @@
}
void DependencyScanningWorkerFilesystem::disableMinimization(
- StringRef RawFilename) {
- llvm::SmallString<256> Filename;
- llvm::sys::path::native(RawFilename, Filename);
- NotToBeMinimized.insert(Filename);
+ StringRef Filename) {
+ // Since we're not done setting up `NotToBeMinimized` yet, we need to disable
+ // minimization explicitly.
+ if (llvm::ErrorOr<EntryRef> Result =
+ getOrCreateFileSystemEntry(Filename, /*DisableMinimization=*/true))
+ NotToBeMinimized.insert(Result->getStatus().getUniqueID());
}
-bool DependencyScanningWorkerFilesystem::shouldMinimize(StringRef RawFilename) {
- if (!shouldMinimizeBasedOnExtension(RawFilename))
- return false;
-
- llvm::SmallString<256> Filename;
- llvm::sys::path::native(RawFilename, Filename);
- return !NotToBeMinimized.contains(Filename);
+bool DependencyScanningWorkerFilesystem::shouldMinimize(
+ const CachedFileSystemEntry &Entry) {
+ return shouldMinimizeBasedOnExtension(Entry.getName()) &&
+ !NotToBeMinimized.contains(Entry.getUniqueID());
}
const CachedFileSystemEntry &
@@ -278,13 +277,13 @@
llvm::ErrorOr<EntryRef>
DependencyScanningWorkerFilesystem::getOrCreateFileSystemEntry(
- StringRef Filename) {
+ StringRef Filename, bool DisableMinimization) {
if (const auto *Entry = findEntryByFilenameWithWriteThrough(Filename))
- return minimizeIfNecessary(*Entry).unwrapError();
+ return minimizeIfNecessary(*Entry, DisableMinimization).unwrapError();
auto MaybeEntry = computeAndStoreResult(Filename);
if (!MaybeEntry)
return MaybeEntry.getError();
- return minimizeIfNecessary(*MaybeEntry).unwrapError();
+ return minimizeIfNecessary(*MaybeEntry, DisableMinimization).unwrapError();
}
llvm::ErrorOr<llvm::vfs::Status>
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"
@@ -305,13 +305,15 @@
private:
/// Check whether the file should be minimized.
- bool shouldMinimize(StringRef Filename);
+ bool shouldMinimize(const CachedFileSystemEntry &Entry);
/// Returns entry for the given filename.
///
/// Attempts to use the local and shared caches first, then falls back to
/// using the underlying filesystem.
- llvm::ErrorOr<EntryRef> getOrCreateFileSystemEntry(StringRef Filename);
+ llvm::ErrorOr<EntryRef>
+ getOrCreateFileSystemEntry(StringRef Filename,
+ bool DisableMinimization = false);
/// For a filename that's not yet associated with any entry in the caches,
/// uses the underlying filesystem to either look up the entry based in the
@@ -321,7 +323,8 @@
/// Minimized the given entry if necessary and returns a wrapper object with
/// reference semantics.
- EntryRef minimizeIfNecessary(const CachedFileSystemEntry &Entry);
+ EntryRef minimizeIfNecessary(const CachedFileSystemEntry &Entry,
+ bool Disable);
/// Represents a filesystem entry that has been stat-ed (and potentially read)
/// and that's about to be inserted into the cache as `CachedFileSystemEntry`.
@@ -397,7 +400,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
