[Lldb-commits] [PATCH] D53788: [FileSystem] Remove GetByteSize() from FileSpec
Zachary Turner via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Mon Oct 29 11:09:19 PDT 2018
zturner added a comment.
In https://reviews.llvm.org/D53788#1279096, @shafik wrote:
> Are we refactoring in the right place? Why not just refactor `FileSpec::GetByteSize()` why is `FileSpec` the wrong place?
There was another review thread where we discussed this just the other day. Basically, It took a herculean effort to get `FileSpec` into the Utility library, which helped greatly with layering and we don't want to undo this. In order to for `GetByteSize` to be able to use the VFS, it has to be able to use `FileSystem`, so either `FileSpec` has to move out of `Utility`, `FileSystem` has to move into `Utility`, or `GetByteSize` has to move out of `FileSpec`. `FileSystem` moving into `Utility` might actually be a worthwhile goal long term, but even putting that aside, I think `FileSpec` should just be a thing for manipulating paths. For example, it has the notion of manipulating paths which might not even make sense on the given host platform (e.g. it has a member variable which represents the path *syntax). You might be on a Windows host debugging a remove Linux target, for example, and your host needs to have some way to manipulate paths on the remote which use a non-host syntax. That's what `FileSpec` is supposed to be. It just deals with strings.
More information about the lldb-commits