[PATCH] D121733: Clean pathnames in FileManager.

Duncan P. N. Exon Smith via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Mar 18 14:49:33 PDT 2022


dexonsmith added reviewers: mstorsjo, rnk.
dexonsmith added subscribers: mstorsjo, rnk.
dexonsmith added a comment.

FWIW, I quite like the clean up effect that this patch has (assuming we are able to convince ourselves that it's safe/correct for `FileManager` to do this).

- Removing the dots feels like pure goodness.
- Canonicalizing the slashes also seems potentially nice (I had missed that `remove_dots()` does this), as long as `FileManager` is respecting any user-specified path style on Windows. Note that the style could be either `sys::path::Style::windows_slash` or `sys::path::Style::windows_backslash`, but your `remove_dots` call will be following `sys::path::Style::native`. Although, it could also be hard to reason about, if some paths in the output are trafficked through `FileManager` and get cleaned up, while others are used directly and don't get cleaned up.

Given the Windows slash behaviour, would be good to get a Windows reviewer in, maybe @mstorsjo or @rnk.

> That's what this change is doing (but there is more Winx64 cleanup pending; sorry, I should have sent this as a draft, but I am still learning the ropes here).

No worries; sorry for jumping in guns blazing. (Drafts are a new feature, and I've been afraid to touch them myself :).)

BTW, if this still isn't ready for review, you can mark it as "changes planned".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121733



More information about the cfe-commits mailing list