[Lldb-commits] [PATCH] D151859: [lldb/Target] Add ability to set a label to targets

2023-06-02 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib updated this revision to Diff 527776.
mib added a comment.

Address @bulbazord comments:

- Make `Target::SetLabel` return an `llvm::Error` to propagate error messages 
to both the `CommandObject` and the `SBAPI` callers.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151859/new/

https://reviews.llvm.org/D151859

Files:
  lldb/include/lldb/API/SBTarget.h
  lldb/include/lldb/Target/Target.h
  lldb/include/lldb/Target/TargetList.h
  lldb/source/API/SBTarget.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetList.cpp
  lldb/test/Shell/Target/target-label.test

Index: lldb/test/Shell/Target/target-label.test
===
--- /dev/null
+++ lldb/test/Shell/Target/target-label.test
@@ -0,0 +1,38 @@
+# RUN: %lldb -b -o 'settings set interpreter.stop-command-source-on-error false' -s %s 2>&1 | FileCheck %s
+
+target create -l "ls" /bin/ls
+target list
+# CHECK: * target #0 (ls): /bin/ls
+
+script lldb.target.SetLabel("")
+target list
+# CHECK: * target #0: /bin/ls
+
+target create -l "cat" /bin/cat
+target list
+# CHECK: target #0: /bin/ls
+# CHECK-NEXT: * target #1 (cat): /bin/cat
+
+target create -l "cat" /bin/cat
+# CHECK: Cannot use label 'cat' since it's set in target #1.
+
+target create -l 42 /bin/cat
+# CHECK: error: Cannot use integer as target label.
+
+target select 0
+# CHECK: * target #0: /bin/ls
+# CHECK-NEXT: target #1 (cat): /bin/cat
+
+target select cat
+# CHECK: target #0: /bin/ls
+# CHECK-NEXT: * target #1 (cat): /bin/cat
+
+script lldb.target.GetLabel()
+# CHECK: 'cat'
+
+script lldb.debugger.GetTargetAtIndex(0).SetLabel('Not cat')
+# CHECK: success
+
+target list
+# CHECK: target #0 (Not cat): /bin/ls
+# CHECK-NEXT: * target #1 (cat): /bin/cat
Index: lldb/source/Target/TargetList.cpp
===
--- lldb/source/Target/TargetList.cpp
+++ lldb/source/Target/TargetList.cpp
@@ -489,7 +489,7 @@
   return num_signals_sent;
 }
 
-int TargetList::GetNumTargets() const {
+size_t TargetList::GetNumTargets() const {
   std::lock_guard guard(m_target_list_mutex);
   return m_target_list.size();
 }
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -69,6 +69,7 @@
 #include 
 #include 
 #include 
+#include 
 
 using namespace lldb;
 using namespace lldb_private;
@@ -2536,6 +2537,27 @@
   Target::GetGlobalProperties().SetDefaultArchitecture(arch);
 }
 
