I'm not sure we have enough instances to decide on better organization, but 
ArchSpec really doesn't feel like a Utility for lldb.  That would be like 
moving the llvm triple handling to ADT, that seems a little weird.  Similarly 
having the register stuff in Utility seems odd as well.  I would never think to 
look for it there.  Pavel asked me a while ago to talk a bit about what the 
strategy for the directories in lldb was originally and I started to answer and 
then got sidetracked by events and never answered.  

The relevant historical bit for this discussion was originally the directory 
layout had nothing to do with building (Xcode doesn't care about files being in 
different directories, and we didn't envision building pieces of lldb 
separately at that point.  I did it originally with the sole intent making an 
organization for people new to the code to find and remember where files were.  
We weren't super-rigorous about this - particularly as Xcode got better at 
finding files for you so finding them in directories was less important on our 
end.  So for the purposes of reflecting code organization, we could probably 
stand another round of organization to make the structure more obvious.  But I 
do think that's still a worthy goal.

Anyway, then just because of the way cmake works (or is used in llvm I don't 
actually know which) we've switch the meaning of the directories to "files that 
are built into a .a file".  And then, because of the need to shrink the code 
size of lldb-server, it became important to make at least the lower-level stuff 
that lldb-server depended on independent of as much of the rest of lldb as 
possible.  So it became important for at least some directories to contain 
"files that are built into .a files that don't depend on some/most of the other 
.a files."  That's introduced a somewhat orthogonal design principle for the 
directory layout.

Note, Greg and I used to argue about the strategy for lldb-server.  My notion 
was on modern systems the actual file size difference between an lldb-server 
that used all of lldb.framework, and one that could use a cut down library was 
really not all that important.  If you're making a stub for an hard embedded 
system, you probably aren't going to use lldb-server, you'll use a much smaller 
gdb-protocol stub, and we didn't really have the intent to provide that 
functionality.  So lldb server as intended for things like phones etc, where 
"small" means small in modern terms, not "a kilobyte matters" type small. 

I always felt the appropriate discipline to impose was making sure that if you 
didn't use something in lldb (symbols for instance) you don't pay memory/time 
cost for it.  If that was true, and the bigger size was not an issue, than 
having lldb-server built with the full lldb.framework would mean that you would 
have flexibility to move work from the host to the lldb-server end of a remote 
debugging session, which could be really convenient since you could offload 
work to the remote end w/o having to come back over the slower remote 
communication channel.  We never really convinced on another either way and 
since he directed most of the development of lldb-server on our end, he won by 
dint of implementation.  But that's why for me the breakup of LLDB.framework 
into independent pieces was never a particularly present goal.

Any, that's water under the bridge at this point.  But I'd also like not to 
lose the benefit of comprehension that was the original reason for the 
directory layout.  And in this instance, ArchSpec really doesn't seem like a 
Utility, which brought this back up in my thinking.  There doesn't seem a 
critical mass of files in Utility to make decisions at this point, but as we 
break up Core, it might be nice to have a place for things that aren't Support 
or ADT type stuff, but more the core business of a debugger, but also don't 
drag in Symbol and other parts of lldb that aren't appropriate for lldb-server.

Jim



> On Nov 10, 2017, at 4:02 AM, Pavel Labath via Phabricator 
> <revi...@reviews.llvm.org> wrote:
> 
> labath created this revision.
> 
> In https://reviews.llvm.org/D39387, I was quick to jump to conclusion that 
> ArchSpec has no
> external dependencies. It turns there still was one call to
> HostInfo::GetArchitecture left -- for implementing the "systemArch32"
> architecture and friends.
> 
> Since GetAugmentedArchSpec is the place we handle these "incomplete"
> triples that don't specify os or vendor and "systemArch" looks very much
> like an incomplete triple, I move its handling there.
> 
> After this ArchSpec *really* does not have external dependencies, and
> I'd like to move it to the Utility module as a follow-up.
> 
> 
> https://reviews.llvm.org/D39896
> 
> Files:
>  include/lldb/Host/HostInfoBase.h
>  source/Core/ArchSpec.cpp
>  source/Host/common/HostInfoBase.cpp
>  source/Target/Platform.cpp
>  unittests/Host/HostInfoTest.cpp
> 
> <D39896.122416.patch>

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to