[PATCH] D68193: In openFileForRead don't cache erroneous entries if the error relates to them being directories. Add tests.

[Kousik Kumar via Phabricator via cfe-commits]

kousikk created this revision.
kousikk added a reviewer: arphaman.
Herald added subscribers: cfe-commits, dexonsmith.
Herald added a project: clang.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68193

Files:
  clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
  clang/test/ClangScanDeps/Inputs/headerwithdirnamefollowedbyinclude.json
  clang/test/ClangScanDeps/headerwithdirnamefollowedbyinclude.cpp


Index: clang/test/ClangScanDeps/headerwithdirnamefollowedbyinclude.cpp
===================================================================
--- /dev/null
+++ clang/test/ClangScanDeps/headerwithdirnamefollowedbyinclude.cpp
@@ -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
Index: clang/test/ClangScanDeps/Inputs/headerwithdirnamefollowedbyinclude.json
===================================================================
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/headerwithdirnamefollowedbyinclude.json
@@ -0,0 +1,7 @@
+[
+    {
+      "directory": "DIR",
+      "command": "clang -c -IDIR -IInputs DIR/headerwithdirname_input.cpp",
+      "file": "DIR/headerwithdirname_input.cpp"
+    }
+]
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===================================================================
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -233,8 +233,15 @@
     CachedFileSystemEntry &CacheEntry = SharedCacheEntry.Value;
 
     if (!CacheEntry.isValid()) {
+      llvm::vfs::FileSystem &FS = getUnderlyingFS();
+      auto MaybeStatus = FS.status(Filename);
+
+      if (MaybeStatus && MaybeStatus->isDirectory())
+        return llvm::ErrorOr<std::unique_ptr<llvm::vfs::File>>(
+          std::make_error_code(std::errc::is_a_directory));
+
       CacheEntry = CachedFileSystemEntry::createFileEntry(
-          Filename, getUnderlyingFS(), !KeepOriginalSource);
+          Filename, FS, !KeepOriginalSource);
     }
 
     Result = &CacheEntry;



Index: clang/test/ClangScanDeps/headerwithdirnamefollowedbyinclude.cpp
===================================================================
--- /dev/null
+++ clang/test/ClangScanDeps/headerwithdirnamefollowedbyinclude.cpp
@@ -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
Index: clang/test/ClangScanDeps/Inputs/headerwithdirnamefollowedbyinclude.json
===================================================================
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/headerwithdirnamefollowedbyinclude.json
@@ -0,0 +1,7 @@
+[
+    {
+      "directory": "DIR",
+      "command": "clang -c -IDIR -IInputs DIR/headerwithdirname_input.cpp",
+      "file": "DIR/headerwithdirname_input.cpp"
+    }
+]
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===================================================================
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -233,8 +233,15 @@
     CachedFileSystemEntry &CacheEntry = SharedCacheEntry.Value;
 
     if (!CacheEntry.isValid()) {
+      llvm::vfs::FileSystem &FS = getUnderlyingFS();
+      auto MaybeStatus = FS.status(Filename);
+
+      if (MaybeStatus && MaybeStatus->isDirectory())
+        return llvm::ErrorOr<std::unique_ptr<llvm::vfs::File>>(
+          std::make_error_code(std::errc::is_a_directory));
+
       CacheEntry = CachedFileSystemEntry::createFileEntry(
-          Filename, getUnderlyingFS(), !KeepOriginalSource);
+          Filename, FS, !KeepOriginalSource);
     }
 
     Result = &CacheEntry;

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits