[Lldb-commits] [lldb] 898b880 - [LLDB] Update AArch64/Windows XFAIl decorators on TestNamespace.py

[Muhammad Omair Javaid via lldb-commits]

Author: Muhammad Omair Javaid
Date: 2023-05-30T13:06:09+04:00
New Revision: 898b880308f1ce31520c939ab19366dc3b82c930

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

LOG: [LLDB] Update AArch64/Windows XFAIl decorators on TestNamespace.py

Added: 


Modified: 
lldb/test/API/lang/cpp/namespace/TestNamespace.py

Removed: 




diff  --git a/lldb/test/API/lang/cpp/namespace/TestNamespace.py 
b/lldb/test/API/lang/cpp/namespace/TestNamespace.py
index 1dc9d00fcd993..3006699b6623a 100644
--- a/lldb/test/API/lang/cpp/namespace/TestNamespace.py
+++ b/lldb/test/API/lang/cpp/namespace/TestNamespace.py
@@ -11,6 +11,7 @@
 
 class NamespaceBreakpointTestCase(TestBase):
 @expectedFailureAll(bugnumber="llvm.org/pr28548", compiler="gcc")
+@expectedFailureAll(oslist=["windows"])
 def test_breakpoints_func_auto(self):
 """Test that we can set breakpoints correctly by basename to find all 
functions whose basename is "func"."""
 self.build()
@@ -37,7 +38,6 @@ def test_breakpoints_func_auto(self):
 )
 
 @expectedFailureAll(bugnumber="llvm.org/pr28548", compiler="gcc")
-@expectedFailureAll(oslist=["windows"])
 def test_breakpoints_func_full(self):
 """Test that we can set breakpoints correctly by fullname to find all 
functions whose fully qualified name is "func"
 (no namespaces)."""



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

[Lldb-commits] [PATCH] D150772: Add code snippet line numbers to TestExprDiagnostics output

[Timm Bäder via Phabricator via lldb-commits]

tbaeder updated this revision to Diff 526557.

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

https://reviews.llvm.org/D150772

Files:
  lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py

Index: lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
===
--- lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
+++ lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
@@ -29,20 +29,38 @@
 self.assertFalse(value.GetError().Success())
 # We should get a nice diagnostic with a caret pointing at the start of
 # the identifier.
-self.assertIn("\nunknown_identifier\n^\n", value.GetError().GetCString())
+self.assertIn(
+"""
+1 | unknown_identifier
+  | ^
+""",
+value.GetError().GetCString(),
+)
 self.assertIn(":1:1", value.GetError().GetCString())
 
 # Same as above but with the identifier in the middle.
-value = frame.EvaluateExpression("1 + unknown_identifier  ")
+value = frame.EvaluateExpression("1 + unknown_identifier")
 self.assertFalse(value.GetError().Success())
-self.assertIn("\n1 + unknown_identifier", value.GetError().GetCString())
-self.assertIn("\n^\n", value.GetError().GetCString())
+self.assertIn(
+"""
+1 | 1 + unknown_identifier
+  | ^
+""",
+value.GetError().GetCString(),
+)
 
 # Multiline expressions.
 value = frame.EvaluateExpression("int a = 0;\nfoobar +=1;\na")
 self.assertFalse(value.GetError().Success())
 # We should still get the right line information and caret position.
-self.assertIn("\nfoobar +=1;\n^\n", value.GetError().GetCString())
+self.assertIn(
+"""
+2 | foobar +=1;
+  | ^
+""",
+value.GetError().GetCString(),
+)
+
 # It's the second line of the user expression.
 self.assertIn(":2:1", value.GetError().GetCString())
 
@@ -52,7 +70,14 @@
 
 value = frame.EvaluateExpression("void foo(unknown_type x) {}", top_level_opts)
 self.assertFalse(value.GetError().Success())
-self.assertIn("\nvoid foo(unknown_type x) {}\n ^\n", value.GetError().GetCString())
+self.assertIn(
+"""
+1 | void foo(unknown_type x) {}
+  |  ^
+""",
+value.GetError().GetCString(),
+)
+
 # Top-level expressions might use a different wrapper code, but the file name should still
 # be the same.
 self.assertIn(":1:10", value.GetError().GetCString())
@@ -60,31 +85,79 @@
 # Multiline top-level expressions.
 value = frame.EvaluateExpression("void x() {}\nvoid foo;", top_level_opts)
 self.assertFalse(value.GetError().Success())
-self.assertIn("\nvoid foo;\n ^", value.GetError().GetCString())
+self.assertIn(
+"""
+2 | void foo;
+  |  ^
+""",
+value.GetError().GetCString(),
+)
+
 self.assertIn(":2:6", value.GetError().GetCString())
 
 # Test that we render Clang's 'notes' correctly.
 value = frame.EvaluateExpression("struct SFoo{}; struct SFoo { int x; };", top_level_opts)
 self.assertFalse(value.GetError().Success())
-self.assertIn(":1:8: previous definition is here\nstruct SFoo{}; struct SFoo { int x; };\n   ^\n", value.GetError().GetCString())
+self.assertIn(
+":1:8: previous definition is here\n",
+value.GetError().GetCString(),
+)
+self.assertIn(
+"""
+1 | struct SFoo{}; struct SFoo { int x; };
+  |^
+""",
+value.GetError().GetCString(),
+)
 
 # Declarations from the debug information currently have no debug information. It's not clear what
 # we should do in this case, but we should at least not print anything that's wrong.
 # In the future our declarations should have valid source locations.
 value = frame.EvaluateExpression("struct FooBar { double x };", top_level_opts)
 self.assertFalse(value.GetError().Success())
-self.assertIn("error: :1:8: redefinition of 'FooBar'\nstruct FooBar { double x };\n   ^\n", value.GetError().GetCString())
+self.assertIn(
+"error: :1:8: redefinition of 'FooBar'\n",
+value.GetError().GetCString(),
+)
+self.assertIn(
+"""
+1 | struct FooBar { double x };
+  |^
+""",
+value.GetError().GetCString(),
+)
 
 value = frame.EvaluateExpression("foo(1, 2)")
 self.assertFalse(value.GetError().Success())
