diff --git a/clang/include/clang/Lex/ModuleMap.h b/clang/include/clang/Lex/ModuleMap.h --- a/clang/include/clang/Lex/ModuleMap.h +++ b/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 &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 diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp --- a/clang/lib/Lex/HeaderSearch.cpp +++ b/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); diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp --- a/clang/lib/Lex/ModuleMap.cpp +++ b/clang/lib/Lex/ModuleMap.cpp @@ -1303,6 +1303,42 @@ InferredModuleAllowedBy[M] = ModMap; } +std::error_code +ModuleMap::canonicalizeModuleMapPath(SmallVectorImpl &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. + + // Remove ., remove redundant separators, and switch to native separators. + // This is needed for separators between CanonicalDir and the filename. + llvm::sys::path::remove_dots(Path); + + return std::error_code(); +} + void ModuleMap::addAdditionalModuleMapFile(const Module *M, const FileEntry *ModuleMap) { AdditionalModMaps[M].insert(ModuleMap); diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp --- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp +++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp @@ -406,15 +406,14 @@ MD.ImplicitModulePCMPath = std::string(M->getASTFile()->getName()); MD.IsSystem = M->IsSystem; - Optional ModuleMap = MDC.ScanInstance.getPreprocessor() - .getHeaderSearchInfo() - .getModuleMap() - .getModuleMapFileForUniquing(M); + ModuleMap &ModMapInfo = + MDC.ScanInstance.getPreprocessor().getHeaderSearchInfo().getModuleMap(); + + Optional ModuleMap = ModMapInfo.getModuleMapFileForUniquing(M); if (ModuleMap) { - StringRef Path = ModuleMap->getFileEntry().tryGetRealPathName(); - if (Path.empty()) - Path = ModuleMap->getName(); + SmallString<128> Path = ModuleMap->getNameAsRequested(); + ModMapInfo.canonicalizeModuleMapPath(Path); MD.ClangModuleMapFile = std::string(Path); } diff --git a/clang/test/ClangScanDeps/modules-symlink-dir.c b/clang/test/ClangScanDeps/modules-symlink-dir.c new file mode 100644 --- /dev/null +++ b/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 \ +// 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"