I don't plan to change debugserver's link requirements. What I'm saying is that debugserver is including StringExtractor.h cross-project from LLDB, and so even something as simple as including an LLVM header from StringExtractor.h will (if I understand correctly) break debugserver. If I'm correct, then I don't think this is an acceptable limitation for an LLDB project header.
I have a patch locally which changes GetNameColonValue() to return two StringRefs instead of two std::strings, eliminating hundreds of string copies and is perfectly safe. So I don't see this as a particularly controversial change to get into LLDB, but it will require debugserver to keep its own local copy of StringExtractor.h, similar to how it already does with JSON.h and JSON.cpp. I hoping to get this change in today since it is a strict improvement over the current StringExtractor. There are still some additional improvements I would like to make independently of this initial change, and they do culminate in changing StringExtractor to store an llvm::StringRef. It turns out that while yes, it uses an std::string, I cannot find one single user (and I have looked at all of them) that depends on this. In every single case, it can use a StringRef with no ownership issues, and the number of string copies is further reduced. So I don't see a convincing argument to keep this as storing a std::string. But maybe there is something I'm not aware of? On Mon, Aug 29, 2016 at 10:51 AM Greg Clayton <gclay...@apple.com> wrote: > > > On Aug 27, 2016, at 3:14 PM, Zachary Turner via lldb-dev < > lldb-dev@lists.llvm.org> wrote: > > > > What is the status of using LLVM from debugserver? AFAICT, it doesn't > use llvm, but it DOES use some lldb private libraries, in which case it is > already implicitly linking against LLVM anyway. > > > > So why can't LLVM headers be included and used from debugserver? Just > now I was changing the signature of an LLDB function to include an llvm > header, and I got everything working and ready to go but searched through > code that doesn't compile on Windows, and I noticed that debugserver > includes some headers from lldb, and since those lldb headers are including > llvm headers, I'm assuming this is not going to work. But I think I'm > missing something, because AFAICT this will already cause a link dependency > from debugserver to llvm, and has been for some time. So I'm not entirely > sure what the status is of this. > > > > In any case, I know the easy way out is to just say don't include an > llvm header from that lldb header, but I don't think that's a great idea as > I think the patch I'm working on is a big win for performance, code > readability, and simplicity. > > > > It looks like there is already precedent for debugserver to copy some > code from lldb and keep a local copy in debugserver, for example JSON.h and > JSON.cpp seem to be copies of the code from lldb/Utility. Can this be done > for StringExtractor.h as well so that I can reference llvm from within > StringExtractor.h in lldb? > > debugserver currently doesn't link against any of the llvm .a files so I > don't think it can be using any llvm stuff unless it uses header file only > inlined functions. We aren't going to spend any time on this and we have > builds of debugserver that link against even less (debugserver-mini) that > the normal debugserver. So I advise you don't change debugserver's link > requirements as we build it in many different places and we don't always > build llvm when we build, so debugserver can't have requirements to link > against llvm or clang .a files. > > For one I would prefer to leave StringExtractor alone. It uses the > std::string to store the packet content and we want that the remain as is. > I am fine with coming up with a replacement StringRefExtractor.h/.cpp that > uses an llvm::StringRef that we cut over to using internally in LLDB, but > again, for anyone needing the data to stay stored somewhere, they should > use StringExtractor.cpp.
_______________________________________________ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
