[PATCH] D70701: Fix more VFS tests on Windows

Duncan P. N. Exon Smith via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 21 16:26:13 PST 2021


dexonsmith added a comment.

@rnk / @amccarth, I've been looking at the history of `makeAbsolute` being virtual, since it's a bit awkward that `RedirectingFileSystem` behaves differently from the others. I'm hoping you can give me a bit more context.

I'm wondering about an alternative implementation where:

- The path style is detected when reading the YAML file (as now).
- Paths in the YAML file are canonicalized to native at parse time.
- The nodes in-memory all use native paths so the non-native style isn't needed after construction is complete.
- `makeAbsolute()` doesn't need to be overridden / special.

Was this considered? If so, why was it rejected? (If it doesn't work, why not?)

If we could limit the scope the special version of `makeAbsolute()` to "construction time" it would simplify the mental model. As it stands it's a bit difficult to reason about `makeAbsolute`, and whether it's safe/correct to send a `makeAbsolute`-d path into `ExternalFS`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70701



More information about the cfe-commits mailing list