================
@@ -1307,6 +1307,9 @@ void ModuleMap::setInferredModuleAllowedBy(Module *M,
std::error_code
ModuleMap::canonicalizeModuleMapPath(SmallVectorImpl<char> &Path) {
+ FileManager &FM = SourceMgr.getFileManager();
+ FM.makeAbsolutePath(Path);
----------------
jansvoboda11 wrote:
> > The canonical module map path would now be absolute. Previously, we'd
> > generate relative paths for module maps found through a relative search
> > paths, or besides includers that themselves were found through a relative
> > path.
>
> Yes, this change is what I'm concerned about.
Sorry, I take that back. (It's been a while since I wrote this patch.) This
function (and therefore the scanner) used to return absolute paths even before
this patch because it always uses `FileManager::getCanonicalName()`.
So the only thing this patch changes is what gets serialized into the PCM file.
When I started canonicalizing the module map path before storing it into the
PCM, some tests started unexpectedly failing with `ASTWriter` [reporting
inconsistencies](https://github.com/llvm/llvm-project/blob/c60ccfbb898148449946f82cbac6140f1e01cb12/clang/lib/Serialization/ASTReader.cpp#L4093).
When I looked into it, I found it was due to the fact that
`FileManager::getCanonicalName()` just forwards the path it's given to
`llvm::vfs::FileSystem::getRealPath()`. The problem is, the VFS might have a
different idea as to what the CWD is, which breaks with relative module map
paths and `-working-directory` (used in `tests/Modules/relative-import-path.c`).
Another issue that popped up was that this function calls
`replace_path_prefix(Path, Dir, CanonicalDir)`. If `Dir` is empty, it just
slaps the absolute real path for CWD in front of the filename without any
separator.
Making the path absolute early on with `FileManager`'s CWD solved both of those
issues. (No usage of VFS's CWD, no empty `Dir`.)
So I think given all of that, my concerns are:
* Should we call `FileManager::makeAbsolutePath()` in
`ModuleMap::canonicalizeModuleMapPath()` or in
`FileManager::getCanonicalName()`?
* 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.
https://github.com/llvm/llvm-project/pull/66389
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
