I prefer free functions as well, but I think switching FileSpec to that would be a non-trivial task. If you want to try it out I would encourage it, but I don't think it's a requirement.
On 15 February 2017 at 15:41, Zachary Turner <ztur...@google.com> wrote: > Having FileSpec in Utility and FileSystem in Host would work for now. > > Any opinions on the FileSpec interface? Llvm's path and file system > libraries use free functions for everything. in a perfect world I feel > that's the best design, and with lldb we could even do slightly better and > make all functions pure (returning copies instead of mutating) since > everything is ConstString. > > If we were starting over from scratch this would definitely be the way to go > imo, but I'm not sure if it's worth the effort now. What do you think? > > On Wed, Feb 15, 2017 at 7:34 AM Pavel Labath <lab...@google.com> wrote: >> >> Right, I see. In that case, I guess my response would be "let's wait >> and see how things look like after FileSpec is moved". >> >> I kinda like how we have the all the host code in a separate module (I >> expect we will have a lot more of it than llvm), but I am not against >> it if it turns out there is no other way to organize dependencies. I >> just don't think we've reached that point yet -- right now I don't see >> why we couldn't have FileSpec in Utility and FileSystem in Host. >> >> Or have you thought ahead and found a problem already? >> >> >> On 15 February 2017 at 15:17, Zachary Turner <ztur...@google.com> wrote: >> > Yes, in fact that mirrors how i had planned to tackle this. >> > >> > The question is, can we put in Utility code that is separated by >> > platform, >> > which typically has been for Host? This mirrors what llvm does >> > On Wed, Feb 15, 2017 at 6:29 AM Pavel Labath <lab...@google.com> wrote: >> >> >> >> I agree that the next module that needs figuring on is the host one. >> >> However I am not sure if the decision on what to put in what module >> >> should be motivated by the FileSpec class, as I think it's current >> >> interface has a some issues, and our choice on how to resolve them can >> >> greatly affect what things it depends on. >> >> >> >> The main thing that bugs me with FileSpec is that it is used for a two >> >> distinct purposes: >> >> - manipulations on abstract path names (e.g. PrependPathComponent) >> >> - manipulations of actual files present on the host (e.g. >> >> MemoryMapFileContents) >> >> For the first one you don't need the files to exist on the host (in >> >> fact they may not even use the same path syntax as the host). For the >> >> other one, they *must* exist and *must* use the same path syntax. This >> >> is currently not very well separated and enforced, and I think it >> >> should be. I believe this is the reason the FileSystem pseudo-class >> >> was created. >> >> >> >> So, my counter-proposal would be to finish moving the host-specific >> >> things out of the FileSpec class (basically just replace >> >> file_spec.Foo(...) with FileSystem::Foo(file_spec, ...). At that point >> >> FileSpec should only depend on string manipulation functions, and we >> >> should be able to move it without much hassle. After that, we can take >> >> another look and decide what to do next. >> >> >> >> The thing I really like about this idea is that we will end up with >> >> two classes that very closely mirror llvm functionality (FileSpec is a >> >> version of Support/Path.h that does not assume host path syntax, >> >> FileSystem is similar to Support/FileSystem.h, but it supports some >> >> more fancy operations like mmap()). Then we could proceed to merge >> >> this functionality with llvm pretty much independently of any other >> >> refactoring we will be doing. >> >> >> >> What do you think? >> >> >> >> >> >> >> >> On 15 February 2017 at 01:48, Zachary Turner via lldb-dev >> >> <lldb-dev@lists.llvm.org> wrote: >> >> > After https://reviews.llvm.org/D29964, we finally have a starting >> >> > point >> >> > at >> >> > which we can begin unravelling the cross-project cyclic dependencies >> >> > in >> >> > LLDB. lldb/Utility now is very similar in spirit to llvm/Support. >> >> > >> >> > But llvmSupport goes one step further and includes what lldb would >> >> > normally >> >> > put under Host. I think this makes some sense. Practically all >> >> > parts >> >> > of a >> >> > codebase have need of a single OS abstraction layer. So, I think >> >> > that a >> >> > lot >> >> > of the functionality currently in lldbHost is in fact needed by the >> >> > rest >> >> > of >> >> > LLDB. >> >> > >> >> > So, I wonder if it makes sense to follow the path that LLVM has >> >> > taken, >> >> > and >> >> > start moving some of this code from Host down to Utility. Doing so >> >> > would >> >> > allow us to break one of the biggest links in the dependency cycle in >> >> > the >> >> > entire codebase, which is that Host depends on everything, and >> >> > everything >> >> > depends on Host. >> >> > >> >> > Of course, it can't just be a straight move. Some things in Host >> >> > interact >> >> > with Target, with CommandInterpreter, and with many other things. >> >> > And >> >> > stuff >> >> > going into Utility can't take a dependency. >> >> > >> >> > So there will be some splitting, some moving, some refactoring, etc. >> >> > But to >> >> > me tackling Host seems like the logical next step, in large part >> >> > because >> >> > Host is where FileSpec is, and it's going to be hard to break any >> >> > dependencies without first addressing FileSpec. >> >> > >> >> > The way LLVM handles cross-platform differences in Support is that >> >> > you >> >> > include a single header and it does conditional includes into a >> >> > platform >> >> > specific subdirectory for the parts that differ. >> >> > >> >> > I'm thinking to follow the same pattern here in lldb/Utility, and >> >> > begin >> >> > looking for ways to get pieces of Host down into Utility this way, >> >> > until >> >> > ultimately I can get FileSpec down there. >> >> > >> >> > Thoughts? >> >> > >> >> > _______________________________________________ >> >> > lldb-dev mailing list >> >> > lldb-dev@lists.llvm.org >> >> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev >> >> > _______________________________________________ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
