bnbarham added a comment. In D121733#3392931 <https://reviews.llvm.org/D121733#3392931>, @dexonsmith wrote:
> However, FileManager changes sometimes have odd side effects... and it's > possible that somewhere in clang relies on having `FileManager::getFileRef()` > return precisely the same path that was requested. Tagging a few other people > that have some context... please share your opinions! Wouldn't that already not be the case though (ie. given `RedirectingFileSystem` and `use-external-names` is currently a thing)? I know we're wanting to change that, but I don't *know* of anywhere that depends on this currently. > @ppluzhnikov, can you give more context on how this interacts with > https://reviews.llvm.org/D121658? I had a quick look but it wasn't > immediately obvious. If I understand correctly, the failing tests in that patch are failing because they're always expecting "/" and since `sys::append` is used, it's now "\\" on Windows. The remove dots change doesn't fix those, since the tests would still need to be updated to remove the "./" (which is most of the tests in this patch). But there's also some others where I wouldn't expect them to be failing in this patch, eg. the ones from `/` -> `{{[/\\]}}`. Are there tests that we can't just fix to expect either `/` or `\\`? Why do we need to change the underlying behaviour here at all? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121733/new/ https://reviews.llvm.org/D121733 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
