[Lldb-commits] [PATCH] D26618: Make some code not manipulate the underlying buffer of a StreamString

[Enrico Granata via lldb-commits]

I will gladly let Todd run tests on your diff, but I just thought I'd 
explain what is going on there

In Cocoa, some NSURL objects are represented as "sub-URLs", e.g. you can 
start from www.apple.com and represent www.apple.com/macos as 
NSURL(base="www.apple.com", next="/macos")

This can be recursively done, so you can have 
NSURL(base=NSURL(base="www.apple.com", next="/macos"), next="/sierra")

The data formatter is attempting to represent the URL as "%s --- %s" % 
(summary(base), summary(next))

On 11/14/16 10:00 AM, Zachary Turner via lldb-commits wrote:
> zturner created this revision.
> zturner added reviewers: tfiala, beanz.
> zturner added a subscriber: lldb-commits.
>
> I have locally a a very large patch which removes the method of `StreamString` that returns a reference to the underlying `std::string` buffer, and instead returns a `StringRef`.  There were 1 or 2 instances where someone was relying on this function to reach into the underlying buffer and do stuff like insert or erase from it.  This seems like a huge hack, and in the future as I move code towards `llvm::raw_ostream` this will be incompatible with LLVM's api anyway since it does not allow such things.  The only non-trivial fix to this was in something Cocoa related.  I don't really understand this code, and I can't test it, but the patch here is intended to be NFC and just re-writing the logic in terms of something that doesn't modify the internal Stream buffer.
>
> If someone could test it for me I would appreciate.
>
>
> https://reviews.llvm.org/D26618
>
> Files:
>    source/Plugins/Language/ObjC/Cocoa.cpp
>
>
> Index: source/Plugins/Language/ObjC/Cocoa.cpp
> ===================================================================
> --- source/Plugins/Language/ObjC/Cocoa.cpp
> +++ source/Plugins/Language/ObjC/Cocoa.cpp
> @@ -559,42 +559,44 @@
>     if (!valobj_addr)
>       return false;
>   
> -  const char *class_name = descriptor->GetClassName().GetCString();
> +  llvm::StringRef class_name = descriptor->GetClassName().GetStringRef();
>   
> -  if (!class_name || !*class_name)
> +  if (!class_name.equals("NSURL"))
>       return false;
>   
> -  if (strcmp(class_name, "NSURL") == 0) {
> -    uint64_t offset_text = ptr_size + ptr_size +
> -                           8; // ISA + pointer + 8 bytes of data (even on 32bit)
> -    uint64_t offset_base = offset_text + ptr_size;
> -    CompilerType type(valobj.GetCompilerType());
> -    ValueObjectSP text(
> -        valobj.GetSyntheticChildAtOffset(offset_text, type, true));
> -    ValueObjectSP base(
> -        valobj.GetSyntheticChildAtOffset(offset_base, type, true));
> -    if (!text)
> -      return false;
> -    if (text->GetValueAsUnsigned(0) == 0)
> -      return false;
> -    StreamString summary;
> -    if (!NSStringSummaryProvider(*text, summary, options))
> -      return false;
> -    if (base && base->GetValueAsUnsigned(0)) {
> -      if (summary.GetSize() > 0)
> -        summary.GetString().resize(summary.GetSize() - 1);
> -      summary.Printf(" -- ");
> -      StreamString base_summary;
> -      if (NSURLSummaryProvider(*base, base_summary, options) &&
> -          base_summary.GetSize() > 0)
> -        summary.Printf("%s", base_summary.GetSize() > 2
> -                                 ? base_summary.GetData() + 2
> -                                 : base_summary.GetData());
> -    }
> -    if (summary.GetSize()) {
> -      stream.Printf("%s", summary.GetData());
> -      return true;
> +  uint64_t offset_text = ptr_size + ptr_size +
> +                         8; // ISA + pointer + 8 bytes of data (even on 32bit)
> +  uint64_t offset_base = offset_text + ptr_size;
> +  CompilerType type(valobj.GetCompilerType());
> +  ValueObjectSP text(valobj.GetSyntheticChildAtOffset(offset_text, type, true));
> +  ValueObjectSP base(valobj.GetSyntheticChildAtOffset(offset_base, type, true));
> +  if (!text)
> +    return false;
> +  if (text->GetValueAsUnsigned(0) == 0)
> +    return false;
> +  StreamString summary;
> +  if (!NSStringSummaryProvider(*text, summary, options))
> +    return false;
> +  if (base && base->GetValueAsUnsigned(0)) {
> +    std::string summary_str = summary.GetString();
> +
> +    if (!summary_str.empty())
> +      summary_str.pop_back();
> +    summary_str += " -- ";
> +    StreamString base_summary;
> +    if (NSURLSummaryProvider(*base, base_summary, options) &&
> +        !base_summary.Empty()) {
> +      llvm::StringRef base_str = base_summary.GetString();
> +      if (base_str.size() > 2)
> +        base_str = base_str.drop_front(2);
> +      summary_str += base_str;
>       }
> +    summary.Clear();
> +    summary.PutCString(summary_str);
> +  }
> +  if (!summary.Empty()) {
> +    stream.PutCString(summary.GetString());
> +    return true;
>     }
>   
>     return false;
>
>
>
>
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20161114/84966f96/attachment.html>