[clang][deps] Handle symlinks in minimizing FS

[jansvoboda11]

Summary

The minimizing and caching filesystem used by the dependency scanner can be configured to not minimize some files. That's necessary when scanning a TU with prebuilt inputs (i.e. PCH) that refer to the original (non-minimized) files. Minimizing such files in the dependency scanner would cause discrepancy between the current perceived state of the filesystem and the file sizes stored in the AST file. By not minimizing such files, we avoid creating the discrepancy.

The problem with the current approach is that files that should not be minimized are identified by their path. This breaks down when the prebuilt input (PCH) and the current TU refer to the same file via different paths (i.e. symlinks). This patch switches from paths to llvm::sys::fs::UniqueID when identifying ignored files. This is consistent with how the rest of Clang treats files.

Depends on D114966.

[jansvoboda11]

Rebase

[tschuett]

clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h

312–313

Do you want to modernise RequestedInformation to an enum class?

[arphaman]

This looks good to me! I agree, an enum class would be cleaner though.

[jansvoboda11]

Rebase on top of new version of D114966.

[jansvoboda11]

Prevent eager minimization of not-to-be-minimized files.

[dexonsmith]

LGTM.

clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h

 //===- DependencyScanningFilesystem.h - clang-scan-deps fs ===---*- 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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_DEPENDENCYSCANNINGFILESYSTEM_H
 #define LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_DEPENDENCYSCANNINGFILESYSTEM_H
 
 #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"
 #include <mutex>
 
 namespace clang {
 namespace tooling {
 namespace dependencies {
public:
 
   /// Disable minimization of the given file.
   void disableMinimization(StringRef Filename);
   /// Enable minimization of all files.
   void enableMinimizationOfAllFiles() { NotToBeMinimized.clear(); }
 
 private:
   /// Check whether the file should be minimized.
-  bool shouldMinimize(StringRef Filename);
+  bool shouldMinimize(StringRef Filename, llvm::sys::fs::UniqueID UID);
 
   /// 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
   /// shared cache indexed by unique ID, or creates new entry from scratch.
   llvm::ErrorOr<const CachedFileSystemEntry &>
   computeAndStoreResult(StringRef Filename);
 
   /// Minimizes the given entry if necessary and returns a wrapper object with
   /// reference semantics.
   EntryRef minimizeIfNecessary(const CachedFileSystemEntry &Entry,
-                               StringRef Filename);
+                               StringRef Filename, 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`.
   struct TentativeEntry {
     llvm::vfs::Status Status;
     std::unique_ptr<llvm::MemoryBuffer> Contents;
 
     TentativeEntry(llvm::vfs::Status Status,
private:
   /// The local cache is used by the worker thread to cache file system queries
   /// locally instead of querying the global cache every time.
   DependencyScanningFilesystemLocalCache LocalCache;
   /// The optional mapping structure which records information about the
   /// excluded conditional directive skip mappings that are used by the
   /// 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
 } // end namespace tooling
 } // end namespace clang
 
 #endif // LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_DEPENDENCYSCANNINGFILESYSTEM_H

clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp

DependencyScanningWorkerFilesystem::readFile(StringRef Filename) {
   // If the file size changed between read and stat, pretend it didn't.
   if (Stat.getSize() != Buffer->getBufferSize())
     Stat = llvm::vfs::Status::copyWithNewSize(Stat, Buffer->getBufferSize());
 
   return TentativeEntry(Stat, std::move(Buffer));
 }
 
 EntryRef DependencyScanningWorkerFilesystem::minimizeIfNecessary(
-    const CachedFileSystemEntry &Entry, StringRef Filename) {
-  if (Entry.isError() || Entry.isDirectory() || !shouldMinimize(Filename))
+    const CachedFileSystemEntry &Entry, StringRef Filename, bool Disable) {
+  if (Entry.isError() || Entry.isDirectory() || Disable ||
+      !shouldMinimize(Filename, Entry.getUniqueID()))
     return EntryRef(/*Minimized=*/false, Filename, Entry);
 
   CachedFileContents *Contents = Entry.getContents();
   assert(Contents && "contents not initialized");
 
   // Double-checked locking.
   if (Contents->MinimizedAccess.load())
     return EntryRef(/*Minimized=*/true, Filename, Entry);
static bool shouldCacheStatFailures(StringRef Filename) {
   StringRef Ext = llvm::sys::path::extension(Filename);
   if (Ext.empty())
     return false; // This may be the module cache directory.
   // Only cache stat failures on source files.
   return shouldMinimizeBasedOnExtension(Filename);
 }
 
 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(
+    StringRef Filename, llvm::sys::fs::UniqueID UID) {
+  return shouldMinimizeBasedOnExtension(Filename) &&
+         !NotToBeMinimized.contains(UID);
 }
 
 const CachedFileSystemEntry &
 DependencyScanningWorkerFilesystem::getOrEmplaceSharedEntryForUID(
     TentativeEntry TEntry) {
   auto &Shard = SharedCache.getShardForUID(TEntry.Status.getUniqueID());
   return Shard.getOrEmplaceEntryForUID(TEntry.Status.getUniqueID(),
                                        std::move(TEntry.Status),
const CachedFileSystemEntry *SharedEntry = [&]() {
     return &getOrEmplaceSharedEntryForFilename(Filename, TEntry.getError());
   }();
 
   return insertLocalEntryForFilename(Filename, *SharedEntry);
 }
 
 llvm::ErrorOr<EntryRef>
 DependencyScanningWorkerFilesystem::getOrCreateFileSystemEntry(
-    StringRef Filename) {
+    StringRef Filename, bool DisableMinimization) {
   if (const auto *Entry = findEntryByFilenameWithWriteThrough(Filename))
-    return minimizeIfNecessary(*Entry, Filename).unwrapError();
+    return minimizeIfNecessary(*Entry, Filename, DisableMinimization)
+        .unwrapError();
   auto MaybeEntry = computeAndStoreResult(Filename);
   if (!MaybeEntry)
     return MaybeEntry.getError();
-  return minimizeIfNecessary(*MaybeEntry, Filename).unwrapError();
+  return minimizeIfNecessary(*MaybeEntry, Filename, DisableMinimization)
+      .unwrapError();
 }
 
 llvm::ErrorOr<llvm::vfs::Status>
 DependencyScanningWorkerFilesystem::status(const Twine &Path) {
   SmallString<256> OwnedFilename;
   StringRef Filename = Path.toStringRef(OwnedFilename);
 
   llvm::ErrorOr<EntryRef> Result = getOrCreateFileSystemEntry(Filename);

clang/test/ClangScanDeps/modules-symlink.c

+// 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