[PATCH] D121733: Clean pathnames in FileManager.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.


================
Comment at: clang/lib/Basic/FileManager.cpp:218
+  llvm::sys::path::remove_dots(CleanFilename, /*remove_dot_dot=*/false);
+  Filename = CleanFilename;
+
----------------
this is actually breaking the [contract of FileEntryRef](https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/FileEntry.h#L58-L59), is this the point?

```
/// A reference to a \c FileEntry that includes the name of the file as it was
/// accessed by the FileManager's client.
```

I don't see mention of this contract being changed explicitly anywhere, if so can you mention it in the commit message and also update the documentation? (the same applies to DirectoryEntryRef changes as well)

I am not able to take a detailed look at this at the moment, but this doesn't feel like a safe change for all clients. Because people might be relying on this contract without explicitly testing the behaviour for "./" in the filenames. So tests passing (especially when you're just updating them to not have `./`) might not be implying it's safe.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121733