benlangmuir updated this revision to Diff 464117.
benlangmuir added a comment.
Simplify modmap checks in test to use PREFIX
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134923/new/
https://reviews.llvm.org/D134923
Files:
clang/include/clang/Lex/ModuleMap.h
clang/lib/Lex/HeaderSearch.cpp
clang/lib/Lex/ModuleMap.cpp
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
clang/test/ClangScanDeps/modules-symlink-dir.c
Index: clang/test/ClangScanDeps/modules-symlink-dir.c
===================================================================
--- /dev/null
+++ clang/test/ClangScanDeps/modules-symlink-dir.c
@@ -0,0 +1,131 @@
+// Check that we canonicalize the module map path without changing the module
+// directory, which would break header lookup.
+
+// REQUIRES: shell
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.in > %t/cdb.json
+// RUN: ln -s module %t/symlink-to-module
+// RUN: ln -s ../actual.modulemap %t/module/module.modulemap
+// RUN: ln -s A %t/module/F.framework/Versions/Current
+// RUN: ln -s Versions/Current/Modules %t/module/F.framework/Modules
+// RUN: ln -s Versions/Current/Headers %t/module/F.framework/Headers
+
+// RUN: clang-scan-deps -compilation-database %t/cdb.json -j 1 -reuse-filemanager=0 \
+// RUN: -format experimental-full -mode=preprocess-dependency-directives \
+// RUN: -optimize-args -module-files-dir %t/build > %t/deps.json
+
+// RUN: cat %t/deps.json | sed 's:\\\\\?:/:g' | FileCheck -DPREFIX=%/t %s
+
+// Check the module commands actually build.
+// RUN: %deps-to-rsp %t/deps.json --module-name=Mod > %t/Mod.rsp
+// RUN: %deps-to-rsp %t/deps.json --module-name=F > %t/F.rsp
+// RUN: %clang @%t/Mod.rsp
+// RUN: %clang @%t/F.rsp
+
+// CHECK: "modules": [
+// CHECK: {
+// CHECK: "clang-module-deps": [],
+// CHECK: "clang-modulemap-file": "[[PREFIX]]/module/F.framework/Modules/module.modulemap"
+// CHECK: "command-line": [
+// CHECK-NOT: symlink-to-module
+// CHECK: "[[PREFIX]]/module/F.framework/Modules/module.modulemap"
+// CHECK-NOT: symlink-to-module
+// CHECK: ]
+// CHECK: "context-hash": "[[F_CONTEXT_HASH:[A-Z0-9]+]]"
+// CHECK: "name": "F"
+// CHECK-NEXT: }
+// CHECK-NEXT: {
+// CHECK: "clang-modulemap-file": "[[PREFIX]]/module/module.modulemap"
+// CHECK: "command-line": [
+// CHECK-NOT: symlink-to-module
+// CHECK: "[[PREFIX]]/module/module.modulemap"
+// CHECK-NOT: symlink-to-module
+// CHECK: ]
+// CHECK: "context-hash": "[[CONTEXT_HASH:[A-Z0-9]+]]"
+// CHECK: "name": "Mod"
+// CHECK-NEXT: }
+// CHECK-NEXT: ]
+// CHECK: "translation-units": [
+// CHECK: "clang-module-deps": [
+// CHECK: {
+// CHECK: "context-hash": "[[CONTEXT_HASH]]"
+// CHECK: "module-name": "Mod"
+// CHECK: }
+// CHECK-NEXT: ],
+// CHECK: "command-line": [
+// CHECK: "-fmodule-map-file=[[PREFIX]]/module/module.modulemap"
+// CHECK: ]
+// CHECK: "clang-module-deps": [
+// CHECK: {
+// CHECK: "context-hash": "[[CONTEXT_HASH]]"
+// CHECK: "module-name": "Mod"
+// CHECK: }
+// CHECK-NEXT: ]
+// CHECK: "command-line": [
+// CHECK: "-fmodule-map-file=[[PREFIX]]/module/module.modulemap"
+// CHECK: ]
+// CHECK: "clang-module-deps": [
+// CHECK: {
+// CHECK: "context-hash": "[[F_CONTEXT_HASH]]"
+// CHECK: "module-name": "F"
+// CHECK: }
+// CHECK-NEXT: ]
+// CHECK: "command-line": [
+// CHECK: "-fmodule-map-file=[[PREFIX]]/module/F.framework/Modules/module.modulemap"
+// CHECK: ]
+// CHECK: "clang-module-deps": [
+// CHECK: {
+// CHECK: "context-hash": "[[F_CONTEXT_HASH]]"
+// CHECK: "module-name": "F"
+// CHECK: }
+// CHECK-NEXT: ]
+// CHECK: "command-line": [
+// CHECK: "-fmodule-map-file=[[PREFIX]]/module/F.framework/Modules/module.modulemap"
+// CHECK: ]
+
+//--- cdb.json.in
+[
+ {
+ "directory": "DIR",
+ "command": "clang -fsyntax-only DIR/tu1.c -fmodules -fimplicit-module-maps -fmodules-cache-path=DIR/cache",
+ "file": "DIR/tu1.c"
+ },
+ {
+ "directory": "DIR",
+ "command": "clang -fsyntax-only DIR/tu2.c -fmodules -fimplicit-module-maps -fmodules-cache-path=DIR/cache",
+ "file": "DIR/tu2.c"
+ },
+ {
+ "directory": "DIR",
+ "command": "clang -fsyntax-only -F DIR/symlink-to-module DIR/tu3.c -fmodules -fimplicit-module-maps -fmodules-cache-path=DIR/cache",
+ "file": "DIR/tu3.c"
+ },
+ {
+ "directory": "DIR",
+ "command": "clang -fsyntax-only -F DIR/module DIR/tu3.c -fmodules -fimplicit-module-maps -fmodules-cache-path=DIR/cache",
+ "file": "DIR/tu3.c"
+ },
+]
+
+//--- actual.modulemap
+module Mod { header "header.h" }
+
+//--- module/header.h
+
+//--- tu1.c
+#include "symlink-to-module/header.h"
+
+//--- tu2.c
+#include "module/header.h"
+
+//--- module/F.framework/Versions/A/Modules/module.modulemap
+framework module F {
+ umbrella header "F.h"
+}
+
+//--- module/F.framework/Versions/A/Headers/F.h
+
+//--- tu3.c
+#include "F/F.h"
Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===================================================================
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -406,15 +406,16 @@
MD.ImplicitModulePCMPath = std::string(M->getASTFile()->getName());
MD.IsSystem = M->IsSystem;
- const FileEntry *ModuleMap = MDC.ScanInstance.getPreprocessor()
- .getHeaderSearchInfo()
- .getModuleMap()
- .getModuleMapFileForUniquing(M);
+ ModuleMap &ModMapInfo =
+ MDC.ScanInstance.getPreprocessor().getHeaderSearchInfo().getModuleMap();
+
+ const FileEntry *ModuleMap = ModMapInfo.getModuleMapFileForUniquing(M);
if (ModuleMap) {
- StringRef Path = ModuleMap->tryGetRealPathName();
- if (Path.empty())
- Path = ModuleMap->getName();
+ // FIXME: Update header search to keep FileEntryRef rather than rely on
+ // getLastRef().
+ SmallString<128> Path = ModuleMap->getLastRef().getNameAsRequested();
+ ModMapInfo.canonicalizeModuleMapPath(Path);
MD.ClangModuleMapFile = std::string(Path);
}
Index: clang/lib/Lex/ModuleMap.cpp
===================================================================
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -1299,6 +1299,38 @@
InferredModuleAllowedBy[M] = ModMap;
}
+std::error_code
+ModuleMap::canonicalizeModuleMapPath(SmallVectorImpl<char> &Path) {
+ StringRef Dir = llvm::sys::path::parent_path({Path.data(), Path.size()});
+
+ // Do not canonicalize within the framework; the module map parser expects
+ // Modules/ not Versions/A/Modules.
+ if (llvm::sys::path::filename(Dir) == "Modules") {
+ StringRef Parent = llvm::sys::path::parent_path(Dir);
+ if (Parent.endswith(".framework"))
+ Dir = Parent;
+ }
+
+ FileManager &FM = SourceMgr.getFileManager();
+ auto DirEntry = FM.getDirectory(Dir.empty() ? "." : Dir);
+ if (!DirEntry)
+ return DirEntry.getError();
+
+ // Canonicalize the directory.
+ StringRef CanonicalDir = FM.getCanonicalName(*DirEntry);
+ if (CanonicalDir != Dir) {
+ bool Done = llvm::sys::path::replace_path_prefix(Path, Dir, CanonicalDir);
+ (void)Done;
+ assert(Done && "Path should always start with Dir");
+ }
+
+ // In theory, the filename component should also be canonicalized if it
+ // on a case-insensitive filesystem. However, the extra canonicalization is
+ // expensive and if clang looked up the filename it will always be lowercase.
+
+ return std::error_code();
+}
+
void ModuleMap::addAdditionalModuleMapFile(const Module *M,
const FileEntry *ModuleMap) {
AdditionalModMaps[M].insert(ModuleMap);
Index: clang/lib/Lex/HeaderSearch.cpp
===================================================================
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -255,18 +255,11 @@
//
// To avoid false-negatives, we form as canonical a path as we can, and map
// to lower-case in case we're on a case-insensitive file system.
- std::string Parent =
- std::string(llvm::sys::path::parent_path(ModuleMapPath));
- if (Parent.empty())
- Parent = ".";
- auto Dir = FileMgr.getDirectory(Parent);
- if (!Dir)
+ SmallString<128> CanonicalPath(ModuleMapPath);
+ if (getModuleMap().canonicalizeModuleMapPath(CanonicalPath))
return {};
- auto DirName = FileMgr.getCanonicalName(*Dir);
- auto FileName = llvm::sys::path::filename(ModuleMapPath);
- llvm::hash_code Hash =
- llvm::hash_combine(DirName.lower(), FileName.lower());
+ llvm::hash_code Hash = llvm::hash_combine(CanonicalPath.str().lower());
SmallString<128> HashStr;
llvm::APInt(64, size_t(Hash)).toStringUnsigned(HashStr, /*Radix*/36);
Index: clang/include/clang/Lex/ModuleMap.h
===================================================================
--- clang/include/clang/Lex/ModuleMap.h
+++ clang/include/clang/Lex/ModuleMap.h
@@ -622,6 +622,15 @@
void setInferredModuleAllowedBy(Module *M, const FileEntry *ModMap);
+ /// Canonicalize \p Path in a manner suitable for a module map file. In
+ /// particular, this canonicalizes the parent directory separately from the
+ /// filename so that it does not affect header resolution relative to the
+ /// modulemap.
+ ///
+ /// \returns an error code if any filesystem operations failed. In this case
+ /// \p Path is not modified.
+ std::error_code canonicalizeModuleMapPath(SmallVectorImpl<char> &Path);
+
/// Get any module map files other than getModuleMapFileForUniquing(M)
/// that define submodules of a top-level module \p M. This is cheaper than
/// getting the module map file for each submodule individually, since the
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
