[lldb-vscode] Don't call SBValue.GetError after generating a summary.

[jgorbe]

Summary

In some occasions, SBValue::GetError can invalidate its cached
m_summary_str member. This in turn invalidates any StringRef variables
pointing to it.

[clayborg]

Looks good!

[yinghuitan]

lldb/tools/lldb-vscode/JSONUtils.cpp

143

Sorry for the late comment. The fix is fine but it depends on an important assumption that GetError() has to be called before GetSummary(). Can we add an explicit comment to document this so that future refactoring won't violate it again.

[clayborg]

lldb/tools/lldb-vscode/JSONUtils.cpp

143

We shouldn't, we need to fix this API bug in LLDB itself. Just calling SBValue::GetError() shouldn't end up clearing the m_summary_str IMHO. This seems like the real bug. This is working around this. IF we add a comment, it won't end up being true when we fix the issue.

lldb/tools/lldb-vscode/JSONUtils.cpp

case llvm::json::Value::Array:
       break;
     }
   }
   return strs;
 }
 
 void SetValueForKey(lldb::SBValue &v, llvm::json::Object &object,
                     llvm::StringRef key) {
-
-  llvm::StringRef value = v.GetValue();
-  llvm::StringRef summary = v.GetSummary();
-  llvm::StringRef type_name = v.GetType().GetDisplayTypeName();
-  lldb::SBError error = v.GetError();
-
   std::string result;
   llvm::raw_string_ostream strm(result);
+
+  lldb::SBError error = v.GetError();
   if (!error.Success()) {
     strm << "<error: " << error.GetCString() << ">";
-  } else if (!value.empty()) {
+  } else {
+    llvm::StringRef value = v.GetValue();
+    llvm::StringRef summary = v.GetSummary();
+    llvm::StringRef type_name = v.GetType().GetDisplayTypeName();
+    if (!value.empty()) {
       strm << value;
       if (!summary.empty())
         strm << ' ' << summary;
     } else if (!summary.empty()) {
       strm << ' ' << summary;
     } else if (!type_name.empty()) {
       strm << type_name;
       lldb::addr_t address = v.GetLoadAddress();
       if (address != LLDB_INVALID_ADDRESS)
         strm << " @ " << llvm::format_hex(address, 0);
     }
+  }
   strm.flush();
   EmplaceSafeString(object, key, result);
 }
 
 void FillResponse(const llvm::json::Object &request,
                   llvm::json::Object &response) {
   // Fill in all of the needed response fields to a "request" and set "success"
   // to true by default.