<div dir="ltr">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.<div><br></div><div>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.</div><div><br></div><div><br></div><div>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?</div></div><br><div class="gmail_quote"><div dir="ltr">On Mon, Aug 29, 2016 at 10:51 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"><br>
> On Aug 27, 2016, at 3:14 PM, Zachary Turner via lldb-dev <<a href="mailto:lldb-dev@lists.llvm.org" target="_blank">lldb-dev@lists.llvm.org</a>> wrote:<br>
><br>
> 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.<br>
><br>
> 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.<br>
><br>
> 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.<br>
><br>
> 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?<br>
<br>
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.<br>
<br>
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. </blockquote></div>