[PATCH] D135849: [llvm] Return canonical virtual path from `RedirectingFileSystem`

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

Module map canonicalization in `clang-scan-deps` causes issues even after D135841 <https://reviews.llvm.org/D135841> landed.

Here's the problematic scenario this patch aims to solve:

- there is a framework `FW` whose headers (but not module maps) are redirected in a VFS overlay file
- there is a TU with `#import <fw/Header.h>` (note the incorrect case of the framework name)
- what follows is how Clang treats the involved file/directory paths:
  - find the incorrectly-cased header through the case-insensitive VFS overlay
  - chop off `Headers/Header.h`
  - call `FileManager::getCanonicalName()` on the `fw.framework` directory
    - `RedirectingFileSystem` calls the underlying FS to get the real path
      - note: this happens since the node is not `RedirectingFileSystem::{FileEntry,DirectoryRemapEntry}`, see the last branch of `RedirectingFileSystem::getRealPath()`
      - the underlying case-sensitive FS doesn't recognize the incorrectly-cased path
    - `FileManager` falls back to just using `Dir.getName()` (`fw.framework`) for that virtual `DirectoryEntry` as the canonical path
  - the `fw.framework/Modules/module.modulemap` file is not in the VFS overlay, and due to the wrong case, the underlying case-sensitive FW doesn't recognize it either
  - no matching module map means `#include <fw/Header.h>` is resolved to textual include
  - that header then includes other headers from the same FW using correct case: `#import <FW/AnotherHeader.h>`
  - the correctly-cased module map is found in the underlying FS and the framework gets compiled into a PCM file
  - at the end of dependency scan, scanner takes the correctly-cased module map path from the PCM file
  - scanner tries to canonicalize the module map path
    - `Modules/module.modulemap` is chopped off and `FileManager::getCanonicalName()` is called on the framework directory
    - `FileManager::getDirectory("FW.framework")` resolves to the same `DirectoryEntry` that `fw.framework` resolved to (VFS overlay is case-insensitive)
    - `FileManager` looks in the cache of canonical names and finds `fw.framework` for that virtual `DirectoryEntry`
    - `FW.framework/Modules/module.modulemap` is canonicalized to `fw.framework/Modules/module.modulemap`
  - scanner asserts that file exists, but it's not in the VFS nor in the underlying case-sensitive FS

This patch fixes this issue by returning the **canonical virtual path** for `fw.framework` at the very beginning. This is achieved by using the as-written spelling from the overlay file.

I can think of two other alternatives to this patch:

- In `ModuleMap::canonicalizeModuleMapPath()`, avoid canonicalization if the new module map path resolves to a different (or no) `FileEntry`.
- In `FileManager`, cache real paths based on the directory name, not the `DirectoryEntry`.

Both of these alternatives are workarounds, whereas the VFS fix seems fairly straightforward.

Note that I plan to add unit tests for this and add `// XFAIL: case-insensitive-filesystem` to the clang-scan-deps test if we get consensus on this approach.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135849/new/

https://reviews.llvm.org/D135849