-self.assertIn("error: :1:1: no matching function for call to 'foo'\nfoo(1, 2)\n^~~\nnote: candidate function not viable: requires single argument 'x', but 2 argumen

[Lldb-commits] [PATCH] D151683: [clang] Enable C++11-style attributes in all language modes

[Erich Keane via Phabricator via lldb-commits]

erichkeane added a comment.

What is the justification for this?  Do other compilers do this?  Was there an 
RFC?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151683

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

[Lldb-commits] [PATCH] D151603: [lldb][NFCI] Refactor Language::GetFormatterPrefixSuffix

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

modulo Felipe's comment, LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151603

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

[Lldb-commits] [PATCH] D143347: [lldb][DWARF] Infer no_unique_address attribute

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

If we can come up with a counterexample where the heuristic in this patch is 
doing the wrong thin then I think emitting a DW_AT_LLVM_no_unique_address 
attribute sounds reasonable to me. But otherwise I don't see a problem with 
using a heuristic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143347

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

[Lldb-commits] [PATCH] D151603: [lldb][NFCI] Refactor Language::GetFormatterPrefixSuffix

[Alex Langford via Phabricator via lldb-commits]

bulbazord added inline comments.



Comment at: lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp:1001
 
-bool ObjCLanguage::GetFormatterPrefixSuffix(ValueObject &valobj,
-ConstString type_hint,
-std::string &prefix,
-std::string &suffix) {
-  static ConstString g_CFBag("CFBag");
-  static ConstString g_CFBinaryHeap("CFBinaryHeap");
-
-  static ConstString g_NSNumberChar("NSNumber:char");
-  static ConstString g_NSNumberShort("NSNumber:short");
-  static ConstString g_NSNumberInt("NSNumber:int");
-  static ConstString g_NSNumberLong("NSNumber:long");
-  static ConstString g_NSNumberInt128("NSNumber:int128_t");
-  static ConstString g_NSNumberFloat("NSNumber:float");
-  static ConstString g_NSNumberDouble("NSNumber:double");
-
-  static ConstString g_NSData("NSData");
-  static ConstString g_NSArray("NSArray");
-  static ConstString g_NSString("NSString");
-  static ConstString g_NSStringStar("NSString*");
-
-  if (type_hint.IsEmpty())
-return false;
-
-  prefix.clear();
-  suffix.clear();
-
-  if (type_hint == g_CFBag || type_hint == g_CFBinaryHeap) {
-prefix = "@";
-return true;
-  }
-
-  if (type_hint == g_NSNumberChar) {
-prefix = "(char)";
-return true;
-  }
-  if (type_hint == g_NSNumberShort) {
-prefix = "(short)";
-return true;
-  }
-  if (type_hint == g_NSNumberInt) {
-prefix = "(int)";
-return true;
-  }
-  if (type_hint == g_NSNumberLong) {
-prefix = "(long)";
-return true;
-  }
-  if (type_hint == g_NSNumberInt128) {
-prefix = "(int128_t)";
-return true;
-  }
-  if (type_hint == g_NSNumberFloat) {
-prefix = "(float)";
-return true;
-  }
-  if (type_hint == g_NSNumberDouble) {
-prefix = "(double)";
-return true;
-  }
-
-  if (type_hint == g_NSData || type_hint == g_NSArray) {
-prefix = "@\"";
-suffix = "\"";
-return true;
-  }
-
-  if (type_hint == g_NSString || type_hint == g_NSStringStar) {
-prefix = "@";
-return true;
-  }
+std::pair
+ObjCLanguage::GetFormatterPrefixSuffix(llvm::StringRef type_hint) {

fdeazeve wrote:
> We can make this whole map const and remove the explicit call_once by folding 
> the `insert` calls into the ctor:
> 
> ```
> static constexpr llvm::StringRef empty;
> static const llvm::StringMap<
> std::pair>
> g_affix_map = {{"CFBag", std::make_pair("@", empty)},
>{"CFBinaryHeap", std::make_pair("@", empty)},
>..., };
> ```
> 
> If you're so inclined, you can even get rid of the final make_pair calls:
> 
> ```
> static const llvm::StringMap<
> std::pair>
> g_affix_map = {{"CFBag", {"@", empty}},
>{"CFBinaryHeap", {"@", empty}}};
> ```
Oh, this looks much nicer. I'll do it this way!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151603

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

[Lldb-commits] [PATCH] D151603: [lldb][NFCI] Refactor Language::GetFormatterPrefixSuffix

[Alex Langford via Phabricator via lldb-commits]

bulbazord updated this revision to Diff 526705.
bulbazord added a comment.

Remove use of `std::call_once` where not needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151603

Files:
  lldb/include/lldb/Target/Language.h
  lldb/source/Plugins/Language/ObjC/CF.cpp
  lldb/source/Plugins/Language/ObjC/Cocoa.cpp
  lldb/source/Plugins/Language/ObjC/NSArray.cpp
  lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
  lldb/source/Plugins/Language/ObjC/NSSet.cpp
  lldb/source/Plugins/Language/ObjC/NSString.cpp
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
  lldb/source/Target/Language.cpp

Index: lldb/source/Target/Language.cpp
===
--- lldb/source/Target/Language.cpp
+++ lldb/source/Target/Language.cpp
@@ -452,11 +452,9 @@
   return result;
 }
 
-bool Language::GetFormatterPrefixSuffix(ValueObject &valobj,
-ConstString type_hint,
-std::string &prefix,
-std::string &suffix) {
-  return false;
+std::pair
+Language::GetFormatterPrefixSuffix(llvm::StringRef type_hint) {
+  return std::pair();
 }
 
 bool Language::DemangledNameContainsPath(llvm::StringRef path, 
Index: lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
===
--- lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
+++ lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
@@ -150,9 +150,8 @@
 
   std::unique_ptr GetTypeScavenger() override;
 
-  bool GetFormatterPrefixSuffix(ValueObject &valobj, ConstString type_hint,
-std::string &prefix,
-std::string &suffix) override;
+  std::pair
+  GetFormatterPrefixSuffix(llvm::StringRef type_hint) override;
 
   bool IsNilReference(ValueObject &valobj) override;
 
Index: lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
===
--- lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
+++ lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
@@ -998,78 +998,27 @@
 ObjCDebugInfoScavenger>());
 }
 
-bool ObjCLanguage::GetFormatterPrefixSuffix(ValueObject &valobj,
-ConstString type_hint,
-std::string &prefix,
-std::string &suffix) {
-  static ConstString g_CFBag("CFBag");
-  static ConstString g_CFBinaryHeap("CFBinaryHeap");
-
-  static ConstString g_NSNumberChar("NSNumber:char");
-  static ConstString g_NSNumberShort("NSNumber:short");
-  static ConstString g_NSNumberInt("NSNumber:int");
-  static ConstString g_NSNumberLong("NSNumber:long");
-  static ConstString g_NSNumberInt128("NSNumber:int128_t");
-  static ConstString g_NSNumberFloat("NSNumber:float");
-  static ConstString g_NSNumberDouble("NSNumber:double");
-
-  static ConstString g_NSData("NSData");
-  static ConstString g_NSArray("NSArray");
-  static ConstString g_NSString("NSString");
-  static ConstString g_NSStringStar("NSString*");
-
-  if (type_hint.IsEmpty())
-return false;
-
-  prefix.clear();
-  suffix.clear();
-
-  if (type_hint == g_CFBag || type_hint == g_CFBinaryHeap) {
-prefix = "@";
-return true;
-  }
-
-  if (type_hint == g_NSNumberChar) {
-prefix = "(char)";
-return true;
-  }
-  if (type_hint == g_NSNumberShort) {
-prefix = "(short)";
-return true;
-  }
-  if (type_hint == g_NSNumberInt) {
-prefix = "(int)";
-return true;
-  }
-  if (type_hint == g_NSNumberLong) {
-prefix = "(long)";
-return true;
-  }
-  if (type_hint == g_NSNumberInt128) {
-prefix = "(int128_t)";
-return true;
-  }
-  if (type_hint == g_NSNumberFloat) {
-prefix = "(float)";
-return true;
-  }
-  if (type_hint == g_NSNumberDouble) {
-prefix = "(double)";
-return true;
-  }
-
-  if (type_hint == g_NSData || type_hint == g_NSArray) {
-prefix = "@\"";
-suffix = "\"";
-return true;
-  }
-
-  if (type_hint == g_NSString || type_hint == g_NSStringStar) {
-prefix = "@";
-return true;
-  }
-
-  return false;
+std::pair
+ObjCLanguage::GetFormatterPrefixSuffix(llvm::StringRef type_hint) {
+  static constexpr llvm::StringRef empty;
+  static const llvm::StringMap<
+  std::pair>
+  g_affix_map = {
+  {"CFBag", {"@", empty}},
+  {"CFBinaryHeap", {"@", empty}},
+  {"NSString", {"@", empty}},
+  {"NSString*", {"@", empty}},
+  {"NSNumber:char", {"(char)", empty}},
+  {"NSNumber:short", {"(short)", empty}},
+  {"NSNumber:int", {"(int)", empty}},
+  {"NSNumber:long", {"(long)", empty}},
+  {"NSNumber:int128_t", {"(int128_t)", empty}},
+  {"NSNumber:float

[Lldb-commits] [PATCH] D150772: Add code snippet line numbers to TestExprDiagnostics output

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM


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

https://reviews.llvm.org/D150772

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

[Lldb-commits] [PATCH] D151603: [lldb][NFCI] Refactor Language::GetFormatterPrefixSuffix

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151603

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

[Lldb-commits] [PATCH] D151599: [lldb][NFCI] Remove use of ConstString from StructuredDataDarwinLog static functions

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: 
lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp:1496-1500
 LLDB_LOGF(log,
   "StructuredDataDarwinLog::%s() warning: no plugin for "
   "feature %s in process uid %u",
-  __FUNCTION__, GetDarwinLogTypeName().AsCString(),
+  __FUNCTION__, GetDarwinLogTypeName().str().c_str(),
   process_sp->GetUniqueID());

You could get rid of the `.str().c_str()` if you used `LLDB_LOG`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151599

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

[Lldb-commits] [lldb] 8e0001e - [lldb][NFCI] Refactor Language::GetFormatterPrefixSuffix

[Alex Langford via lldb-commits]

Author: Alex Langford
Date: 2023-05-30T13:11:55-07:00
New Revision: 8e0001eb95ce8654660510ddb06f5a8a3c5c6d68

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

LOG: [lldb][NFCI] Refactor Language::GetFormatterPrefixSuffix

- Remove unused parameter `valobj` (I checked downstream, not
  even swift is using it).
- Return a std::pair insted of having 2 out
  parameter strings.
- Remove the use of ConstStrings.

This change was primarily mechanical except in
`ObjCLanguage::GetFormatterPrefixSuffix`. To keep this fast, we
construct an llvm::StringMap> so that we
can look things up quickly. There is some amount of cost to setting up
the map the first time it is called, but subsequent lookups should be
as fast as a hash + string comparison (the cost of looking up something
in an llvm::StringMap).

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

Added: 


Modified: 
lldb/include/lldb/Target/Language.h
lldb/source/Plugins/Language/ObjC/CF.cpp
lldb/source/Plugins/Language/ObjC/Cocoa.cpp
lldb/source/Plugins/Language/ObjC/NSArray.cpp
lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
lldb/source/Plugins/Language/ObjC/NSSet.cpp
lldb/source/Plugins/Language/ObjC/NSString.cpp
lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
lldb/source/Target/Language.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Language.h 
b/lldb/include/lldb/Target/Language.h
index bbb3e7c0cc8d1..a6b9ccaf31b3c 100644
--- a/lldb/include/lldb/Target/Language.h
+++ b/lldb/include/lldb/Target/Language.h
@@ -208,14 +208,21 @@ class Language : public PluginInterface {
   /// that the name actually belongs to this language.
   virtual bool SymbolNameFitsToLanguage(Mangled name) const { return false; }
 
-  // if an individual data formatter can apply to several types and cross a
-  // language boundary it makes sense for individual languages to want to
-  // customize the printing of values of that type by appending proper
-  // prefix/suffix information in language-specific ways
-  virtual bool GetFormatterPrefixSuffix(ValueObject &valobj,
-ConstString type_hint,
-std::string &prefix,
-std::string &suffix);
+  /// An individual data formatter may apply to several types and cross 
language
+  /// boundaries. Each of those languages may want to customize the display of
+  /// values of said types by appending proper prefix/suffix information in
+  /// language-specific ways. This function returns that prefix and suffix.
+  ///
+  /// \param[in] type_hint
+  ///   A StringRef used to determine what the prefix and suffix should be. It
+  ///   is called a hint because some types may have multiple variants for 
which
+  ///   the prefix and/or suffix may vary.
+  ///
+  /// \return
+  ///   A std::pair, the first being the prefix and the
+  ///   second being the suffix. They may be empty.
+  virtual std::pair
+  GetFormatterPrefixSuffix(llvm::StringRef type_hint);
 
   // When looking up functions, we take a user provided string which may be a
   // partial match to the full demangled name and compare it to the actual

diff  --git a/lldb/source/Plugins/Language/ObjC/CF.cpp 
b/lldb/source/Plugins/Language/ObjC/CF.cpp
index fa2130e4b01e3..0926192a4f384 100644
--- a/lldb/source/Plugins/Language/ObjC/CF.cpp
+++ b/lldb/source/Plugins/Language/ObjC/CF.cpp
@@ -44,7 +44,7 @@ bool lldb_private::formatters::CFAbsoluteTimeSummaryProvider(
 
 bool lldb_private::formatters::CFBagSummaryProvider(
 ValueObject &valobj, Stream &stream, const TypeSummaryOptions &options) {
-  static ConstString g_TypeHint("CFBag");
+  static constexpr llvm::StringLiteral g_TypeHint("CFBag");
 
   ProcessSP process_sp = valobj.GetProcessSP();
   if (!process_sp)
@@ -92,17 +92,13 @@ bool lldb_private::formatters::CFBagSummaryProvider(
   } else
 return false;
 
-  std::string prefix, suffix;
-  if (Language *language = Language::FindPlugin(options.GetLanguage())) {
-if (!language->GetFormatterPrefixSuffix(valobj, g_TypeHint, prefix,
-suffix)) {
-  prefix.clear();
-  suffix.clear();
-}
-  }
+  llvm::StringRef prefix, suffix;
+  if (Language *language = Language::FindPlugin(options.GetLanguage()))
+std::tie(prefix, suffix) = language->GetFormatterPrefixSuffix(g_TypeHint);
 
-  stream.Printf("%s\"%u value%s\"%s", prefix.c_str(), count,
-(count == 1 ? "" : "s"), suffix.c_str());
+  stream << prefix;
+  stream.Printf("\"%u value%s\"", count, (count == 1 ? "" : "s"));
+  stream << suffix;
   return true;
 }
 
@@ -226,7 +222,7 @@ bool lldb_private::formatters::CFBit

[Lldb-commits] [PATCH] D151603: [lldb][NFCI] Refactor Language::GetFormatterPrefixSuffix

[Alex Langford via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8e0001eb95ce: [lldb][NFCI] Refactor 
Language::GetFormatterPrefixSuffix (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151603

Files:
  lldb/include/lldb/Target/Language.h
  lldb/source/Plugins/Language/ObjC/CF.cpp
  lldb/source/Plugins/Language/ObjC/Cocoa.cpp
  lldb/source/Plugins/Language/ObjC/NSArray.cpp
  lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
  lldb/source/Plugins/Language/ObjC/NSSet.cpp
  lldb/source/Plugins/Language/ObjC/NSString.cpp
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
  lldb/source/Target/Language.cpp

Index: lldb/source/Target/Language.cpp
===
--- lldb/source/Target/Language.cpp
+++ lldb/source/Target/Language.cpp
@@ -452,11 +452,9 @@
   return result;
 }
 
-bool Language::GetFormatterPrefixSuffix(ValueObject &valobj,
-ConstString type_hint,
-std::string &prefix,
-std::string &suffix) {
-  return false;
+std::pair
+Language::GetFormatterPrefixSuffix(llvm::StringRef type_hint) {
+  return std::pair();
 }
 
 bool Language::DemangledNameContainsPath(llvm::StringRef path, 
Index: lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
===
--- lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
+++ lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
@@ -150,9 +150,8 @@
 
   std::unique_ptr GetTypeScavenger() override;
 
-  bool GetFormatterPrefixSuffix(ValueObject &valobj, ConstString type_hint,
-std::string &prefix,
-std::string &suffix) override;
+  std::pair
+  GetFormatterPrefixSuffix(llvm::StringRef type_hint) override;
 
   bool IsNilReference(ValueObject &valobj) override;
 
Index: lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
===
--- lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
+++ lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
@@ -998,78 +998,27 @@
 ObjCDebugInfoScavenger>());
 }
 
-bool ObjCLanguage::GetFormatterPrefixSuffix(ValueObject &valobj,
-ConstString type_hint,
-std::string &prefix,
-std::string &suffix) {
-  static ConstString g_CFBag("CFBag");
-  static ConstString g_CFBinaryHeap("CFBinaryHeap");
-
-  static ConstString g_NSNumberChar("NSNumber:char");
-  static ConstString g_NSNumberShort("NSNumber:short");
-  static ConstString g_NSNumberInt("NSNumber:int");
-  static ConstString g_NSNumberLong("NSNumber:long");
-  static ConstString g_NSNumberInt128("NSNumber:int128_t");
-  static ConstString g_NSNumberFloat("NSNumber:float");
-  static ConstString g_NSNumberDouble("NSNumber:double");
-
-  static ConstString g_NSData("NSData");
-  static ConstString g_NSArray("NSArray");
-  static ConstString g_NSString("NSString");
-  static ConstString g_NSStringStar("NSString*");
-
-  if (type_hint.IsEmpty())
-return false;
-
-  prefix.clear();
-  suffix.clear();
-
-  if (type_hint == g_CFBag || type_hint == g_CFBinaryHeap) {
-prefix = "@";
-return true;
-  }
-
-  if (type_hint == g_NSNumberChar) {
-prefix = "(char)";
-return true;
-  }
-  if (type_hint == g_NSNumberShort) {
-prefix = "(short)";
-return true;
-  }
-  if (type_hint == g_NSNumberInt) {
-prefix = "(int)";
-return true;
-  }
-  if (type_hint == g_NSNumberLong) {
-prefix = "(long)";
-return true;
-  }
-  if (type_hint == g_NSNumberInt128) {
-prefix = "(int128_t)";
-return true;
-  }
-  if (type_hint == g_NSNumberFloat) {
-prefix = "(float)";
-return true;
-  }
-  if (type_hint == g_NSNumberDouble) {
-prefix = "(double)";
-return true;
-  }
-
-  if (type_hint == g_NSData || type_hint == g_NSArray) {
-prefix = "@\"";
-suffix = "\"";
-return true;
-  }
-
-  if (type_hint == g_NSString || type_hint == g_NSStringStar) {
-prefix = "@";
-return true;
-  }
-
-  return false;
+std::pair
+ObjCLanguage::GetFormatterPrefixSuffix(llvm::StringRef type_hint) {
+  static constexpr llvm::StringRef empty;
+  static const llvm::StringMap<
+  std::pair>
+  g_affix_map = {
+  {"CFBag", {"@", empty}},
+  {"CFBinaryHeap", {"@", empty}},
+  {"NSString", {"@", empty}},
+  {"NSString*", {"@", empty}},
+  {"NSNumber:char", {"(char)", empty}},
+  {"NSNumber:short", {"(short)", empty}},
+  {"NSNumber:int", {"(int)", empty}},
+  {"NSNumber:long", {"(long)", empty}},
+  {"NSNumbe

[Lldb-commits] [PATCH] D151748: [lldb] Consult summary provider before printing children of root references

[Dave Lee via Phabricator via lldb-commits]

kastiglione created this revision.
kastiglione added reviewers: jingham, bulbazord, aprantl, Michael137.
Herald added a project: All.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

When printing the root of a value, if it's a reference its children are 
unconditionally
printed - in contrast to pointers whose children are only printed if a 
sufficient
pointer depth is given.

However, the children are printed even when there's a summary provider that 
says not to.
If a summary provider exists, this change consults it to determine if children 
should be
printed.

For example, given a variable of type `std::string &`, this change has the 
following
effect:

Before:

  (lldb) p string_ref
  (std::string &) string_ref = "one two three four five six seven eight nine 
ten": {
__r_ = {
  std::__1::__compressed_pair_elem, std::__1::allocator >::__rep, 0, false> = {
__value_ = {
   = {
__l = (__data_ = "one two three four five six seven eight nine 
ten", __size_ = 48, __cap_ = 64, __is_long_ = 1)
__s = (__data_ = "@\0p\U0001\0`\0\00\0\0\0\0\0\0\0@", 
__padding_ = "\x80t<", __size_ = '\0', __is_long_ = '\x01')
__r = {
  __words ={...}
}
  }
}
  }
}
  }

After:

  (lldb) p string_ref
  (std::string &) string_ref = "one two three four five six seven eight nine 
ten"

rdar://73248786


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151748

Files:
  lldb/source/DataFormatters/ValueObjectPrinter.cpp
  lldb/test/API/functionalities/data-formatter/root-reference-children/Makefile
  
lldb/test/API/functionalities/data-formatter/root-reference-children/TestRootReferenceChildren.py
  lldb/test/API/functionalities/data-formatter/root-reference-children/main.cpp

Index: lldb/test/API/functionalities/data-formatter/root-reference-children/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/root-reference-children/main.cpp
@@ -0,0 +1,24 @@
+#include 
+
+struct SummaryAndChildren {
+  int child = 30;
+};
+
+struct SummaryOnly {
+  int child = 30;
+};
+
+struct ChildrenOnly {
+  int child = 30;
+};
+
+int main() {
+  SummaryAndChildren summary_and_children;
+  SummaryOnly summary_only;
+  ChildrenOnly children_only;
+  auto &summary_and_children_ref = summary_and_children;
+  auto &summary_only_ref = summary_only;
+  auto &children_only_ref = children_only;
+  printf("break here\n");
+  return 0;
+}
Index: lldb/test/API/functionalities/data-formatter/root-reference-children/TestRootReferenceChildren.py
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/root-reference-children/TestRootReferenceChildren.py
@@ -0,0 +1,27 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestCase(TestBase):
+def test(self):
+self.build()
+lldbutil.run_to_source_breakpoint(
+self, "break here", lldb.SBFileSpec("main.cpp")
+)
+
+self.dbg.HandleCommand(
+f"type summary add --expand -s 'some summary' SummaryAndChildren"
+)
+self.dbg.HandleCommand(f"type summary add -s 'some summary' SummaryOnly")
+
+self.expect(
+"v summary_and_children_ref", substrs=["some summary", "child = 30"]
+)
+self.expect(
+"v summary_only_ref", patterns=["some summary", "(?s)^(?!.*child = )"]
+)
+self.expect(
+"v children_only_ref", patterns=["(?s)^(?!.*some summary)", "child = 30"]
+)
Index: lldb/test/API/functionalities/data-formatter/root-reference-children/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/root-reference-children/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/DataFormatters/ValueObjectPrinter.cpp
===
--- lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -516,11 +516,13 @@
   if (m_options.m_pointer_as_array)
 return true;
 
-  TypeSummaryImpl *entry = GetSummaryFormatter();
-
   if (m_options.m_use_objc)
 return false;
 
+  bool print_children = true;
+  if (TypeSummaryImpl *type_summary = GetSummaryFormatter())
+print_children = type_summary->DoesPrintChildren(m_valobj);
+
   if (is_failed_description || !HasReachedMaximumDepth()) {
 // We will show children for all concrete types. We won't show pointer
 // contents unless a pointer depth has been specified. We won't reference
@@ -538,7 +540,7 @@
 
   const bool is_root_level = m_curr_depth == 0;
 
-  if (is_ref && is

[Lldb-commits] [lldb] f46638b - [lldb][NFCI] Change type of SBDebugger::m_instance_name

[Alex Langford via lldb-commits]

Author: Alex Langford
Date: 2023-05-30T13:21:56-07:00
New Revision: f46638b01d1bd66aa879188132e0d19a0a7f5928

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

LOG: [lldb][NFCI] Change type of SBDebugger::m_instance_name

This doesn't need to be in the ConstString StringPool. There's little
benefit to having these be unique, and we don't need fast comparisons on
them.

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

Added: 


Modified: 
lldb/include/lldb/Core/Debugger.h
lldb/source/API/SBDebugger.cpp
lldb/source/Core/Debugger.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/Debugger.h 
b/lldb/include/lldb/Core/Debugger.h
index 54f7d5c0edb4a..b63597fc71b4c 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -116,7 +116,7 @@ class Debugger : public 
std::enable_shared_from_this,
   static lldb::DebuggerSP FindDebuggerWithID(lldb::user_id_t id);
 
   static lldb::DebuggerSP
-  FindDebuggerWithInstanceName(ConstString instance_name);
+  FindDebuggerWithInstanceName(llvm::StringRef instance_name);
 
   static size_t GetNumDebuggers();
 
@@ -359,7 +359,7 @@ class Debugger : public 
std::enable_shared_from_this,
 
   bool GetNotifyVoid() const;
 
-  ConstString GetInstanceName() { return m_instance_name; }
+  const std::string &GetInstanceName() { return m_instance_name; }
 
   bool LoadPlugin(const FileSpec &spec, Status &error);
 
@@ -644,7 +644,7 @@ class Debugger : public 
std::enable_shared_from_this,
 
   llvm::StringMap> m_stream_handlers;
   std::shared_ptr m_callback_handler_sp;
-  ConstString m_instance_name;
+  const std::string m_instance_name;
   static LoadPluginCallbackType g_load_plugin_callback;
   typedef std::vector LoadedPluginsList;
   LoadedPluginsList m_loaded_plugins;

diff  --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index 9e9b01f830b59..9641e2f9c8a08 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -1326,7 +1326,10 @@ SBDebugger SBDebugger::FindDebuggerWithID(int id) {
 const char *SBDebugger::GetInstanceName() {
   LLDB_INSTRUMENT_VA(this);
 
-  return (m_opaque_sp ? m_opaque_sp->GetInstanceName().AsCString() : nullptr);
+  if (!m_opaque_sp)
+return nullptr;
+
+  return ConstString(m_opaque_sp->GetInstanceName()).AsCString();
 }
 
 SBError SBDebugger::SetInternalVariable(const char *var_name, const char 
*value,
@@ -1334,8 +1337,8 @@ SBError SBDebugger::SetInternalVariable(const char 
*var_name, const char *value,
   LLDB_INSTRUMENT_VA(var_name, value, debugger_instance_name);
 
   SBError sb_error;
-  DebuggerSP debugger_sp(Debugger::FindDebuggerWithInstanceName(
-  ConstString(debugger_instance_name)));
+  DebuggerSP debugger_sp(
+  Debugger::FindDebuggerWithInstanceName(debugger_instance_name));
   Status error;
   if (debugger_sp) {
 ExecutionContext exe_ctx(
@@ -1356,8 +1359,8 @@ SBDebugger::GetInternalVariableValue(const char *var_name,
  const char *debugger_instance_name) {
   LLDB_INSTRUMENT_VA(var_name, debugger_instance_name);
 
-  DebuggerSP debugger_sp(Debugger::FindDebuggerWithInstanceName(
-  ConstString(debugger_instance_name)));
+  DebuggerSP debugger_sp(
+  Debugger::FindDebuggerWithInstanceName(debugger_instance_name));
   Status error;
   if (debugger_sp) {
 ExecutionContext exe_ctx(
@@ -1487,7 +1490,7 @@ bool SBDebugger::GetDescription(SBStream &description) {
   Stream &strm = description.ref();
 
   if (m_opaque_sp) {
-const char *name = m_opaque_sp->GetInstanceName().AsCString();
+const char *name = m_opaque_sp->GetInstanceName().c_str();
 user_id_t id = m_opaque_sp->GetID();
 strm.Printf("Debugger (instance: \"%s\", id: %" PRIu64 ")", name, id);
   } else

diff  --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 1d92f2f52c2f7..ad177637f45b4 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -740,19 +740,20 @@ void Debugger::Destroy(DebuggerSP &debugger_sp) {
   }
 }
 
-DebuggerSP Debugger::FindDebuggerWithInstanceName(ConstString instance_name) {
-  DebuggerSP debugger_sp;
-  if (g_debugger_list_ptr && g_debugger_list_mutex_ptr) {
-std::lock_guard guard(*g_debugger_list_mutex_ptr);
-DebuggerList::iterator pos, end = g_debugger_list_ptr->end();
-for (pos = g_debugger_list_ptr->begin(); pos != end; ++pos) {
-  if ((*pos)->m_instance_name == instance_name) {
-debugger_sp = *pos;
-break;
-  }
-}
+DebuggerSP
+Debugger::FindDebuggerWithInstanceName(llvm::StringRef instance_name) {
+  if (!g_debugger_list_ptr || !g_debugger_list_mutex_ptr)
+return DebuggerSP();
+
+  

[Lldb-commits] [PATCH] D151524: [lldb][NFCI] Change type of SBDebugger::m_instance_name

[Alex Langford via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf46638b01d1b: [lldb][NFCI] Change type of 
SBDebugger::m_instance_name (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151524

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -408,7 +408,7 @@
   m_session_dict(PyInitialValue::Invalid),
   m_sys_module_dict(PyInitialValue::Invalid), m_run_one_line_function(),
   m_run_one_line_str_global(),
-  m_dictionary_name(m_debugger.GetInstanceName().AsCString()),
+  m_dictionary_name(m_debugger.GetInstanceName()),
   m_active_io_handler(eIOHandlerNone), m_session_is_active(false),
   m_pty_secondary_is_open(false), m_valid_session(true), m_lock_count(0),
   m_command_thread_state(nullptr) {
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -740,19 +740,20 @@
   }
 }
 
-DebuggerSP Debugger::FindDebuggerWithInstanceName(ConstString instance_name) {
-  DebuggerSP debugger_sp;
-  if (g_debugger_list_ptr && g_debugger_list_mutex_ptr) {
-std::lock_guard guard(*g_debugger_list_mutex_ptr);
-DebuggerList::iterator pos, end = g_debugger_list_ptr->end();
-for (pos = g_debugger_list_ptr->begin(); pos != end; ++pos) {
-  if ((*pos)->m_instance_name == instance_name) {
-debugger_sp = *pos;
-break;
-  }
-}
+DebuggerSP
+Debugger::FindDebuggerWithInstanceName(llvm::StringRef instance_name) {
+  if (!g_debugger_list_ptr || !g_debugger_list_mutex_ptr)
+return DebuggerSP();
+
+  std::lock_guard guard(*g_debugger_list_mutex_ptr);
+  for (const DebuggerSP &debugger_sp : *g_debugger_list_ptr) {
+if (!debugger_sp)
+  continue;
+
+if (llvm::StringRef(debugger_sp->GetInstanceName()) == instance_name)
+  return debugger_sp;
   }
-  return debugger_sp;
+  return DebuggerSP();
 }
 
 TargetSP Debugger::FindTargetWithProcessID(lldb::pid_t pid) {
@@ -801,13 +802,13 @@
   m_source_manager_up(), m_source_file_cache(),
   m_command_interpreter_up(
   std::make_unique(*this, false)),
-  m_io_handler_stack(), m_instance_name(), m_loaded_plugins(),
-  m_event_handler_thread(), m_io_handler_thread(),
+  m_io_handler_stack(),
+  m_instance_name(llvm::formatv("debugger_{0}", GetID()).str()),
+  m_loaded_plugins(), m_event_handler_thread(), m_io_handler_thread(),
   m_sync_broadcaster(nullptr, "lldb.debugger.sync"),
   m_broadcaster(m_broadcaster_manager_sp,
 GetStaticBroadcasterClass().AsCString()),
   m_forward_listener_sp(), m_clear_once() {
-  m_instance_name.SetString(llvm::formatv("debugger_{0}", GetID()).str());
   // Initialize the debugger properties as early as possible as other parts of
   // LLDB will start querying them during construction.
   m_collection_sp->Initialize(g_debugger_properties);
Index: lldb/source/API/SBDebugger.cpp
===
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -1326,7 +1326,10 @@
 const char *SBDebugger::GetInstanceName() {
   LLDB_INSTRUMENT_VA(this);
 
-  return (m_opaque_sp ? m_opaque_sp->GetInstanceName().AsCString() : nullptr);
+  if (!m_opaque_sp)
+return nullptr;
+
+  return ConstString(m_opaque_sp->GetInstanceName()).AsCString();
 }
 
 SBError SBDebugger::SetInternalVariable(const char *var_name, const char *value,
@@ -1334,8 +1337,8 @@
   LLDB_INSTRUMENT_VA(var_name, value, debugger_instance_name);
 
   SBError sb_error;
-  DebuggerSP debugger_sp(Debugger::FindDebuggerWithInstanceName(
-  ConstString(debugger_instance_name)));
+  DebuggerSP debugger_sp(
+  Debugger::FindDebuggerWithInstanceName(debugger_instance_name));
   Status error;
   if (debugger_sp) {
 ExecutionContext exe_ctx(
@@ -1356,8 +1359,8 @@
  const char *debugger_instance_name) {
   LLDB_INSTRUMENT_VA(var_name, debugger_instance_name);
 
-  DebuggerSP debugger_sp(Debugger::FindDebuggerWithInstanceName(
-  ConstString(debugger_instance_name)));
+  DebuggerSP debugger_sp(
+  Debugger::FindDebuggerWithInstanceName(debugger_instance_name));
   Status error;
   if (debugger_sp) {
 ExecutionContext exe_ctx(
@@ -1487,7 +1490,7 @@
   Stream &strm = description.ref();
 
   if (m_opaque_sp) {
-const char *name = m_opaque_sp->GetInstanceName()

[Lldb-commits] [PATCH] D151599: [lldb][NFCI] Remove use of ConstString from StructuredDataDarwinLog static functions

[Alex Langford via Phabricator via lldb-commits]

bulbazord updated this revision to Diff 526768.
bulbazord added a comment.

LLDB_LOGF -> LLDB_LOG to avoid a temporary string construction.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151599

Files:
  lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp

Index: lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
===
--- lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
+++ lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
@@ -162,13 +162,13 @@
 // used to format message text
 };
 
-static ConstString GetDarwinLogTypeName() {
-  static const ConstString s_key_name("DarwinLog");
+static llvm::StringRef GetDarwinLogTypeName() {
+  static constexpr llvm::StringLiteral s_key_name("DarwinLog");
   return s_key_name;
 }
 
-static ConstString GetLogEventType() {
-  static const ConstString s_event_type("log");
+static llvm::StringRef GetLogEventType() {
+  static constexpr llvm::StringLiteral s_event_type("log");
   return s_event_type;
 }
 
@@ -799,8 +799,8 @@
 }
 
 // Get the plugin for the process.
-auto plugin_sp =
-process_sp->GetStructuredDataPlugin(GetDarwinLogTypeName());
+auto plugin_sp = process_sp->GetStructuredDataPlugin(
+ConstString(GetDarwinLogTypeName()));
 if (!plugin_sp || (plugin_sp->GetPluginName() !=
StructuredDataDarwinLog::GetStaticPluginName())) {
   result.AppendError("failed to get StructuredDataPlugin for "
@@ -822,8 +822,8 @@
 // Send configuration to the feature by way of the process. Construct the
 // options we will use.
 auto config_sp = m_options_sp->BuildConfigurationData(m_enable);
-const Status error =
-process_sp->ConfigureStructuredData(GetDarwinLogTypeName(), config_sp);
+const Status error = process_sp->ConfigureStructuredData(
+ConstString(GetDarwinLogTypeName()), config_sp);
 
 // Report results.
 if (!error.Success()) {
@@ -871,8 +871,8 @@
   stream.PutCString("Enabled: not applicable "
 "(requires process)\n");
 } else {
-  auto plugin_sp =
-  process_sp->GetStructuredDataPlugin(GetDarwinLogTypeName());
+  auto plugin_sp = process_sp->GetStructuredDataPlugin(
+  ConstString(GetDarwinLogTypeName()));
   stream.Printf("Availability: %s\n",
 plugin_sp ? "available" : "unavailable");
   llvm::StringRef plugin_name = StructuredDataDarwinLog::GetStaticPluginName();
@@ -1089,7 +1089,7 @@
 LLDB_LOGF(log,
   "StructuredDataDarwinLog::%s() StructuredData type "
   "expected to be %s but was %s, ignoring",
-  __FUNCTION__, GetDarwinLogTypeName().AsCString(),
+  __FUNCTION__, GetDarwinLogTypeName().str().c_str(),
   type_name.AsCString());
 return;
   }
@@ -1142,7 +1142,7 @@
   }
 
   // Validate this is really a message for our plugin.
-  ConstString type_name;
+  llvm::StringRef type_name;
   if (!dictionary->GetValueForKeyAsString("type", type_name)) {
 SetErrorWithJSON(error, "Structured data doesn't contain mandatory "
 "type field",
@@ -1490,13 +1490,11 @@
   LLDB_LOGF(log, "StructuredDataDarwinLog::%s() call is for process uid %d",
 __FUNCTION__, process_sp->GetUniqueID());
 
-  auto plugin_sp = process_sp->GetStructuredDataPlugin(GetDarwinLogTypeName());
+  auto plugin_sp =
+  process_sp->GetStructuredDataPlugin(ConstString(GetDarwinLogTypeName()));
   if (!plugin_sp) {
-LLDB_LOGF(log,
-  "StructuredDataDarwinLog::%s() warning: no plugin for "
-  "feature %s in process uid %u",
-  __FUNCTION__, GetDarwinLogTypeName().AsCString(),
-  process_sp->GetUniqueID());
+LLDB_LOG(log, "warning: no plugin for feature {0} in process uid {1}",
+ GetDarwinLogTypeName(), process_sp->GetUniqueID());
 return false;
   }
 
@@ -1736,7 +1734,7 @@
 size_t StructuredDataDarwinLog::HandleDisplayOfEvent(
 const StructuredData::Dictionary &event, Stream &stream) {
   // Check the type of the event.
-  ConstString event_type;
+  llvm::StringRef event_type;
   if (!event.GetValueForKeyAsString("type", event_type)) {
 // Hmm, we expected to get events that describe what they are.  Continue
 // anyway.
@@ -1836,8 +1834,8 @@
 
   // We can run it directly.
   // Send configuration to the feature by way of the process.
-  const Status error =
-  process_sp->ConfigureStructuredData(GetDarwinLogTypeName(), config_sp);
+  const Status error = process_sp->ConfigureStructuredData(
+  ConstString(GetDarwinLogTypeName()), config_sp);
 
   // Report results.
   if (!error.Success()) {
___
lldb-commits mailing list
lldb-commits@lists.llvm

[Lldb-commits] [lldb] 9e8a412 - [lldb][NFCI] Remove use of ConstString from StructuredDataDarwinLog static functions

[Alex Langford via lldb-commits]

Author: Alex Langford
Date: 2023-05-30T13:47:09-07:00
New Revision: 9e8a412cb37d2a1201bd33878fce0993587ef335

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

LOG: [lldb][NFCI] Remove use of ConstString from StructuredDataDarwinLog static 
functions

The strings "DarwinLog" and "log" probably do not need to be in the
ConstString StringPool. We still create ConstStrings from them in some
places (for now) but that's because we don't have an implicit
constructor to convert a StringRef to a ConstString.

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

Added: 


Modified: 
lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp 
b/lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
index deebf0700f947..a9f88233d9463 100644
--- a/lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
+++ b/lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
@@ -162,13 +162,13 @@ const char *const s_filter_attributes[] = {
 // used to format message text
 };
 
-static ConstString GetDarwinLogTypeName() {
-  static const ConstString s_key_name("DarwinLog");
+static llvm::StringRef GetDarwinLogTypeName() {
+  static constexpr llvm::StringLiteral s_key_name("DarwinLog");
   return s_key_name;
 }
 
-static ConstString GetLogEventType() {
-  static const ConstString s_event_type("log");
+static llvm::StringRef GetLogEventType() {
+  static constexpr llvm::StringLiteral s_event_type("log");
   return s_event_type;
 }
 
@@ -799,8 +799,8 @@ class EnableCommand : public CommandObjectParsed {
 }
 
 // Get the plugin for the process.
-auto plugin_sp =
-process_sp->GetStructuredDataPlugin(GetDarwinLogTypeName());
+auto plugin_sp = process_sp->GetStructuredDataPlugin(
+ConstString(GetDarwinLogTypeName()));
 if (!plugin_sp || (plugin_sp->GetPluginName() !=
StructuredDataDarwinLog::GetStaticPluginName())) {
   result.AppendError("failed to get StructuredDataPlugin for "
@@ -822,8 +822,8 @@ class EnableCommand : public CommandObjectParsed {
 // Send configuration to the feature by way of the process. Construct the
 // options we will use.
 auto config_sp = m_options_sp->BuildConfigurationData(m_enable);
-const Status error =
-process_sp->ConfigureStructuredData(GetDarwinLogTypeName(), config_sp);
+const Status error = process_sp->ConfigureStructuredData(
+ConstString(GetDarwinLogTypeName()), config_sp);
 
 // Report results.
 if (!error.Success()) {
@@ -871,8 +871,8 @@ class StatusCommand : public CommandObjectParsed {
   stream.PutCString("Enabled: not applicable "
 "(requires process)\n");
 } else {
-  auto plugin_sp =
-  process_sp->GetStructuredDataPlugin(GetDarwinLogTypeName());
+  auto plugin_sp = process_sp->GetStructuredDataPlugin(
+  ConstString(GetDarwinLogTypeName()));
   stream.Printf("Availability: %s\n",
 plugin_sp ? "available" : "unavailable");
   llvm::StringRef plugin_name = 
StructuredDataDarwinLog::GetStaticPluginName();
@@ -1089,7 +1089,7 @@ void 
StructuredDataDarwinLog::HandleArrivalOfStructuredData(
 LLDB_LOGF(log,
   "StructuredDataDarwinLog::%s() StructuredData type "
   "expected to be %s but was %s, ignoring",
-  __FUNCTION__, GetDarwinLogTypeName().AsCString(),
+  __FUNCTION__, GetDarwinLogTypeName().str().c_str(),
   type_name.AsCString());
 return;
   }
@@ -1142,7 +1142,7 @@ Status StructuredDataDarwinLog::GetDescription(
   }
 
   // Validate this is really a message for our plugin.
-  ConstString type_name;
+  llvm::StringRef type_name;
   if (!dictionary->GetValueForKeyAsString("type", type_name)) {
 SetErrorWithJSON(error, "Structured data doesn't contain mandatory "
 "type field",
@@ -1490,13 +1490,11 @@ bool 
StructuredDataDarwinLog::InitCompletionHookCallback(
   LLDB_LOGF(log, "StructuredDataDarwinLog::%s() call is for process uid %d",
 __FUNCTION__, process_sp->GetUniqueID());
 
-  auto plugin_sp = process_sp->GetStructuredDataPlugin(GetDarwinLogTypeName());
+  auto plugin_sp =
+  process_sp->GetStructuredDataPlugin(ConstString(GetDarwinLogTypeName()));
   if (!plugin_sp) {
-LLDB_LOGF(log,
-  "StructuredDataDarwinLog::%s() warning: no plugin for "
-  "feature %s in process uid %u",
-  __FUNCTION__, GetDarwinLogTypeName().AsCString(),
-  process_sp->GetUniqueID());
+LLDB_LOG(log, "warning: no plugin for feature {0} in process uid {1

[Lldb-commits] [PATCH] D151599: [lldb][NFCI] Remove use of ConstString from StructuredDataDarwinLog static functions

[Alex Langford via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9e8a412cb37d: [lldb][NFCI] Remove use of ConstString from 
StructuredDataDarwinLog static… (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151599

Files:
  lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp

Index: lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
===
--- lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
+++ lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
@@ -162,13 +162,13 @@
 // used to format message text
 };
 
-static ConstString GetDarwinLogTypeName() {
-  static const ConstString s_key_name("DarwinLog");
+static llvm::StringRef GetDarwinLogTypeName() {
+  static constexpr llvm::StringLiteral s_key_name("DarwinLog");
   return s_key_name;
 }
 
-static ConstString GetLogEventType() {
-  static const ConstString s_event_type("log");
+static llvm::StringRef GetLogEventType() {
+  static constexpr llvm::StringLiteral s_event_type("log");
   return s_event_type;
 }
 
@@ -799,8 +799,8 @@
 }
 
 // Get the plugin for the process.
-auto plugin_sp =
-process_sp->GetStructuredDataPlugin(GetDarwinLogTypeName());
+auto plugin_sp = process_sp->GetStructuredDataPlugin(
+ConstString(GetDarwinLogTypeName()));
 if (!plugin_sp || (plugin_sp->GetPluginName() !=
StructuredDataDarwinLog::GetStaticPluginName())) {
   result.AppendError("failed to get StructuredDataPlugin for "
@@ -822,8 +822,8 @@
 // Send configuration to the feature by way of the process. Construct the
 // options we will use.
 auto config_sp = m_options_sp->BuildConfigurationData(m_enable);
-const Status error =
-process_sp->ConfigureStructuredData(GetDarwinLogTypeName(), config_sp);
+const Status error = process_sp->ConfigureStructuredData(
+ConstString(GetDarwinLogTypeName()), config_sp);
 
 // Report results.
 if (!error.Success()) {
@@ -871,8 +871,8 @@
   stream.PutCString("Enabled: not applicable "
 "(requires process)\n");
 } else {
-  auto plugin_sp =
-  process_sp->GetStructuredDataPlugin(GetDarwinLogTypeName());
+  auto plugin_sp = process_sp->GetStructuredDataPlugin(
+  ConstString(GetDarwinLogTypeName()));
   stream.Printf("Availability: %s\n",
 plugin_sp ? "available" : "unavailable");
   llvm::StringRef plugin_name = StructuredDataDarwinLog::GetStaticPluginName();
@@ -1089,7 +1089,7 @@
 LLDB_LOGF(log,
   "StructuredDataDarwinLog::%s() StructuredData type "
   "expected to be %s but was %s, ignoring",
-  __FUNCTION__, GetDarwinLogTypeName().AsCString(),
+  __FUNCTION__, GetDarwinLogTypeName().str().c_str(),
   type_name.AsCString());
 return;
   }
@@ -1142,7 +1142,7 @@
   }
 
   // Validate this is really a message for our plugin.
-  ConstString type_name;
+  llvm::StringRef type_name;
   if (!dictionary->GetValueForKeyAsString("type", type_name)) {
 SetErrorWithJSON(error, "Structured data doesn't contain mandatory "
 "type field",
@@ -1490,13 +1490,11 @@
   LLDB_LOGF(log, "StructuredDataDarwinLog::%s() call is for process uid %d",
 __FUNCTION__, process_sp->GetUniqueID());
 
-  auto plugin_sp = process_sp->GetStructuredDataPlugin(GetDarwinLogTypeName());
+  auto plugin_sp =
+  process_sp->GetStructuredDataPlugin(ConstString(GetDarwinLogTypeName()));
   if (!plugin_sp) {
-LLDB_LOGF(log,
-  "StructuredDataDarwinLog::%s() warning: no plugin for "
-  "feature %s in process uid %u",
-  __FUNCTION__, GetDarwinLogTypeName().AsCString(),
-  process_sp->GetUniqueID());
+LLDB_LOG(log, "warning: no plugin for feature {0} in process uid {1}",
+ GetDarwinLogTypeName(), process_sp->GetUniqueID());
 return false;
   }
 
@@ -1736,7 +1734,7 @@
 size_t StructuredDataDarwinLog::HandleDisplayOfEvent(
 const StructuredData::Dictionary &event, Stream &stream) {
   // Check the type of the event.
-  ConstString event_type;
+  llvm::StringRef event_type;
   if (!event.GetValueForKeyAsString("type", event_type)) {
 // Hmm, we expected to get events that describe what they are.  Continue
 // anyway.
@@ -1836,8 +1834,8 @@
 
   // We can run it directly.
   // Send configuration to the feature by way of the process.
-  const Status error =
-  process_sp->ConfigureStructuredData(GetDarwinLogTypeName(), config_sp);
+  const Status error = process_sp->ConfigureStructuredData(
+  ConstString(GetDarwinLogTypeName()), config_sp);
 
   // Report results.
   if (!error.Success()) {
_

[Lldb-commits] [lldb] e348dbc - [lldb] Fix build after Clang API change

[Jan Svoboda via lldb-commits]

Author: Jan Svoboda
Date: 2023-05-30T14:08:04-07:00
New Revision: e348dbc4b2766f17c251b6c305a3b34fbdb9be96

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

LOG: [lldb] Fix build after Clang API change

This fixes breakage introduced by 769d282d.

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
index 98c1b1a73b782..7895fc6d59ef7 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -333,7 +333,7 @@ bool ClangModulesDeclVendorImpl::AddModule(const 
SourceModule &module,
   HS.getFileMgr().getDirectory(module.search_path.GetStringRef());
   if (!dir)
 return error();
-  auto *file = HS.lookupModuleMapFile(*dir, is_framework);
+  auto file = HS.lookupModuleMapFile(*dir, is_framework);
   if (!file)
 return error();
   if (!HS.loadModuleMapFile(file, is_system))



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

[Lldb-commits] [PATCH] D151748: [lldb] Consult summary provider before printing children of root references

[Michael Buch via Phabricator via lldb-commits]

Michael137 accepted this revision.
Michael137 added a comment.
This revision is now accepted and ready to land.

makes sense


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151748

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

[Lldb-commits] [PATCH] D151748: [lldb] Consult summary provider before printing children of root references

[Alex Langford via Phabricator via lldb-commits]

bulbazord accepted this revision.
bulbazord added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151748

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

[Lldb-commits] [PATCH] D150928: Two bug fixes for loading process save-core created core files, plus perf improvements

[Alex Langford via Phabricator via lldb-commits]

bulbazord accepted this revision.
bulbazord added a comment.
This revision is now accepted and ready to land.

Looks like my concerns were addressed. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150928

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

[Lldb-commits] [PATCH] D151351: [lldb] Prevent dwim-print from showing kNoResult error

[Adrian Prantl via Phabricator via lldb-commits]

aprantl accepted this revision.
aprantl added inline comments.
This revision is now accepted and ready to land.



Comment at: lldb/source/Commands/CommandObjectDWIMPrint.cpp:1
 //===-- CommandObjectDWIMPrint.cpp --*- C++ 
-*-===//
 //

nit: C++ marker is only needed in a .h file ;-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151351

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

[Lldb-commits] [lldb] a79b0f9 - [lldb] Fix build after Clang API change at rev 769d282d7292

[Jorge Gorbe Moya via lldb-commits]

Author: Jorge Gorbe Moya
Date: 2023-05-30T14:21:21-07:00
New Revision: a79b0f9f1d8275b023bcd2bf1763b148d088ad97

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

LOG: [lldb] Fix build after Clang API change at rev 769d282d7292

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
index 7895fc6d59ef..0af5de4702df 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -336,7 +336,7 @@ bool ClangModulesDeclVendorImpl::AddModule(const 
SourceModule &module,
   auto file = HS.lookupModuleMapFile(*dir, is_framework);
   if (!file)
 return error();
-  if (!HS.loadModuleMapFile(file, is_system))
+  if (!HS.loadModuleMapFile(*file, is_system))
 return error();
 }
   }



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

[Lldb-commits] [PATCH] D151683: [clang] Enable C++11-style attributes in all language modes

[Nikolas Klauser via Phabricator via lldb-commits]

philnik added a comment.

In D151683#4380877 , @erichkeane 
wrote:

> What is the justification for this?

What exactly are you asking for? Why I'd like to back port it? This would make 
quite a bit of code in libc++ simpler and avoids pit-falls where an attribute 
works in some place in some version but not in another.

> Do other compilers do this?

ICC and NVC++ support this: https://godbolt.org/z/TeMnGdGsY

> Was there an RFC?

No. I guess, since you are asking I should write one for this? Only for the 
removal of `-fdouble-square-bracket-attributes`, or also for back porting this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151683

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

[Lldb-commits] [lldb] 57154a6 - [lldb] Introduce FileSpec::GetComponents

[Alex Langford via lldb-commits]

Author: Alex Langford
Date: 2023-05-30T15:13:17-07:00
New Revision: 57154a63a07f732552968141136279350bcdf99d

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

LOG: [lldb] Introduce FileSpec::GetComponents

This patch introduces FileSpec::GetComponents, a method that splits a
FileSpec's path into its individual components. For example, given
/foo/bar/baz, you'll get back a vector of strings {"foo", "bar", baz"}.

The motivation here is to reduce the use of
`FileSpec::RemoveLastPathComponent`. Mutating a FileSpec is expensive,
so providing a way of doing this without mutation is useful.

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

Added: 


Modified: 
lldb/include/lldb/Utility/FileSpec.h
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
lldb/source/Utility/FileSpec.cpp
lldb/unittests/Utility/FileSpecTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/FileSpec.h 
b/lldb/include/lldb/Utility/FileSpec.h
index 919b5e8564583..6eb5b805d9d9f 100644
--- a/lldb/include/lldb/Utility/FileSpec.h
+++ b/lldb/include/lldb/Utility/FileSpec.h
@@ -408,6 +408,18 @@ class FileSpec {
   /// A boolean value indicating whether the path was updated.
   bool RemoveLastPathComponent();
 
+  /// Gets the components of the FileSpec's path.
+  /// For example, given the path:
+  ///   /System/Library/PrivateFrameworks/UIFoundation.framework/UIFoundation
+  ///
+  /// This function returns:
+  ///   {"System", "Library", "PrivateFrameworks", "UIFoundation.framework",
+  ///   "UIFoundation"}
+  /// \return
+  ///   A std::vector of llvm::StringRefs for each path component.
+  ///   The lifetime of the StringRefs is tied to the lifetime of the FileSpec.
+  std::vector GetComponents() const;
+
 protected:
   // Convenience method for setting the file without changing the style.
   void SetFile(llvm::StringRef path);

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp 
b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
index 76c6b535679a6..60327fbe3124f 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1236,13 +1236,9 @@ lldb_private::Status 
PlatformDarwin::FindBundleBinaryInExecSearchPaths(
 // "UIFoundation" and "UIFoundation.framework" -- most likely the latter
 // will be the one we find there.
 
-FileSpec platform_pull_upart(platform_file);
-std::vector path_parts;
-path_parts.push_back(platform_pull_upart.GetFilename().AsCString());
-while (platform_pull_upart.RemoveLastPathComponent()) {
-  ConstString part = platform_pull_upart.GetFilename();
-  path_parts.push_back(part.AsCString());
-}
+std::vector path_parts = platform_file.GetComponents();
+// We want the components in reverse order.
+std::reverse(path_parts.begin(), path_parts.end());
 const size_t path_parts_size = path_parts.size();
 
 size_t num_module_search_paths = module_search_paths_ptr->GetSize();

diff  --git a/lldb/source/Utility/FileSpec.cpp 
b/lldb/source/Utility/FileSpec.cpp
index 6688e45650505..707033c1f8d6b 100644
--- a/lldb/source/Utility/FileSpec.cpp
+++ b/lldb/source/Utility/FileSpec.cpp
@@ -463,6 +463,26 @@ bool FileSpec::RemoveLastPathComponent() {
   }
   return false;
 }
+
+std::vector FileSpec::GetComponents() const {
+  std::vector components;
+
+  auto dir_begin = llvm::sys::path::begin(m_directory.GetStringRef(), m_style);
+  auto dir_end = llvm::sys::path::end(m_directory.GetStringRef());
+
+  for (auto iter = dir_begin; iter != dir_end; ++iter) {
+if (*iter == "/" || *iter == ".")
+  continue;
+
+components.push_back(*iter);
+  }
+
+  if (!m_filename.IsEmpty() && m_filename != "/" && m_filename != ".")
+components.push_back(m_filename.GetStringRef());
+
+  return components;
+}
+
 /// Returns true if the filespec represents an implementation source
 /// file (files with a ".c", ".cpp", ".m", ".mm" (many more)
 /// extension).

diff  --git a/lldb/unittests/Utility/FileSpecTest.cpp 
b/lldb/unittests/Utility/FileSpecTest.cpp
index ffd3d343dc188..2a62f6b1e7612 100644
--- a/lldb/unittests/Utility/FileSpecTest.cpp
+++ b/lldb/unittests/Utility/FileSpecTest.cpp
@@ -504,3 +504,33 @@ TEST(FileSpecTest, TestIsSourceImplementationFile) {
   EXPECT_FALSE(win_noext.IsSourceImplementationFile());
   EXPECT_FALSE(exe.IsSourceImplementationFile());
 }
+
+TEST(FileSpecTest, TestGetComponents) {
+  std::pair> PosixTests[] = {
+  {"/", {}},
+  {"/foo", {"foo"}},
+  {"/foo/", {"foo"}},
+  {"/foo/bar", {"foo", "bar"}},
+  {"/llvm-project/lldb/unittests/Utility/FileSpecTest.cpp",
+   {"llvm-project", "lldb", "unittests", "Utility", "FileSpecTest.cpp"}},
+  };
+
+  for (const auto &pair : PosixTests)

[Lldb-commits] [PATCH] D151399: [lldb] Introduce FileSpec::GetComponents

[Alex Langford via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG57154a63a07f: [lldb] Introduce FileSpec::GetComponents 
(authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151399

Files:
  lldb/include/lldb/Utility/FileSpec.h
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Utility/FileSpec.cpp
  lldb/unittests/Utility/FileSpecTest.cpp

Index: lldb/unittests/Utility/FileSpecTest.cpp
===
--- lldb/unittests/Utility/FileSpecTest.cpp
+++ lldb/unittests/Utility/FileSpecTest.cpp
@@ -504,3 +504,33 @@
   EXPECT_FALSE(win_noext.IsSourceImplementationFile());
   EXPECT_FALSE(exe.IsSourceImplementationFile());
 }
+
+TEST(FileSpecTest, TestGetComponents) {
+  std::pair> PosixTests[] = {
+  {"/", {}},
+  {"/foo", {"foo"}},
+  {"/foo/", {"foo"}},
+  {"/foo/bar", {"foo", "bar"}},
+  {"/llvm-project/lldb/unittests/Utility/FileSpecTest.cpp",
+   {"llvm-project", "lldb", "unittests", "Utility", "FileSpecTest.cpp"}},
+  };
+
+  for (const auto &pair : PosixTests) {
+FileSpec file_spec = PosixSpec(pair.first);
+EXPECT_EQ(file_spec.GetComponents(), pair.second);
+  }
+
+  std::pair> WindowsTests[] = {
+  {"C:\\", {"C:"}},
+  {"C:\\Windows\\", {"C:", "Windows"}},
+  {"C:\\Windows\\System32", {"C:", "Windows", "System32"}},
+  {"C:\\llvm-project\\lldb\\unittests\\Utility\\FileSpecTest.cpp",
+   {"C:", "llvm-project", "lldb", "unittests", "Utility",
+"FileSpecTest.cpp"}},
+  };
+
+  for (const auto &pair : WindowsTests) {
+FileSpec file_spec = WindowsSpec(pair.first);
+EXPECT_EQ(file_spec.GetComponents(), pair.second);
+  }
+}
Index: lldb/source/Utility/FileSpec.cpp
===
--- lldb/source/Utility/FileSpec.cpp
+++ lldb/source/Utility/FileSpec.cpp
@@ -463,6 +463,26 @@
   }
   return false;
 }
+
+std::vector FileSpec::GetComponents() const {
+  std::vector components;
+
+  auto dir_begin = llvm::sys::path::begin(m_directory.GetStringRef(), m_style);
+  auto dir_end = llvm::sys::path::end(m_directory.GetStringRef());
+
+  for (auto iter = dir_begin; iter != dir_end; ++iter) {
+if (*iter == "/" || *iter == ".")
+  continue;
+
+components.push_back(*iter);
+  }
+
+  if (!m_filename.IsEmpty() && m_filename != "/" && m_filename != ".")
+components.push_back(m_filename.GetStringRef());
+
+  return components;
+}
+
 /// Returns true if the filespec represents an implementation source
 /// file (files with a ".c", ".cpp", ".m", ".mm" (many more)
 /// extension).
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1236,13 +1236,9 @@
 // "UIFoundation" and "UIFoundation.framework" -- most likely the latter
 // will be the one we find there.
 
-FileSpec platform_pull_upart(platform_file);
-std::vector path_parts;
-path_parts.push_back(platform_pull_upart.GetFilename().AsCString());
-while (platform_pull_upart.RemoveLastPathComponent()) {
-  ConstString part = platform_pull_upart.GetFilename();
-  path_parts.push_back(part.AsCString());
-}
+std::vector path_parts = platform_file.GetComponents();
+// We want the components in reverse order.
+std::reverse(path_parts.begin(), path_parts.end());
 const size_t path_parts_size = path_parts.size();
 
 size_t num_module_search_paths = module_search_paths_ptr->GetSize();
Index: lldb/include/lldb/Utility/FileSpec.h
===
--- lldb/include/lldb/Utility/FileSpec.h
+++ lldb/include/lldb/Utility/FileSpec.h
@@ -408,6 +408,18 @@
   /// A boolean value indicating whether the path was updated.
   bool RemoveLastPathComponent();
 
+  /// Gets the components of the FileSpec's path.
+  /// For example, given the path:
+  ///   /System/Library/PrivateFrameworks/UIFoundation.framework/UIFoundation
+  ///
+  /// This function returns:
+  ///   {"System", "Library", "PrivateFrameworks", "UIFoundation.framework",
+  ///   "UIFoundation"}
+  /// \return
+  ///   A std::vector of llvm::StringRefs for each path component.
+  ///   The lifetime of the StringRefs is tied to the lifetime of the FileSpec.
+  std::vector GetComponents() const;
+
 protected:
   // Convenience method for setting the file without changing the style.
   void SetFile(llvm::StringRef path);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] c7eb1b0 - [lldb] Consult summary provider before printing children of root references

[Dave Lee via lldb-commits]

Author: Dave Lee
Date: 2023-05-30T15:35:03-07:00
New Revision: c7eb1b07470b9babfcd258f014df3661e5f84b30

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

LOG: [lldb] Consult summary provider before printing children of root references

When printing the root of a value, if it's a reference its children are 
unconditionally
printed - in contrast to pointers whose children are only printed if a 
sufficient
pointer depth is given.

However, the children are printed even when there's a summary provider that 
says not to.
If a summary provider exists, this change consults it to determine if children 
should be
printed.

For example, given a variable of type `std::string &`, this change has the 
following
effect:

Before:

```
(lldb) p string_ref
(std::string &) string_ref = "one two three four five six seven eight nine 
ten": {
  __r_ = {
std::__1::__compressed_pair_elem, std::__1::allocator >::__rep, 0, false> = {
  __value_ = {
 = {
  __l = (__data_ = "one two three four five six seven eight nine ten", 
__size_ = 48, __cap_ = 64, __is_long_ = 1)
  __s = (__data_ = "@\0p\U0001\0`\0\00\0\0\0\0\0\0\0@", __padding_ 
= "\x80t<", __size_ = '\0', __is_long_ = '\x01')
  __r = {
__words ={...}
  }
}
  }
}
  }
}
```

After:

```
(lldb) p string_ref
(std::string &) string_ref = "one two three four five six seven eight nine ten"
```

rdar://73248786

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

Added: 

lldb/test/API/functionalities/data-formatter/root-reference-children/Makefile

lldb/test/API/functionalities/data-formatter/root-reference-children/TestRootReferenceChildren.py

lldb/test/API/functionalities/data-formatter/root-reference-children/main.cpp

Modified: 
lldb/source/DataFormatters/ValueObjectPrinter.cpp

Removed: 




diff  --git a/lldb/source/DataFormatters/ValueObjectPrinter.cpp 
b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
index bde999a7a8bcf..fac319f67c805 100644
--- a/lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -516,11 +516,13 @@ bool ValueObjectPrinter::ShouldPrintChildren(
   if (m_options.m_pointer_as_array)
 return true;
 
-  TypeSummaryImpl *entry = GetSummaryFormatter();
-
   if (m_options.m_use_objc)
 return false;
 
+  bool print_children = true;
+  if (TypeSummaryImpl *type_summary = GetSummaryFormatter())
+print_children = type_summary->DoesPrintChildren(m_valobj);
+
   if (is_failed_description || !HasReachedMaximumDepth()) {
 // We will show children for all concrete types. We won't show pointer
 // contents unless a pointer depth has been specified. We won't reference
@@ -538,7 +540,7 @@ bool ValueObjectPrinter::ShouldPrintChildren(
 
   const bool is_root_level = m_curr_depth == 0;
 
-  if (is_ref && is_root_level) {
+  if (is_ref && is_root_level && print_children) {
 // If this is the root object (depth is zero) that we are showing and
 // it is a reference, and no pointer depth has been supplied print out
 // what it references. Don't do this at deeper depths otherwise we can
@@ -549,7 +551,7 @@ bool ValueObjectPrinter::ShouldPrintChildren(
   return curr_ptr_depth.CanAllowExpansion();
 }
 
-return (!entry || entry->DoesPrintChildren(m_valobj) || m_summary.empty());
+return print_children || m_summary.empty();
   }
   return false;
 }

diff  --git 
a/lldb/test/API/functionalities/data-formatter/root-reference-children/Makefile 
b/lldb/test/API/functionalities/data-formatter/root-reference-children/Makefile
new file mode 100644
index 0..8b20bcb05
--- /dev/null
+++ 
b/lldb/test/API/functionalities/data-formatter/root-reference-children/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules

diff  --git 
a/lldb/test/API/functionalities/data-formatter/root-reference-children/TestRootReferenceChildren.py
 
b/lldb/test/API/functionalities/data-formatter/root-reference-children/TestRootReferenceChildren.py
new file mode 100644
index 0..5de66177e7cad
--- /dev/null
+++ 
b/lldb/test/API/functionalities/data-formatter/root-reference-children/TestRootReferenceChildren.py
@@ -0,0 +1,27 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestCase(TestBase):
+def test(self):
+self.build()
+lldbutil.run_to_source_breakpoint(
+self, "break here", lldb.SBFileSpec("main.cpp")
+)
+
+self.dbg.HandleCommand(
+f"type summary add --expand -s 'some summary' SummaryAndChildren"
+)
+self.dbg.HandleCommand(f"type summary add -s 'some 

[Lldb-commits] [PATCH] D151748: [lldb] Consult summary provider before printing children of root references

[Dave Lee via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc7eb1b07470b: [lldb] Consult summary provider before 
printing children of root references (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151748

Files:
  lldb/source/DataFormatters/ValueObjectPrinter.cpp
  lldb/test/API/functionalities/data-formatter/root-reference-children/Makefile
  
lldb/test/API/functionalities/data-formatter/root-reference-children/TestRootReferenceChildren.py
  lldb/test/API/functionalities/data-formatter/root-reference-children/main.cpp

Index: lldb/test/API/functionalities/data-formatter/root-reference-children/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/root-reference-children/main.cpp
@@ -0,0 +1,24 @@
+#include 
+
+struct SummaryAndChildren {
+  int child = 30;
+};
+
+struct SummaryOnly {
+  int child = 30;
+};
+
+struct ChildrenOnly {
+  int child = 30;
+};
+
+int main() {
+  SummaryAndChildren summary_and_children;
+  SummaryOnly summary_only;
+  ChildrenOnly children_only;
+  auto &summary_and_children_ref = summary_and_children;
+  auto &summary_only_ref = summary_only;
+  auto &children_only_ref = children_only;
+  printf("break here\n");
+  return 0;
+}
Index: lldb/test/API/functionalities/data-formatter/root-reference-children/TestRootReferenceChildren.py
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/root-reference-children/TestRootReferenceChildren.py
@@ -0,0 +1,27 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestCase(TestBase):
+def test(self):
+self.build()
+lldbutil.run_to_source_breakpoint(
+self, "break here", lldb.SBFileSpec("main.cpp")
+)
+
+self.dbg.HandleCommand(
+f"type summary add --expand -s 'some summary' SummaryAndChildren"
+)
+self.dbg.HandleCommand(f"type summary add -s 'some summary' SummaryOnly")
+
+self.expect(
+"v summary_and_children_ref", substrs=["some summary", "child = 30"]
+)
+self.expect(
+"v summary_only_ref", patterns=["some summary", "(?s)^(?!.*child = )"]
+)
+self.expect(
+"v children_only_ref", patterns=["(?s)^(?!.*some summary)", "child = 30"]
+)
Index: lldb/test/API/functionalities/data-formatter/root-reference-children/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/root-reference-children/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/DataFormatters/ValueObjectPrinter.cpp
===
--- lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -516,11 +516,13 @@
   if (m_options.m_pointer_as_array)
 return true;
 
-  TypeSummaryImpl *entry = GetSummaryFormatter();
-
   if (m_options.m_use_objc)
 return false;
 
+  bool print_children = true;
+  if (TypeSummaryImpl *type_summary = GetSummaryFormatter())
+print_children = type_summary->DoesPrintChildren(m_valobj);
+
   if (is_failed_description || !HasReachedMaximumDepth()) {
 // We will show children for all concrete types. We won't show pointer
 // contents unless a pointer depth has been specified. We won't reference
@@ -538,7 +540,7 @@
 
   const bool is_root_level = m_curr_depth == 0;
 
-  if (is_ref && is_root_level) {
+  if (is_ref && is_root_level && print_children) {
 // If this is the root object (depth is zero) that we are showing and
 // it is a reference, and no pointer depth has been supplied print out
 // what it references. Don't do this at deeper depths otherwise we can
@@ -549,7 +551,7 @@
   return curr_ptr_depth.CanAllowExpansion();
 }
 
-return (!entry || entry->DoesPrintChildren(m_valobj) || m_summary.empty());
+return print_children || m_summary.empty();
   }
   return false;
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 48a12ae - Fix a few bugs with Mach-O corefile loading, plus perf

[Jason Molenda via lldb-commits]

Author: Jason Molenda
Date: 2023-05-30T15:36:40-07:00
New Revision: 48a12ae8212c22d9d1d84270db659ac76ecfa972

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

LOG: Fix a few bugs with Mach-O corefile loading, plus perf

In ProcessMachCore::LoadBinariesViaMetadata(), if we did
load some binaries via metadata in the core file, don't
then search for a userland dyld in the corefile / kernel
and throw away that binary list.  Also fix a little bug
with correctly recognizing corefiles using a `main bin spec`
LC_NOTE that explicitly declare that this is a userland
corefile.

LocateSymbolFileMacOSX.cpp's Symbols::DownloadObjectAndSymbolFile
clarify the comments on how the force_lookup and how the
dbgshell_command local both have the same effect.

In PlatformDarwinKernel::LoadPlatformBinaryAndSetup, don't
log a message unless we actually found a kernel fileset.

Reorganize ObjectFileMachO::LoadCoreFileImages so that it delegates
binary searching to DynamicLoader::LoadBinaryWithUUIDAndAddress and
doesn't duplicate those searches.  For searches that fail, we would
perform them multiple times in both methods.  When we have the
mach-o segment vmaddrs for a binary, don't let LoadBinaryWithUUIDAndAddress
load the binary first at its mach-o header address in the Target;
we'll load the segments at the correct addresses individually later
in this method.

DynamicLoaderDarwin::ImageInfo::PutToLog fix a LLDB_LOG logging
formatter.

In DynamicLoader::LoadBinaryWithUUIDAndAddress, instead of using
Target::GetOrCreateModule as a way to find a binary already registered
in lldb's global module cache (and implicitly add it to the Target
image list), use ModuleList::GetSharedModule() which only searches
the global module cache, don't add it to the Target.  We may not
want to add an unstripped binary to the Target.

Add a call to Symbols::DownloadObjectAndSymbolFile() even if
"force_symbol_search" isn't set -- this will turn into a
DebugSymbols call / Spotlight search on a macOS system, which
we want.

Only set the Module's LoadAddress if the caller asked us to do that.

Differential Revision: https://reviews.llvm.org/D150928
rdar://109186357

Added: 


Modified: 
lldb/include/lldb/Target/DynamicLoader.h
lldb/source/Core/DynamicLoader.cpp
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
lldb/source/Symbol/LocateSymbolFileMacOSX.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/DynamicLoader.h 
b/lldb/include/lldb/Target/DynamicLoader.h
index 22d047ab4b616..3aa92398d0130 100644
--- a/lldb/include/lldb/Target/DynamicLoader.h
+++ b/lldb/include/lldb/Target/DynamicLoader.h
@@ -256,11 +256,21 @@ class DynamicLoader : public PluginInterface {
   /// to the Target.  The caller may prefer to batch up these when loading
   /// multiple binaries.
   ///
+  /// \param[in] set_address_in_target
+  /// Whether the address of the binary should be set in the Target if it
+  /// is added.  The caller may want to set the section addresses
+  /// individually, instead of loading the binary the entire based on the
+  /// start address or slide.  The caller is responsible for setting the
+  /// load address for the binary or its segments in the Target if it 
passes
+  /// true.
+  ///
   /// \return
   /// Returns a shared pointer for the Module that has been added.
-  static lldb::ModuleSP LoadBinaryWithUUIDAndAddress(
-  Process *process, llvm::StringRef name, UUID uuid, lldb::addr_t value,
-  bool value_is_offset, bool force_symbol_search, bool notify);
+  static lldb::ModuleSP
+  LoadBinaryWithUUIDAndAddress(Process *process, llvm::StringRef name,
+   UUID uuid, lldb::addr_t value,
+   bool value_is_offset, bool force_symbol_search,
+   bool notify, bool set_address_in_target);
 
   /// Get information about the shared cache for a process, if possible.
   ///

diff  --git a/lldb/source/Core/DynamicLoader.cpp 
b/lldb/source/Core/DynamicLoader.cpp
index 8849ccedbd481..2e5378f654a51 100644
--- a/lldb/source/Core/DynamicLoader.cpp
+++ b/lldb/source/Core/DynamicLoader.cpp
@@ -187,14 +187,13 @@ static ModuleSP ReadUnnamedMemoryModule(Process *process, 
addr_t addr,
 
 ModuleSP DynamicLoader::LoadBinaryWithUUIDAndAddress(
 Process *process, llvm::StringRef name, UUID uuid, addr_t value,
-bool value_is_offset, bool force_symbol_search, bool notify) {
+bool value_is_offset, bool force_symbol_search, bool 

[Lldb-commits] [PATCH] D150928: Two bug fixes for loading process save-core created core files, plus perf improvements

[Jason Molenda via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG48a12ae8212c: Fix a few bugs with Mach-O corefile loading, 
plus perf (authored by jasonmolenda).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150928

Files:
  lldb/include/lldb/Target/DynamicLoader.h
  lldb/source/Core/DynamicLoader.cpp
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
  lldb/source/Symbol/LocateSymbolFileMacOSX.cpp

Index: lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
===
--- lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
+++ lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
@@ -559,14 +559,17 @@
   const UUID *uuid_ptr = module_spec.GetUUIDPtr();
   const FileSpec *file_spec_ptr = module_spec.GetFileSpecPtr();
 
+  // If \a dbgshell_command is set, the user has specified
+  // forced symbol lookup via that command.  We'll get the
+  // path back from GetDsymForUUIDExecutable() later.
   llvm::StringRef dbgshell_command = GetDbgShellCommand();
 
-  // When dbgshell_command is empty, the user has not enabled the use of an
-  // external program to find the symbols, don't run it for them.
+  // If forced lookup isn't set, by the user's \a dbgshell_command or
+  // by the \a force_lookup argument, exit this method.
   if (!force_lookup && dbgshell_command.empty())
 return false;
 
-  // We need a UUID or valid (existing FileSpec.
+  // We need a UUID or valid existing FileSpec.
   if (!uuid_ptr &&
   (!file_spec_ptr || !FileSystem::Instance().Exists(*file_spec_ptr)))
 return false;
Index: lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
===
--- lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
+++ lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
@@ -252,20 +252,20 @@
   m_mach_kernel_addr = objfile_binary_value;
   m_dyld_plugin_name = DynamicLoaderDarwinKernel::GetPluginNameStatic();
   found_main_binary_definitively = true;
+} else if (type == ObjectFile::eBinaryTypeUser) {
+  m_dyld_addr = objfile_binary_value;
+  m_dyld_plugin_name = DynamicLoaderMacOSXDYLD::GetPluginNameStatic();
 } else {
   const bool force_symbol_search = true;
   const bool notify = true;
+  const bool set_address_in_target = true;
   if (DynamicLoader::LoadBinaryWithUUIDAndAddress(
   this, llvm::StringRef(), objfile_binary_uuid,
   objfile_binary_value, objfile_binary_value_is_offset,
-  force_symbol_search, notify)) {
+  force_symbol_search, notify, set_address_in_target)) {
 found_main_binary_definitively = true;
 m_dyld_plugin_name = DynamicLoaderStatic::GetPluginNameStatic();
   }
-  if (type == ObjectFile::eBinaryTypeUser) {
-m_dyld_addr = objfile_binary_value;
-m_dyld_plugin_name = DynamicLoaderMacOSXDYLD::GetPluginNameStatic();
-  }
 }
   }
 
@@ -314,9 +314,11 @@
   const bool value_is_offset = false;
   const bool force_symbol_search = true;
   const bool notify = true;
+  const bool set_address_in_target = true;
   if (DynamicLoader::LoadBinaryWithUUIDAndAddress(
   this, llvm::StringRef(), ident_uuid, ident_binary_addr,
-  value_is_offset, force_symbol_search, notify)) {
+  value_is_offset, force_symbol_search, notify,
+  set_address_in_target)) {
 found_main_binary_definitively = true;
 m_dyld_plugin_name = DynamicLoaderStatic::GetPluginNameStatic();
   }
@@ -325,7 +327,10 @@
 
   // Finally, load any binaries noted by "load binary" LC_NOTEs in the
   // corefile
-  core_objfile->LoadCoreFileImages(*this);
+  if (core_objfile->LoadCoreFileImages(*this)) {
+found_main_binary_definitively = true;
+m_dyld_plugin_name = DynamicLoaderStatic::GetPluginNameStatic();
+  }
 
   // LoadCoreFileImges may have set the dynamic loader, e.g. in
   // PlatformDarwinKernel::LoadPlatformBinaryAndSetup().
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -995,9 +995,11 @@
 if (standalone_uuid.IsValid()) {
   const bool force_symbol_search = true;
   const bool notify = true;
+  const bool set_address_in_target = true;
   DynamicLoader::LoadBinaryWithUUIDAndAddress(
   this, "", standalone_uuid, standalone_value,
-  standalone_value_is_offset, fo

[Lldb-commits] [lldb] 061a839 - [lldb] Prevent dwim-print from showing kNoResult error

[Dave Lee via lldb-commits]

Author: Dave Lee
Date: 2023-05-30T15:38:42-07:00
New Revision: 061a839033dc5f11c4e43fb64ed49cc85e1e5f32

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

LOG: [lldb] Prevent dwim-print from showing kNoResult error

Expression evaluation for `void` valued expressions sets an error using the 
`kNoResult`
code. Like the `expression` command, `dwim-print` should also not print such 
errors.

Before:

```
(lldb) dwim-print (void)printf("hi\n")
hi
Error: 'unknown error'
```

After:

```
(lldb) dwim-print (void)printf("hi\n")
hi
```

rdar://109746544

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

Added: 


Modified: 
lldb/source/Commands/CommandObjectDWIMPrint.cpp
lldb/test/API/commands/dwim-print/TestDWIMPrint.py

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp 
b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index 8fc702a1a220e..7cb95fd622ba1 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -11,6 +11,7 @@
 #include "lldb/Core/ValueObject.h"
 #include "lldb/DataFormatters/DumpValueObjectOptions.h"
 #include "lldb/Expression/ExpressionVariable.h"
+#include "lldb/Expression/UserExpression.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandObject.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
@@ -135,7 +136,8 @@ bool CommandObjectDWIMPrint::DoExecute(StringRef command,
 expr);
   }
 
-  valobj_sp->Dump(result.GetOutputStream(), dump_options);
+  if (valobj_sp->GetError().GetError() != UserExpression::kNoResult)
+valobj_sp->Dump(result.GetOutputStream(), dump_options);
 
   if (suppress_result)
 if (auto result_var_sp =

diff  --git a/lldb/test/API/commands/dwim-print/TestDWIMPrint.py 
b/lldb/test/API/commands/dwim-print/TestDWIMPrint.py
index f2799ef53d49c..9cb99a2a817d6 100644
--- a/lldb/test/API/commands/dwim-print/TestDWIMPrint.py
+++ b/lldb/test/API/commands/dwim-print/TestDWIMPrint.py
@@ -138,3 +138,9 @@ def test_summary_strings(self):
 self.runCmd("type summary add -e -s 'stub summary' Structure")
 self._expect_cmd(f"dwim-print s", "frame variable")
 self._expect_cmd(f"dwim-print (struct Structure)s", "expression")
+
+def test_void_result(self):
+"""Test dwim-print does not surface an error message for void 
expressions."""
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here", 
lldb.SBFileSpec("main.c"))
+self.expect("dwim-print (void)15", matching=False, 
patterns=["(?i)error"])



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

[Lldb-commits] [PATCH] D151351: [lldb] Prevent dwim-print from showing kNoResult error

[Dave Lee via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG061a839033dc: [lldb] Prevent dwim-print from showing 
kNoResult error (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151351

Files:
  lldb/source/Commands/CommandObjectDWIMPrint.cpp
  lldb/test/API/commands/dwim-print/TestDWIMPrint.py


Index: lldb/test/API/commands/dwim-print/TestDWIMPrint.py
===
--- lldb/test/API/commands/dwim-print/TestDWIMPrint.py
+++ lldb/test/API/commands/dwim-print/TestDWIMPrint.py
@@ -138,3 +138,9 @@
 self.runCmd("type summary add -e -s 'stub summary' Structure")
 self._expect_cmd(f"dwim-print s", "frame variable")
 self._expect_cmd(f"dwim-print (struct Structure)s", "expression")
+
+def test_void_result(self):
+"""Test dwim-print does not surface an error message for void 
expressions."""
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here", 
lldb.SBFileSpec("main.c"))
+self.expect("dwim-print (void)15", matching=False, 
patterns=["(?i)error"])
Index: lldb/source/Commands/CommandObjectDWIMPrint.cpp
===
--- lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -11,6 +11,7 @@
 #include "lldb/Core/ValueObject.h"
 #include "lldb/DataFormatters/DumpValueObjectOptions.h"
 #include "lldb/Expression/ExpressionVariable.h"
+#include "lldb/Expression/UserExpression.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandObject.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
@@ -135,7 +136,8 @@
 expr);
   }
 
-  valobj_sp->Dump(result.GetOutputStream(), dump_options);
+  if (valobj_sp->GetError().GetError() != UserExpression::kNoResult)
+valobj_sp->Dump(result.GetOutputStream(), dump_options);
 
   if (suppress_result)
 if (auto result_var_sp =


Index: lldb/test/API/commands/dwim-print/TestDWIMPrint.py
===
--- lldb/test/API/commands/dwim-print/TestDWIMPrint.py
+++ lldb/test/API/commands/dwim-print/TestDWIMPrint.py
@@ -138,3 +138,9 @@
 self.runCmd("type summary add -e -s 'stub summary' Structure")
 self._expect_cmd(f"dwim-print s", "frame variable")
 self._expect_cmd(f"dwim-print (struct Structure)s", "expression")
+
+def test_void_result(self):
+"""Test dwim-print does not surface an error message for void expressions."""
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here", lldb.SBFileSpec("main.c"))
+self.expect("dwim-print (void)15", matching=False, patterns=["(?i)error"])
Index: lldb/source/Commands/CommandObjectDWIMPrint.cpp
===
--- lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -11,6 +11,7 @@
 #include "lldb/Core/ValueObject.h"
 #include "lldb/DataFormatters/DumpValueObjectOptions.h"
 #include "lldb/Expression/ExpressionVariable.h"
+#include "lldb/Expression/UserExpression.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandObject.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
@@ -135,7 +136,8 @@
 expr);
   }
 
-  valobj_sp->Dump(result.GetOutputStream(), dump_options);
+  if (valobj_sp->GetError().GetError() != UserExpression::kNoResult)
+valobj_sp->Dump(result.GetOutputStream(), dump_options);
 
   if (suppress_result)
 if (auto result_var_sp =
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D151762: [lldb] Remove __FUNCTION__ from log messages in lldbHost (NFC)

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added a reviewer: LLDB.
Herald added a project: All.
JDevlieghere requested review of this revision.

LLDB's logging infrastructure supports prepending log messages with the name of 
the file and function that generates the log (see `help log enable`). Therefore 
it's unnecessary to include the current `__FUNCTION__` in the log message 
itself. This patch removes `__FUNCTION__` from log messages in the `Host` 
library.


https://reviews.llvm.org/D151762

Files:
  lldb/source/Host/common/HostInfoBase.cpp
  lldb/source/Host/common/NativeRegisterContext.cpp
  lldb/source/Host/common/TCPSocket.cpp
  lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm

Index: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
===
--- lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
+++ lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
@@ -152,8 +152,7 @@
 FileSystem::Instance().Resolve(support_dir_spec);
 if (!FileSystem::Instance().IsDirectory(support_dir_spec)) {
   Log *log = GetLog(LLDBLog::Host);
-  LLDB_LOGF(log, "HostInfoMacOSX::%s(): failed to find support directory",
-__FUNCTION__);
+  LLDB_LOG(log, "failed to find support directory");
   return false;
 }
 
Index: lldb/source/Host/common/TCPSocket.cpp
===
--- lldb/source/Host/common/TCPSocket.cpp
+++ lldb/source/Host/common/TCPSocket.cpp
@@ -150,7 +150,7 @@
 Status TCPSocket::Connect(llvm::StringRef name) {
 
   Log *log = GetLog(LLDBLog::Communication);
-  LLDB_LOGF(log, "TCPSocket::%s (host/port = %s)", __FUNCTION__, name.data());
+  LLDB_LOGF(log, "Connect to host/port %s", name.data());
 
   Status error;
   llvm::Expected host_port = DecodeHostAndPort(name);
@@ -189,7 +189,7 @@
 
 Status TCPSocket::Listen(llvm::StringRef name, int backlog) {
   Log *log = GetLog(LLDBLog::Connection);
-  LLDB_LOGF(log, "TCPSocket::%s (%s)", __FUNCTION__, name.data());
+  LLDB_LOGF(log, "Listen to %s", name.data());
 
   Status error;
   llvm::Expected host_port = DecodeHostAndPort(name);
Index: lldb/source/Host/common/NativeRegisterContext.cpp
===
--- lldb/source/Host/common/NativeRegisterContext.cpp
+++ lldb/source/Host/common/NativeRegisterContext.cpp
@@ -125,15 +125,12 @@
 
   uint32_t reg = ConvertRegisterKindToRegisterNumber(eRegisterKindGeneric,
  LLDB_REGNUM_GENERIC_PC);
-  LLDB_LOGF(log,
-"NativeRegisterContext::%s using reg index %" PRIu32
-" (default %" PRIu64 ")",
-__FUNCTION__, reg, fail_value);
+  LLDB_LOGF(log, "Using reg index %" PRIu32 " (default %" PRIu64 ")", reg,
+fail_value);
 
   const uint64_t retval = ReadRegisterAsUnsigned(reg, fail_value);
 
-  LLDB_LOGF(log, "NativeRegisterContext::%s " PRIu32 " retval %" PRIu64,
-__FUNCTION__, retval);
+  LLDB_LOGF(log, PRIu32 " retval %" PRIu64, retval);
 
   return retval;
 }
@@ -203,18 +200,15 @@
 Status error = ReadRegister(reg_info, value);
 if (error.Success()) {
   LLDB_LOGF(log,
-"NativeRegisterContext::%s ReadRegister() succeeded, value "
+"Read register succeeded: value "
 "%" PRIu64,
-__FUNCTION__, value.GetAsUInt64());
+value.GetAsUInt64());
   return value.GetAsUInt64();
 } else {
-  LLDB_LOGF(log,
-"NativeRegisterContext::%s ReadRegister() failed, error %s",
-__FUNCTION__, error.AsCString());
+  LLDB_LOGF(log, "Read register failed: error %s", error.AsCString());
 }
   } else {
-LLDB_LOGF(log, "NativeRegisterContext::%s ReadRegister() null reg_info",
-  __FUNCTION__);
+LLDB_LOGF(log, "Read register failed: null reg_info");
   }
   return fail_value;
 }
@@ -222,7 +216,7 @@
 Status NativeRegisterContext::WriteRegisterFromUnsigned(uint32_t reg,
 uint64_t uval) {
   if (reg == LLDB_INVALID_REGNUM)
-return Status("NativeRegisterContext::%s (): reg is invalid", __FUNCTION__);
+return Status("Write register failed: reg is invalid");
   return WriteRegisterFromUnsigned(GetRegisterInfoAtIndex(reg), uval);
 }
 
Index: lldb/source/Host/common/HostInfoBase.cpp
===
--- lldb/source/Host/common/HostInfoBase.cpp
+++ lldb/source/Host/common/HostInfoBase.cpp
@@ -225,24 +225,20 @@
 return false;
 
   std::string raw_path = lldb_file_spec.GetPath();
-  LLDB_LOGF(log,
-"HostInfo::%s() attempting to "
-"derive the path %s relative to liblldb install path: %s",
-__FUNCTION__, dir.data(), raw_path.c_str());
+  LLDB_LOGF(
+  log,
+  "Attempting to derive the path %s relative to liblldb install path: %s",
+  dir.

[Lldb-commits] [PATCH] D151762: [lldb] Remove __FUNCTION__ from log messages in lldbHost (NFC)

[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 change itself looks fine and is fairly mechanical. Easy to verify.

One thing that I see here is we're treating the result of 
`llvm::StringRef::data()` as a c-string which isn't guaranteed to be true. That 
makes me a little nervous. As frustrating as temporary allocations are, the 
right thing to do would be something like `foo.str().c_str()` where `foo` is a 
StringRef. If you think it's appropriate, can you change those instances in 
this patch? If not we can do it in a follow-up.


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

https://reviews.llvm.org/D151762

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

[Lldb-commits] [PATCH] D151762: [lldb] Remove __FUNCTION__ from log messages in lldbHost (NFC)

[Med Ismail Bennani via Phabricator via lldb-commits]

mib accepted this revision as: mib.
mib added a comment.

LGTM! Are you considering doing it for the rest of the codebase as well ? That 
would be very nice :)


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

https://reviews.llvm.org/D151762

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

[Lldb-commits] [PATCH] D151764: [lldb] Support file and function names in LLDB_LOGF macro

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added a reviewer: LLDB.
Herald added a project: All.
JDevlieghere requested review of this revision.

LLDB's logging machinery supports prepending log messages with the name of the 
file and function that generates the log. However, currently this functionality 
is limited to the `LLDB_LOG` macro. I meant to do this as a follow up to D65128 
 but never got around to it.


https://reviews.llvm.org/D151764

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


Index: lldb/source/Utility/Log.cpp
===
--- lldb/source/Utility/Log.cpp
+++ lldb/source/Utility/Log.cpp
@@ -155,6 +155,21 @@
   PutString(Content);
 }
 
+void Log::Formatf(llvm::StringRef file, llvm::StringRef function,
+  const char *format, ...) {
+  va_list args;
+  va_start(args, format);
+  VAFormatf(file, function, format, args);
+  va_end(args);
+}
+
+void Log::VAFormatf(llvm::StringRef file, llvm::StringRef function,
+const char *format, va_list args) {
+  llvm::SmallString<64> Content;
+  lldb_private::VASprintf(Content, format, args);
+  Format(file, function, llvm::formatv("{0}", Content));
+}
+
 // Printing of errors that are not fatal.
 void Log::Error(const char *format, ...) {
   va_list args;
Index: lldb/include/lldb/Utility/Log.h
===
--- lldb/include/lldb/Utility/Log.h
+++ lldb/include/lldb/Utility/Log.h
@@ -232,6 +232,9 @@
  std::forward(args)...));
   }
 
+  void Formatf(llvm::StringRef file, llvm::StringRef function,
+   const char *format, ...) __attribute__((format(printf, 4, 5)));
+
   /// Prefer using LLDB_LOGF whenever possible.
   void Printf(const char *format, ...) __attribute__((format(printf, 2, 3)));
 
@@ -249,6 +252,8 @@
 
   void VAPrintf(const char *format, va_list args);
   void VAError(const char *format, va_list args);
+  void VAFormatf(llvm::StringRef file, llvm::StringRef function,
+ const char *format, va_list args);
 
 private:
   Channel &m_channel;
@@ -345,7 +350,7 @@
   do { 
\
 ::lldb_private::Log *log_private = (log);  
\
 if (log_private)   
\
-  log_private->Printf(__VA_ARGS__);
\
+  log_private->Formatf(__FILE__, __func__, __VA_ARGS__);   
\
   } while (0)
 
 #define LLDB_LOGV(log, ...)
\


Index: lldb/source/Utility/Log.cpp
===
--- lldb/source/Utility/Log.cpp
+++ lldb/source/Utility/Log.cpp
@@ -155,6 +155,21 @@
   PutString(Content);
 }
 
+void Log::Formatf(llvm::StringRef file, llvm::StringRef function,
+  const char *format, ...) {
+  va_list args;
+  va_start(args, format);
+  VAFormatf(file, function, format, args);
+  va_end(args);
+}
+
+void Log::VAFormatf(llvm::StringRef file, llvm::StringRef function,
+const char *format, va_list args) {
+  llvm::SmallString<64> Content;
+  lldb_private::VASprintf(Content, format, args);
+  Format(file, function, llvm::formatv("{0}", Content));
+}
+
 // Printing of errors that are not fatal.
 void Log::Error(const char *format, ...) {
   va_list args;
Index: lldb/include/lldb/Utility/Log.h
===
--- lldb/include/lldb/Utility/Log.h
+++ lldb/include/lldb/Utility/Log.h
@@ -232,6 +232,9 @@
  std::forward(args)...));
   }
 
+  void Formatf(llvm::StringRef file, llvm::StringRef function,
+   const char *format, ...) __attribute__((format(printf, 4, 5)));
+
   /// Prefer using LLDB_LOGF whenever possible.
   void Printf(const char *format, ...) __attribute__((format(printf, 2, 3)));
 
@@ -249,6 +252,8 @@
 
   void VAPrintf(const char *format, va_list args);
   void VAError(const char *format, va_list args);
+  void VAFormatf(llvm::StringRef file, llvm::StringRef function,
+ const char *format, va_list args);
 
 private:
   Channel &m_channel;
@@ -345,7 +350,7 @@
   do { \
 ::lldb_private::Log *log_private = (log);  \
 if (log_private)   \
-  log_private->Printf(__VA_ARGS__);\
+  log_private->Formatf(__FILE__, __func__, __VA_ARGS__);   \
   } while (0)
 
 #define LLDB_LOGV(log, ...)\
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://l

[Lldb-commits] [PATCH] D151764: [lldb] Support file and function names in LLDB_LOGF macro

[Med Ismail Bennani via Phabricator via lldb-commits]

mib accepted this revision as: mib.
mib added a comment.
This revision is now accepted and ready to land.

LGTM! Can we add a test for this ?


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

https://reviews.llvm.org/D151764

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

[Lldb-commits] [lldb] 1418677 - Fix SBValue::FindValue for file static variables

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2023-05-30T17:12:35-07:00
New Revision: 14186773e79b8c6787afac2f9ee69738151377ec

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

LOG: Fix SBValue::FindValue for file static variables

This was just a thinko. The API StackFrame::GetVariableList takes a
bool for "get_file_globals" which if true will also find file statics
and file globals. But we only were passing that as true if the
ValueType was eValueTypeVariableGlobal, which meant that we never find
file statics. It's okay if we cast too wide a net when we do
GetVariableList as later on we check against the ValueType to filter
globals from statics.

There was a test that had a whole bunch of globals and tested
FindValue on all of them, but had no statics. So I just made one of
the globals a file static, which verifies the fix.

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

Added: 


Modified: 
lldb/source/API/SBFrame.cpp
lldb/test/API/python_api/process/TestProcessAPI.py
lldb/test/API/python_api/process/main.cpp

Removed: 




diff  --git a/lldb/source/API/SBFrame.cpp b/lldb/source/API/SBFrame.cpp
index 285469c1063b3..e31297446007a 100644
--- a/lldb/source/API/SBFrame.cpp
+++ b/lldb/source/API/SBFrame.cpp
@@ -602,7 +602,8 @@ SBValue SBFrame::FindValue(const char *name, ValueType 
value_type,
 stop_if_block_is_inlined_function,
 [frame](Variable *v) { return v->IsInScope(frame); },
 &variable_list);
-  if (value_type == eValueTypeVariableGlobal) {
+  if (value_type == eValueTypeVariableGlobal 
+  || value_type == eValueTypeVariableStatic) {
 const bool get_file_globals = true;
 VariableList *frame_vars = frame->GetVariableList(get_file_globals,
   nullptr);

diff  --git a/lldb/test/API/python_api/process/TestProcessAPI.py 
b/lldb/test/API/python_api/process/TestProcessAPI.py
index df41397eb32b8..65330e5163f72 100644
--- a/lldb/test/API/python_api/process/TestProcessAPI.py
+++ b/lldb/test/API/python_api/process/TestProcessAPI.py
@@ -49,8 +49,8 @@ def test_read_memory(self):
 )
 frame = thread.GetFrameAtIndex(0)
 
-# Get the SBValue for the global variable 'my_char'.
-val = frame.FindValue("my_char", lldb.eValueTypeVariableGlobal)
+# Get the SBValue for the file static variable 'my_char'.
+val = frame.FindValue("my_char", lldb.eValueTypeVariableStatic)
 self.DebugSBValue(val)
 
 # Due to the typemap magic (see lldb.swig), we pass in 1 to ReadMemory 
and
@@ -149,8 +149,8 @@ def test_write_memory(self):
 )
 frame = thread.GetFrameAtIndex(0)
 
-# Get the SBValue for the global variable 'my_char'.
-val = frame.FindValue("my_char", lldb.eValueTypeVariableGlobal)
+# Get the SBValue for the static variable 'my_char'.
+val = frame.FindValue("my_char", lldb.eValueTypeVariableStatic)
 self.DebugSBValue(val)
 
 # If the variable does not have a load address, there's no sense

diff  --git a/lldb/test/API/python_api/process/main.cpp 
b/lldb/test/API/python_api/process/main.cpp
index 07cde05e2a054..54bf3590ad431 100644
--- a/lldb/test/API/python_api/process/main.cpp
+++ b/lldb/test/API/python_api/process/main.cpp
@@ -3,7 +3,7 @@
 
 // This simple program is to test the lldb Python API related to process.
 
-char my_char = 'u';
+static char my_char = 'u';
 char my_cstring[] = "lldb.SBProcess.ReadCStringFromMemory() works!";
 char *my_char_ptr = (char *)"Does it work?";
 uint32_t my_uint32 = 12345;



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

[Lldb-commits] [PATCH] D151392: Fix SBValue::FindValue for file static variables

[Jim Ingham via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG14186773e79b: Fix SBValue::FindValue for file static 
variables (authored by jingham).

Changed prior to commit:
  https://reviews.llvm.org/D151392?vs=525391&id=526852#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151392

Files:
  lldb/source/API/SBFrame.cpp
  lldb/test/API/python_api/process/TestProcessAPI.py
  lldb/test/API/python_api/process/main.cpp


Index: lldb/test/API/python_api/process/main.cpp
===
--- lldb/test/API/python_api/process/main.cpp
+++ lldb/test/API/python_api/process/main.cpp
@@ -3,7 +3,7 @@
 
 // This simple program is to test the lldb Python API related to process.
 
-char my_char = 'u';
+static char my_char = 'u';
 char my_cstring[] = "lldb.SBProcess.ReadCStringFromMemory() works!";
 char *my_char_ptr = (char *)"Does it work?";
 uint32_t my_uint32 = 12345;
Index: lldb/test/API/python_api/process/TestProcessAPI.py
===
--- lldb/test/API/python_api/process/TestProcessAPI.py
+++ lldb/test/API/python_api/process/TestProcessAPI.py
@@ -49,8 +49,8 @@
 )
 frame = thread.GetFrameAtIndex(0)
 
-# Get the SBValue for the global variable 'my_char'.
-val = frame.FindValue("my_char", lldb.eValueTypeVariableGlobal)
+# Get the SBValue for the file static variable 'my_char'.
+val = frame.FindValue("my_char", lldb.eValueTypeVariableStatic)
 self.DebugSBValue(val)
 
 # Due to the typemap magic (see lldb.swig), we pass in 1 to ReadMemory 
and
@@ -149,8 +149,8 @@
 )
 frame = thread.GetFrameAtIndex(0)
 
-# Get the SBValue for the global variable 'my_char'.
-val = frame.FindValue("my_char", lldb.eValueTypeVariableGlobal)
+# Get the SBValue for the static variable 'my_char'.
+val = frame.FindValue("my_char", lldb.eValueTypeVariableStatic)
 self.DebugSBValue(val)
 
 # If the variable does not have a load address, there's no sense
Index: lldb/source/API/SBFrame.cpp
===
--- lldb/source/API/SBFrame.cpp
+++ lldb/source/API/SBFrame.cpp
@@ -602,7 +602,8 @@
 stop_if_block_is_inlined_function,
 [frame](Variable *v) { return v->IsInScope(frame); },
 &variable_list);
-  if (value_type == eValueTypeVariableGlobal) {
+  if (value_type == eValueTypeVariableGlobal 
+  || value_type == eValueTypeVariableStatic) {
 const bool get_file_globals = true;
 VariableList *frame_vars = frame->GetVariableList(get_file_globals,
   nullptr);


Index: lldb/test/API/python_api/process/main.cpp
===
--- lldb/test/API/python_api/process/main.cpp
+++ lldb/test/API/python_api/process/main.cpp
@@ -3,7 +3,7 @@
 
 // This simple program is to test the lldb Python API related to process.
 
-char my_char = 'u';
+static char my_char = 'u';
 char my_cstring[] = "lldb.SBProcess.ReadCStringFromMemory() works!";
 char *my_char_ptr = (char *)"Does it work?";
 uint32_t my_uint32 = 12345;
Index: lldb/test/API/python_api/process/TestProcessAPI.py
===
--- lldb/test/API/python_api/process/TestProcessAPI.py
+++ lldb/test/API/python_api/process/TestProcessAPI.py
@@ -49,8 +49,8 @@
 )
 frame = thread.GetFrameAtIndex(0)
 
-# Get the SBValue for the global variable 'my_char'.
-val = frame.FindValue("my_char", lldb.eValueTypeVariableGlobal)
+# Get the SBValue for the file static variable 'my_char'.
+val = frame.FindValue("my_char", lldb.eValueTypeVariableStatic)
 self.DebugSBValue(val)
 
 # Due to the typemap magic (see lldb.swig), we pass in 1 to ReadMemory and
@@ -149,8 +149,8 @@
 )
 frame = thread.GetFrameAtIndex(0)
 
-# Get the SBValue for the global variable 'my_char'.
-val = frame.FindValue("my_char", lldb.eValueTypeVariableGlobal)
+# Get the SBValue for the static variable 'my_char'.
+val = frame.FindValue("my_char", lldb.eValueTypeVariableStatic)
 self.DebugSBValue(val)
 
 # If the variable does not have a load address, there's no sense
Index: lldb/source/API/SBFrame.cpp
===
--- lldb/source/API/SBFrame.cpp
+++ lldb/source/API/SBFrame.cpp
@@ -602,7 +602,8 @@
 stop_if_block_is_inlined_function,
 [frame](Variable *v) { return v->IsInScope(frame); },
 &variable_list);
-  if (value_type == eValueTypeVariableGlobal) {
+  if (value_typ

[Lldb-commits] [PATCH] D151764: [lldb] Support file and function names in LLDB_LOGF macro

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 526853.
JDevlieghere added a comment.

Add unit test


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

https://reviews.llvm.org/D151764

Files:
  lldb/include/lldb/Utility/Log.h
  lldb/source/Utility/Log.cpp
  lldb/unittests/Utility/LogTest.cpp

Index: lldb/unittests/Utility/LogTest.cpp
===
--- lldb/unittests/Utility/LogTest.cpp
+++ lldb/unittests/Utility/LogTest.cpp
@@ -100,6 +100,7 @@
   Log *getLog() { return m_log; }
   llvm::StringRef takeOutput();
   llvm::StringRef logAndTakeOutput(llvm::StringRef Message);
+  llvm::StringRef logAndTakeOutputf(llvm::StringRef Message);
 
 public:
   void SetUp() override;
@@ -136,6 +137,12 @@
   return takeOutput();
 }
 
+llvm::StringRef
+LogChannelEnabledTest::logAndTakeOutputf(llvm::StringRef Message) {
+  LLDB_LOGF(m_log, "%s", Message.str().c_str());
+  return takeOutput();
+}
+
 TEST(LogTest, LLDB_LOG_nullptr) {
   Log *log = nullptr;
   LLDB_LOG(log, "{0}", 0); // Shouldn't crash
@@ -296,6 +303,21 @@
 EXPECT_STREQ("logAndTakeOutput", Function);
   }
 
+  {
+EXPECT_TRUE(EnableChannel(getLogHandler(),
+  LLDB_LOG_OPTION_PREPEND_FILE_FUNCTION, "chan", {},
+  Err));
+llvm::StringRef Msg = logAndTakeOutputf("Hello World");
+char File[12];
+char Function[17];
+
+sscanf(Msg.str().c_str(),
+   "%[^:]:%s Hello World", File,
+   Function);
+EXPECT_STRCASEEQ("LogTest.cpp", File);
+EXPECT_STREQ("logAndTakeOutputf", Function);
+  }
+
   EXPECT_TRUE(EnableChannel(getLogHandler(),
 LLDB_LOG_OPTION_PREPEND_PROC_AND_THREAD, "chan", {},
 Err));
Index: lldb/source/Utility/Log.cpp
===
--- lldb/source/Utility/Log.cpp
+++ lldb/source/Utility/Log.cpp
@@ -155,6 +155,21 @@
   PutString(Content);
 }
 
+void Log::Formatf(llvm::StringRef file, llvm::StringRef function,
+  const char *format, ...) {
+  va_list args;
+  va_start(args, format);
+  VAFormatf(file, function, format, args);
+  va_end(args);
+}
+
+void Log::VAFormatf(llvm::StringRef file, llvm::StringRef function,
+const char *format, va_list args) {
+  llvm::SmallString<64> Content;
+  lldb_private::VASprintf(Content, format, args);
+  Format(file, function, llvm::formatv("{0}", Content));
+}
+
 // Printing of errors that are not fatal.
 void Log::Error(const char *format, ...) {
   va_list args;
Index: lldb/include/lldb/Utility/Log.h
===
--- lldb/include/lldb/Utility/Log.h
+++ lldb/include/lldb/Utility/Log.h
@@ -232,6 +232,9 @@
  std::forward(args)...));
   }
 
+  void Formatf(llvm::StringRef file, llvm::StringRef function,
+   const char *format, ...) __attribute__((format(printf, 4, 5)));
+
   /// Prefer using LLDB_LOGF whenever possible.
   void Printf(const char *format, ...) __attribute__((format(printf, 2, 3)));
 
@@ -249,6 +252,8 @@
 
   void VAPrintf(const char *format, va_list args);
   void VAError(const char *format, va_list args);
+  void VAFormatf(llvm::StringRef file, llvm::StringRef function,
+ const char *format, va_list args);
 
 private:
   Channel &m_channel;
@@ -345,7 +350,7 @@
   do { \
 ::lldb_private::Log *log_private = (log);  \
 if (log_private)   \
-  log_private->Printf(__VA_ARGS__);\
+  log_private->Formatf(__FILE__, __func__, __VA_ARGS__);   \
   } while (0)
 
 #define LLDB_LOGV(log, ...)\
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D151762: [lldb] Remove __FUNCTION__ from log messages in lldbHost (NFC)

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D151762#4382679 , @mib wrote:

> LGTM! Are you considering doing it for the rest of the codebase as well ? 
> That would be very nice :)

Yup, that's the goal. I wanted to gather some initial feedback before I go 
through the other libs.


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

https://reviews.llvm.org/D151762

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

[Lldb-commits] [PATCH] D151762: [lldb] Remove __FUNCTION__ from log messages in lldbHost (NFC)

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 526854.
JDevlieghere added a comment.

Use `LLDB_LOG` when `StringRef`s are involved.


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

https://reviews.llvm.org/D151762

Files:
  lldb/source/Host/common/HostInfoBase.cpp
  lldb/source/Host/common/NativeRegisterContext.cpp
  lldb/source/Host/common/TCPSocket.cpp
  lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm

Index: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
===
--- lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
+++ lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
@@ -152,8 +152,7 @@
 FileSystem::Instance().Resolve(support_dir_spec);
 if (!FileSystem::Instance().IsDirectory(support_dir_spec)) {
   Log *log = GetLog(LLDBLog::Host);
-  LLDB_LOGF(log, "HostInfoMacOSX::%s(): failed to find support directory",
-__FUNCTION__);
+  LLDB_LOG(log, "failed to find support directory");
   return false;
 }
 
@@ -342,8 +341,8 @@
 HostInfo::GetSDKRoot(SDKOptions{XcodeSDK::GetAnyMacOS()});
 if (!sdk_path_or_err) {
   Log *log = GetLog(LLDBLog::Host);
-  LLDB_LOGF(log, "Error while searching for Xcode SDK: %s",
-toString(sdk_path_or_err.takeError()).c_str());
+  LLDB_LOG_ERROR(log, sdk_path_or_err.takeError(),
+ "Error while searching for Xcode SDK: {0}");
   return;
 }
 FileSpec fspec(*sdk_path_or_err);
@@ -493,7 +492,7 @@
   if (!path.empty())
 break;
 }
-LLDB_LOGF(log, "Couldn't find SDK %s on host", sdk_name.c_str());
+LLDB_LOG(log, "Couldn't find SDK {0} on host", sdk_name);
 
 // Try without the version.
 if (!info.version.empty()) {
@@ -507,13 +506,13 @@
 break;
 }
 
-LLDB_LOGF(log, "Couldn't find any matching SDK on host");
+LLDB_LOG(log, "Couldn't find any matching SDK on host");
 return "";
   }
 
   // Whatever is left in output should be a valid path.
   if (!FileSystem::Instance().Exists(path)) {
-LLDB_LOGF(log, "SDK returned by xcrun doesn't exist");
+LLDB_LOG(log, "SDK returned by xcrun doesn't exist");
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
"SDK returned by xcrun doesn't exist");
   }
Index: lldb/source/Host/common/TCPSocket.cpp
===
--- lldb/source/Host/common/TCPSocket.cpp
+++ lldb/source/Host/common/TCPSocket.cpp
@@ -150,7 +150,7 @@
 Status TCPSocket::Connect(llvm::StringRef name) {
 
   Log *log = GetLog(LLDBLog::Communication);
-  LLDB_LOGF(log, "TCPSocket::%s (host/port = %s)", __FUNCTION__, name.data());
+  LLDB_LOG(log, "Connect to host/port {0}", name);
 
   Status error;
   llvm::Expected host_port = DecodeHostAndPort(name);
@@ -189,7 +189,7 @@
 
 Status TCPSocket::Listen(llvm::StringRef name, int backlog) {
   Log *log = GetLog(LLDBLog::Connection);
-  LLDB_LOGF(log, "TCPSocket::%s (%s)", __FUNCTION__, name.data());
+  LLDB_LOG(log, "Listen to {0}", name);
 
   Status error;
   llvm::Expected host_port = DecodeHostAndPort(name);
Index: lldb/source/Host/common/NativeRegisterContext.cpp
===
--- lldb/source/Host/common/NativeRegisterContext.cpp
+++ lldb/source/Host/common/NativeRegisterContext.cpp
@@ -125,15 +125,12 @@
 
   uint32_t reg = ConvertRegisterKindToRegisterNumber(eRegisterKindGeneric,
  LLDB_REGNUM_GENERIC_PC);
-  LLDB_LOGF(log,
-"NativeRegisterContext::%s using reg index %" PRIu32
-" (default %" PRIu64 ")",
-__FUNCTION__, reg, fail_value);
+  LLDB_LOGF(log, "Using reg index %" PRIu32 " (default %" PRIu64 ")", reg,
+fail_value);
 
   const uint64_t retval = ReadRegisterAsUnsigned(reg, fail_value);
 
-  LLDB_LOGF(log, "NativeRegisterContext::%s " PRIu32 " retval %" PRIu64,
-__FUNCTION__, retval);
+  LLDB_LOGF(log, PRIu32 " retval %" PRIu64, retval);
 
   return retval;
 }
@@ -203,18 +200,15 @@
 Status error = ReadRegister(reg_info, value);
 if (error.Success()) {
   LLDB_LOGF(log,
-"NativeRegisterContext::%s ReadRegister() succeeded, value "
+"Read register succeeded: value "
 "%" PRIu64,
-__FUNCTION__, value.GetAsUInt64());
+value.GetAsUInt64());
   return value.GetAsUInt64();
 } else {
-  LLDB_LOGF(log,
-"NativeRegisterContext::%s ReadRegister() failed, error %s",
-__FUNCTION__, error.AsCString());
+  LLDB_LOGF(log, "Read register failed: error %s", error.AsCString());
 }
   } else {
-LLDB_LOGF(log, "NativeRegisterContext::%s ReadRegister() null reg_info",
-  __FUNCTION__);
+LLDB_LOGF(log, "Read register failed: null reg_info");
   }
   return fail_value;
 }
@@ -222,7 

[Lldb-commits] [lldb] be9b79f - [lldb] Remove commented out code/logging in BreakpointSiteList (NFC)

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2023-05-30T17:27:58-07:00
New Revision: be9b79fb14ce0776e147860c9fbffc84ea7b39e6

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

LOG: [lldb] Remove commented out code/logging in BreakpointSiteList (NFC)

Added: 


Modified: 
lldb/source/Breakpoint/BreakpointSiteList.cpp

Removed: 




diff  --git a/lldb/source/Breakpoint/BreakpointSiteList.cpp 
b/lldb/source/Breakpoint/BreakpointSiteList.cpp
index 32a2f24d411a1..ab15da82ea450 100644
--- a/lldb/source/Breakpoint/BreakpointSiteList.cpp
+++ b/lldb/source/Breakpoint/BreakpointSiteList.cpp
@@ -48,15 +48,8 @@ bool BreakpointSiteList::ShouldStop(StoppointCallbackContext 
*context,
   return true;
 }
 lldb::break_id_t BreakpointSiteList::FindIDByAddress(lldb::addr_t addr) {
-  BreakpointSiteSP bp = FindByAddress(addr);
-  if (bp) {
-// DBLogIf(PD_LOG_BREAKPOINTS, "BreakpointSiteList::%s ( addr = 0x%8.8"
-// PRIx64 " ) => %u", __FUNCTION__, (uint64_t)addr, bp->GetID());
+  if (BreakpointSiteSP bp = FindByAddress(addr))
 return bp.get()->GetID();
-  }
-  // DBLogIf(PD_LOG_BREAKPOINTS, "BreakpointSiteList::%s ( addr = 0x%8.8"
-  // PRIx64
-  // " ) => NONE", __FUNCTION__, (uint64_t)addr);
   return LLDB_INVALID_BREAK_ID;
 }
 



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

[Lldb-commits] [PATCH] D151765: [lldb] Introduce the FileSpecBuilder abstraction

[Alex Langford via Phabricator via lldb-commits]

bulbazord created this revision.
bulbazord added reviewers: JDevlieghere, mib, jingham, jasonmolenda, clayborg.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Mutating a FileSpec is quite expensive. Every time you add or remove a
component of a path, we need to re-parse the entire path and insert a
new directory and filename into the ConstString StringPool. In many
cases, we take an existing FilePath and slowly morph it into the one we
want to actually use.

In order to improve performance, I want to introduce a new abstraction
called the `FileSpecBuilder`. The idea is that it maintains a
SmallString to store the entire path and a path style. It uses llvm's
path manipulation code to quickly append and remove things so we don't
have to re-parse the path every single time. When you're done
manipulating paths and want a FileSpec for sure, you can invoke
`FileSpecBuilder::CreateFileSpec` to create a new FileSpec, only parsing
the path once.

This patch primarily is to gather feedback about the idea. I wanted to
keep this patch small and targeted to get specific feedback, and for
that reason I avoided doing any kind of refactoring as most changes will
be quite invasive. Instead I opted to add unit tests to show how I want
FileSpecBuilder to be used in code. If this abstraction goes into LLDB,
I will refactor every place where we mutate a FileSpec until
FileSpecBuilder is the default way of manipulating paths.

For further motivation of this change, please see:
https://reviews.llvm.org/D149096


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151765

Files:
  lldb/include/lldb/Utility/FileSpecBuilder.h
  lldb/source/Utility/CMakeLists.txt
  lldb/source/Utility/FileSpecBuilder.cpp
  lldb/unittests/Utility/FileSpecBuilderTest.cpp

Index: lldb/unittests/Utility/FileSpecBuilderTest.cpp
===
--- /dev/null
+++ lldb/unittests/Utility/FileSpecBuilderTest.cpp
@@ -0,0 +1,118 @@
+//===-- FileSpecBuilderTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/FileSpecBuilder.h"
+
+using namespace lldb_private;
+
+static FileSpec PosixSpec(llvm::StringRef path) {
+  return FileSpec(path, FileSpec::Style::posix);
+}
+
+static FileSpec WindowsSpec(llvm::StringRef path) {
+  return FileSpec(path, FileSpec::Style::windows);
+}
+
+TEST(FileSpecBuilderTest, AppendPathComponent) {
+  FileSpecBuilder from_scratch_posix("/", FileSpecBuilder::Style::posix);
+  EXPECT_EQ("/", from_scratch_posix.GetCurrentPath());
+  from_scratch_posix.AppendPathComponent("foo");
+  EXPECT_EQ("/foo", from_scratch_posix.GetCurrentPath());
+  from_scratch_posix.AppendPathComponent("bar");
+  EXPECT_EQ("/foo/bar", from_scratch_posix.GetCurrentPath());
+  from_scratch_posix.AppendPathComponent("baz");
+  EXPECT_EQ("/foo/bar/baz", from_scatch_posix.GetCurrentPath());
+  const FileSpec posix_scratch_result = from_scratch_posix.CreateFileSpec();
+  EXPECT_STREQ("/foo/bar/baz", posix_scratch_result.GetPath().c_str());
+
+  const FileSpec fs_posix = PosixSpec("/foo");
+  FileSpecBuilder builder_posix(fs_posix);
+  builder_posix.AppendPathComponent("bar");
+  EXPECT_EQ("/foo/bar", builder_posix.GetCurrentPath());
+  const FileSpec posix_result = builder_posix.CreateFileSpec();
+  EXPECT_STREQ("/foo/bar", posix_result.GetPath().c_str());
+
+  const FileSpec fs_windows = WindowsSpec("F:\\");
+  FileSpecBuilder builder_windows(fs_windows);
+  builder_windows.AppendPathComponent("bar");
+  EXPECT_EQ("F:\\bar", builder_windows.GetCurrentPath());
+  builder_windows.AppendPathComponent("baz");
+  EXPECT_EQ("F:\\bar\\baz", builder_windows.GetCurrentPath());
+  const FileSpec windows_result = builder_windows.CreateFileSpec();
+  EXPECT_STREQ("F:\\bar\\baz", windows_result.GetPath().c_str());
+}
+
+TEST(FileSpecBuilderTest, RemoveLastPathComponent) {
+  const FileSpec fs_posix = PosixSpec("/foo/bar/baz");
+  FileSpecBuilder builder_posix(fs_posix);
+
+  EXPECT_EQ("/foo/bar/baz", builder_posix.GetCurrentPath());
+  EXPECT_TRUE(fs_posix.RemoveLastPathComponent());
+  EXPECT_EQ("/foo/bar", builder_posix.GetCurrentPath());
+  EXPECT_TRUE(fs_posix.RemoveLastPathComponent());
+  EXPECT_EQ("/foo", builder_posix.GetCurrentPath());
+  EXPECT_TRUE(fs_posix.RemoveLastPathComponent());
+  EXPECT_EQ("/", builder_posix.GetCurrentPath());
+  EXPECT_FALSE(fs_posix.RemoveLastPathComponent());
+  EXPECT_EQ("/", builder_posix.GetCurrentPath());
+
+  const FileSpec fs_posix_relative = PosixSpec("./foo/bar/baz");
+  FileSpecBuilder builder_

[Lldb-commits] [lldb] f63155a - [clang] Show line numbers in diagnostic code snippets

[Timm Bäder via lldb-commits]

Author: Timm Bäder
Date: 2023-05-31T07:26:03+02:00
New Revision: f63155aaa6467bd2610820dfd1996af3bb6029a7

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

LOG: [clang] Show line numbers in diagnostic code snippets

Show line numbers to the left of diagnostic code snippets and increase
the numbers of lines shown from 1 to 16.

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

Added: 


Modified: 

clang-tools-extra/test/clang-tidy/checkers/cert/uppercase-literal-suffix-integer.cpp

clang-tools-extra/test/clang-tidy/checkers/readability/uppercase-literal-suffix-float16.cpp

clang-tools-extra/test/clang-tidy/checkers/readability/uppercase-literal-suffix-floating-point-opencl-half.cpp

clang-tools-extra/test/clang-tidy/checkers/readability/uppercase-literal-suffix-floating-point.cpp

clang-tools-extra/test/clang-tidy/checkers/readability/uppercase-literal-suffix-hexadecimal-floating-point.cpp

clang-tools-extra/test/clang-tidy/checkers/readability/uppercase-literal-suffix-integer-custom-list.cpp

clang-tools-extra/test/clang-tidy/checkers/readability/uppercase-literal-suffix-integer-ms.cpp

clang-tools-extra/test/clang-tidy/checkers/readability/uppercase-literal-suffix-integer.cpp
clang/docs/ReleaseNotes.rst
clang/docs/UsersManual.rst
clang/include/clang/Basic/DiagnosticOptions.def
clang/include/clang/Basic/DiagnosticOptions.h
clang/include/clang/Driver/Options.td
clang/include/clang/Frontend/TextDiagnostic.h
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/TextDiagnostic.cpp
clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
clang/test/FixIt/fixit-function-call.cpp
clang/test/FixIt/fixit-newline-style.c
clang/test/FixIt/fixit-unicode-with-utf8-output.c
clang/test/FixIt/fixit-unicode.c
clang/test/Frontend/source-col-map.c
clang/test/Lexer/header.cpp
clang/test/Lexer/string-literal-errors.cpp
clang/test/Misc/caret-diags-macros.c
clang/test/Misc/caret-diags-multiline.cpp
clang/test/Misc/diag-macro-backtrace.c
clang/test/Misc/message-length.c
clang/test/Misc/tabstop.c
clang/test/Misc/unnecessary-elipses.cpp
clang/test/Misc/unprintable.c
clang/test/Misc/wrong-encoding.c
clang/test/Parser/brackets.c
clang/test/Parser/brackets.cpp
clang/test/Preprocessor/ucn-pp-identifier.c
clang/test/Sema/caret-diags-complex-init.cpp
clang/test/SemaCXX/struct-class-redecl.cpp
lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py

Removed: 




diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/cert/uppercase-literal-suffix-integer.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/cert/uppercase-literal-suffix-integer.cpp
index 0dc06df4f18b4..6fa700bf06d4f 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/cert/uppercase-literal-suffix-integer.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/cert/uppercase-literal-suffix-integer.cpp
@@ -31,7 +31,7 @@ void integer_suffix() {
   // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal has suffix 'l', 
which is not uppercase
   // CHECK-MESSAGES-NEXT: static constexpr auto v5 = 1l;
   // CHECK-MESSAGES-NEXT: ^~
-  // CHECK-MESSAGES-NEXT: {{^ *}}L{{$}}
+  // CHECK-MESSAGES-NEXT: {{^ *| *}}L{{$}}
   // CHECK-FIXES: static constexpr auto v5 = 1L;
   static_assert(is_same::value, "");
   static_assert(v5 == 1, "");
@@ -46,7 +46,7 @@ void integer_suffix() {
   // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal has suffix 
'll', which is not uppercase
   // CHECK-MESSAGES-NEXT: static constexpr auto v7 = 1ll;
   // CHECK-MESSAGES-NEXT: ^~~
-  // CHECK-MESSAGES-NEXT: {{^ *}}LL{{$}}
+  // CHECK-MESSAGES-NEXT: {{^ *| *}}LL{{$}}
   // CHECK-FIXES: static constexpr auto v7 = 1LL;
   static_assert(is_same::value, "");
   static_assert(v7 == 1, "");
@@ -79,7 +79,7 @@ void integer_suffix() {
   // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: integer literal has suffix 
'lu', which is not uppercase
   // CHECK-MESSAGES-NEXT: static constexpr auto v13 = 1lu;
   // CHECK-MESSAGES-NEXT: ^~~
-  // CHECK-MESSAGES-NEXT: {{^ *}}LU{{$}}
+  // CHECK-MESSAGES-NEXT: {{^ *| *}}LU{{$}}
   // CHECK-FIXES: static constexpr auto v13 = 1LU;
   static_assert(is_same::value, "");
   static_assert(v13 == 1, "");
@@ -88,7 +88,7 @@ void integer_suffix() {
   // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: integer literal has suffix 
'Lu', which is not uppercase
   // CHECK-MESSAGES-NEXT: static constexpr auto v14 = 1Lu;
   // CHECK-MESSAGES-NEXT: ^~~
-  // CHECK-MESSAGES-NEXT: {{^ *}}LU{{$}}
+  // CHECK-MESSAGES-NEXT: {{^ *| *}}LU{{$}}
   // CHECK-FIXES: static constexpr auto v14 = 1LU;
   static_assert(is_same::value, "");
   static_assert(v14 == 1, "");
@@ -97,7 +97,7 @@ voi

[Lldb-commits] [PATCH] D150772: Add code snippet line numbers to TestExprDiagnostics output

[Timm Bäder via Phabricator via lldb-commits]

tbaeder closed this revision.
tbaeder added a comment.

I merged the changes into the original line number commit and pushed them as 
https://github.com/llvm/llvm-project/commit/f63155aaa6467bd2610820dfd1996af3bb6029a7


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

https://reviews.llvm.org/D150772

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