swiftlang
[cxx-interop] Fix -verify-additional-file on Windows
SourceManager uses associates paths to buffer IDs using BufIdentIDMap. However, on Windows, both '' and '/' are valid path separators, so foo\bar\baz.cpp foo/bar\baz.cpp and foo/bar/baz.cpp all refer to the same file, but will be stored as separate entries in BufIdentIDMap. This led -verify-additional-file to misbehave because it would create multiple buffer ID numbers for the same path, which confused the diagnostic verifier logic.
This patch normalizes paths before looking them up in BufIdentIDMap, so that buffer IDs are uniquely associated with each file rather than each path.
There are other ways for paths to be denormalised than just forward-slash vs backward-slash. Do we care about those? E.g:
- some file systems are case insensitive
- relative vs absolute paths
- redundant dots:
Cxx/include/cxx-header.hvs./Cxx/include/cxx-header.hvsCxx/./include/cxx-header.hvsCxx/../Cxx/include/cxx-header.h
There are other ways for paths to be denormalised than just forward-slash vs backward-slash. Do we care about those?
The intention of this change is only to resolve the issue we face in -verify, which, as far as I'm aware, is for our own test suite. So unless purposely specify path collisions like that, we probably won't run into those kinds of issues (and if we do, we can normalize as necessary).
In the meantime, I'd like not to add too much overhead/complexity to this code path.
I forgot that the StringRef key is also saved in the BufIdentIDMap, so creating a new entry with a stack-allocated std::string causes a lifetime issue (which is what is causing CI to fail).
Lifetime issues aside: upon discussions with some other folks, I've come around to the fact that canonicalization may not be the right approach here, since it comes with other pitfalls. I'm closing this PR in favor of simply refactoring the tests to use %{fs-sep}. That should only affect the handful of tests using -verify-additional-file.