[lldb-vscode] Follow up of D99989 - store some strings more safely

[wallace]

Summary

As a follow up of https://reviews.llvm.org/D99989#inline-953343, I'm now
storing std::string instead of char *. I know it might never break as char *,
but if it does, chasing that bug might be dauting.

[JDevlieghere]

lldb/tools/lldb-vscode/lldb-vscode.cpp

2934

Have you considered an`llvm::StringMap`, it should be more performant.

[wallace]

TIL. I'll give it a try

[wallace]

lldb/tools/lldb-vscode/lldb-vscode.cpp

2934

I've been reading https://llvm.org/devmtg/2014-04/PDFs/LightningTalks/data_structure_llvm.pdf and it seems that it's not more performant speed-wise. I imagine it's more performance space-wise, but here I mostly care about time.

[clayborg]

lldb/tools/lldb-vscode/lldb-vscode.cpp

2939

This will crash if "variable.GetName()" return nullptr. We should check and only add if not nullptr

[wallace]

now checking if the strings gotten from the API are null or not

[wallace]

lldb/tools/lldb-vscode/JSONUtils.cpp

925

i had to change this because std::string(nullptr) throws on my machine

[shafik]

lldb/tools/lldb-vscode/JSONUtils.cpp

925

Yes, the std::string constructor taking a const CharT* expects a null terminated C-string.

lldb/tools/lldb-vscode/JSONUtils.h

 //===-- JSONUtils.h ---------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_TOOLS_LLDB_VSCODE_JSONUTILS_H
 #define LLDB_TOOLS_LLDB_VSCODE_JSONUTILS_H
 
 #include "VSCodeForward.h"
 #include "lldb/API/SBModule.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/JSON.h"
 #include <stdint.h>
 
 namespace lldb_vscode {
 
 /// Emplace a StringRef in a json::Object after enusring that the
 /// string is valid UTF8. If not, first call llvm::json::fixUTF8

 ///     The LLDB thread to use when populating out the "StoppedEvent"
 ///     object.
 ///
 /// \return
 ///     A "StoppedEvent" JSON object with that follows the formal JSON
 ///     definition outlined by Microsoft.
 llvm::json::Value CreateThreadStopped(lldb::SBThread &thread, uint32_t stop_id);
 
+/// \return
+///     The variable name of \a value or a default placeholder.
+const char *GetNonNullVariableName(lldb::SBValue value);
+
 /// VSCode can't display two variables with the same name, so we need to
 /// distinguish them by using a suffix.
 ///
 /// If the source and line information is present, we use it as the suffix.
 /// Otherwise, we fallback to the variable address or register location.
 std::string CreateUniqueVariableNameForDisplay(lldb::SBValue v,
                                                bool is_name_duplicated);
 

lldb/tools/lldb-vscode/JSONUtils.cpp

 #include <iomanip>
 #include <sstream>
 
 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/FormatAdapters.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ScopedPrinter.h"
 
 #include "lldb/API/SBBreakpoint.h"
 #include "lldb/API/SBBreakpointLocation.h"
 #include "lldb/API/SBDeclaration.h"
 #include "lldb/API/SBValue.h"
 #include "lldb/Host/PosixApi.h"
 
 #include "ExceptionBreakpoint.h"
 #include "JSONUtils.h"
 #include "LLDBUtils.h"
if (tid == g_vsc.focus_tid) {
     body.try_emplace("threadCausedFocus", true);
   }
   body.try_emplace("preserveFocusHint", tid != g_vsc.focus_tid);
   body.try_emplace("allThreadsStopped", true);
   event.try_emplace("body", std::move(body));
   return llvm::json::Value(std::move(event));
 }
 
+const char *GetNonNullVariableName(lldb::SBValue v) {
+  const char *name = v.GetName();
+  return name ? name : "<null>";
+}
+
 std::string CreateUniqueVariableNameForDisplay(lldb::SBValue v,
                                                bool is_name_duplicated) {
   lldb::SBStream name_builder;
-  const char *name = v.GetName();
-  name_builder.Print(name ? name : "<null>");
+  name_builder.Print(GetNonNullVariableName(v));
   if (is_name_duplicated) {
-    name_builder.Print(" @ ");
     lldb::SBDeclaration declaration = v.GetDeclaration();
-    std::string file_name(declaration.GetFileSpec().GetFilename());
+    const char *file_name = declaration.GetFileSpec().GetFilename();
     const uint32_t line = declaration.GetLine();
 
-    if (!file_name.empty() && line > 0)
-      name_builder.Printf("%s:%u", file_name.c_str(), line);
-    else
-      name_builder.Print(v.GetLocation());
+    if (file_name != nullptr && line > 0)
+      name_builder.Printf(" @ %s:%u", file_name, line);
+    else if (const char *location = v.GetLocation())
+      name_builder.Printf(" @ %s", location);
   }
   return name_builder.GetData();
 }
 
 // "Variable": {
 //   "type": "object",
 //   "description": "A Variable is a name/value pair. Optionally a variable
 //                   can have a 'type' that is shown if space permits or when

lldb/tools/lldb-vscode/lldb-vscode.cpp

case VARREF_REGS:
       num_children = g_vsc.num_regs;
       break;
     default:
       break;
     }
     const int64_t end_idx = start_idx + ((count == 0) ? num_children : count);
 
     // We first find out which variable names are duplicated
-    llvm::DenseMap<const char *, int> variable_name_counts;
+    std::map<std::string, int> variable_name_counts;
     for (auto i = start_idx; i < end_idx; ++i) {
       lldb::SBValue variable = g_vsc.variables.GetValueAtIndex(i);
       if (!variable.IsValid())
         break;
-      variable_name_counts[variable.GetName()]++;
+      variable_name_counts[GetNonNullVariableName(variable)]++;
     }
 
     // Now we construct the result with unique display variable names
     for (auto i = start_idx; i < end_idx; ++i) {
       lldb::SBValue variable = g_vsc.variables.GetValueAtIndex(i);
-      const char *name = variable.GetName();
 
       if (!variable.IsValid())
         break;
       variables.emplace_back(CreateVariable(variable, VARIDX_TO_VARREF(i), i,
                                             hex,
-                                            variable_name_counts[name] > 1));
+          variable_name_counts[GetNonNullVariableName(variable)] > 1));
     }
   } else {
     // We are expanding a variable that has children, so we will return its
     // children.
     const int64_t var_idx = VARREF_TO_VARIDX(variablesReference);
     lldb::SBValue variable = g_vsc.variables.GetValueAtIndex(var_idx);
     if (variable.IsValid()) {
       const auto num_children = variable.GetNumChildren();