[clang] [clang][deps] Load module map file from PCM (PR #66389)

Ben Langmuir via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 15 13:31:05 PDT 2023


================
@@ -1307,6 +1307,9 @@ void ModuleMap::setInferredModuleAllowedBy(Module *M,
 
 std::error_code
 ModuleMap::canonicalizeModuleMapPath(SmallVectorImpl<char> &Path) {
+  FileManager &FM = SourceMgr.getFileManager();
+  FM.makeAbsolutePath(Path);
----------------
benlangmuir wrote:

> This function (and therefore the scanner) used to return absolute paths even before this patch because it always uses FileManager::getCanonicalName().

Heh, my bad. As the person who wrote that code I should've realized this was already absolute.

> Should we call FileManager::makeAbsolutePath() in ModuleMap::canonicalizeModuleMapPath() or in FileManager::getCanonicalName()?

I think the correct behaviour would be that `FileManager::getCanonicalName` would use `FixupRelativePath` to apply `-working-directory`, but not use `makeAbsolutePath` since that would be inconsistent with how it handles paths in other APIs.  The VFS itself is responsible for resolving CWD in `getRealPath`.

That being said, if this causes other issues I'm fine with your current approach in the meantime.

> Is using ASTWriter::AddPath() with the real path problematic? It only performs textual prefix match with a directory path that's not necessarily the real path.

I don't know how we avoid that without the same issues that motivated canonicalizing in the first place. We don't want to be compiling separate modules for /a/Foo, /a/foo, /a/b/../foo, /symlink/foo, etc.

https://github.com/llvm/llvm-project/pull/66389


More information about the cfe-commits mailing list