[PATCH] D30858: Teach llvm's path library to support both windows and posix paths at the same time.

Pavel Labath via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 14 08:28:20 PDT 2017


labath added a comment.

@zturner said (in email)

> I actually think that should live in lldb. All of llvm's path handling code is stateless which makes the most sense from an api perspective. Lldb is just "special" in that regard

I think that makes sense when you don't have to worry about path syntax, as then there isn't actually any state to maintain. But if you support different path syntaxes, then it sounds to me like you should maintain that explicitly, as without it, there is no way to interpret the path correctly. I was thinking of having a class (let's call it `FileSpec` for lack of a better name), which would basically be a glorified `std::pair<SmallString, Style>` wrapper.

All the manipulation functions would remain freestanding as they are now, but they would accept either a `SmallString` (in which case they assume native path syntax) or the `FileSpec` object, where the syntax is stored explicitly. (this would probably require a helper type FileSpecRef, constructible from both, to enable code sharing in the implementation of these functions.)

I can live with this implementation, if that's what people consider better, but I think I'd prefer something where you have the path syntax stored explicitly.


https://reviews.llvm.org/D30858





More information about the llvm-commits mailing list