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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to