<div dir="ltr">Yes. In some cases I want to try deleting LLDB's implementation and seeing if we can fall back on LLVM. For example, MemoryMapFileContents comes to mind. LLVM has some code to memory map files as well. I've spent 0 time looking into how widely FileSpec::MemoryMapFileContents is used, so it's possible that after 1 minute I realize it's not possible / too much work. But I'd like to at least do due diligence and see if there's any parts of the FileSpec interface that can be removed in favor of LLVM classes first.</div><br><div class="gmail_quote"><div dir="ltr">On Wed, Feb 15, 2017 at 10:20 AM Greg Clayton <<a href="mailto:gclayton@apple.com">gclayton@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg">On Feb 15, 2017, at 10:14 AM, Zachary Turner <<a href="mailto:zturner@google.com" class="gmail_msg" target="_blank">zturner@google.com</a>> wrote:</div><br class="m_6646888128330930912Apple-interchange-newline gmail_msg"><div class="gmail_msg"><div dir="ltr" class="gmail_msg">I think the main improvement that it provides is that you need *something* which represents only a path. Because we use FileSpecs to refer to paths on remote machines, and then what does it mean to call Exists or Resolve? So, we could have something like PathSpec and then have FileSpec contain a PathSpec, and PathSpec's only purpose would be to know about the grammar of a path.</div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg">Agreed. Are you ok with starting with moving as much stuff from FileSpec over into File to start with?</div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg"><div dir="ltr" class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg"><div class="gmail_msg">I agree though, in the spirit of doing things incrementally, purely moving things around and not making major substantive changes is a good starting point and reduces the scope of the problem.</div></div></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg">Agreed.</div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"><div class="gmail_quote gmail_msg"><div dir="ltr" class="gmail_msg">On Wed, Feb 15, 2017 at 10:07 AM Greg Clayton <<a href="mailto:gclayton@apple.com" class="gmail_msg" target="_blank">gclayton@apple.com</a>> wrote:<br class="gmail_msg"></div><blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I would prefer things stay object oriented when possible. I don't think it adds anything by making everything a static function. It is ok where it makes sense, but I would vote to add static functions in the FileSpec class over making purely stand alone functions as they indicate that it should be used on a FileSpec object. It is also ok to have both methods and static functions where the methods (like AppendPathComponent() can call through to the static versions if needed.<br class="gmail_msg">
<br class="gmail_msg">
We also have the "File.h" class that defines lldb_private::File. This is the object that wraps the access to the data in the file (the file descriptor integers and the "FILE *" abstraction. It would be interesting to move any functions in lldb_private::FileSpec, like the mmap functions, over into lldb_private::File where needed. And then see if there is even a need for the FileSystem class at all. If there is we can divvy it up as needed.<br class="gmail_msg">
<br class="gmail_msg">
I would say lets start with purely moving things around and get the file locations all set and the correct abstractions where FileSpec is for representing a file specification efficiently and all operations on that affect file spec mutations, moving anything that is accessing file contents over into lldb_private::File, and then seeing if we have any functions left over and if we even need FileSystem.<br class="gmail_msg">
<br class="gmail_msg">
That being said I generally find it useful to do something like:<br class="gmail_msg">
<br class="gmail_msg">
FileSpec f("/tmp/a.txt");<br class="gmail_msg">
if (f.Exists())<br class="gmail_msg">
...<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
It seems like the suggestions would be to move to something like:<br class="gmail_msg">
<br class="gmail_msg">
FileSpec f("/tmp/a.txt");<br class="gmail_msg">
if (File::Exists(f))<br class="gmail_msg">
...<br class="gmail_msg">
<br class="gmail_msg">
Both are fine, but the latter is much less discoverable. Think about when you use IDEs with auto completion. So while it might be cleaner from a code linking standpoint, I don't think it actually improves the code for users. I much prefer the object oriented methodology and I would prefer code linking considerations don't override the clean API we currently have.<br class="gmail_msg">
<br class="gmail_msg">
> On Feb 15, 2017, at 8:05 AM, Pavel Labath via lldb-dev <<a href="mailto:lldb-dev@lists.llvm.org" class="gmail_msg" target="_blank">lldb-dev@lists.llvm.org</a>> wrote:<br class="gmail_msg">
><br class="gmail_msg">
> I prefer free functions as well, but I think switching FileSpec to<br class="gmail_msg">
> that would be a non-trivial task. If you want to try it out I would<br class="gmail_msg">
> encourage it, but I don't think it's a requirement.<br class="gmail_msg">
><br class="gmail_msg">
> On 15 February 2017 at 15:41, Zachary Turner <<a href="mailto:zturner@google.com" class="gmail_msg" target="_blank">zturner@google.com</a>> wrote:<br class="gmail_msg">
>> Having FileSpec in Utility and FileSystem in Host would work for now.<br class="gmail_msg">
>><br class="gmail_msg">
>> Any opinions on the FileSpec interface? Llvm's path and file system<br class="gmail_msg">
>> libraries use free functions for everything. in a perfect world I feel<br class="gmail_msg">
>> that's the best design, and with lldb we could even do slightly better and<br class="gmail_msg">
>> make all functions pure (returning copies instead of mutating) since<br class="gmail_msg">
>> everything is ConstString.<br class="gmail_msg">
>><br class="gmail_msg">
>> If we were starting over from scratch this would definitely be the way to go<br class="gmail_msg">
>> imo, but I'm not sure if it's worth the effort now. What do you think?<br class="gmail_msg">
>><br class="gmail_msg">
>> On Wed, Feb 15, 2017 at 7:34 AM Pavel Labath <<a href="mailto:labath@google.com" class="gmail_msg" target="_blank">labath@google.com</a>> wrote:<br class="gmail_msg">
>>><br class="gmail_msg">
>>> Right, I see. In that case, I guess my response would be "let's wait<br class="gmail_msg">
>>> and see how things look like after FileSpec is moved".<br class="gmail_msg">
>>><br class="gmail_msg">
>>> I kinda like how we have the all the host code in a separate module (I<br class="gmail_msg">
>>> expect we will have a lot more of it than llvm), but I am not against<br class="gmail_msg">
>>> it if it turns out there is no other way to organize dependencies. I<br class="gmail_msg">
>>> just don't think we've reached that point yet -- right now I don't see<br class="gmail_msg">
>>> why we couldn't have FileSpec in Utility and FileSystem in Host.<br class="gmail_msg">
>>><br class="gmail_msg">
>>> Or have you thought ahead and found a problem already?<br class="gmail_msg">
>>><br class="gmail_msg">
>>><br class="gmail_msg">
>>> On 15 February 2017 at 15:17, Zachary Turner <<a href="mailto:zturner@google.com" class="gmail_msg" target="_blank">zturner@google.com</a>> wrote:<br class="gmail_msg">
>>>> Yes, in fact that mirrors how i had planned to tackle this.<br class="gmail_msg">
>>>><br class="gmail_msg">
>>>> The question is, can we put in Utility code that is separated by<br class="gmail_msg">
>>>> platform,<br class="gmail_msg">
>>>> which typically has been for Host? This mirrors what llvm does<br class="gmail_msg">
>>>> On Wed, Feb 15, 2017 at 6:29 AM Pavel Labath <<a href="mailto:labath@google.com" class="gmail_msg" target="_blank">labath@google.com</a>> wrote:<br class="gmail_msg">
>>>>><br class="gmail_msg">
>>>>> I agree that the next module that needs figuring on is the host one.<br class="gmail_msg">
>>>>> However I am not sure if the decision on what to put in what module<br class="gmail_msg">
>>>>> should be motivated by the FileSpec class, as I think it's current<br class="gmail_msg">
>>>>> interface has a some issues, and our choice on how to resolve them can<br class="gmail_msg">
>>>>> greatly affect what things it depends on.<br class="gmail_msg">
>>>>><br class="gmail_msg">
>>>>> The main thing that bugs me with FileSpec is that it is used for a two<br class="gmail_msg">
>>>>> distinct purposes:<br class="gmail_msg">
>>>>> - manipulations on abstract path names (e.g. PrependPathComponent)<br class="gmail_msg">
>>>>> - manipulations of actual files present on the host (e.g.<br class="gmail_msg">
>>>>> MemoryMapFileContents)<br class="gmail_msg">
>>>>> For the first one you don't need the files to exist on the host (in<br class="gmail_msg">
>>>>> fact they may not even use the same path syntax as the host). For the<br class="gmail_msg">
>>>>> other one, they *must* exist and *must* use the same path syntax. This<br class="gmail_msg">
>>>>> is currently not very well separated and enforced, and I think it<br class="gmail_msg">
>>>>> should be. I believe this is the reason the FileSystem pseudo-class<br class="gmail_msg">
>>>>> was created.<br class="gmail_msg">
>>>>><br class="gmail_msg">
>>>>> So, my counter-proposal would be to finish moving the host-specific<br class="gmail_msg">
>>>>> things out of the FileSpec class (basically just replace<br class="gmail_msg">
>>>>> file_spec.Foo(...) with FileSystem::Foo(file_spec, ...). At that point<br class="gmail_msg">
>>>>> FileSpec should only depend on string manipulation functions, and we<br class="gmail_msg">
>>>>> should be able to move it without much hassle. After that, we can take<br class="gmail_msg">
>>>>> another look and decide what to do next.<br class="gmail_msg">
>>>>><br class="gmail_msg">
>>>>> The thing I really like about this idea is that we will end up with<br class="gmail_msg">
>>>>> two classes that very closely mirror llvm functionality (FileSpec is a<br class="gmail_msg">
>>>>> version of Support/Path.h that does not assume host path syntax,<br class="gmail_msg">
>>>>> FileSystem is similar to Support/FileSystem.h, but it supports some<br class="gmail_msg">
>>>>> more fancy operations like mmap()). Then we could proceed to merge<br class="gmail_msg">
>>>>> this functionality with llvm pretty much independently of any other<br class="gmail_msg">
>>>>> refactoring we will be doing.<br class="gmail_msg">
>>>>><br class="gmail_msg">
>>>>> What do you think?<br class="gmail_msg">
>>>>><br class="gmail_msg">
>>>>><br class="gmail_msg">
>>>>><br class="gmail_msg">
>>>>> On 15 February 2017 at 01:48, Zachary Turner via lldb-dev<br class="gmail_msg">
>>>>> <<a href="mailto:lldb-dev@lists.llvm.org" class="gmail_msg" target="_blank">lldb-dev@lists.llvm.org</a>> wrote:<br class="gmail_msg">
>>>>>> After <a href="https://reviews.llvm.org/D29964" rel="noreferrer" class="gmail_msg" target="_blank">https://reviews.llvm.org/D29964</a>, we finally have a starting<br class="gmail_msg">
>>>>>> point<br class="gmail_msg">
>>>>>> at<br class="gmail_msg">
>>>>>> which we can begin unravelling the cross-project cyclic dependencies<br class="gmail_msg">
>>>>>> in<br class="gmail_msg">
>>>>>> LLDB. lldb/Utility now is very similar in spirit to llvm/Support.<br class="gmail_msg">
>>>>>><br class="gmail_msg">
>>>>>> But llvmSupport goes one step further and includes what lldb would<br class="gmail_msg">
>>>>>> normally<br class="gmail_msg">
>>>>>> put under Host. I think this makes some sense. Practically all<br class="gmail_msg">
>>>>>> parts<br class="gmail_msg">
>>>>>> of a<br class="gmail_msg">
>>>>>> codebase have need of a single OS abstraction layer. So, I think<br class="gmail_msg">
>>>>>> that a<br class="gmail_msg">
>>>>>> lot<br class="gmail_msg">
>>>>>> of the functionality currently in lldbHost is in fact needed by the<br class="gmail_msg">
>>>>>> rest<br class="gmail_msg">
>>>>>> of<br class="gmail_msg">
>>>>>> LLDB.<br class="gmail_msg">
>>>>>><br class="gmail_msg">
>>>>>> So, I wonder if it makes sense to follow the path that LLVM has<br class="gmail_msg">
>>>>>> taken,<br class="gmail_msg">
>>>>>> and<br class="gmail_msg">
>>>>>> start moving some of this code from Host down to Utility. Doing so<br class="gmail_msg">
>>>>>> would<br class="gmail_msg">
>>>>>> allow us to break one of the biggest links in the dependency cycle in<br class="gmail_msg">
>>>>>> the<br class="gmail_msg">
>>>>>> entire codebase, which is that Host depends on everything, and<br class="gmail_msg">
>>>>>> everything<br class="gmail_msg">
>>>>>> depends on Host.<br class="gmail_msg">
>>>>>><br class="gmail_msg">
>>>>>> Of course, it can't just be a straight move. Some things in Host<br class="gmail_msg">
>>>>>> interact<br class="gmail_msg">
>>>>>> with Target, with CommandInterpreter, and with many other things.<br class="gmail_msg">
>>>>>> And<br class="gmail_msg">
>>>>>> stuff<br class="gmail_msg">
>>>>>> going into Utility can't take a dependency.<br class="gmail_msg">
>>>>>><br class="gmail_msg">
>>>>>> So there will be some splitting, some moving, some refactoring, etc.<br class="gmail_msg">
>>>>>> But to<br class="gmail_msg">
>>>>>> me tackling Host seems like the logical next step, in large part<br class="gmail_msg">
>>>>>> because<br class="gmail_msg">
>>>>>> Host is where FileSpec is, and it's going to be hard to break any<br class="gmail_msg">
>>>>>> dependencies without first addressing FileSpec.<br class="gmail_msg">
>>>>>><br class="gmail_msg">
>>>>>> The way LLVM handles cross-platform differences in Support is that<br class="gmail_msg">
>>>>>> you<br class="gmail_msg">
>>>>>> include a single header and it does conditional includes into a<br class="gmail_msg">
>>>>>> platform<br class="gmail_msg">
>>>>>> specific subdirectory for the parts that differ.<br class="gmail_msg">
>>>>>><br class="gmail_msg">
>>>>>> I'm thinking to follow the same pattern here in lldb/Utility, and<br class="gmail_msg">
>>>>>> begin<br class="gmail_msg">
>>>>>> looking for ways to get pieces of Host down into Utility this way,<br class="gmail_msg">
>>>>>> until<br class="gmail_msg">
>>>>>> ultimately I can get FileSpec down there.<br class="gmail_msg">
>>>>>><br class="gmail_msg">
>>>>>> Thoughts?<br class="gmail_msg">
>>>>>><br class="gmail_msg">
>>>>>> _______________________________________________<br class="gmail_msg">
>>>>>> lldb-dev mailing list<br class="gmail_msg">
>>>>>> <a href="mailto:lldb-dev@lists.llvm.org" class="gmail_msg" target="_blank">lldb-dev@lists.llvm.org</a><br class="gmail_msg">
>>>>>> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev" rel="noreferrer" class="gmail_msg" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev</a><br class="gmail_msg">
>>>>>><br class="gmail_msg">
> _______________________________________________<br class="gmail_msg">
> lldb-dev mailing list<br class="gmail_msg">
> <a href="mailto:lldb-dev@lists.llvm.org" class="gmail_msg" target="_blank">lldb-dev@lists.llvm.org</a><br class="gmail_msg">
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev" rel="noreferrer" class="gmail_msg" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev</a><br class="gmail_msg">
<br class="gmail_msg">
</blockquote></div>
</div></blockquote></div><br class="gmail_msg"></div></blockquote></div>