+llvm::Error Target::SetLabel(llvm::StringRef label) {
+  size_t n = LLDB_INVALID_INDEX32;
+  if (llvm::to_integer(label, n))
+return llvm::make_error(
+"Cannot use integer as target label.", llvm::inconvertibleErrorCode());
+  TargetList &targets = GetDebugger().GetTargetList();
+  for (size_t i = 0; i < targets.GetNumTargets(); i++) {
+TargetSP target_sp = targets.GetTargetAtIndex(i);
+if (target_sp && target_sp->GetLabel() == label) {
+return llvm::make_error(
+llvm::formatv(
+"Cannot use label '{0}' since it's set in target #{1}.", label,
+i),
+llvm::inconvertibleErrorCode());
+}
+  }
+
+  m_label = label.str();
+  return llvm::Error::success();
+}
+
 Target *Target::GetTargetFromContexts(const ExecutionContext *exe_ctx_ptr,
   const SymbolContext *sc_ptr) {
   // The target can either exist in the "process" of ExecutionContext, or in
Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -82,8 +82,14 @@
   if (!exe_valid)
 ::strcpy(exe_path, "");
 
-  strm.Printf("%starget #%u: %s", prefix_cstr ? prefix_cstr : "", target_idx,
-  exe_path);
+  std::string formatted_label = "";
+  const std::string &label = target->GetLabel();
+  if (!label.empty()) {
+formatted_label = " (" + label + ")";
+  }
+
+  strm.Printf("%starget #%u%s: %s", prefix_cstr ? prefix_cstr : "", target_idx,
+  formatted_label.data(), exe_path);
 
   uint32_t properties = 0;
   if (target_arch.IsValid()) {
@@ -209,6 +215,8 @@
 m_platform_options(true), // Include the --platform option.
 m_core_file(LLDB_OPT_SET_1, false, "core", 'c', 0, eArgTypeFilename,
 "Fullpath to a core file to use for this target."),
+m_label(LLDB_OPT_SET_1, false, "label", 'l', 0, eArgTypeName,
+"Optional name for this target.", nullptr),
 m_symbol_file(LLDB_OPT_SET_1, false, "symfile", 's', 0,
   eArgTypeFilename,
   "Fullpath to a stand alone debug "
@@ -234,6 +242,7 @@
 m_option_group.Append(&m_arch_option, LLDB_OPT_SET_ALL, LLDB_OPT_SET_1);
 m_option_group.Append(&m_platform_optio

[Lldb-commits] [PATCH] D151919: [lldb][NFCI] Apply IndexEntry to DWARFUnitHeader outside of extraction

2023-06-02 Thread Felipe de Azevedo Piovezan via Phabricator via lldb-commits
fdeazeve accepted this revision.
fdeazeve added a comment.

I'm not exactly familiar with DWOs, but the code motions LGTM!




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:882
+  // we have a valid one to set it to.
+  assert(index_entry);
+  assert(!m_index_entry);

If LLVM's function is like this as well, we shouldn't do anything now and just 
refactor it later once we make the switch. But a pointer parameter in a 
function that asserts not null immediately is more expressive as a reference. 
Note that, in the code where this is called, we have a:

```
if (ptr)
  ApplyIndex(ptr)
```

So the assert is not needed, as the callee respects the contract and can do a 
`*ptr`



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151919/new/

https://reviews.llvm.org/D151919

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D151292: lldb WIP/RFC: Adding support for address fixing on AArch64 with high and low memory addresses

2023-06-02 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment.

> None of this should change behavior on linux, but if folks from the linux 
> world have a comment or reaction to this change, I would be interested to 
> hear opinions. I haven't done much testing beyond the one test corefile, and 
> I still need to work out how the two values are communicated in corefiles & 
> live debug, but those are trivial details on the core idea.

I primarily work with lldb in Linux userspace, so this isn't an issue. Also 
seems unlikely that the Linux kernel would decide to change it, but if it did 
the scheme implemented here looks fine.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151292/new/

https://reviews.llvm.org/D151292

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D149784: [Damangle] convert rustDemangle to use std::string_view

2023-06-02 Thread Nick Desaulniers via Phabricator via lldb-commits
nickdesaulniers updated this revision to Diff 527895.
nickdesaulniers added a comment.

- rebase for presubmits


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149784/new/

https://reviews.llvm.org/D149784

Files:
  lldb/include/lldb/Utility/ConstString.h
  lldb/source/Core/Mangled.cpp
  llvm/include/llvm/Demangle/Demangle.h
  llvm/lib/Demangle/RustDemangle.cpp
  llvm/tools/llvm-rust-demangle-fuzzer/llvm-rust-demangle-fuzzer.cpp

Index: llvm/tools/llvm-rust-demangle-fuzzer/llvm-rust-demangle-fuzzer.cpp
===
--- llvm/tools/llvm-rust-demangle-fuzzer/llvm-rust-demangle-fuzzer.cpp
+++ llvm/tools/llvm-rust-demangle-fuzzer/llvm-rust-demangle-fuzzer.cpp
@@ -13,7 +13,7 @@
 
 extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) {
   std::string NullTerminatedString((const char *)Data, Size);
-  char *Demangled = llvm::rustDemangle(NullTerminatedString.c_str());
+  char *Demangled = llvm::rustDemangle(NullTerminatedString);
   std::free(Demangled);
   return 0;
 }
Index: llvm/lib/Demangle/RustDemangle.cpp
===
--- llvm/lib/Demangle/RustDemangle.cpp
+++ llvm/lib/Demangle/RustDemangle.cpp
@@ -20,11 +20,13 @@
 #include 
 #include 
 #include 
+#include 
 
 using namespace llvm;
 
 using llvm::itanium_demangle::OutputBuffer;
 using llvm::itanium_demangle::ScopedOverride;
+using llvm::itanium_demangle::starts_with;
 
 namespace {
 
@@ -146,17 +148,13 @@
 
 } // namespace
 
-char *llvm::rustDemangle(const char *MangledName) {
-  if (MangledName == nullptr)
-return nullptr;
-
+char *llvm::rustDemangle(std::string_view MangledName) {
   // Return early if mangled name doesn't look like a Rust symbol.
-  std::string_view Mangled(MangledName);
-  if (!llvm::itanium_demangle::starts_with(Mangled, "_R"))
+  if (MangledName.empty() || !starts_with(MangledName, "_R"))
 return nullptr;
 
   Demangler D;
-  if (!D.demangle(Mangled)) {
+  if (!D.demangle(MangledName)) {
 std::free(D.Output.getBuffer());
 return nullptr;
   }
@@ -196,7 +194,7 @@
   RecursionLevel = 0;
   BoundLifetimes = 0;
 
-  if (!llvm::itanium_demangle::starts_with(Mangled, "_R")) {
+  if (!starts_with(Mangled, "_R")) {
 Error = true;
 return false;
   }
Index: llvm/include/llvm/Demangle/Demangle.h
===
--- llvm/include/llvm/Demangle/Demangle.h
+++ llvm/include/llvm/Demangle/Demangle.h
@@ -11,6 +11,7 @@
 
 #include 
 #include 
+#include 
 
 namespace llvm {
 /// This is a llvm local version of __cxa_demangle. Other than the name and
@@ -54,7 +55,7 @@
 MSDemangleFlags Flags = MSDF_None);
 
 // Demangles a Rust v0 mangled symbol.
-char *rustDemangle(const char *MangledName);
+char *rustDemangle(std::string_view MangledName);
 
 // Demangles a D mangled symbol.
 char *dlangDemangle(const char *MangledName);
Index: lldb/source/Core/Mangled.cpp
===
--- lldb/source/Core/Mangled.cpp
+++ lldb/source/Core/Mangled.cpp
@@ -25,6 +25,7 @@
 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -150,7 +151,7 @@
   return demangled_cstr;
 }
 
-static char *GetRustV0DemangledStr(const char *M) {
+static char *GetRustV0DemangledStr(std::string_view M) {
   char *demangled_cstr = llvm::rustDemangle(M);
 
   if (Log *log = GetLog(LLDBLog::Demangle)) {
@@ -259,7 +260,7 @@
 break;
   }
   case eManglingSchemeRustV0:
-demangled_name = GetRustV0DemangledStr(mangled_name);
+demangled_name = GetRustV0DemangledStr(m_mangled);
 break;
   case eManglingSchemeD:
 demangled_name = GetDLangDemangledStr(mangled_name);
Index: lldb/include/lldb/Utility/ConstString.h
===
--- lldb/include/lldb/Utility/ConstString.h
+++ lldb/include/lldb/Utility/ConstString.h
@@ -14,6 +14,7 @@
 #include "llvm/Support/FormatVariadic.h"
 
 #include 
+#include 
 
 namespace lldb_private {
 class Stream;
@@ -180,8 +181,11 @@
 
   bool operator<(ConstString rhs) const;
 
-  // Implicitly convert \class ConstString instances to \class StringRef.
+  // Implicit conversion functions.
   operator llvm::StringRef() const { return GetStringRef(); }
+  operator std::string_view() const {
+return std::string_view(m_string, GetLength());
+  }
 
   /// Get the string value as a C string.
   ///
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D151003: [Damangle] convert dlangDemangle to use std::string_view

2023-06-02 Thread Nick Desaulniers via Phabricator via lldb-commits
nickdesaulniers marked an inline comment as done.
nickdesaulniers added inline comments.



Comment at: llvm/lib/Demangle/Demangle.cpp:49
 bool llvm::nonMicrosoftDemangle(const char *MangledName, std::string &Result) {
+  if (!MangledName)
+return false;

MaskRay wrote:
> Why is this change? The original contract is that `MangledName` must be 
> non-null.
https://fosstodon.org/@llvm/110397650834821908

It's insidious; implicitly constructing a `std::string_view` from a `char*` 
which is `nullptr` invokes a call to `strlen` upon construction, which 
segfaults on my system.  Therefor, all of the `nullptr` checks need to be 
hoisted into the callers or stated another way exist at the boundary of `char*` 
to `std::string_view` API boundaries (such as right here).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151003/new/

https://reviews.llvm.org/D151003

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D151003: [Damangle] convert dlangDemangle to use std::string_view

2023-06-02 Thread Nick Desaulniers via Phabricator via lldb-commits
nickdesaulniers marked an inline comment as done.
nickdesaulniers added inline comments.



Comment at: llvm/lib/Demangle/Demangle.cpp:49
 bool llvm::nonMicrosoftDemangle(const char *MangledName, std::string &Result) {
+  if (!MangledName)
+return false;

nickdesaulniers wrote:
> MaskRay wrote:
> > Why is this change? The original contract is that `MangledName` must be 
> > non-null.
> https://fosstodon.org/@llvm/110397650834821908
> 
> It's insidious; implicitly constructing a `std::string_view` from a `char*` 
> which is `nullptr` invokes a call to `strlen` upon construction, which 
> segfaults on my system.  Therefor, all of the `nullptr` checks need to be 
> hoisted into the callers or stated another way exist at the boundary of 
> `char*` to `std::string_view` API boundaries (such as right here).
This is also why the `nullptr` input test case must be removed from 
llvm/unittests/Demangle/DLangDemangleTest.cpp below.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151003/new/

https://reviews.llvm.org/D151003

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D151003: [Damangle] convert dlangDemangle to use std::string_view

2023-06-02 Thread Nick Desaulniers via Phabricator via lldb-commits
nickdesaulniers updated this revision to Diff 527900.
nickdesaulniers added a comment.

- rebase for presubmits


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151003/new/

https://reviews.llvm.org/D151003

Files:
  lldb/source/Core/Mangled.cpp
  llvm/include/llvm/Demangle/Demangle.h
  llvm/lib/Demangle/DLangDemangle.cpp
  llvm/lib/Demangle/Demangle.cpp
  llvm/tools/llvm-dlang-demangle-fuzzer/llvm-dlang-demangle-fuzzer.cpp
  llvm/unittests/Demangle/DLangDemangleTest.cpp

Index: llvm/unittests/Demangle/DLangDemangleTest.cpp
===
--- llvm/unittests/Demangle/DLangDemangleTest.cpp
+++ llvm/unittests/Demangle/DLangDemangleTest.cpp
@@ -11,10 +11,11 @@
 #include "gtest/gtest.h"
 
 #include 
+#include 
 #include 
 
 struct DLangDemangleTestFixture
-: public testing::TestWithParam> {
+: public testing::TestWithParam> {
   char *Demangled;
 
   void SetUp() override { Demangled = llvm::dlangDemangle(GetParam().first); }
@@ -29,9 +30,8 @@
 INSTANTIATE_TEST_SUITE_P(
 DLangDemangleTest, DLangDemangleTestFixture,
 testing::Values(
-std::make_pair("_Dmain", "D main"), std::make_pair(nullptr, nullptr),
-std::make_pair("_Z", nullptr), std::make_pair("_DDD", nullptr),
-std::make_pair("_D88", nullptr),
+std::make_pair("_Dmain", "D main"), std::make_pair("_Z", nullptr),
+std::make_pair("_DDD", nullptr), std::make_pair("_D88", nullptr),
 std::make_pair("_D8demangleZ", "demangle"),
 std::make_pair("_D8demangle4testZ", "demangle.test"),
 std::make_pair("_D8demangle4test5test2Z", "demangle.test.test2"),
Index: llvm/tools/llvm-dlang-demangle-fuzzer/llvm-dlang-demangle-fuzzer.cpp
===
--- llvm/tools/llvm-dlang-demangle-fuzzer/llvm-dlang-demangle-fuzzer.cpp
+++ llvm/tools/llvm-dlang-demangle-fuzzer/llvm-dlang-demangle-fuzzer.cpp
@@ -13,7 +13,7 @@
 
 extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) {
   std::string NullTerminatedString((const char *)Data, Size);
-  char *Demangled = llvm::dlangDemangle(NullTerminatedString.c_str());
+  char *Demangled = llvm::dlangDemangle(NullTerminatedString);
   std::free(Demangled);
   return 0;
 }
Index: llvm/lib/Demangle/Demangle.cpp
===
--- llvm/lib/Demangle/Demangle.cpp
+++ llvm/lib/Demangle/Demangle.cpp
@@ -46,6 +46,9 @@
 }
 
 bool llvm::nonMicrosoftDemangle(const char *MangledName, std::string &Result) {
+  if (!MangledName)
+return false;
+
   char *Demangled = nullptr;
   if (isItaniumEncoding(MangledName))
 Demangled = itaniumDemangle(MangledName);
Index: llvm/lib/Demangle/DLangDemangle.cpp
===
--- llvm/lib/Demangle/DLangDemangle.cpp
+++ llvm/lib/Demangle/DLangDemangle.cpp
@@ -14,6 +14,7 @@
 //===--===//
 
 #include "llvm/Demangle/Demangle.h"
+#include "llvm/Demangle/StringViewExtras.h"
 #include "llvm/Demangle/Utility.h"
 
 #include 
@@ -22,6 +23,7 @@
 
 using namespace llvm;
 using llvm::itanium_demangle::OutputBuffer;
+using llvm::itanium_demangle::starts_with;
 
 namespace {
 
@@ -541,20 +543,20 @@
   return parseMangle(Demangled, this->Str);
 }
 
-char *llvm::dlangDemangle(const char *MangledName) {
-  if (MangledName == nullptr || strncmp(MangledName, "_D", 2) != 0)
+char *llvm::dlangDemangle(std::string_view MangledName) {
+  if (MangledName.empty() || !starts_with(MangledName, "_D"))
 return nullptr;
 
   OutputBuffer Demangled;
-  if (strcmp(MangledName, "_Dmain") == 0) {
+  if (MangledName == "_Dmain") {
 Demangled << "D main";
   } else {
 
-Demangler D = Demangler(MangledName);
-MangledName = D.parseMangle(&Demangled);
+Demangler D(MangledName.data());
+const char *M = D.parseMangle(&Demangled);
 
 // Check that the entire symbol was successfully demangled.
-if (MangledName == nullptr || *MangledName != '\0') {
+if (M == nullptr || *M != '\0') {
   std::free(Demangled.getBuffer());
   return nullptr;
 }
Index: llvm/include/llvm/Demangle/Demangle.h
===
--- llvm/include/llvm/Demangle/Demangle.h
+++ llvm/include/llvm/Demangle/Demangle.h
@@ -58,7 +58,7 @@
 char *rustDemangle(std::string_view MangledName);
 
 // Demangles a D mangled symbol.
-char *dlangDemangle(const char *MangledName);
+char *dlangDemangle(std::string_view MangledName);
 
 /// Attempt to demangle a string using different demangling schemes.
 /// The function uses heuristics to determine which demangling scheme to use.
Index: lldb/source/Core/Mangled.cpp
===
--- lldb/source/Core/Mangled.cpp
+++ lldb/source/Core/Mangled.cpp
@@ -164,7 +164,7 @@
   return demangled_cstr;
 

[Lldb-commits] [PATCH] D152010: [lldb][NFCI] ConstString methods should take StringRefs by value

2023-06-02 Thread Alex Langford via Phabricator via lldb-commits
bulbazord created this revision.
bulbazord added reviewers: fdeazeve, kastiglione, mib, jasonmolenda.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

StringRef was made to be passed by value efficiently.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152010

Files:
  lldb/include/lldb/Utility/ConstString.h
  lldb/source/Utility/ConstString.cpp


Index: lldb/source/Utility/ConstString.cpp
===
--- lldb/source/Utility/ConstString.cpp
+++ lldb/source/Utility/ConstString.cpp
@@ -98,7 +98,7 @@
 return nullptr;
   }
 
-  const char *GetConstCStringWithStringRef(const llvm::StringRef &string_ref) {
+  const char *GetConstCStringWithStringRef(const llvm::StringRef string_ref) {
 if (string_ref.data()) {
   const uint8_t h = hash(string_ref);
 
@@ -171,7 +171,7 @@
   }
 
 protected:
-  uint8_t hash(const llvm::StringRef &s) const {
+  uint8_t hash(const llvm::StringRef s) const {
 uint32_t h = llvm::djbHash(s);
 return ((h >> 24) ^ (h >> 16) ^ (h >> 8) ^ h) & 0xff;
   }
@@ -208,7 +208,7 @@
 ConstString::ConstString(const char *cstr, size_t cstr_len)
 : m_string(StringPool().GetConstCStringWithLength(cstr, cstr_len)) {}
 
-ConstString::ConstString(const llvm::StringRef &s)
+ConstString::ConstString(const llvm::StringRef s)
 : m_string(StringPool().GetConstCStringWithStringRef(s)) {}
 
 bool ConstString::operator<(ConstString rhs) const {
@@ -302,8 +302,8 @@
   m_string = StringPool().GetConstCString(cstr);
 }
 
-void ConstString::SetString(const llvm::StringRef &s) {
-  m_string = StringPool().GetConstCStringWithLength(s.data(), s.size());
+void ConstString::SetString(const llvm::StringRef s) {
+  m_string = StringPool().GetConstCStringWithStringRef(s);
 }
 
 void ConstString::SetStringWithMangledCounterpart(llvm::StringRef demangled,
Index: lldb/include/lldb/Utility/ConstString.h
===
--- lldb/include/lldb/Utility/ConstString.h
+++ lldb/include/lldb/Utility/ConstString.h
@@ -43,7 +43,7 @@
   /// Initializes the string to an empty string.
   ConstString() = default;
 
-  explicit ConstString(const llvm::StringRef &s);
+  explicit ConstString(const llvm::StringRef s);
 
   /// Construct with C String value
   ///
@@ -325,7 +325,7 @@
   /// A NULL terminated C string to add to the string pool.
   void SetCString(const char *cstr);
 
-  void SetString(const llvm::StringRef &s);
+  void SetString(const llvm::StringRef s);
 
   /// Set the C string value and its mangled counterpart.
   ///


Index: lldb/source/Utility/ConstString.cpp
===
--- lldb/source/Utility/ConstString.cpp
+++ lldb/source/Utility/ConstString.cpp
@@ -98,7 +98,7 @@
 return nullptr;
   }
 
-  const char *GetConstCStringWithStringRef(const llvm::StringRef &string_ref) {
+  const char *GetConstCStringWithStringRef(const llvm::StringRef string_ref) {
 if (string_ref.data()) {
   const uint8_t h = hash(string_ref);
 
@@ -171,7 +171,7 @@
   }
 
 protected:
-  uint8_t hash(const llvm::StringRef &s) const {
+  uint8_t hash(const llvm::StringRef s) const {
 uint32_t h = llvm::djbHash(s);
 return ((h >> 24) ^ (h >> 16) ^ (h >> 8) ^ h) & 0xff;
   }
@@ -208,7 +208,7 @@
 ConstString::ConstString(const char *cstr, size_t cstr_len)
 : m_string(StringPool().GetConstCStringWithLength(cstr, cstr_len)) {}
 
-ConstString::ConstString(const llvm::StringRef &s)
+ConstString::ConstString(const llvm::StringRef s)
 : m_string(StringPool().GetConstCStringWithStringRef(s)) {}
 
 bool ConstString::operator<(ConstString rhs) const {
@@ -302,8 +302,8 @@
   m_string = StringPool().GetConstCString(cstr);
 }
 
-void ConstString::SetString(const llvm::StringRef &s) {
-  m_string = StringPool().GetConstCStringWithLength(s.data(), s.size());
+void ConstString::SetString(const llvm::StringRef s) {
+  m_string = StringPool().GetConstCStringWithStringRef(s);
 }
 
 void ConstString::SetStringWithMangledCounterpart(llvm::StringRef demangled,
Index: lldb/include/lldb/Utility/ConstString.h
===
--- lldb/include/lldb/Utility/ConstString.h
+++ lldb/include/lldb/Utility/ConstString.h
@@ -43,7 +43,7 @@
   /// Initializes the string to an empty string.
   ConstString() = default;
 
-  explicit ConstString(const llvm::StringRef &s);
+  explicit ConstString(const llvm::StringRef s);
 
   /// Construct with C String value
   ///
@@ -325,7 +325,7 @@
   /// A NULL terminated C string to add to the string pool.
   void SetCString(const char *cstr);
 
-  void SetString(const llvm::StringRef &s);
+  void SetString(const llvm::StringRef s);
 
   /// Set the C string value and its mangled counterpart.
   ///
___
lldb-commits mailing list
l

[Lldb-commits] [PATCH] D152010: [lldb][NFCI] ConstString methods should take StringRefs by value

2023-06-02 Thread Dave Lee via Phabricator via lldb-commits
kastiglione added inline comments.
Herald added a subscriber: JDevlieghere.



Comment at: lldb/source/Utility/ConstString.cpp:305
 
-void ConstString::SetString(const llvm::StringRef &s) {
-  m_string = StringPool().GetConstCStringWithLength(s.data(), s.size());
+void ConstString::SetString(const llvm::StringRef s) {
+  m_string = StringPool().GetConstCStringWithStringRef(s);

I think the `const` can be removed too, since `StringRef`s are immutable.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152010/new/

https://reviews.llvm.org/D152010

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152010: [lldb][NFCI] ConstString methods should take StringRefs by value

2023-06-02 Thread Alex Langford via Phabricator via lldb-commits
bulbazord added inline comments.



Comment at: lldb/source/Utility/ConstString.cpp:305
 
-void ConstString::SetString(const llvm::StringRef &s) {
-  m_string = StringPool().GetConstCStringWithLength(s.data(), s.size());
+void ConstString::SetString(const llvm::StringRef s) {
+  m_string = StringPool().GetConstCStringWithStringRef(s);

kastiglione wrote:
> I think the `const` can be removed too, since `StringRef`s are immutable.
Yeah, I can remove `const` and that should be fine too. But `StringRef` objects 
aren't immutable, you can modify them with methods like `consume_front` and 
`consume_back`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152010/new/

https://reviews.llvm.org/D152010

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152010: [lldb][NFCI] ConstString methods should take StringRefs by value

2023-06-02 Thread Dave Lee via Phabricator via lldb-commits
kastiglione added inline comments.



Comment at: lldb/source/Utility/ConstString.cpp:305
 
-void ConstString::SetString(const llvm::StringRef &s) {
-  m_string = StringPool().GetConstCStringWithLength(s.data(), s.size());
+void ConstString::SetString(const llvm::StringRef s) {
+  m_string = StringPool().GetConstCStringWithStringRef(s);

bulbazord wrote:
> kastiglione wrote:
> > I think the `const` can be removed too, since `StringRef`s are immutable.
> Yeah, I can remove `const` and that should be fine too. But `StringRef` 
> objects aren't immutable, you can modify them with methods like 
> `consume_front` and `consume_back`.
thanks for pointing that out, I was thinking about the underlying `char *`, 
which isn't mutable. Now that the `StringRef` is passed by copy, then the 
caller's copy can't be modified.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152010/new/

https://reviews.llvm.org/D152010

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152011: [lldb/Commands] Add support to auto-completion for user commands

2023-06-02 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib created this revision.
mib added reviewers: bulbazord, jingham, JDevlieghere.
mib added a project: LLDB.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch should allow the user to set specific auto-completion type
for their custom commands.

To do so, we had to hoist the `CompletionType` enum so the user can
access it and add a new completion type flag to the CommandScriptAdd
Command Object.

So now, the user can specify which completion type will be used with
their custom command, when they register it.

This also makes the `crashlog` custom commands use disk-file completion
type, to browse through the user file system and load the report.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152011

Files:
  lldb/docs/python_api_enums.rst
  lldb/examples/python/crashlog.py
  lldb/include/lldb/Interpreter/CommandCompletions.h
  lldb/include/lldb/Interpreter/CommandObject.h
  lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h
  lldb/include/lldb/Interpreter/OptionValueFileColonLine.h
  lldb/include/lldb/Interpreter/OptionValueFileSpec.h
  lldb/include/lldb/Utility/OptionDefinition.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/Commands/CommandCompletions.cpp
  lldb/source/Commands/CommandObjectBreakpoint.cpp
  lldb/source/Commands/CommandObjectCommands.cpp
  lldb/source/Commands/CommandObjectDWIMPrint.cpp
  lldb/source/Commands/CommandObjectFrame.cpp
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Commands/CommandObjectPlugin.cpp
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/CommandObjectRegexCommand.cpp
  lldb/source/Commands/CommandObjectRegister.cpp
  lldb/source/Commands/CommandObjectSession.cpp
  lldb/source/Commands/CommandObjectSettings.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/CommandObjectTrace.cpp
  lldb/source/Commands/CommandObjectType.cpp
  lldb/source/Commands/CommandObjectWatchpoint.cpp
  lldb/source/Commands/Options.td
  lldb/source/Core/IOHandler.cpp
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Interpreter/OptionValueArch.cpp
  lldb/source/Interpreter/OptionValueFileColonLine.cpp
  lldb/source/Interpreter/OptionValueFileSpec.cpp
  lldb/source/Interpreter/Options.cpp
  lldb/utils/TableGen/LLDBOptionDefEmitter.cpp

Index: lldb/utils/TableGen/LLDBOptionDefEmitter.cpp
===
--- lldb/utils/TableGen/LLDBOptionDefEmitter.cpp
+++ lldb/utils/TableGen/LLDBOptionDefEmitter.cpp
@@ -120,12 +120,11 @@
   if (!O.Completions.empty()) {
 std::vector CompletionArgs;
 for (llvm::StringRef Completion : O.Completions)
-  CompletionArgs.push_back("CommandCompletions::e" + Completion.str() +
-   "Completion");
+  CompletionArgs.push_back("e" + Completion.str() + "Completion");
 
 OS << llvm::join(CompletionArgs.begin(), CompletionArgs.end(), " | ");
   } else
-OS << "CommandCompletions::eNoCompletion";
+OS << "CompletionType::eNoCompletion";
 
   // Add the argument type.
   OS << ", eArgType";
Index: lldb/source/Interpreter/Options.cpp
===
--- lldb/source/Interpreter/Options.cpp
+++ lldb/source/Interpreter/Options.cpp
@@ -714,8 +714,8 @@
 }
   }
 
-  if (completion_mask & CommandCompletions::eSourceFileCompletion ||
-  completion_mask & CommandCompletions::eSymbolCompletion) {
+  if (completion_mask & lldb::eSourceFileCompletion ||
+  completion_mask & lldb::eSymbolCompletion) {
 for (size_t i = 0; i < opt_element_vector.size(); i++) {
   int cur_defs_index = opt_element_vector[i].opt_defs_index;
 
@@ -748,7 +748,7 @@
 }
   }
 
-  CommandCompletions::InvokeCommonCompletionCallbacks(
+  lldb_private::CommandCompletions::InvokeCommonCompletionCallbacks(
   interpreter, completion_mask, request, filter_up.get());
 }
 
Index: lldb/source/Interpreter/OptionValueFileSpec.cpp
===
--- lldb/source/Interpreter/OptionValueFileSpec.cpp
+++ lldb/source/Interpreter/OptionValueFileSpec.cpp
@@ -84,7 +84,7 @@
 
 void OptionValueFileSpec::AutoComplete(CommandInterpreter &interpreter,
CompletionRequest &request) {
-  CommandCompletions::InvokeCommonCompletionCallbacks(
+  lldb_private::CommandCompletions::InvokeCommonCompletionCallbacks(
   interpreter, m_completion_mask, request, nullptr);
 }
 
Index: lldb/source/Interpreter/OptionValueFileColonLine.cpp
===
--- lldb/source/Interpreter/OptionValueFileColonLine.cpp
+++ lldb/source/Interpreter/OptionValueFileColonLine.cpp
@@ -132,6 +132,6 @@
 
 void OptionValueFileColonLine::AutoComplete(Comma

[Lldb-commits] [PATCH] D152012: [lldb/crashlog] Expand crash report file path before parsing

2023-06-02 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib created this revision.
mib added reviewers: bulbazord, kastiglione.
mib added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch should fix a crash in the opening a crash report that was
passed with a relative path.

This patch expands the crash report path before parsing it and raises a
`FileNotFoundError` exception if the file doesn't exist.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152012

Files:
  lldb/examples/python/crashlog.py


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -1346,13 +1346,7 @@
 print(error)
 
 
-def load_crashlog_in_scripted_process(debugger, crash_log_file, options, 
result):
-crashlog_path = os.path.expanduser(crash_log_file)
-if not os.path.exists(crashlog_path):
-raise InteractiveCrashLogException(
-"crashlog file %s does not exist" % crashlog_path
-)
-
+def load_crashlog_in_scripted_process(debugger, crashlog_path, options, 
result):
 crashlog = CrashLogParser.create(debugger, crashlog_path, False).parse()
 
 target = lldb.SBTarget()
@@ -1641,17 +1635,22 @@
 ci = debugger.GetCommandInterpreter()
 
 if args:
-for crash_log_file in args:
+for crashlog_file in args:
+crashlog_path = os.path.expanduser(crashlog_file)
+if not os.path.exists(crashlog_path):
+raise FileNotFoundError(
+"crashlog file %s does not exist" % crashlog_path
+)
 if should_run_in_interactive_mode(options, ci):
 try:
 load_crashlog_in_scripted_process(
-debugger, crash_log_file, options, result
+debugger, crashlog_path, options, result
 )
 except InteractiveCrashLogException as e:
 result.SetError(str(e))
 else:
 crash_log = CrashLogParser.create(
-debugger, crash_log_file, options.verbose
+debugger, crashlog_path, options.verbose
 ).parse()
 SymbolicateCrashLog(crash_log, options)
 


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -1346,13 +1346,7 @@
 print(error)
 
 
-def load_crashlog_in_scripted_process(debugger, crash_log_file, options, result):
-crashlog_path = os.path.expanduser(crash_log_file)
-if not os.path.exists(crashlog_path):
-raise InteractiveCrashLogException(
-"crashlog file %s does not exist" % crashlog_path
-)
-
+def load_crashlog_in_scripted_process(debugger, crashlog_path, options, result):
 crashlog = CrashLogParser.create(debugger, crashlog_path, False).parse()
 
 target = lldb.SBTarget()
@@ -1641,17 +1635,22 @@
 ci = debugger.GetCommandInterpreter()
 
 if args:
-for crash_log_file in args:
+for crashlog_file in args:
+crashlog_path = os.path.expanduser(crashlog_file)
+if not os.path.exists(crashlog_path):
+raise FileNotFoundError(
+"crashlog file %s does not exist" % crashlog_path
+)
 if should_run_in_interactive_mode(options, ci):
 try:
 load_crashlog_in_scripted_process(
-debugger, crash_log_file, options, result
+debugger, crashlog_path, options, result
 )
 except InteractiveCrashLogException as e:
 result.SetError(str(e))
 else:
 crash_log = CrashLogParser.create(
-debugger, crash_log_file, options.verbose
+debugger, crashlog_path, options.verbose
 ).parse()
 SymbolicateCrashLog(crash_log, options)
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152013: [lldb/Commands] Fix disk completion from root directory

2023-06-02 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib created this revision.
mib added reviewers: JDevlieghere, bulbazord, kastiglione.
mib added a project: LLDB.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch should fix path completion starting from the root directory.

To do so, this patch adds a special case when setting the search
directory when the completion buffer points to the root directory.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152013

Files:
  lldb/source/Commands/CommandCompletions.cpp


Index: lldb/source/Commands/CommandCompletions.cpp
===
--- lldb/source/Commands/CommandCompletions.cpp
+++ lldb/source/Commands/CommandCompletions.cpp
@@ -381,6 +381,8 @@
   Storage.append(RemainderDir);
 }
 SearchDir = Storage;
+  } else if (CompletionBuffer == path::root_directory(CompletionBuffer)) {
+SearchDir = CompletionBuffer;
   } else {
 SearchDir = path::parent_path(CompletionBuffer);
   }
@@ -390,9 +392,11 @@
   PartialItem = path::filename(CompletionBuffer);
 
   // path::filename() will return "." when the passed path ends with a
-  // directory separator. We have to filter those out, but only when the
-  // "." doesn't come from the completion request itself.
-  if (PartialItem == "." && path::is_separator(CompletionBuffer.back()))
+  // directory separator or the separator when passed the disk root directory.
+  // We have to filter those out, but only when the "." doesn't come from the
+  // completion request itself.
+  if ((PartialItem == "." || PartialItem == path::get_separator()) &&
+  path::is_separator(CompletionBuffer.back()))
 PartialItem = llvm::StringRef();
 
   if (SearchDir.empty()) {


Index: lldb/source/Commands/CommandCompletions.cpp
===
--- lldb/source/Commands/CommandCompletions.cpp
+++ lldb/source/Commands/CommandCompletions.cpp
@@ -381,6 +381,8 @@
   Storage.append(RemainderDir);
 }
 SearchDir = Storage;
+  } else if (CompletionBuffer == path::root_directory(CompletionBuffer)) {
+SearchDir = CompletionBuffer;
   } else {
 SearchDir = path::parent_path(CompletionBuffer);
   }
@@ -390,9 +392,11 @@
   PartialItem = path::filename(CompletionBuffer);
 
   // path::filename() will return "." when the passed path ends with a
-  // directory separator. We have to filter those out, but only when the
-  // "." doesn't come from the completion request itself.
-  if (PartialItem == "." && path::is_separator(CompletionBuffer.back()))
+  // directory separator or the separator when passed the disk root directory.
+  // We have to filter those out, but only when the "." doesn't come from the
+  // completion request itself.
+  if ((PartialItem == "." || PartialItem == path::get_separator()) &&
+  path::is_separator(CompletionBuffer.back()))
 PartialItem = llvm::StringRef();
 
   if (SearchDir.empty()) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152010: [lldb][NFCI] ConstString methods should take StringRefs by value

2023-06-02 Thread Alex Langford via Phabricator via lldb-commits
bulbazord updated this revision to Diff 527908.
bulbazord added a comment.

Remove `const` where unneeded.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152010/new/

https://reviews.llvm.org/D152010

Files:
  lldb/include/lldb/Utility/ConstString.h
  lldb/source/Utility/ConstString.cpp


Index: lldb/source/Utility/ConstString.cpp
===
--- lldb/source/Utility/ConstString.cpp
+++ lldb/source/Utility/ConstString.cpp
@@ -98,7 +98,7 @@
 return nullptr;
   }
 
-  const char *GetConstCStringWithStringRef(const llvm::StringRef &string_ref) {
+  const char *GetConstCStringWithStringRef(llvm::StringRef string_ref) {
 if (string_ref.data()) {
   const uint8_t h = hash(string_ref);
 
@@ -171,7 +171,7 @@
   }
 
 protected:
-  uint8_t hash(const llvm::StringRef &s) const {
+  uint8_t hash(llvm::StringRef s) const {
 uint32_t h = llvm::djbHash(s);
 return ((h >> 24) ^ (h >> 16) ^ (h >> 8) ^ h) & 0xff;
   }
@@ -208,7 +208,7 @@
 ConstString::ConstString(const char *cstr, size_t cstr_len)
 : m_string(StringPool().GetConstCStringWithLength(cstr, cstr_len)) {}
 
-ConstString::ConstString(const llvm::StringRef &s)
+ConstString::ConstString(llvm::StringRef s)
 : m_string(StringPool().GetConstCStringWithStringRef(s)) {}
 
 bool ConstString::operator<(ConstString rhs) const {
@@ -302,8 +302,8 @@
   m_string = StringPool().GetConstCString(cstr);
 }
 
-void ConstString::SetString(const llvm::StringRef &s) {
-  m_string = StringPool().GetConstCStringWithLength(s.data(), s.size());
+void ConstString::SetString(llvm::StringRef s) {
+  m_string = StringPool().GetConstCStringWithStringRef(s);
 }
 
 void ConstString::SetStringWithMangledCounterpart(llvm::StringRef demangled,
Index: lldb/include/lldb/Utility/ConstString.h
===
--- lldb/include/lldb/Utility/ConstString.h
+++ lldb/include/lldb/Utility/ConstString.h
@@ -43,7 +43,7 @@
   /// Initializes the string to an empty string.
   ConstString() = default;
 
-  explicit ConstString(const llvm::StringRef &s);
+  explicit ConstString(llvm::StringRef s);
 
   /// Construct with C String value
   ///
@@ -325,7 +325,7 @@
   /// A NULL terminated C string to add to the string pool.
   void SetCString(const char *cstr);
 
-  void SetString(const llvm::StringRef &s);
+  void SetString(llvm::StringRef s);
 
   /// Set the C string value and its mangled counterpart.
   ///


Index: lldb/source/Utility/ConstString.cpp
===
--- lldb/source/Utility/ConstString.cpp
+++ lldb/source/Utility/ConstString.cpp
@@ -98,7 +98,7 @@
 return nullptr;
   }
 
-  const char *GetConstCStringWithStringRef(const llvm::StringRef &string_ref) {
+  const char *GetConstCStringWithStringRef(llvm::StringRef string_ref) {
 if (string_ref.data()) {
   const uint8_t h = hash(string_ref);
 
@@ -171,7 +171,7 @@
   }
 
 protected:
-  uint8_t hash(const llvm::StringRef &s) const {
+  uint8_t hash(llvm::StringRef s) const {
 uint32_t h = llvm::djbHash(s);
 return ((h >> 24) ^ (h >> 16) ^ (h >> 8) ^ h) & 0xff;
   }
@@ -208,7 +208,7 @@
 ConstString::ConstString(const char *cstr, size_t cstr_len)
 : m_string(StringPool().GetConstCStringWithLength(cstr, cstr_len)) {}
 
-ConstString::ConstString(const llvm::StringRef &s)
+ConstString::ConstString(llvm::StringRef s)
 : m_string(StringPool().GetConstCStringWithStringRef(s)) {}
 
 bool ConstString::operator<(ConstString rhs) const {
@@ -302,8 +302,8 @@
   m_string = StringPool().GetConstCString(cstr);
 }
 
-void ConstString::SetString(const llvm::StringRef &s) {
-  m_string = StringPool().GetConstCStringWithLength(s.data(), s.size());
+void ConstString::SetString(llvm::StringRef s) {
+  m_string = StringPool().GetConstCStringWithStringRef(s);
 }
 
 void ConstString::SetStringWithMangledCounterpart(llvm::StringRef demangled,
Index: lldb/include/lldb/Utility/ConstString.h
===
--- lldb/include/lldb/Utility/ConstString.h
+++ lldb/include/lldb/Utility/ConstString.h
@@ -43,7 +43,7 @@
   /// Initializes the string to an empty string.
   ConstString() = default;
 
-  explicit ConstString(const llvm::StringRef &s);
+  explicit ConstString(llvm::StringRef s);
 
   /// Construct with C String value
   ///
@@ -325,7 +325,7 @@
   /// A NULL terminated C string to add to the string pool.
   void SetCString(const char *cstr);
 
-  void SetString(const llvm::StringRef &s);
+  void SetString(llvm::StringRef s);
 
   /// Set the C string value and its mangled counterpart.
   ///
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D151859: [lldb/Target] Add ability to set a label to targets

2023-06-02 Thread Alex Langford via Phabricator via lldb-commits
bulbazord accepted this revision.
bulbazord added a comment.
This revision is now accepted and ready to land.

LGTM!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151859/new/

https://reviews.llvm.org/D151859

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152013: [lldb/Commands] Fix disk completion from root directory

2023-06-02 Thread Alex Langford via Phabricator via lldb-commits
bulbazord added a comment.

The change itself looks ok to me. Could you add a test for this? We do have 
some tests to test for completion so the machinery is there, but it's mostly 
for the expression evaluator right now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152013/new/

https://reviews.llvm.org/D152013

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152012: [lldb/crashlog] Expand crash report file path before parsing

2023-06-02 Thread Alex Langford via Phabricator via lldb-commits
bulbazord accepted this revision.
bulbazord added a comment.
This revision is now accepted and ready to land.

The key here is that you're expanding the path and checking for the validity 
before calling `CrashLogParser.create()` right? It looks like for the 
`load_crashlog_in_scripted_process` case nothing has changed. If my 
understanding is correct, then this looks good to me.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152012/new/

https://reviews.llvm.org/D152012

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152011: [lldb/Commands] Add support to auto-completion for user commands

2023-06-02 Thread Alex Langford via Phabricator via lldb-commits
bulbazord added a comment.

So the idea behind this is fine to me, allowing custom commands to specify a 
completion style seems like a very nifty feature to have.

We should definitely add a test though. An API test or a Shell test where we 
create a custom command and then try to do some kind of completion on it would 
be good.




Comment at: lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h:153-179
+{lldb::eNoCompletion, "no-completion", nullptr},
+{lldb::eSourceFileCompletion, "source-file", nullptr},
+{lldb::eDiskFileCompletion, "disk-file", nullptr},
+{lldb::eDiskDirectoryCompletion, "disk-directory", nullptr},
+{lldb::eSymbolCompletion, "symbol", nullptr},
+{lldb::eModuleCompletion, "module", nullptr},
+{lldb::eSettingsNameCompletion, "settings-name", nullptr},

The `nullptr` argument for these is for the `usage`, yeah? We should probably 
add something... For example, `eSourceFileCompletion` could be something like 
"Completes to a source file" or something like this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152011/new/

https://reviews.llvm.org/D152011

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D151919: [lldb][NFCI] Apply IndexEntry to DWARFUnitHeader outside of extraction

2023-06-02 Thread Alex Langford via Phabricator via lldb-commits
bulbazord added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:882
+  // we have a valid one to set it to.
+  assert(index_entry);
+  assert(!m_index_entry);

fdeazeve wrote:
> If LLVM's function is like this as well, we shouldn't do anything now and 
> just refactor it later once we make the switch. But a pointer parameter in a 
> function that asserts not null immediately is more expressive as a reference. 
> Note that, in the code where this is called, we have a:
> 
> ```
> if (ptr)
>   ApplyIndex(ptr)
> ```
> 
> So the assert is not needed, as the callee respects the contract and can do a 
> `*ptr`
> 
Yes, this was my thought process as well. I wrote it this way because LLVM's 
function is just like this. I'm also touching that equivalent function in LLVM 
in a different way (adding error handling, see 
https://reviews.llvm.org/D151933). Let's rework this in a follow-up?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151919/new/

https://reviews.llvm.org/D151919

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152012: [lldb/crashlog] Expand crash report file path before parsing

2023-06-02 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib added a comment.

In D152012#4391336 , @bulbazord wrote:

> The key here is that you're expanding the path and checking for the validity 
> before calling `CrashLogParser.create()` right? It looks like for the 
> `load_crashlog_in_scripted_process` case nothing has changed. If my 
> understanding is correct, then this looks good to me.

Yep, I had to fixed this in the past for interactive mode in a07a751 
 but not 
for the regular crashlog command. This patch refactors it to cover both cases.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152012/new/

https://reviews.llvm.org/D152012

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D151003: [Damangle] convert dlangDemangle to use std::string_view

2023-06-02 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments.



Comment at: llvm/lib/Demangle/Demangle.cpp:49
 bool llvm::nonMicrosoftDemangle(const char *MangledName, std::string &Result) {
+  if (!MangledName)
+return false;

nickdesaulniers wrote:
> nickdesaulniers wrote:
> > MaskRay wrote:
> > > Why is this change? The original contract is that `MangledName` must be 
> > > non-null.
> > https://fosstodon.org/@llvm/110397650834821908
> > 
> > It's insidious; implicitly constructing a `std::string_view` from a `char*` 
> > which is `nullptr` invokes a call to `strlen` upon construction, which 
> > segfaults on my system.  Therefor, all of the `nullptr` checks need to be 
> > hoisted into the callers or stated another way exist at the boundary of 
> > `char*` to `std::string_view` API boundaries (such as right here).
> This is also why the `nullptr` input test case must be removed from 
> llvm/unittests/Demangle/DLangDemangleTest.cpp below.
It's usually code smell if a function taking a C string argument additionally 
accepts nullptr. 
Previously, passing `nullptr` to `nonMicrosoftDemangle` will crash as 
`MangledName[0]` is accessed. We should retain this strictness.

`ninja check-llvm-demangle` passes if I remove the 2 lines.

If future refactoring will possibly pass `nullptr` to `nonMicrosoftDemangle`, I 
think we should fix those call sites not to do that...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151003/new/

https://reviews.llvm.org/D151003

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D149784: [Damangle] convert rustDemangle to use std::string_view

2023-06-02 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments.



Comment at: llvm/lib/Demangle/RustDemangle.cpp:150
-char *llvm::rustDemangle(const char *MangledName) {
-  if (MangledName == nullptr)
-return nullptr;

I see that this patch drops `if (MangledName == nullptr)` (D101444). This is a 
right direction. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149784/new/

https://reviews.llvm.org/D149784

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D151966: [lldb] Default can_create to true in GetChildMemberWithName (NFC)

2023-06-02 Thread Dave Lee via Phabricator via lldb-commits
kastiglione added a comment.

correction: there's still an overload of `GetChildAtNamePath`, which takes a 
can_create value and passes it through to `GetChildMemberWithName`. However 
that function isn't used, and could be deleted. To delete the parameter, that 
overload of `GetChildMemberWithName` would need to be deleted too. In general, 
I'm still in favor of the conservative choice of giving it a default. Deleting 
it altogether is an easy follow up, if so desired.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151966/new/

https://reviews.llvm.org/D151966

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152031: [lldb] Default can_create to true in GetChildAtIndex (NFC)

2023-06-02 Thread Dave Lee via Phabricator via lldb-commits
kastiglione created this revision.
kastiglione added reviewers: bulbazord, jingham.
Herald added a subscriber: arphaman.
Herald added a project: All.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Existing callers of `GetChildAtIndex` pass true for can_create. This change
makes true the default value, callers don't have to pass an opaque true.

See also D151966  for the same change to 
`GetChildMemberWithName`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152031

Files:
  lldb/include/lldb/Core/ValueObject.h
  lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
  lldb/source/API/SBValue.cpp
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Core/ValueObject.cpp
  lldb/source/Core/ValueObjectSyntheticFilter.cpp
  lldb/source/DataFormatters/FormatManager.cpp
  lldb/source/DataFormatters/ValueObjectPrinter.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
  lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
  lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
  lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
  lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxQueue.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
  lldb/source/Plugins/Language/ObjC/Cocoa.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
  lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp
  lldb/source/Target/StackFrame.cpp

Index: lldb/source/Target/StackFrame.cpp
===
--- lldb/source/Target/StackFrame.cpp
+++ lldb/source/Target/StackFrame.cpp
@@ -814,7 +814,7 @@
   // extract bit low out of it. reading array item low would be done by
   // saying arr[low], without a deref * sign
   Status error;
-  ValueObjectSP temp(valobj_sp->GetChildAtIndex(0, true));
+  ValueObjectSP temp(valobj_sp->GetChildAtIndex(0));
   if (error.Fail()) {
 valobj_sp->GetExpressionPath(var_expr_path_strm);
 error.SetErrorStringWithFormat(
@@ -868,7 +868,7 @@
   valobj_sp->GetTypeName().AsCString(""),
   var_expr_path_strm.GetData());
 } else {
-  child_valobj_sp = synthetic->GetChildAtIndex(child_index, true);
+  child_valobj_sp = synthetic->GetChildAtIndex(child_index);
   if (!child_valobj_sp) {
 valobj_sp->GetExpressionPath(var_expr_path_strm);
 error.SetErrorStringWithFormat(
@@ -894,7 +894,7 @@
nullptr, nullptr, &is_incomplete_array)) {
   // Pass false to dynamic_value here so we can tell the difference
   // between no dynamic value and no member of this type...
-  child_valobj_sp = valobj_sp->GetChildAtIndex(child_index, true);
+  child_valobj_sp = valobj_sp->GetChildAtIndex(child_index);
   if (!child_valobj_sp && (is_incomplete_array || !no_synth_child))
 child_valobj_sp =
 valobj_sp->GetSyntheticArrayMember(child_index, true);
@@ -940,7 +940,7 @@
 valobj_sp->GetTypeName().AsCString(""),
 var_expr_path_strm.GetData());
   } else {
-child_valobj_sp = synthetic->GetChildAtIndex(child_index, true);
+child_valobj_sp = synthetic->GetChildAtIndex(child_index);
 if (!child_valobj_sp) {
   valobj_sp->GetExpressionPath(var_expr_path_strm);
   error.SetErrorStringWithFormat(
@@ -1012,7 +1012,7 @@
 // extract bits low thru high out of it. reading array items low thru
 // high would be done by saying arr[low-high], without a deref * sign
 Status error;
-ValueObjectSP temp(valobj_sp->GetChildAtIndex(0, true));
+ValueObjectSP temp(valobj_sp->GetChildAtIndex(0));
 if (error.Fail()) {
   valobj_sp->GetExpressionPath(var_expr_path_strm);
   error.SetErrorStringWithFormat(
@@ -1400,8 +1400,7 @@
   }
 
   for (int ci = 0, ce = parent->GetNumChildren(); ci != ce; ++ci) {
-const bool can_create = true;
-ValueObjectSP child_sp = parent->GetChildAtIndex(ci, can_create);
+ValueObjectSP child_sp = parent->GetChildAtIndex(ci);
 
 if (!child_sp) {
   return ValueObjectSP();
Index: lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp
=

[Lldb-commits] [PATCH] D151966: [lldb] Default can_create to true in GetChildMemberWithName (NFC)

2023-06-02 Thread Dave Lee via Phabricator via lldb-commits
kastiglione added a comment.

To expand the conversation, I have also opened D152031 
 which makes the same change to 
`GetChildAtIndex`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151966/new/

https://reviews.llvm.org/D151966

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152010: [lldb][NFCI] ConstString methods should take StringRefs by value

2023-06-02 Thread Felipe de Azevedo Piovezan via Phabricator via lldb-commits
fdeazeve accepted this revision.
fdeazeve added a comment.

LGTM, thanks for picking this up!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152010/new/

https://reviews.llvm.org/D152010

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D151003: [Damangle] convert dlangDemangle to use std::string_view

2023-06-02 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

If `nonMicrosoftDemangle` is going to be changed to take a `string_view` 
argument, adding `if (!MangledName) return false;` should be fine.
If possible, it's best for the change to be deferred to a later patch on the 
stack, but I won't push strongly on this. You did the work...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151003/new/

https://reviews.llvm.org/D151003

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D151003: [Damangle] convert dlangDemangle to use std::string_view

2023-06-02 Thread Nick Desaulniers via Phabricator via lldb-commits
nickdesaulniers added inline comments.



Comment at: llvm/lib/Demangle/Demangle.cpp:49
 bool llvm::nonMicrosoftDemangle(const char *MangledName, std::string &Result) {
+  if (!MangledName)
+return false;

MaskRay wrote:
> nickdesaulniers wrote:
> > nickdesaulniers wrote:
> > > MaskRay wrote:
> > > > Why is this change? The original contract is that `MangledName` must be 
> > > > non-null.
> > > https://fosstodon.org/@llvm/110397650834821908
> > > 
> > > It's insidious; implicitly constructing a `std::string_view` from a 
> > > `char*` which is `nullptr` invokes a call to `strlen` upon construction, 
> > > which segfaults on my system.  Therefor, all of the `nullptr` checks need 
> > > to be hoisted into the callers or stated another way exist at the 
> > > boundary of `char*` to `std::string_view` API boundaries (such as right 
> > > here).
> > This is also why the `nullptr` input test case must be removed from 
> > llvm/unittests/Demangle/DLangDemangleTest.cpp below.
> It's usually code smell if a function taking a C string argument additionally 
> accepts nullptr. 
> Previously, passing `nullptr` to `nonMicrosoftDemangle` will crash as 
> `MangledName[0]` is accessed. We should retain this strictness.
> 
> `ninja check-llvm-demangle` passes if I remove the 2 lines.
> 
> If future refactoring will possibly pass `nullptr` to `nonMicrosoftDemangle`, 
> I think we should fix those call sites not to do that...
>  We should retain this strictness.
> If future refactoring will possibly pass nullptr to nonMicrosoftDemangle, I 
> think we should fix those call sites not to do that...

Ok, I will drop it then, and add nullptr checks to callers.

> ninja check-llvm-demangle passes if I remove the 2 lines.

Then `check-llvm-demangle` must not be running 
llvm/unittests/Demangle/DLangDemangleTest.cpp, or you tested with my change to 
llvm/unittests/Demangle/DLangDemangleTest.cpp from below applied.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151003/new/

https://reviews.llvm.org/D151003

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D149784: [Damangle] convert rustDemangle to use std::string_view

2023-06-02 Thread Nick Desaulniers via Phabricator via lldb-commits
nickdesaulniers updated this revision to Diff 528006.
nickdesaulniers added a comment.

- rebase on top of D149675  (I wasn't sure 
which would land first)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149784/new/

https://reviews.llvm.org/D149784

Files:
  lldb/include/lldb/Utility/ConstString.h
  lldb/source/Core/Mangled.cpp
  llvm/include/llvm/Demangle/Demangle.h
  llvm/lib/Demangle/Demangle.cpp
  llvm/lib/Demangle/RustDemangle.cpp
  llvm/tools/llvm-rust-demangle-fuzzer/llvm-rust-demangle-fuzzer.cpp

Index: llvm/tools/llvm-rust-demangle-fuzzer/llvm-rust-demangle-fuzzer.cpp
===
--- llvm/tools/llvm-rust-demangle-fuzzer/llvm-rust-demangle-fuzzer.cpp
+++ llvm/tools/llvm-rust-demangle-fuzzer/llvm-rust-demangle-fuzzer.cpp
@@ -13,7 +13,7 @@
 
 extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) {
   std::string NullTerminatedString((const char *)Data, Size);
-  char *Demangled = llvm::rustDemangle(NullTerminatedString.c_str());
+  char *Demangled = llvm::rustDemangle(NullTerminatedString);
   std::free(Demangled);
   return 0;
 }
Index: llvm/lib/Demangle/RustDemangle.cpp
===
--- llvm/lib/Demangle/RustDemangle.cpp
+++ llvm/lib/Demangle/RustDemangle.cpp
@@ -20,11 +20,13 @@
 #include 
 #include 
 #include 
+#include 
 
 using namespace llvm;
 
 using llvm::itanium_demangle::OutputBuffer;
 using llvm::itanium_demangle::ScopedOverride;
+using llvm::itanium_demangle::starts_with;
 
 namespace {
 
@@ -146,17 +148,13 @@
 
 } // namespace
 
-char *llvm::rustDemangle(const char *MangledName) {
-  if (MangledName == nullptr)
-return nullptr;
-
+char *llvm::rustDemangle(std::string_view MangledName) {
   // Return early if mangled name doesn't look like a Rust symbol.
-  std::string_view Mangled(MangledName);
-  if (!llvm::itanium_demangle::starts_with(Mangled, "_R"))
+  if (MangledName.empty() || !starts_with(MangledName, "_R"))
 return nullptr;
 
   Demangler D;
-  if (!D.demangle(Mangled)) {
+  if (!D.demangle(MangledName)) {
 std::free(D.Output.getBuffer());
 return nullptr;
   }
@@ -196,7 +194,7 @@
   RecursionLevel = 0;
   BoundLifetimes = 0;
 
-  if (!llvm::itanium_demangle::starts_with(Mangled, "_R")) {
+  if (!starts_with(Mangled, "_R")) {
 Error = true;
 return false;
   }
Index: llvm/lib/Demangle/Demangle.cpp
===
--- llvm/lib/Demangle/Demangle.cpp
+++ llvm/lib/Demangle/Demangle.cpp
@@ -51,7 +51,7 @@
   if (isItaniumEncoding(MangledName.data()))
 Demangled = itaniumDemangle(MangledName);
   else if (isRustEncoding(MangledName.data()))
-Demangled = rustDemangle(MangledName.data());
+Demangled = rustDemangle(MangledName);
   else if (isDLangEncoding(MangledName.data()))
 Demangled = dlangDemangle(MangledName.data());
 
Index: llvm/include/llvm/Demangle/Demangle.h
===
--- llvm/include/llvm/Demangle/Demangle.h
+++ llvm/include/llvm/Demangle/Demangle.h
@@ -11,6 +11,7 @@
 
 #include 
 #include 
+#include 
 
 namespace llvm {
 /// This is a llvm local version of __cxa_demangle. Other than the name and
@@ -54,7 +55,7 @@
 MSDemangleFlags Flags = MSDF_None);
 
 // Demangles a Rust v0 mangled symbol.
-char *rustDemangle(const char *MangledName);
+char *rustDemangle(std::string_view MangledName);
 
 // Demangles a D mangled symbol.
 char *dlangDemangle(const char *MangledName);
Index: lldb/source/Core/Mangled.cpp
===
--- lldb/source/Core/Mangled.cpp
+++ lldb/source/Core/Mangled.cpp
@@ -25,6 +25,7 @@
 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -150,7 +151,7 @@
   return demangled_cstr;
 }
 
-static char *GetRustV0DemangledStr(const char *M) {
+static char *GetRustV0DemangledStr(std::string_view M) {
   char *demangled_cstr = llvm::rustDemangle(M);
 
   if (Log *log = GetLog(LLDBLog::Demangle)) {
@@ -259,7 +260,7 @@
 break;
   }
   case eManglingSchemeRustV0:
-demangled_name = GetRustV0DemangledStr(mangled_name);
+demangled_name = GetRustV0DemangledStr(m_mangled);
 break;
   case eManglingSchemeD:
 demangled_name = GetDLangDemangledStr(mangled_name);
Index: lldb/include/lldb/Utility/ConstString.h
===
--- lldb/include/lldb/Utility/ConstString.h
+++ lldb/include/lldb/Utility/ConstString.h
@@ -14,6 +14,7 @@
 #include "llvm/Support/FormatVariadic.h"
 
 #include 
+#include 
 
 namespace lldb_private {
 class Stream;
@@ -182,6 +183,8 @@
 
   // Implicitly convert \class ConstString instances to \class StringRef.
   operator llvm::StringRef() const { return GetStringRef(); }
+  // Implicitly convert \class ConstString insta

[Lldb-commits] [lldb] 12d967c - [Damangle] convert rustDemangle to use std::string_view

2023-06-02 Thread Nick Desaulniers via lldb-commits

Author: Nick Desaulniers
Date: 2023-06-02T15:08:14-07:00
New Revision: 12d967c95f1633bebd1b225ddd53573951a7ca43

URL: 
https://github.com/llvm/llvm-project/commit/12d967c95f1633bebd1b225ddd53573951a7ca43
DIFF: 
https://github.com/llvm/llvm-project/commit/12d967c95f1633bebd1b225ddd53573951a7ca43.diff

LOG: [Damangle] convert rustDemangle to use std::string_view

I was doing this API conversion to use std::string_view top-down in
D149104, but this exposed issues in individual demanglers that needed to
get fixed first. There's no issue with the conversion for the Rust
demangler, so convert it first.

Reviewed By: MaskRay

Differential Revision: https://reviews.llvm.org/D149784

Added: 


Modified: 
lldb/include/lldb/Utility/ConstString.h
lldb/source/Core/Mangled.cpp
llvm/include/llvm/Demangle/Demangle.h
llvm/lib/Demangle/Demangle.cpp
llvm/lib/Demangle/RustDemangle.cpp
llvm/tools/llvm-rust-demangle-fuzzer/llvm-rust-demangle-fuzzer.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/ConstString.h 
b/lldb/include/lldb/Utility/ConstString.h
index a4b959b14f15a..332cca5fdbf12 100644
--- a/lldb/include/lldb/Utility/ConstString.h
+++ b/lldb/include/lldb/Utility/ConstString.h
@@ -14,6 +14,7 @@
 #include "llvm/Support/FormatVariadic.h"
 
 #include 
+#include 
 
 namespace lldb_private {
 class Stream;
@@ -182,6 +183,8 @@ class ConstString {
 
   // Implicitly convert \class ConstString instances to \class StringRef.
   operator llvm::StringRef() const { return GetStringRef(); }
+  // Implicitly convert \class ConstString instances to \calss 
std::string_view.
+  operator std::string_view() const { return std::string_view(m_string, 
GetLength()); }
 
   /// Get the string value as a C string.
   ///

diff  --git a/lldb/source/Core/Mangled.cpp b/lldb/source/Core/Mangled.cpp
index 30c8e1a3330d2..bf9014daf8a26 100644
--- a/lldb/source/Core/Mangled.cpp
+++ b/lldb/source/Core/Mangled.cpp
@@ -25,6 +25,7 @@
 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -150,7 +151,7 @@ static char *GetItaniumDemangledStr(const char *M) {
   return demangled_cstr;
 }
 
-static char *GetRustV0DemangledStr(const char *M) {
+static char *GetRustV0DemangledStr(std::string_view M) {
   char *demangled_cstr = llvm::rustDemangle(M);
 
   if (Log *log = GetLog(LLDBLog::Demangle)) {
@@ -259,7 +260,7 @@ ConstString Mangled::GetDemangledName() const {
 break;
   }
   case eManglingSchemeRustV0:
-demangled_name = GetRustV0DemangledStr(mangled_name);
+demangled_name = GetRustV0DemangledStr(m_mangled);
 break;
   case eManglingSchemeD:
 demangled_name = GetDLangDemangledStr(mangled_name);

diff  --git a/llvm/include/llvm/Demangle/Demangle.h 
b/llvm/include/llvm/Demangle/Demangle.h
index 1fd286bfad4c0..9ba96c093eaca 100644
--- a/llvm/include/llvm/Demangle/Demangle.h
+++ b/llvm/include/llvm/Demangle/Demangle.h
@@ -11,6 +11,7 @@
 
 #include 
 #include 
+#include 
 
 namespace llvm {
 /// This is a llvm local version of __cxa_demangle. Other than the name and
@@ -54,7 +55,7 @@ char *microsoftDemangle(const char *mangled_name, size_t 
*n_read, int *status,
 MSDemangleFlags Flags = MSDF_None);
 
 // Demangles a Rust v0 mangled symbol.
-char *rustDemangle(const char *MangledName);
+char *rustDemangle(std::string_view MangledName);
 
 // Demangles a D mangled symbol.
 char *dlangDemangle(const char *MangledName);

diff  --git a/llvm/lib/Demangle/Demangle.cpp b/llvm/lib/Demangle/Demangle.cpp
index f83117f3b7e05..4627ca822f676 100644
--- a/llvm/lib/Demangle/Demangle.cpp
+++ b/llvm/lib/Demangle/Demangle.cpp
@@ -51,7 +51,7 @@ bool llvm::nonMicrosoftDemangle(std::string_view MangledName,
   if (isItaniumEncoding(MangledName.data()))
 Demangled = itaniumDemangle(MangledName);
   else if (isRustEncoding(MangledName.data()))
-Demangled = rustDemangle(MangledName.data());
+Demangled = rustDemangle(MangledName);
   else if (isDLangEncoding(MangledName.data()))
 Demangled = dlangDemangle(MangledName.data());
 

diff  --git a/llvm/lib/Demangle/RustDemangle.cpp 
b/llvm/lib/Demangle/RustDemangle.cpp
index 697673f0564b7..f0d70de3abb53 100644
--- a/llvm/lib/Demangle/RustDemangle.cpp
+++ b/llvm/lib/Demangle/RustDemangle.cpp
@@ -20,11 +20,13 @@
 #include 
 #include 
 #include 
+#include 
 
 using namespace llvm;
 
 using llvm::itanium_demangle::OutputBuffer;
 using llvm::itanium_demangle::ScopedOverride;
+using llvm::itanium_demangle::starts_with;
 
 namespace {
 
@@ -146,17 +148,13 @@ class Demangler {
 
 } // namespace
 
-char *llvm::rustDemangle(const char *MangledName) {
-  if (MangledName == nullptr)
-return nullptr;
-
+char *llvm::rustDemangle(std::string_view MangledName) {
   // Return early if mangled name doesn't look like a Rust symbol.
-  std::string_view Mangled(MangledName);
-  if (!llvm::itanium_demangle::starts_with(Mangled, "_R"))
+  if (

[Lldb-commits] [PATCH] D149784: [Damangle] convert rustDemangle to use std::string_view

2023-06-02 Thread Nick Desaulniers via Phabricator via lldb-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG12d967c95f16: [Damangle] convert rustDemangle to use 
std::string_view (authored by nickdesaulniers).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149784/new/

https://reviews.llvm.org/D149784

Files:
  lldb/include/lldb/Utility/ConstString.h
  lldb/source/Core/Mangled.cpp
  llvm/include/llvm/Demangle/Demangle.h
  llvm/lib/Demangle/Demangle.cpp
  llvm/lib/Demangle/RustDemangle.cpp
  llvm/tools/llvm-rust-demangle-fuzzer/llvm-rust-demangle-fuzzer.cpp

Index: llvm/tools/llvm-rust-demangle-fuzzer/llvm-rust-demangle-fuzzer.cpp
===
--- llvm/tools/llvm-rust-demangle-fuzzer/llvm-rust-demangle-fuzzer.cpp
+++ llvm/tools/llvm-rust-demangle-fuzzer/llvm-rust-demangle-fuzzer.cpp
@@ -13,7 +13,7 @@
 
 extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) {
   std::string NullTerminatedString((const char *)Data, Size);
-  char *Demangled = llvm::rustDemangle(NullTerminatedString.c_str());
+  char *Demangled = llvm::rustDemangle(NullTerminatedString);
   std::free(Demangled);
   return 0;
 }
Index: llvm/lib/Demangle/RustDemangle.cpp
===
--- llvm/lib/Demangle/RustDemangle.cpp
+++ llvm/lib/Demangle/RustDemangle.cpp
@@ -20,11 +20,13 @@
 #include 
 #include 
 #include 
+#include 
 
 using namespace llvm;
 
 using llvm::itanium_demangle::OutputBuffer;
 using llvm::itanium_demangle::ScopedOverride;
+using llvm::itanium_demangle::starts_with;
 
 namespace {
 
@@ -146,17 +148,13 @@
 
 } // namespace
 
-char *llvm::rustDemangle(const char *MangledName) {
-  if (MangledName == nullptr)
-return nullptr;
-
+char *llvm::rustDemangle(std::string_view MangledName) {
   // Return early if mangled name doesn't look like a Rust symbol.
-  std::string_view Mangled(MangledName);
-  if (!llvm::itanium_demangle::starts_with(Mangled, "_R"))
+  if (MangledName.empty() || !starts_with(MangledName, "_R"))
 return nullptr;
 
   Demangler D;
-  if (!D.demangle(Mangled)) {
+  if (!D.demangle(MangledName)) {
 std::free(D.Output.getBuffer());
 return nullptr;
   }
@@ -196,7 +194,7 @@
   RecursionLevel = 0;
   BoundLifetimes = 0;
 
-  if (!llvm::itanium_demangle::starts_with(Mangled, "_R")) {
+  if (!starts_with(Mangled, "_R")) {
 Error = true;
 return false;
   }
Index: llvm/lib/Demangle/Demangle.cpp
===
--- llvm/lib/Demangle/Demangle.cpp
+++ llvm/lib/Demangle/Demangle.cpp
@@ -51,7 +51,7 @@
   if (isItaniumEncoding(MangledName.data()))
 Demangled = itaniumDemangle(MangledName);
   else if (isRustEncoding(MangledName.data()))
-Demangled = rustDemangle(MangledName.data());
+Demangled = rustDemangle(MangledName);
   else if (isDLangEncoding(MangledName.data()))
 Demangled = dlangDemangle(MangledName.data());
 
Index: llvm/include/llvm/Demangle/Demangle.h
===
--- llvm/include/llvm/Demangle/Demangle.h
+++ llvm/include/llvm/Demangle/Demangle.h
@@ -11,6 +11,7 @@
 
 #include 
 #include 
+#include 
 
 namespace llvm {
 /// This is a llvm local version of __cxa_demangle. Other than the name and
@@ -54,7 +55,7 @@
 MSDemangleFlags Flags = MSDF_None);
 
 // Demangles a Rust v0 mangled symbol.
-char *rustDemangle(const char *MangledName);
+char *rustDemangle(std::string_view MangledName);
 
 // Demangles a D mangled symbol.
 char *dlangDemangle(const char *MangledName);
Index: lldb/source/Core/Mangled.cpp
===
--- lldb/source/Core/Mangled.cpp
+++ lldb/source/Core/Mangled.cpp
@@ -25,6 +25,7 @@
 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -150,7 +151,7 @@
   return demangled_cstr;
 }
 
-static char *GetRustV0DemangledStr(const char *M) {
+static char *GetRustV0DemangledStr(std::string_view M) {
   char *demangled_cstr = llvm::rustDemangle(M);
 
   if (Log *log = GetLog(LLDBLog::Demangle)) {
@@ -259,7 +260,7 @@
 break;
   }
   case eManglingSchemeRustV0:
-demangled_name = GetRustV0DemangledStr(mangled_name);
+demangled_name = GetRustV0DemangledStr(m_mangled);
 break;
   case eManglingSchemeD:
 demangled_name = GetDLangDemangledStr(mangled_name);
Index: lldb/include/lldb/Utility/ConstString.h
===
--- lldb/include/lldb/Utility/ConstString.h
+++ lldb/include/lldb/Utility/ConstString.h
@@ -14,6 +14,7 @@
 #include "llvm/Support/FormatVariadic.h"
 
 #include 
+#include 
 
 namespace lldb_private {
 class Stream;
@@ -182,6 +183,8 @@
 
   // Implicitly convert \class ConstString instances to \class StringRef.
   operator llvm::StringRef() const { return GetStri

[Lldb-commits] [PATCH] D151003: [Damangle] convert dlangDemangle to use std::string_view

2023-06-02 Thread Nick Desaulniers via Phabricator via lldb-commits
nickdesaulniers updated this revision to Diff 528011.
nickdesaulniers added a comment.
This revision is now accepted and ready to land.

- rebase on D149675 , remove nullptr check

No, I don't want no nullptr-check
A nullptr-check is a guard that can't get no love from me
Hangin' out the passenger side
Of their best friend's ride
Trying to holla at me


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151003/new/

https://reviews.llvm.org/D151003

Files:
  lldb/source/Core/Mangled.cpp
  llvm/include/llvm/Demangle/Demangle.h
  llvm/lib/Demangle/DLangDemangle.cpp
  llvm/lib/Demangle/Demangle.cpp
  llvm/tools/llvm-dlang-demangle-fuzzer/llvm-dlang-demangle-fuzzer.cpp
  llvm/unittests/Demangle/DLangDemangleTest.cpp

Index: llvm/unittests/Demangle/DLangDemangleTest.cpp
===
--- llvm/unittests/Demangle/DLangDemangleTest.cpp
+++ llvm/unittests/Demangle/DLangDemangleTest.cpp
@@ -11,10 +11,11 @@
 #include "gtest/gtest.h"
 
 #include 
+#include 
 #include 
 
 struct DLangDemangleTestFixture
-: public testing::TestWithParam> {
+: public testing::TestWithParam> {
   char *Demangled;
 
   void SetUp() override { Demangled = llvm::dlangDemangle(GetParam().first); }
@@ -29,9 +30,8 @@
 INSTANTIATE_TEST_SUITE_P(
 DLangDemangleTest, DLangDemangleTestFixture,
 testing::Values(
-std::make_pair("_Dmain", "D main"), std::make_pair(nullptr, nullptr),
-std::make_pair("_Z", nullptr), std::make_pair("_DDD", nullptr),
-std::make_pair("_D88", nullptr),
+std::make_pair("_Dmain", "D main"), std::make_pair("_Z", nullptr),
+std::make_pair("_DDD", nullptr), std::make_pair("_D88", nullptr),
 std::make_pair("_D8demangleZ", "demangle"),
 std::make_pair("_D8demangle4testZ", "demangle.test"),
 std::make_pair("_D8demangle4test5test2Z", "demangle.test.test2"),
Index: llvm/tools/llvm-dlang-demangle-fuzzer/llvm-dlang-demangle-fuzzer.cpp
===
--- llvm/tools/llvm-dlang-demangle-fuzzer/llvm-dlang-demangle-fuzzer.cpp
+++ llvm/tools/llvm-dlang-demangle-fuzzer/llvm-dlang-demangle-fuzzer.cpp
@@ -13,7 +13,7 @@
 
 extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) {
   std::string NullTerminatedString((const char *)Data, Size);
-  char *Demangled = llvm::dlangDemangle(NullTerminatedString.c_str());
+  char *Demangled = llvm::dlangDemangle(NullTerminatedString);
   std::free(Demangled);
   return 0;
 }
Index: llvm/lib/Demangle/Demangle.cpp
===
--- llvm/lib/Demangle/Demangle.cpp
+++ llvm/lib/Demangle/Demangle.cpp
@@ -53,7 +53,7 @@
   else if (isRustEncoding(MangledName.data()))
 Demangled = rustDemangle(MangledName);
   else if (isDLangEncoding(MangledName.data()))
-Demangled = dlangDemangle(MangledName.data());
+Demangled = dlangDemangle(MangledName);
 
   if (!Demangled)
 return false;
Index: llvm/lib/Demangle/DLangDemangle.cpp
===
--- llvm/lib/Demangle/DLangDemangle.cpp
+++ llvm/lib/Demangle/DLangDemangle.cpp
@@ -14,6 +14,7 @@
 //===--===//
 
 #include "llvm/Demangle/Demangle.h"
+#include "llvm/Demangle/StringViewExtras.h"
 #include "llvm/Demangle/Utility.h"
 
 #include 
@@ -22,6 +23,7 @@
 
 using namespace llvm;
 using llvm::itanium_demangle::OutputBuffer;
+using llvm::itanium_demangle::starts_with;
 
 namespace {
 
@@ -541,20 +543,20 @@
   return parseMangle(Demangled, this->Str);
 }
 
-char *llvm::dlangDemangle(const char *MangledName) {
-  if (MangledName == nullptr || strncmp(MangledName, "_D", 2) != 0)
+char *llvm::dlangDemangle(std::string_view MangledName) {
+  if (MangledName.empty() || !starts_with(MangledName, "_D"))
 return nullptr;
 
   OutputBuffer Demangled;
-  if (strcmp(MangledName, "_Dmain") == 0) {
+  if (MangledName == "_Dmain") {
 Demangled << "D main";
   } else {
 
-Demangler D = Demangler(MangledName);
-MangledName = D.parseMangle(&Demangled);
+Demangler D(MangledName.data());
+const char *M = D.parseMangle(&Demangled);
 
 // Check that the entire symbol was successfully demangled.
-if (MangledName == nullptr || *MangledName != '\0') {
+if (M == nullptr || *M != '\0') {
   std::free(Demangled.getBuffer());
   return nullptr;
 }
Index: llvm/include/llvm/Demangle/Demangle.h
===
--- llvm/include/llvm/Demangle/Demangle.h
+++ llvm/include/llvm/Demangle/Demangle.h
@@ -58,7 +58,7 @@
 char *rustDemangle(std::string_view MangledName);
 
 // Demangles a D mangled symbol.
-char *dlangDemangle(const char *MangledName);
+char *dlangDemangle(std::string_view MangledName);
 
 /// Attempt to demangle a string using different demang

[Lldb-commits] [lldb] f5371eb - [Damangle] convert dlangDemangle to use std::string_view

2023-06-02 Thread Nick Desaulniers via lldb-commits

Author: Nick Desaulniers
Date: 2023-06-02T15:19:41-07:00
New Revision: f5371eb3d3aed06ba84a69533586a60243ad2f24

URL: 
https://github.com/llvm/llvm-project/commit/f5371eb3d3aed06ba84a69533586a60243ad2f24
DIFF: 
https://github.com/llvm/llvm-project/commit/f5371eb3d3aed06ba84a69533586a60243ad2f24.diff

LOG: [Damangle] convert dlangDemangle to use std::string_view

I was doing this API conversion to use std::string_view top-down in
D149104, but this exposed issues in individual demanglers that needed to
get fixed first. There's no issue with the conversion for the D language
demangler, so convert it.

I have a more aggressive refactoring of the entire D language demangler
to use std::string_view more extensively, but the interface with
llvm::nonMicrosoftDemangle is the more interesting one.

Reviewed By: MaskRay

Differential Revision: https://reviews.llvm.org/D151003

Added: 


Modified: 
lldb/source/Core/Mangled.cpp
llvm/include/llvm/Demangle/Demangle.h
llvm/lib/Demangle/DLangDemangle.cpp
llvm/lib/Demangle/Demangle.cpp
llvm/tools/llvm-dlang-demangle-fuzzer/llvm-dlang-demangle-fuzzer.cpp
llvm/unittests/Demangle/DLangDemangleTest.cpp

Removed: 




diff  --git a/lldb/source/Core/Mangled.cpp b/lldb/source/Core/Mangled.cpp
index bf9014daf8a26..3294b246ae743 100644
--- a/lldb/source/Core/Mangled.cpp
+++ b/lldb/source/Core/Mangled.cpp
@@ -164,7 +164,7 @@ static char *GetRustV0DemangledStr(std::string_view M) {
   return demangled_cstr;
 }
 
-static char *GetDLangDemangledStr(const char *M) {
+static char *GetDLangDemangledStr(std::string_view M) {
   char *demangled_cstr = llvm::dlangDemangle(M);
 
   if (Log *log = GetLog(LLDBLog::Demangle)) {
@@ -263,7 +263,7 @@ ConstString Mangled::GetDemangledName() const {
 demangled_name = GetRustV0DemangledStr(m_mangled);
 break;
   case eManglingSchemeD:
-demangled_name = GetDLangDemangledStr(mangled_name);
+demangled_name = GetDLangDemangledStr(m_mangled);
 break;
   case eManglingSchemeNone:
 llvm_unreachable("eManglingSchemeNone was handled already");

diff  --git a/llvm/include/llvm/Demangle/Demangle.h 
b/llvm/include/llvm/Demangle/Demangle.h
index 9ba96c093eaca..d9b830c660047 100644
--- a/llvm/include/llvm/Demangle/Demangle.h
+++ b/llvm/include/llvm/Demangle/Demangle.h
@@ -58,7 +58,7 @@ char *microsoftDemangle(const char *mangled_name, size_t 
*n_read, int *status,
 char *rustDemangle(std::string_view MangledName);
 
 // Demangles a D mangled symbol.
-char *dlangDemangle(const char *MangledName);
+char *dlangDemangle(std::string_view MangledName);
 
 /// Attempt to demangle a string using 
diff erent demangling schemes.
 /// The function uses heuristics to determine which demangling scheme to use.

diff  --git a/llvm/lib/Demangle/DLangDemangle.cpp 
b/llvm/lib/Demangle/DLangDemangle.cpp
index ad583b86946fd..8b94d40354b43 100644
--- a/llvm/lib/Demangle/DLangDemangle.cpp
+++ b/llvm/lib/Demangle/DLangDemangle.cpp
@@ -14,6 +14,7 @@
 
//===--===//
 
 #include "llvm/Demangle/Demangle.h"
+#include "llvm/Demangle/StringViewExtras.h"
 #include "llvm/Demangle/Utility.h"
 
 #include 
@@ -22,6 +23,7 @@
 
 using namespace llvm;
 using llvm::itanium_demangle::OutputBuffer;
+using llvm::itanium_demangle::starts_with;
 
 namespace {
 
@@ -541,20 +543,20 @@ const char *Demangler::parseMangle(OutputBuffer 
*Demangled) {
   return parseMangle(Demangled, this->Str);
 }
 
-char *llvm::dlangDemangle(const char *MangledName) {
-  if (MangledName == nullptr || strncmp(MangledName, "_D", 2) != 0)
+char *llvm::dlangDemangle(std::string_view MangledName) {
+  if (MangledName.empty() || !starts_with(MangledName, "_D"))
 return nullptr;
 
   OutputBuffer Demangled;
-  if (strcmp(MangledName, "_Dmain") == 0) {
+  if (MangledName == "_Dmain") {
 Demangled << "D main";
   } else {
 
-Demangler D = Demangler(MangledName);
-MangledName = D.parseMangle(&Demangled);
+Demangler D(MangledName.data());
+const char *M = D.parseMangle(&Demangled);
 
 // Check that the entire symbol was successfully demangled.
-if (MangledName == nullptr || *MangledName != '\0') {
+if (M == nullptr || *M != '\0') {
   std::free(Demangled.getBuffer());
   return nullptr;
 }

diff  --git a/llvm/lib/Demangle/Demangle.cpp b/llvm/lib/Demangle/Demangle.cpp
index 4627ca822f676..3dd4f31268f4a 100644
--- a/llvm/lib/Demangle/Demangle.cpp
+++ b/llvm/lib/Demangle/Demangle.cpp
@@ -53,7 +53,7 @@ bool llvm::nonMicrosoftDemangle(std::string_view MangledName,
   else if (isRustEncoding(MangledName.data()))
 Demangled = rustDemangle(MangledName);
   else if (isDLangEncoding(MangledName.data()))
-Demangled = dlangDemangle(MangledName.data());
+Demangled = dlangDemangle(MangledName);
 
   if (!Demangled)
 return false;

diff  --git 
a/llvm/tools/llvm

[Lldb-commits] [PATCH] D151003: [Damangle] convert dlangDemangle to use std::string_view

2023-06-02 Thread Nick Desaulniers via Phabricator via lldb-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf5371eb3d3ae: [Damangle] convert dlangDemangle to use 
std::string_view (authored by nickdesaulniers).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151003/new/

https://reviews.llvm.org/D151003

Files:
  lldb/source/Core/Mangled.cpp
  llvm/include/llvm/Demangle/Demangle.h
  llvm/lib/Demangle/DLangDemangle.cpp
  llvm/lib/Demangle/Demangle.cpp
  llvm/tools/llvm-dlang-demangle-fuzzer/llvm-dlang-demangle-fuzzer.cpp
  llvm/unittests/Demangle/DLangDemangleTest.cpp

Index: llvm/unittests/Demangle/DLangDemangleTest.cpp
===
--- llvm/unittests/Demangle/DLangDemangleTest.cpp
+++ llvm/unittests/Demangle/DLangDemangleTest.cpp
@@ -11,10 +11,11 @@
 #include "gtest/gtest.h"
 
 #include 
+#include 
 #include 
 
 struct DLangDemangleTestFixture
-: public testing::TestWithParam> {
+: public testing::TestWithParam> {
   char *Demangled;
 
   void SetUp() override { Demangled = llvm::dlangDemangle(GetParam().first); }
@@ -29,9 +30,8 @@
 INSTANTIATE_TEST_SUITE_P(
 DLangDemangleTest, DLangDemangleTestFixture,
 testing::Values(
-std::make_pair("_Dmain", "D main"), std::make_pair(nullptr, nullptr),
-std::make_pair("_Z", nullptr), std::make_pair("_DDD", nullptr),
-std::make_pair("_D88", nullptr),
+std::make_pair("_Dmain", "D main"), std::make_pair("_Z", nullptr),
+std::make_pair("_DDD", nullptr), std::make_pair("_D88", nullptr),
 std::make_pair("_D8demangleZ", "demangle"),
 std::make_pair("_D8demangle4testZ", "demangle.test"),
 std::make_pair("_D8demangle4test5test2Z", "demangle.test.test2"),
Index: llvm/tools/llvm-dlang-demangle-fuzzer/llvm-dlang-demangle-fuzzer.cpp
===
--- llvm/tools/llvm-dlang-demangle-fuzzer/llvm-dlang-demangle-fuzzer.cpp
+++ llvm/tools/llvm-dlang-demangle-fuzzer/llvm-dlang-demangle-fuzzer.cpp
@@ -13,7 +13,7 @@
 
 extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) {
   std::string NullTerminatedString((const char *)Data, Size);
-  char *Demangled = llvm::dlangDemangle(NullTerminatedString.c_str());
+  char *Demangled = llvm::dlangDemangle(NullTerminatedString);
   std::free(Demangled);
   return 0;
 }
Index: llvm/lib/Demangle/Demangle.cpp
===
--- llvm/lib/Demangle/Demangle.cpp
+++ llvm/lib/Demangle/Demangle.cpp
@@ -53,7 +53,7 @@
   else if (isRustEncoding(MangledName.data()))
 Demangled = rustDemangle(MangledName);
   else if (isDLangEncoding(MangledName.data()))
-Demangled = dlangDemangle(MangledName.data());
+Demangled = dlangDemangle(MangledName);
 
   if (!Demangled)
 return false;
Index: llvm/lib/Demangle/DLangDemangle.cpp
===
--- llvm/lib/Demangle/DLangDemangle.cpp
+++ llvm/lib/Demangle/DLangDemangle.cpp
@@ -14,6 +14,7 @@
 //===--===//
 
 #include "llvm/Demangle/Demangle.h"
+#include "llvm/Demangle/StringViewExtras.h"
 #include "llvm/Demangle/Utility.h"
 
 #include 
@@ -22,6 +23,7 @@
 
 using namespace llvm;
 using llvm::itanium_demangle::OutputBuffer;
+using llvm::itanium_demangle::starts_with;
 
 namespace {
 
@@ -541,20 +543,20 @@
   return parseMangle(Demangled, this->Str);
 }
 
-char *llvm::dlangDemangle(const char *MangledName) {
-  if (MangledName == nullptr || strncmp(MangledName, "_D", 2) != 0)
+char *llvm::dlangDemangle(std::string_view MangledName) {
+  if (MangledName.empty() || !starts_with(MangledName, "_D"))
 return nullptr;
 
   OutputBuffer Demangled;
-  if (strcmp(MangledName, "_Dmain") == 0) {
+  if (MangledName == "_Dmain") {
 Demangled << "D main";
   } else {
 
-Demangler D = Demangler(MangledName);
-MangledName = D.parseMangle(&Demangled);
+Demangler D(MangledName.data());
+const char *M = D.parseMangle(&Demangled);
 
 // Check that the entire symbol was successfully demangled.
-if (MangledName == nullptr || *MangledName != '\0') {
+if (M == nullptr || *M != '\0') {
   std::free(Demangled.getBuffer());
   return nullptr;
 }
Index: llvm/include/llvm/Demangle/Demangle.h
===
--- llvm/include/llvm/Demangle/Demangle.h
+++ llvm/include/llvm/Demangle/Demangle.h
@@ -58,7 +58,7 @@
 char *rustDemangle(std::string_view MangledName);
 
 // Demangles a D mangled symbol.
-char *dlangDemangle(const char *MangledName);
+char *dlangDemangle(std::string_view MangledName);
 
 /// Attempt to demangle a string using different demangling schemes.
 /// The function uses heuristics to determine which demangling scheme to use.
Index: lldb/source/Core/Mangled.cpp

[Lldb-commits] [PATCH] D151570: Fix Build error on Mac M1 : error: use of undeclared identifier 'getopt_long_only'; did you mean 'getopt_long'?

2023-06-02 Thread Nicklas Boman via Phabricator via lldb-commits
smurfd abandoned this revision.
smurfd added a comment.

Hey, sorry for wasting your time with this. Should have read the error message 
more carefully.
Its related to `pkgsrc` missing `getopt_long_only()` in `getopt.h`
Checking/filing a bug towards them instead


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151570/new/

https://reviews.llvm.org/D151570

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152013: [lldb/Commands] Fix disk completion from root directory

2023-06-02 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib updated this revision to Diff 528058.
mib added a comment.

Add test


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152013/new/

https://reviews.llvm.org/D152013

Files:
  lldb/source/Commands/CommandCompletions.cpp
  lldb/test/API/functionalities/completion/TestCompletion.py


Index: lldb/test/API/functionalities/completion/TestCompletion.py
===
--- lldb/test/API/functionalities/completion/TestCompletion.py
+++ lldb/test/API/functionalities/completion/TestCompletion.py
@@ -436,6 +436,19 @@
 self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
 self.complete_from_to("target modules dump line-table main.cp", 
["main.cpp"])
 
+def test_completion_target_create_from_root_dir(self):
+"""Tests source file completion by completing ."""
+root_dir = os.path.abspath(os.sep)
+self.completions_contain(
+"target create /",
+list(
+filter(
+lambda x: os.path.exists(x),
+map(lambda x: root_dir + x + os.sep, os.listdir(root_dir)),
+)
+),
+)
+
 def test_target_modules_load_aout(self):
 """Tests modules completion by completing the target modules load 
argument."""
 self.build()
Index: lldb/source/Commands/CommandCompletions.cpp
===
--- lldb/source/Commands/CommandCompletions.cpp
+++ lldb/source/Commands/CommandCompletions.cpp
@@ -381,6 +381,8 @@
   Storage.append(RemainderDir);
 }
 SearchDir = Storage;
+  } else if (CompletionBuffer == path::root_directory(CompletionBuffer)) {
+SearchDir = CompletionBuffer;
   } else {
 SearchDir = path::parent_path(CompletionBuffer);
   }
@@ -390,9 +392,11 @@
   PartialItem = path::filename(CompletionBuffer);
 
   // path::filename() will return "." when the passed path ends with a
-  // directory separator. We have to filter those out, but only when the
-  // "." doesn't come from the completion request itself.
-  if (PartialItem == "." && path::is_separator(CompletionBuffer.back()))
+  // directory separator or the separator when passed the disk root directory.
+  // We have to filter those out, but only when the "." doesn't come from the
+  // completion request itself.
+  if ((PartialItem == "." || PartialItem == path::get_separator()) &&
+  path::is_separator(CompletionBuffer.back()))
 PartialItem = llvm::StringRef();
 
   if (SearchDir.empty()) {


Index: lldb/test/API/functionalities/completion/TestCompletion.py
===
--- lldb/test/API/functionalities/completion/TestCompletion.py
+++ lldb/test/API/functionalities/completion/TestCompletion.py
@@ -436,6 +436,19 @@
 self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
 self.complete_from_to("target modules dump line-table main.cp", ["main.cpp"])
 
+def test_completion_target_create_from_root_dir(self):
+"""Tests source file completion by completing ."""
+root_dir = os.path.abspath(os.sep)
+self.completions_contain(
+"target create /",
+list(
+filter(
+lambda x: os.path.exists(x),
+map(lambda x: root_dir + x + os.sep, os.listdir(root_dir)),
+)
+),
+)
+
 def test_target_modules_load_aout(self):
 """Tests modules completion by completing the target modules load argument."""
 self.build()
Index: lldb/source/Commands/CommandCompletions.cpp
===
--- lldb/source/Commands/CommandCompletions.cpp
+++ lldb/source/Commands/CommandCompletions.cpp
@@ -381,6 +381,8 @@
   Storage.append(RemainderDir);
 }
 SearchDir = Storage;
+  } else if (CompletionBuffer == path::root_directory(CompletionBuffer)) {
+SearchDir = CompletionBuffer;
   } else {
 SearchDir = path::parent_path(CompletionBuffer);
   }
@@ -390,9 +392,11 @@
   PartialItem = path::filename(CompletionBuffer);
 
   // path::filename() will return "." when the passed path ends with a
-  // directory separator. We have to filter those out, but only when the
-  // "." doesn't come from the completion request itself.
-  if (PartialItem == "." && path::is_separator(CompletionBuffer.back()))
+  // directory separator or the separator when passed the disk root directory.
+  // We have to filter those out, but only when the "." doesn't come from the
+  // completion request itself.
+  if ((PartialItem == "." || PartialItem == path::get_separator()) &&
+  path::is_separator(CompletionBuffer.back()))
 PartialItem = llvm::StringRef();
 
   if (SearchDir.empty()) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D152011: [lldb/Commands] Add support to auto-completion for user commands

2023-06-02 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib updated this revision to Diff 528069.
mib added a comment.

Address @bulbazord comments:

- Add test & usage descriptions


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152011/new/

https://reviews.llvm.org/D152011

Files:
  lldb/docs/python_api_enums.rst
  lldb/examples/python/crashlog.py
  lldb/include/lldb/Interpreter/CommandCompletions.h
  lldb/include/lldb/Interpreter/CommandObject.h
  lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h
  lldb/include/lldb/Interpreter/OptionValueFileColonLine.h
  lldb/include/lldb/Interpreter/OptionValueFileSpec.h
  lldb/include/lldb/Utility/OptionDefinition.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/Commands/CommandCompletions.cpp
  lldb/source/Commands/CommandObjectBreakpoint.cpp
  lldb/source/Commands/CommandObjectCommands.cpp
  lldb/source/Commands/CommandObjectDWIMPrint.cpp
  lldb/source/Commands/CommandObjectFrame.cpp
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Commands/CommandObjectPlugin.cpp
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/CommandObjectRegexCommand.cpp
  lldb/source/Commands/CommandObjectRegister.cpp
  lldb/source/Commands/CommandObjectSession.cpp
  lldb/source/Commands/CommandObjectSettings.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/CommandObjectTrace.cpp
  lldb/source/Commands/CommandObjectType.cpp
  lldb/source/Commands/CommandObjectWatchpoint.cpp
  lldb/source/Commands/Options.td
  lldb/source/Core/IOHandler.cpp
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Interpreter/OptionValueArch.cpp
  lldb/source/Interpreter/OptionValueFileColonLine.cpp
  lldb/source/Interpreter/OptionValueFileSpec.cpp
  lldb/source/Interpreter/Options.cpp
  lldb/test/API/functionalities/completion/TestCompletion.py
  lldb/test/API/functionalities/completion/my_test_cmd.py
  lldb/utils/TableGen/LLDBOptionDefEmitter.cpp

Index: lldb/utils/TableGen/LLDBOptionDefEmitter.cpp
===
--- lldb/utils/TableGen/LLDBOptionDefEmitter.cpp
+++ lldb/utils/TableGen/LLDBOptionDefEmitter.cpp
@@ -120,12 +120,11 @@
   if (!O.Completions.empty()) {
 std::vector CompletionArgs;
 for (llvm::StringRef Completion : O.Completions)
-  CompletionArgs.push_back("CommandCompletions::e" + Completion.str() +
-   "Completion");
+  CompletionArgs.push_back("e" + Completion.str() + "Completion");
 
 OS << llvm::join(CompletionArgs.begin(), CompletionArgs.end(), " | ");
   } else
-OS << "CommandCompletions::eNoCompletion";
+OS << "CompletionType::eNoCompletion";
 
   // Add the argument type.
   OS << ", eArgType";
Index: lldb/test/API/functionalities/completion/my_test_cmd.py
===
--- /dev/null
+++ lldb/test/API/functionalities/completion/my_test_cmd.py
@@ -0,0 +1,2 @@
+def my_test_cmd(debugger, command, exe_ctx, result, dict):
+result.Print(command)
Index: lldb/test/API/functionalities/completion/TestCompletion.py
===
--- lldb/test/API/functionalities/completion/TestCompletion.py
+++ lldb/test/API/functionalities/completion/TestCompletion.py
@@ -436,6 +436,47 @@
 self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
 self.complete_from_to("target modules dump line-table main.cp", ["main.cpp"])
 
+def test_custom_command_completion(self):
+"""Tests completion in custom user provided commands."""
+completion_types = [
+"none",
+"source-file",
+"disk-file",
+"disk-directory",
+"symbol",
+"module",
+"settings-name",
+"platform-plugin",
+"architecture",
+"variable-path",
+"register",
+"breakpoint",
+"process-plugin",
+"disassembly-flavor",
+"type-language",
+"frame-index",
+"module-uuid",
+"stophook-id",
+"thread-index",
+"watchpoint-id",
+"breakpoint-name",
+"process-id",
+"process-name",
+"remote-disk-file",
+"remote-disk-directory",
+"type-category-name",
+"custom",
+]
+self.completions_contain("command script add -C ", completion_types)
+
+source_path = os.path.join(self.getSourceDir(), "my_test_cmd.py")
+self.runCmd("command script import '%s'" % (source_path))
+self.runCmd(
+"command script add -C disk-file -f my_test_cmd.my_test_cmd my_test_cmd"
+)
+self.complete_from_to("my_test_cmd main.cp", ["main.cpp"])
+self.expect("my_test_cmd main.cpp", substrs=["main.cpp"])
+
 def test_target_modules_load_aout(self):

[Lldb-commits] [PATCH] D152013: [lldb/Commands] Fix disk completion from root directory

2023-06-02 Thread Alex Langford via Phabricator via lldb-commits
bulbazord added inline comments.



Comment at: lldb/test/API/functionalities/completion/TestCompletion.py:439-450
+def test_completion_target_create_from_root_dir(self):
+"""Tests source file completion by completing ."""
+root_dir = os.path.abspath(os.sep)
+self.completions_contain(
+"target create /",
+list(
+filter(

I'm curious to know if this will work for windows? I don't know how lldb treats 
`/` as a path when on a windows host.

`root_dir` should be `os.path.abspath('\\')` on windows, which might give you 
`C:\` or whatever the current drive is, so that path might possibly give you 
something? Idk, you might have to fiddle with the test to get it to work 
correctly or disable it.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152013/new/

https://reviews.llvm.org/D152013

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

2023-06-02 Thread Junchang Liu via Phabricator via lldb-commits
paperchalice added a comment.

In D141907#4361594 , @tstellar wrote:

> In D141907#4355229 , @paperchalice 
> wrote:
>
>> In D141907#4355228 , @tstellar 
>> wrote:
>>
>>> @paperchalice Do you need someone to commit this for you?
>>
>> Sure, I don't have the commit access.
>
> What name / email address do you want me to use for the commit author field?

Sorry for the late, the information in phabricator is OK, and 
`GetClangResourceDir.cmake` is not landed correctly.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141907/new/

https://reviews.llvm.org/D141907

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits