[Lldb-commits] [PATCH] D149949: [lldb][TypeSystem] ForEach: Don't hold the TypeSystemMap lock across callback

2023-05-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

Just as a drive-by, since we're constructing a copy anyway it would be possible 
to merge this construction with the "uniqueing" behavior of the `visited` set. 
I.e., instead of making a literal copy of the map, just construct a 
`to_be_visited` set of /unique/ objects, and then iterate over that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149949

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


[Lldb-commits] [PATCH] D149949: [lldb][TypeSystem] ForEach: Don't hold the TypeSystemMap lock across callback

2023-05-15 Thread Michael Buch via Phabricator via lldb-commits
Michael137 added a comment.

In D149949#4341608 , @labath wrote:

> Just as a drive-by, since we're constructing a copy anyway it would be 
> possible to merge this construction with the "uniqueing" behavior of the 
> `visited` set. I.e., instead of making a literal copy of the map, just 
> construct a `to_be_visited` set of /unique/ objects, and then iterate over 
> that.

Agreed that would be a nice cleanup. I considered it initially but thought I'd 
split it out into a separate patch (but never did)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149949

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


[Lldb-commits] [PATCH] D150589: [lldb][DWARFASTParserClang][NFC] Simplify unnamed bitfield condition

2023-05-15 Thread Michael Buch via Phabricator via lldb-commits
Michael137 created this revision.
Michael137 added a reviewer: aprantl.
Herald added a reviewer: shafik.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Minor cleanup of redundant variable initialization and
if-condition. These are leftovers/oversights from previous
cleanup in this area:

- https://reviews.llvm.org/D72953
- https://reviews.llvm.org/D76808


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150589

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2881,9 +2881,8 @@
 
 if (detect_unnamed_bitfields) {
   std::optional unnamed_field_info;
-  uint64_t last_field_end = 0;
-
-  last_field_end = last_field_info.bit_offset + last_field_info.bit_size;
+  uint64_t last_field_end =
+  last_field_info.bit_offset + last_field_info.bit_size;
 
   if (!last_field_info.IsBitfield()) {
 // The last field was not a bit-field...
@@ -2903,10 +2902,8 @@
   // indeed an unnamed bit-field. We currently do not have the
   // machinary to track the offset of the last field of classes we
   // have seen before, so we are not handling this case.
-  if (this_field_info.bit_offset != last_field_end &&
-  this_field_info.bit_offset > last_field_end &&
-  !(last_field_info.bit_offset == 0 &&
-last_field_info.bit_size == 0 &&
+  if (this_field_info.bit_offset > last_field_end &&
+  !(last_field_info.bit_offset == 0 && last_field_info.bit_size == 0 &&
 layout_info.base_offsets.size() != 0)) {
 unnamed_field_info = FieldInfo{};
 unnamed_field_info->bit_size =


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2881,9 +2881,8 @@
 
 if (detect_unnamed_bitfields) {
   std::optional unnamed_field_info;
-  uint64_t last_field_end = 0;
-
-  last_field_end = last_field_info.bit_offset + last_field_info.bit_size;
+  uint64_t last_field_end =
+  last_field_info.bit_offset + last_field_info.bit_size;
 
   if (!last_field_info.IsBitfield()) {
 // The last field was not a bit-field...
@@ -2903,10 +2902,8 @@
   // indeed an unnamed bit-field. We currently do not have the
   // machinary to track the offset of the last field of classes we
   // have seen before, so we are not handling this case.
-  if (this_field_info.bit_offset != last_field_end &&
-  this_field_info.bit_offset > last_field_end &&
-  !(last_field_info.bit_offset == 0 &&
-last_field_info.bit_size == 0 &&
+  if (this_field_info.bit_offset > last_field_end &&
+  !(last_field_info.bit_offset == 0 && last_field_info.bit_size == 0 &&
 layout_info.base_offsets.size() != 0)) {
 unnamed_field_info = FieldInfo{};
 unnamed_field_info->bit_size =
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D150590: [lldb][DWARFASTParserClang][NFC] Extract condition for unnamed bitfield creation into helper function

2023-05-15 Thread Michael Buch via Phabricator via lldb-commits
Michael137 created this revision.
Michael137 added a reviewer: aprantl.
Herald added a reviewer: shafik.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This patch adds a new private helper
`DWARFASTParserClang::ShouldCreateUnnamedBitfield` which
`ParseSingleMember` whether we should fill the current gap
in a structure layout with unnamed bitfields.

Extracting this logic will allow us to add additional
conditions in upcoming patches without jeoperdizing readability
of `ParseSingleMember`.

We also store some of the boolean conditions in local variables
to make the intent more obvious.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150590

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -236,6 +236,11 @@
 }
   };
 
+  bool ShouldCreateUnnamedBitfield(
+  FieldInfo const &last_field_info, uint64_t last_field_end,
+  FieldInfo const &this_field_info,
+  lldb_private::ClangASTImporter::LayoutInfo const &layout_info) const;
+
   /// Parses a DW_TAG_APPLE_property DIE and appends the parsed data to the
   /// list of delayed Objective-C properties.
   ///
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2893,18 +2893,8 @@
   last_field_end += word_width - (last_field_end % word_width);
   }
 
-  // If we have a gap between the last_field_end and the current
-  // field we have an unnamed bit-field.
-  // If we have a base class, we assume there is no unnamed
-  // bit-field if this is the first field since the gap can be
-  // attributed to the members from the base class. This assumption
-  // is not correct if the first field of the derived class is
-  // indeed an unnamed bit-field. We currently do not have the
-  // machinary to track the offset of the last field of classes we
-  // have seen before, so we are not handling this case.
-  if (this_field_info.bit_offset > last_field_end &&
-  !(last_field_info.bit_offset == 0 && last_field_info.bit_size == 0 &&
-layout_info.base_offsets.size() != 0)) {
+  if (ShouldCreateUnnamedBitfield(last_field_info, last_field_end,
+  this_field_info, layout_info)) {
 unnamed_field_info = FieldInfo{};
 unnamed_field_info->bit_size =
 this_field_info.bit_offset - last_field_end;
@@ -3696,3 +3686,29 @@
 
   return !failures.empty();
 }
+
+bool DWARFASTParserClang::ShouldCreateUnnamedBitfield(
+FieldInfo const &last_field_info, uint64_t last_field_end,
+FieldInfo const &this_field_info,
+lldb_private::ClangASTImporter::LayoutInfo const &layout_info) const {
+  // If we have a gap between the last_field_end and the current
+  // field we have an unnamed bit-field.
+  if (this_field_info.bit_offset <= last_field_end)
+return false;
+
+  // If we have a base class, we assume there is no unnamed
+  // bit-field if this is the first field since the gap can be
+  // attributed to the members from the base class. This assumption
+  // is not correct if the first field of the derived class is
+  // indeed an unnamed bit-field. We currently do not have the
+  // machinary to track the offset of the last field of classes we
+  // have seen before, so we are not handling this case.
+  const bool have_base = layout_info.base_offsets.size() != 0;
+  const bool this_is_first_field =
+  last_field_info.bit_offset == 0 && last_field_info.bit_size == 0;
+
+  if (have_base && this_is_first_field)
+return false;
+
+  return true;
+}


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -236,6 +236,11 @@
 }
   };
 
+  bool ShouldCreateUnnamedBitfield(
+  FieldInfo const &last_field_info, uint64_t last_field_end,
+  FieldInfo const &this_field_info,
+  lldb_private::ClangASTImporter::LayoutInfo const &layout_info) const;
+
   /// Parses a DW_TAG_APPLE_property DIE and appends the parsed data to the
   /// list of delayed Objective-C properties.
   ///
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWA

[Lldb-commits] [PATCH] D150591: [lldb][DWARFASTParserClang] Don't create unnamed bitfields to account for vtable pointer

2023-05-15 Thread Michael Buch via Phabricator via lldb-commits
Michael137 created this revision.
Michael137 added a reviewer: aprantl.
Herald added a reviewer: shafik.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

**Summary**

When filling out the LayoutInfo for a structure with the offsets
from DWARF, LLDB fills gaps in the layout by creating unnamed
bitfields and adding them to the AST. If we don't do this correctly
and our layout has overlapping fields, we will hat an assertion
in `clang::CGRecordLowering::lower()`. Specifically, if we have
a derived class with a VTable and a bitfield immediately following
the vtable pointer, we create a layout with overlapping fields.

This is an oversight in some of the previous cleanups done around this
area.

In `D76808`, we prevented LLDB from creating unnamed bitfields if there
was a gap between the last field of a base class and the start of a bitfield
in the derived class.

In `D112697`, we started accounting for the vtable pointer. The intention
there was to make sure the offset bookkeeping accounted for the
existence of a vtable pointer (but we didn't actually want to create
any AST nodes for it). Now that `last_field_info.bit_size` was being
set even for artifical fields, the previous fix `D76808` broke
specifically for cases where the bitfield was the first member of a
derived class with a vtable (this scenario wasn't tested so we didn't
notice it). I.e., we started creating redundant unnamed bitfields for
where the vtable pointer usually sits. This confused the lowering logic
in clang.

This patch adds a condition to `ShouldCreateUnnamedBitfield` which
checks whether the first field in the derived class is a vtable ptr.

**Testing**

- Added API test case


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150591

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
  lldb/test/API/lang/cpp/bitfields/main.cpp

Index: lldb/test/API/lang/cpp/bitfields/main.cpp
===
--- lldb/test/API/lang/cpp/bitfields/main.cpp
+++ lldb/test/API/lang/cpp/bitfields/main.cpp
@@ -113,6 +113,12 @@
 };
 HasBaseWithVTable base_with_vtable;
 
+struct DerivedWithVTable : public Base {
+  virtual ~DerivedWithVTable() {}
+  unsigned a : 1;
+};
+DerivedWithVTable derived_with_vtable;
+
 int main(int argc, char const *argv[]) {
   lba.a = 2;
 
@@ -153,5 +159,8 @@
   base_with_vtable.b = 0;
   base_with_vtable.c = 5;
 
+  derived_with_vtable.b_a = 2;
+  derived_with_vtable.a = 1;
+
   return 0; // break here
 }
Index: lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
===
--- lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
+++ lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
@@ -168,3 +168,14 @@
  result_children=with_vtable_and_unnamed_children)
 self.expect_var_path("with_vtable_and_unnamed",
  children=with_vtable_and_unnamed_children)
+
+derived_with_vtable_children = [
+ValueCheck(name="Base", children=[
+  ValueCheck(name="b_a", value="2", type="uint32_t")
+]),
+ValueCheck(name="a", value="1", type="unsigned int:1")
+]
+self.expect_expr("derived_with_vtable",
+ result_children=derived_with_vtable_children)
+self.expect_var_path("derived_with_vtable",
+ children=derived_with_vtable_children)
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -223,12 +223,16 @@
 uint64_t bit_size = 0;
 uint64_t bit_offset = 0;
 bool is_bitfield = false;
+bool is_artificial = false;
 
 FieldInfo() = default;
 
 void SetIsBitfield(bool flag) { is_bitfield = flag; }
 bool IsBitfield() { return is_bitfield; }
 
+void SetIsArtificial(bool flag) { is_artificial = flag; }
+bool IsArtificial() const { return is_artificial; }
+
 bool NextBitfieldOffsetIsValid(const uint64_t next_bit_offset) const {
   // Any subsequent bitfields must not overlap and must be at a higher
   // bit offset than any previous bitfield + size.
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2935,8 +2935,10 @@
   // artificial member with (unnamed bitfield) padding.
   // FIXME: This check should verify that this is indeed an artificial member
   // we are supposed

[Lldb-commits] [PATCH] D150590: [lldb][DWARFASTParserClang][NFC] Extract condition for unnamed bitfield creation into helper function

2023-05-15 Thread 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/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h:239
 
+  bool ShouldCreateUnnamedBitfield(
+  FieldInfo const &last_field_info, uint64_t last_field_end,

Doxygen comment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150590

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


[Lldb-commits] [PATCH] D149625: [lldb] Refactor SBFileSpec::GetDirectory

2023-05-15 Thread Muhammad Omair Javaid via Phabricator via lldb-commits
omjavaid reopened this revision.
omjavaid added a comment.
This revision is now accepted and ready to land.

This introduced test failures on windows lldb-aarch64-windows buildbot.

  lldb-api :: functionalities/process_save_core/TestProcessSaveCore.py
  lldb-api :: python_api/symbol-context/TestSymbolContext.py

https://lab.llvm.org/buildbot/#/builders/219/builds/2485

I am reverting it, kindly revisit it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149625

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


[Lldb-commits] [lldb] 58e6caa - Revert "[lldb] Refactor SBFileSpec::GetDirectory"

2023-05-15 Thread Muhammad Omair Javaid via lldb-commits

Author: Muhammad Omair Javaid
Date: 2023-05-15T22:49:00+04:00
New Revision: 58e6caaba1cf623292c8898be30a5a56722432b3

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

LOG: Revert "[lldb] Refactor SBFileSpec::GetDirectory"

This reverts commit 2bea2d7b070dc5df723ce2b92dbc654b8bb1847e.

It introduced following failures on buildbot lldb-aarch64-windows:

lldb-api :: functionalities/process_save_core/TestProcessSaveCore.py
lldb-api :: python_api/symbol-context/TestSymbolContext.py

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

Added: 


Modified: 
lldb/source/API/SBFileSpec.cpp

Removed: 




diff  --git a/lldb/source/API/SBFileSpec.cpp b/lldb/source/API/SBFileSpec.cpp
index 8668b64b4ce76..a7df9afc4b8eb 100644
--- a/lldb/source/API/SBFileSpec.cpp
+++ b/lldb/source/API/SBFileSpec.cpp
@@ -114,7 +114,9 @@ const char *SBFileSpec::GetFilename() const {
 const char *SBFileSpec::GetDirectory() const {
   LLDB_INSTRUMENT_VA(this);
 
-  return m_opaque_up->GetDirectory().GetCString();
+  FileSpec directory{*m_opaque_up};
+  directory.ClearFilename();
+  return directory.GetPathAsConstString().GetCString();
 }
 
 void SBFileSpec::SetFilename(const char *filename) {



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


[Lldb-commits] [PATCH] D149625: [lldb] Refactor SBFileSpec::GetDirectory

2023-05-15 Thread Muhammad Omair Javaid via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG58e6caaba1cf: Revert "[lldb] Refactor 
SBFileSpec::GetDirectory" (authored by omjavaid).

Changed prior to commit:
  https://reviews.llvm.org/D149625?vs=518782&id=522285#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149625

Files:
  lldb/source/API/SBFileSpec.cpp


Index: lldb/source/API/SBFileSpec.cpp
===
--- lldb/source/API/SBFileSpec.cpp
+++ lldb/source/API/SBFileSpec.cpp
@@ -114,7 +114,9 @@
 const char *SBFileSpec::GetDirectory() const {
   LLDB_INSTRUMENT_VA(this);
 
-  return m_opaque_up->GetDirectory().GetCString();
+  FileSpec directory{*m_opaque_up};
+  directory.ClearFilename();
+  return directory.GetPathAsConstString().GetCString();
 }
 
 void SBFileSpec::SetFilename(const char *filename) {


Index: lldb/source/API/SBFileSpec.cpp
===
--- lldb/source/API/SBFileSpec.cpp
+++ lldb/source/API/SBFileSpec.cpp
@@ -114,7 +114,9 @@
 const char *SBFileSpec::GetDirectory() const {
   LLDB_INSTRUMENT_VA(this);
 
-  return m_opaque_up->GetDirectory().GetCString();
+  FileSpec directory{*m_opaque_up};
+  directory.ClearFilename();
+  return directory.GetPathAsConstString().GetCString();
 }
 
 void SBFileSpec::SetFilename(const char *filename) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D150366: [lldb][NFCI] Use llvm's libDebugInfo for DebugRanges

2023-05-15 Thread 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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150366

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


[Lldb-commits] [PATCH] D149625: [lldb] Refactor SBFileSpec::GetDirectory

2023-05-15 Thread Alex Langford via Phabricator via lldb-commits
bulbazord added a comment.

> AssertionError: 
> 'C:\\Users\\tcwg\\llvm-worker\\lldb-aarch64-windows\\build\\lldb-test-build.noindex\\functionalities\\process_save_core\\TestProcessSaveCore.test_save_windows_mini_dump_dwarf\\a.out'
>  not found in 
> ['C:/Users/tcwg/llvm-worker/lldb-aarch64-windows/build/lldb-test-build.noindex/functionalities/process_save_core/TestProcessSaveCore.test_save_windows_mini_dump_dwarf\\a.out',
>  'C:/Windows/System32\\ntdll.dll', 'C:/Windows/System32\\kernel32.dll', 
> 'C:/Windows/System32\\KernelBase.dll']

Looks like a path normalization issue... `GetPathAsConstString()` has a bool 
parameter that defaults to true for normalizing paths. The right thing for this 
patch to do is actually return 
`m_opaque_up->GetPathAsConstString().GetCString()` then instead of invoking 
`GetDirectory()` directly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149625

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


[Lldb-commits] [PATCH] D149625: [lldb] Refactor SBFileSpec::GetDirectory

2023-05-15 Thread Alex Langford via Phabricator via lldb-commits
bulbazord added a comment.

Ok, after looking at this more closely, it's a little clearer to me why 
`SBFileSpec::GetDirectory` was written this way to begin with. The method 
itself requires its return value to be denormalized (something not explicitly 
documented in the header nor the SWIG docstrings for this class/method). The 
only way to get the FileSpec's path in a denormalized form is by using 
`FileSpec::GetPath` and its variants... That's why we create a new FileSpec, 
remove its filename, and then get the path as a ConstString. I won't be 
addressing that in this patch because I view this as a flaw of the existing 
FileSpec implementation.

Thank you for finding and reverting this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149625

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


[Lldb-commits] [lldb] 039b28e - [LLDB] Fix TestDataFormatterSynthVal.py for AArch64/Windows

2023-05-15 Thread Muhammad Omair Javaid via lldb-commits

Author: Muhammad Omair Javaid
Date: 2023-05-16T00:14:20+04:00
New Revision: 039b28e14e6d5a4b9b9b333695dabe9dc57233b0

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

LOG: [LLDB] Fix TestDataFormatterSynthVal.py for AArch64/Windows

Since 44363f2 various tests have started passing but introduced a
expression evaluation failure in TestDataFormatterSynthVal.py.
This patch marks the expression evaluation part as skipped while rest
of the test passes.
This patch aslo introduces a new helper isAArch64Windows in lldbtest.py.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/lldbtest.py

lldb/test/API/functionalities/data-formatter/data-formatter-synthval/TestDataFormatterSynthVal.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 31539f6a768bd..0ca0fac7bb3f6 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1252,6 +1252,13 @@ def isAArch64PAuth(self):
 return True
 return self.isAArch64() and "paca" in self.getCPUInfo()
 
+def isAArch64Windows(self):
+"""Returns true if the architecture is AArch64 and platform windows."""
+if self.getPlatform() == 'windows':
+arch = self.getArchitecture().lower()
+return arch in ["aarch64", "arm64", "arm64e"]
+return False
+
 def getArchitecture(self):
 """Returns the architecture in effect the test suite is running 
with."""
 module = builder_module()

diff  --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-synthval/TestDataFormatterSynthVal.py
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-synthval/TestDataFormatterSynthVal.py
index 2e1367563cd7b..67619287ef65e 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-synthval/TestDataFormatterSynthVal.py
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-synthval/TestDataFormatterSynthVal.py
@@ -96,9 +96,10 @@ def cleanup():
 
 # check that an aptly defined synthetic provider does not affect
 # one-lining
-self.expect(
-"expression struct Struct { myInt theInt{12}; }; Struct()",
-substrs=['(theInt = 12)'])
+if self.isAArch64Windows():
+self.expect(
+"expression struct Struct { myInt theInt{12}; }; Struct()",
+substrs=['(theInt = 12)'])
 
 # check that we can use a synthetic value in a summary
 self.runCmd("type summary add hasAnInt -s ${var.theInt}")



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


[Lldb-commits] [PATCH] D150485: [lldb] Add support for negative integer to {SB, }StructuredData

2023-05-15 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added inline comments.



Comment at: lldb/include/lldb/API/SBStructuredData.h:70
   /// Return the integer value if this data structure is an integer type.
-  uint64_t GetIntegerValue(uint64_t fail_value = 0) const;
+  template  T GetIntegerValue(T fail_value = {}) const {
+if constexpr (!std::is_integral_v)

bulbazord wrote:
> > All the SB API classes are non-virtual, single inheritance classes. They 
> > should only include SBDefines.h or other SB headers as needed. **There 
> > should be no inlined method implementations in the header files, they 
> > should all be in the implementation files**. And there should be no direct 
> > ivar access.
> 
> Emphasis mine, from: https://lldb.llvm.org/design/sbapi.html
> 
> We should be able to move this into the implementation file.
I'm open to the idea of adding templates to the SB API, but I think that will 
require careful considerations in terms of impact on the ABI, the bindings, 
etc. I'm not convinced this use case really warrants that: it looks like 
there's only really two meaningful instantiations. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150485

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


[Lldb-commits] [lldb] f464b7c - [lldb] Change definition of DisassemblerCreateInstance

2023-05-15 Thread Alex Langford via lldb-commits

Author: Alex Langford
Date: 2023-05-15T13:31:26-07:00
New Revision: f464b7c764bcb8f29f28025919800f49405e4e93

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

LOG: [lldb] Change definition of DisassemblerCreateInstance

DissassemblerCreateInstance is a function pointer whos return type is
`Disassembler *`. But Disassembler::FindPlugin always returns a
DisassemblerSP, so there's no reason why we can't just create a
DisassemblerSP in the first place.

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

Added: 


Modified: 
lldb/include/lldb/lldb-private-interfaces.h
lldb/source/Core/Disassembler.cpp
lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h

Removed: 




diff  --git a/lldb/include/lldb/lldb-private-interfaces.h 
b/lldb/include/lldb/lldb-private-interfaces.h
index 236a7f85c2c37..51bdfd360ab4a 100644
--- a/lldb/include/lldb/lldb-private-interfaces.h
+++ b/lldb/include/lldb/lldb-private-interfaces.h
@@ -30,8 +30,8 @@ typedef lldb::ABISP (*ABICreateInstance)(lldb::ProcessSP 
process_sp,
  const ArchSpec &arch);
 typedef std::unique_ptr (*ArchitectureCreateInstance)(
 const ArchSpec &arch);
-typedef Disassembler *(*DisassemblerCreateInstance)(const ArchSpec &arch,
-const char *flavor);
+typedef lldb::DisassemblerSP (*DisassemblerCreateInstance)(const ArchSpec 
&arch,
+   const char *flavor);
 typedef DynamicLoader *(*DynamicLoaderCreateInstance)(Process *process,
   bool force);
 typedef lldb::JITLoaderSP (*JITLoaderCreateInstance)(Process *process,

diff  --git a/lldb/source/Core/Disassembler.cpp 
b/lldb/source/Core/Disassembler.cpp
index bc9bf4f45f932..09eee082bc394 100644
--- a/lldb/source/Core/Disassembler.cpp
+++ b/lldb/source/Core/Disassembler.cpp
@@ -67,20 +67,16 @@ DisassemblerSP Disassembler::FindPlugin(const ArchSpec 
&arch,
 create_callback =
 PluginManager::GetDisassemblerCreateCallbackForPluginName(plugin_name);
 if (create_callback) {
-  DisassemblerSP disassembler_sp(create_callback(arch, flavor));
-
-  if (disassembler_sp)
-return disassembler_sp;
+  if (auto disasm_sp = create_callback(arch, flavor))
+return disasm_sp;
 }
   } else {
 for (uint32_t idx = 0;
  (create_callback = 
PluginManager::GetDisassemblerCreateCallbackAtIndex(
   idx)) != nullptr;
  ++idx) {
-  DisassemblerSP disassembler_sp(create_callback(arch, flavor));
-
-  if (disassembler_sp)
-return disassembler_sp;
+  if (auto disasm_sp = create_callback(arch, flavor))
+return disasm_sp;
 }
   }
   return DisassemblerSP();

diff  --git a/lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp 
b/lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
index 8a4fd08a9268b..09115cc670da7 100644
--- a/lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
+++ b/lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
@@ -1572,16 +1572,14 @@ DisassemblerLLVMC::DisassemblerLLVMC(const ArchSpec 
&arch,
 
 DisassemblerLLVMC::~DisassemblerLLVMC() = default;
 
-Disassembler *DisassemblerLLVMC::CreateInstance(const ArchSpec &arch,
-const char *flavor) {
+lldb::DisassemblerSP DisassemblerLLVMC::CreateInstance(const ArchSpec &arch,
+   const char *flavor) {
   if (arch.GetTriple().getArch() != llvm::Triple::UnknownArch) {
-std::unique_ptr disasm_up(
-new DisassemblerLLVMC(arch, flavor));
-
-if (disasm_up.get() && disasm_up->IsValid())
-  return disasm_up.release();
+auto disasm_sp = std::make_shared(arch, flavor);
+if (disasm_sp && disasm_sp->IsValid())
+  return disasm_sp;
   }
-  return nullptr;
+  return lldb::DisassemblerSP();
 }
 
 size_t DisassemblerLLVMC::DecodeInstructions(const Address &base_addr,

diff  --git a/lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h 
b/lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
index 68ae3a32e18fd..30c69de81dfc8 100644
--- a/lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
+++ b/lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
@@ -34,8 +34,8 @@ class DisassemblerLLVMC : public lldb_private::Disassembler {
 
   static llvm::StringRef GetPluginNameStatic() { return "llvm-mc"; }
 
-  static lldb_private::Disassembler *
-  CreateInstance(const lldb_private::ArchSpec &arch, const char *flavor);
+  static lldb::DisassemblerSP CreateInstance(const lldb_private::ArchSpec 
&arch,
+

[Lldb-commits] [PATCH] D150235: [lldb] Change definition of DisassemblerCreateInstance

2023-05-15 Thread Alex Langford via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf464b7c764bc: [lldb] Change definition of 
DisassemblerCreateInstance (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150235

Files:
  lldb/include/lldb/lldb-private-interfaces.h
  lldb/source/Core/Disassembler.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h


Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
===
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
+++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
@@ -34,8 +34,8 @@
 
   static llvm::StringRef GetPluginNameStatic() { return "llvm-mc"; }
 
-  static lldb_private::Disassembler *
-  CreateInstance(const lldb_private::ArchSpec &arch, const char *flavor);
+  static lldb::DisassemblerSP CreateInstance(const lldb_private::ArchSpec 
&arch,
+ const char *flavor);
 
   size_t DecodeInstructions(const lldb_private::Address &base_addr,
 const lldb_private::DataExtractor &data,
Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
===
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
+++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
@@ -1572,16 +1572,14 @@
 
 DisassemblerLLVMC::~DisassemblerLLVMC() = default;
 
-Disassembler *DisassemblerLLVMC::CreateInstance(const ArchSpec &arch,
-const char *flavor) {
+lldb::DisassemblerSP DisassemblerLLVMC::CreateInstance(const ArchSpec &arch,
+   const char *flavor) {
   if (arch.GetTriple().getArch() != llvm::Triple::UnknownArch) {
-std::unique_ptr disasm_up(
-new DisassemblerLLVMC(arch, flavor));
-
-if (disasm_up.get() && disasm_up->IsValid())
-  return disasm_up.release();
+auto disasm_sp = std::make_shared(arch, flavor);
+if (disasm_sp && disasm_sp->IsValid())
+  return disasm_sp;
   }
-  return nullptr;
+  return lldb::DisassemblerSP();
 }
 
 size_t DisassemblerLLVMC::DecodeInstructions(const Address &base_addr,
Index: lldb/source/Core/Disassembler.cpp
===
--- lldb/source/Core/Disassembler.cpp
+++ lldb/source/Core/Disassembler.cpp
@@ -67,20 +67,16 @@
 create_callback =
 PluginManager::GetDisassemblerCreateCallbackForPluginName(plugin_name);
 if (create_callback) {
-  DisassemblerSP disassembler_sp(create_callback(arch, flavor));
-
-  if (disassembler_sp)
-return disassembler_sp;
+  if (auto disasm_sp = create_callback(arch, flavor))
+return disasm_sp;
 }
   } else {
 for (uint32_t idx = 0;
  (create_callback = 
PluginManager::GetDisassemblerCreateCallbackAtIndex(
   idx)) != nullptr;
  ++idx) {
-  DisassemblerSP disassembler_sp(create_callback(arch, flavor));
-
-  if (disassembler_sp)
-return disassembler_sp;
+  if (auto disasm_sp = create_callback(arch, flavor))
+return disasm_sp;
 }
   }
   return DisassemblerSP();
Index: lldb/include/lldb/lldb-private-interfaces.h
===
--- lldb/include/lldb/lldb-private-interfaces.h
+++ lldb/include/lldb/lldb-private-interfaces.h
@@ -30,8 +30,8 @@
  const ArchSpec &arch);
 typedef std::unique_ptr (*ArchitectureCreateInstance)(
 const ArchSpec &arch);
-typedef Disassembler *(*DisassemblerCreateInstance)(const ArchSpec &arch,
-const char *flavor);
+typedef lldb::DisassemblerSP (*DisassemblerCreateInstance)(const ArchSpec 
&arch,
+   const char *flavor);
 typedef DynamicLoader *(*DynamicLoaderCreateInstance)(Process *process,
   bool force);
 typedef lldb::JITLoaderSP (*JITLoaderCreateInstance)(Process *process,


Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
===
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
+++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
@@ -34,8 +34,8 @@
 
   static llvm::StringRef GetPluginNameStatic() { return "llvm-mc"; }
 
-  static lldb_private::Disassembler *
-  CreateInstance(const lldb_private::ArchSpec &arch, const char *flavor);
+  static lldb::DisassemblerSP CreateInstance(const lldb_private::ArchSpec &arch,
+ const char *flavor);
 
   size_t DecodeInstructions(const lldb_

[Lldb-commits] [lldb] 870eb04 - [lldb] Set CMAKE_CXX_STANDARD before including LLDBStandalone

2023-05-15 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2023-05-15T14:32:15-07:00
New Revision: 870eb04f1005da8278673f3cd1d1a640d16b63e6

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

LOG: [lldb] Set CMAKE_CXX_STANDARD before including LLDBStandalone

Set the C++ language standard before including LLDBStandalone.cmake.
Otherwise we risk building some of our dependencies (such as llvm_gtest)
without C++ 17 support.

This should fix the standalone bot [1] which is currently failing with the
following error:

  test-port.h:841:12: error: no member named 'tuple' in namespace 'std'
  using std::tuple;

[1] https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake-standalone

Added: 


Modified: 
lldb/CMakeLists.txt

Removed: 




diff  --git a/lldb/CMakeLists.txt b/lldb/CMakeLists.txt
index 9ae6722295ac..4a53d7ef3d0d 100644
--- a/lldb/CMakeLists.txt
+++ b/lldb/CMakeLists.txt
@@ -27,11 +27,11 @@ include(GNUInstallDirs)
 option(LLDB_INCLUDE_TESTS "Generate build targets for the LLDB unit tests." 
${LLVM_INCLUDE_TESTS})
 
 if(LLDB_BUILT_STANDALONE)
-  include(LLDBStandalone)
-
   set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to conform to")
   set(CMAKE_CXX_STANDARD_REQUIRED YES)
   set(CMAKE_CXX_EXTENSIONS NO)
+
+  include(LLDBStandalone)
 endif()
 
 include(LLDBConfig)



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


[Lldb-commits] [PATCH] D149914: [lldb] Refactor ObjCLanguage::MethodName

2023-05-15 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

Your test measured setting a found simple breakpoint.  That should measure 
filling all the names caches - we do that the first time you try to set a 
breakpoint of any sort.  But doesn't measure the effects on lookup.  I am 
guessing you will find the same "not much difference" here as well, but it 
would be good to test that.  So it would be good to also ensure you aren't 
slowing down looking for a selector by name, and looking for a selector you 
aren't going to find by name, and looking by full ObjC name.  But if that's 
also true, then I'm fine with this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149914

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


[Lldb-commits] [PATCH] D149379: [lldb] Add tests for command removal

2023-05-15 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

In D149379#4331059 , @wallace wrote:

> The alias still works because it still holds a reference to it. I could add 
> that as a test

The alias works but the original command is gone?  Interesting.  We should 
definitely test that that works correctly.  We should also test what happens 
when a regex command dispatches to a command that has been deleted.

Also, should the "delete" operation fail or return an error if the actual 
command is still made available because of an alias.  If your intention was to 
remove a built-in command (still not sure why that is an operation that makes 
sense, but that aside...) then you haven't if you can still get to it through 
an alias.  Should we tell the user that?

, removing user commands is cosmetic if you don't remove `script` as well, 
since there's pretty much nothing that can be done on the command line that 
can't be done with scripts.  So I don't have a sense for how strict you are 
trying to be.  But if I can defeat the removal of a command by putting an alias 
in my .lldbinit, you are being very not strict...




Comment at: lldb/include/lldb/API/SBCommandInterpreter.h:316
 
+  /// Remove a command if it is removable (python or regex command). If \b 
force
+  /// is provided, the command is removed regardless of its removable status.

You should probably say explicitly what the "removable status" of a built-in 
command is.  You (correctly) have to explicitly pass force=true for that to 
work but it would be good to state that explicitly.  We should also emphasize 
here there isn't any way to get a built-in command back once you've deleted it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149379

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


[Lldb-commits] [PATCH] D150619: [lldb] Delay removal of persistent results

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

Follow up to "Suppress persistent result when running po" (D144044 
).

This change delays removal of the persistent result until after `Dump` has been 
called.
In doing so, the persistent result is available for the purpose of getting its 
object
description.

In the original change, the persistent result removal happens indirectly, by 
setting
`EvaluateExpressionOptions::SetSuppressPersistentResult`. In practice this has 
worked,
however this exposed a latent bug in swift-lldb. The subtlety, and the bug, 
depend on
when the persisteted result variable is removed.

When the result is removed via `SetSuppressPersistentResult`, it happens within 
the call
to `Target::EvaluateExpression`. That is, by the time the call returns, the 
persistent
result is already removed.

The issue occurs shortly thereafter, when `ValueObject::Dump` is called, it 
cannot make
use of the persistent result variable (instead it uses the 
`ValueObjectConstResult`). In
swift-lldb, this causes an additional expression evaluation to happen. It first 
tries an
expression that reference `$R0` etc, but that always fails because `$R0` is 
removed. The
fallback to this failure does work most of the time, but there's at least one 
bug
involving imported Clang types.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150619

Files:
  lldb/source/Commands/CommandObjectDWIMPrint.cpp
  lldb/source/Commands/CommandObjectExpression.cpp
  lldb/source/Commands/CommandObjectExpression.h

Index: lldb/source/Commands/CommandObjectExpression.h
===
--- lldb/source/Commands/CommandObjectExpression.h
+++ lldb/source/Commands/CommandObjectExpression.h
@@ -41,6 +41,9 @@
 const Target &target,
 const OptionGroupValueObjectDisplay &display_opts);
 
+bool ShouldSuppressResult(
+const OptionGroupValueObjectDisplay &display_opts) const;
+
 bool top_level;
 bool unwind_on_error;
 bool ignore_breakpoints;
Index: lldb/source/Commands/CommandObjectExpression.cpp
===
--- lldb/source/Commands/CommandObjectExpression.cpp
+++ lldb/source/Commands/CommandObjectExpression.cpp
@@ -10,6 +10,7 @@
 
 #include "CommandObjectExpression.h"
 #include "lldb/Core/Debugger.h"
+#include "lldb/Expression/ExpressionVariable.h"
 #include "lldb/Expression/REPL.h"
 #include "lldb/Expression/UserExpression.h"
 #include "lldb/Host/OptionParser.h"
@@ -21,6 +22,7 @@
 #include "lldb/Target/Process.h"
 #include "lldb/Target/StackFrame.h"
 #include "lldb/Target/Target.h"
+#include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-private-enumerations.h"
 
 using namespace lldb;
@@ -200,13 +202,6 @@
 const Target &target, const OptionGroupValueObjectDisplay &display_opts) {
   EvaluateExpressionOptions options;
   options.SetCoerceToId(display_opts.use_objc);
-  // Explicitly disabling persistent results takes precedence over the
-  // m_verbosity/use_objc logic.
-  if (suppress_persistent_result != eLazyBoolCalculate)
-options.SetSuppressPersistentResult(suppress_persistent_result ==
-eLazyBoolYes);
-  else if (m_verbosity == eLanguageRuntimeDescriptionDisplayVerbosityCompact)
-options.SetSuppressPersistentResult(display_opts.use_objc);
   options.SetUnwindOnError(unwind_on_error);
   options.SetIgnoreBreakpoints(ignore_breakpoints);
   options.SetKeepInMemory(true);
@@ -242,6 +237,17 @@
   return options;
 }
 
+bool CommandObjectExpression::CommandOptions::ShouldSuppressResult(
+const OptionGroupValueObjectDisplay &display_opts) const {
+  // Explicitly disabling persistent results takes precedence over the
+  // m_verbosity/use_objc logic.
+  if (suppress_persistent_result != eLazyBoolCalculate)
+return suppress_persistent_result == eLazyBoolYes;
+
+  return display_opts.use_objc &&
+ m_verbosity == eLanguageRuntimeDescriptionDisplayVerbosityCompact;
+}
+
 CommandObjectExpression::CommandObjectExpression(
 CommandInterpreter &interpreter)
 : CommandObjectRaw(interpreter, "expression",
@@ -454,14 +460,27 @@
   }
 }
 
+bool suppress_result =
+m_command_options.ShouldSuppressResult(m_varobj_options);
+
 DumpValueObjectOptions options(m_varobj_options.GetAsDumpOptions(
 m_command_options.m_verbosity, format));
-options.SetHideRootName(eval_options.GetSuppressPersistentResult());
+options.SetHideRootName(suppress_result);
 options.SetVariableFormatDisplayLanguage(
 result_valobj_sp->GetPreferredDisplayLanguage());
 
 result_valobj_sp->Dump(output_stream, options);
 
+if (suppress

[Lldb-commits] [PATCH] D150619: [lldb] Delay removal of persistent results

2023-05-15 Thread Dave Lee via Phabricator via lldb-commits
kastiglione updated this revision to Diff 522354.
kastiglione added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150619

Files:
  lldb/source/Commands/CommandObjectDWIMPrint.cpp
  lldb/source/Commands/CommandObjectExpression.cpp
  lldb/source/Commands/CommandObjectExpression.h

Index: lldb/source/Commands/CommandObjectExpression.h
===
--- lldb/source/Commands/CommandObjectExpression.h
+++ lldb/source/Commands/CommandObjectExpression.h
@@ -41,6 +41,9 @@
 const Target &target,
 const OptionGroupValueObjectDisplay &display_opts);
 
+bool ShouldSuppressResult(
+const OptionGroupValueObjectDisplay &display_opts) const;
+
 bool top_level;
 bool unwind_on_error;
 bool ignore_breakpoints;
Index: lldb/source/Commands/CommandObjectExpression.cpp
===
--- lldb/source/Commands/CommandObjectExpression.cpp
+++ lldb/source/Commands/CommandObjectExpression.cpp
@@ -10,6 +10,7 @@
 
 #include "CommandObjectExpression.h"
 #include "lldb/Core/Debugger.h"
+#include "lldb/Expression/ExpressionVariable.h"
 #include "lldb/Expression/REPL.h"
 #include "lldb/Expression/UserExpression.h"
 #include "lldb/Host/OptionParser.h"
@@ -21,6 +22,7 @@
 #include "lldb/Target/Process.h"
 #include "lldb/Target/StackFrame.h"
 #include "lldb/Target/Target.h"
+#include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-private-enumerations.h"
 
 using namespace lldb;
@@ -200,13 +202,6 @@
 const Target &target, const OptionGroupValueObjectDisplay &display_opts) {
   EvaluateExpressionOptions options;
   options.SetCoerceToId(display_opts.use_objc);
-  // Explicitly disabling persistent results takes precedence over the
-  // m_verbosity/use_objc logic.
-  if (suppress_persistent_result != eLazyBoolCalculate)
-options.SetSuppressPersistentResult(suppress_persistent_result ==
-eLazyBoolYes);
-  else if (m_verbosity == eLanguageRuntimeDescriptionDisplayVerbosityCompact)
-options.SetSuppressPersistentResult(display_opts.use_objc);
   options.SetUnwindOnError(unwind_on_error);
   options.SetIgnoreBreakpoints(ignore_breakpoints);
   options.SetKeepInMemory(true);
@@ -242,6 +237,17 @@
   return options;
 }
 
+bool CommandObjectExpression::CommandOptions::ShouldSuppressResult(
+const OptionGroupValueObjectDisplay &display_opts) const {
+  // Explicitly disabling persistent results takes precedence over the
+  // m_verbosity/use_objc logic.
+  if (suppress_persistent_result != eLazyBoolCalculate)
+return suppress_persistent_result == eLazyBoolYes;
+
+  return display_opts.use_objc &&
+ m_verbosity == eLanguageRuntimeDescriptionDisplayVerbosityCompact;
+}
+
 CommandObjectExpression::CommandObjectExpression(
 CommandInterpreter &interpreter)
 : CommandObjectRaw(interpreter, "expression",
@@ -454,14 +460,27 @@
   }
 }
 
+bool suppress_result =
+m_command_options.ShouldSuppressResult(m_varobj_options);
+
 DumpValueObjectOptions options(m_varobj_options.GetAsDumpOptions(
 m_command_options.m_verbosity, format));
-options.SetHideRootName(eval_options.GetSuppressPersistentResult());
+options.SetHideRootName(suppress_result);
 options.SetVariableFormatDisplayLanguage(
 result_valobj_sp->GetPreferredDisplayLanguage());
 
 result_valobj_sp->Dump(output_stream, options);
 
+if (suppress_result)
+  if (auto result_var_sp =
+  target.GetPersistentVariable(result_valobj_sp->GetName())) {
+auto language = eval_options.GetLanguage();
+if (language == lldb::eLanguageTypeUnknown)
+  language = frame->GuessLanguage();
+if (auto *persistent_state =
+target.GetPersistentExpressionStateForLanguage(language))
+  persistent_state->RemovePersistentVariable(result_var_sp);
+  }
 result.SetStatus(eReturnStatusSuccessFinishResult);
   }
 } else {
Index: lldb/source/Commands/CommandObjectDWIMPrint.cpp
===
--- lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -10,6 +10,7 @@
 
 #include "lldb/Core/ValueObject.h"
 #include "lldb/DataFormatters/DumpValueObjectOptions.h"
+#include "lldb/Expression/ExpressionVariable.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandObject.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
@@ -76,6 +77,7 @@
   // If the user has not specified, default to disabling persistent results.
   if (m_expr_options.suppress_persistent_result == eLazyBoolCalculate)
 m_expr_options.suppress_persistent_result = eLazyBoolYes;
+  bool suppress

[Lldb-commits] [lldb] 8d4f0e0 - [lldb] Refine call to decl printing helper (NFC)

2023-05-15 Thread Dave Lee via lldb-commits

Author: Dave Lee
Date: 2023-05-15T15:20:48-07:00
New Revision: 8d4f0e079554c5eb5c9effda58dbda3b17f03160

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

LOG: [lldb] Refine call to decl printing helper (NFC)

When `ValueObjectPrinter` calls its `m_decl_printing_helper`, not all state is 
passed to
the helper. In particular, the helper doesn't have access to `m_curr_depth`, 
and thus
can't act on the logic within `ShouldShowName`.

To address this, this change passes in a modified copy of `m_options`. The 
modified copy
has has `m_hide_name` set according to the results of `ShouldShowName`. This 
allows
helper functions to know whether the name should be shown or hidden, without 
having
access to `ValueObjectPrinter`'s full state.

This is NFC in mainline lldb, as the only decl printing helper doesn't make use 
of this.
However in swift-lldb at least, there are decl printing helpers that do need 
this
information passed to them. See https://github.com/apple/llvm-project/pull/6795 
where a
test is also included.

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

Added: 


Modified: 
lldb/source/DataFormatters/ValueObjectPrinter.cpp

Removed: 




diff  --git a/lldb/source/DataFormatters/ValueObjectPrinter.cpp 
b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
index 8e44f723165b7..bde999a7a8bcf 100644
--- a/lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -300,9 +300,14 @@ void ValueObjectPrinter::PrintDecl() {
 ConstString type_name_cstr(typeName.GetString());
 ConstString var_name_cstr(varName.GetString());
 
+DumpValueObjectOptions decl_print_options = m_options;
+// Pass printing helpers an option object that indicates whether the name
+// should be shown or hidden.
+decl_print_options.SetHideName(!ShouldShowName());
+
 StreamString dest_stream;
 if (m_options.m_decl_printing_helper(type_name_cstr, var_name_cstr,
- m_options, dest_stream)) {
+ decl_print_options, dest_stream)) {
   decl_printed = true;
   m_stream->PutCString(dest_stream.GetString());
 }



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


[Lldb-commits] [PATCH] D150129: [lldb] Refine call to decl printing helper (NFC)

2023-05-15 Thread Dave Lee via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8d4f0e079554: [lldb] Refine call to decl printing helper 
(NFC) (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150129

Files:
  lldb/source/DataFormatters/ValueObjectPrinter.cpp


Index: lldb/source/DataFormatters/ValueObjectPrinter.cpp
===
--- lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -300,9 +300,14 @@
 ConstString type_name_cstr(typeName.GetString());
 ConstString var_name_cstr(varName.GetString());
 
+DumpValueObjectOptions decl_print_options = m_options;
+// Pass printing helpers an option object that indicates whether the name
+// should be shown or hidden.
+decl_print_options.SetHideName(!ShouldShowName());
+
 StreamString dest_stream;
 if (m_options.m_decl_printing_helper(type_name_cstr, var_name_cstr,
- m_options, dest_stream)) {
+ decl_print_options, dest_stream)) {
   decl_printed = true;
   m_stream->PutCString(dest_stream.GetString());
 }


Index: lldb/source/DataFormatters/ValueObjectPrinter.cpp
===
--- lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -300,9 +300,14 @@
 ConstString type_name_cstr(typeName.GetString());
 ConstString var_name_cstr(varName.GetString());
 
+DumpValueObjectOptions decl_print_options = m_options;
+// Pass printing helpers an option object that indicates whether the name
+// should be shown or hidden.
+decl_print_options.SetHideName(!ShouldShowName());
+
 StreamString dest_stream;
 if (m_options.m_decl_printing_helper(type_name_cstr, var_name_cstr,
- m_options, dest_stream)) {
+ decl_print_options, dest_stream)) {
   decl_printed = true;
   m_stream->PutCString(dest_stream.GetString());
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D150621: lldb PlatformDarwinKernel, delay local filesystem scan until needed

2023-05-15 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda created this revision.
jasonmolenda added a reviewer: bulbazord.
jasonmolenda added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.

The darwin kernel debugging platform plugin has a feature where it will scan a 
list of directories on the local filesystem for kernel binaries and kexts, 
loadable kernel extensions.  When it finds a kernel/kext with a given UUID, it 
looks over the list of binaries it found, and loads them automatically.

In https://reviews.llvm.org/D133680 I added a feature to PlatformDarwinKernel 
where the platform is force-created, and then a method is called on it with an 
address in memory.  If the address is a Mach-O fileset, it looks for a kernel 
in the fileset and returns the address of the kernel binary.  This allows for 
generic code (live debugging, corefile loading) to handle this 
platform-specific case in a generic way.

In the ctor for PlatformDarwinKernel, I was doing this local filesystem scan.  
And if a generic bit of code is force creating this platform to run that method 
for each binary, we could end up iterating over the local filesystem multiple 
times.  In extreme cases, it was a noticeable perf hit.

This patch moves the filesystem scan out of the ctor and into a method which is 
called in the two places where this platform actually needs the list of 
binaries.  It grabs a mutex before doing the scan, to avoid a race if multiple 
threads print the platform status or try to find a kext/kernel on the local 
filesystem.  There isn't any change in behavior beyond the performance 
difference when this platform is force-created multiple times in a debug 
session.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150621

Files:
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.h


Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.h
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.h
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.h
@@ -158,6 +158,8 @@
   bool LoadPlatformBinaryAndSetup(Process *process, lldb::addr_t addr,
   bool notify) override;
 
+  void UpdateKextandKernelsLocalScan();
+
   // Most of the ivars are assembled under FileSystem::EnumerateDirectory calls
   // where the function being called for each file/directory must be static.
   // We'll pass a this pointer as a baton and access the ivars directly.
@@ -194,6 +196,9 @@
 
   LazyBool m_ios_debug_session;
 
+  std::mutex m_kext_scan_mutex;
+  bool m_did_kext_scan;
+
   PlatformDarwinKernel(const PlatformDarwinKernel &) = delete;
   const PlatformDarwinKernel &operator=(const PlatformDarwinKernel &) = delete;
 };
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
@@ -229,12 +229,8 @@
   m_name_to_kext_path_map_without_dsyms(), m_search_directories(),
   m_search_directories_no_recursing(), m_kernel_binaries_with_dsyms(),
   m_kernel_binaries_without_dsyms(), m_kernel_dsyms_no_binaries(),
-  m_kernel_dsyms_yaas(), m_ios_debug_session(is_ios_debug_session)
-
-{
-  CollectKextAndKernelDirectories();
-  SearchForKextsAndKernelsRecursively();
-}
+  m_kernel_dsyms_yaas(), m_ios_debug_session(is_ios_debug_session),
+  m_kext_scan_mutex(), m_did_kext_scan(false) {}
 
 /// Destructor.
 ///
@@ -243,6 +239,7 @@
 PlatformDarwinKernel::~PlatformDarwinKernel() = default;
 
 void PlatformDarwinKernel::GetStatus(Stream &strm) {
+  UpdateKextandKernelsLocalScan();
   Platform::GetStatus(strm);
   strm.Printf(" Debug session type: ");
   if (m_ios_debug_session == eLazyBoolYes)
@@ -709,6 +706,15 @@
   return results;
 }
 
+void PlatformDarwinKernel::UpdateKextandKernelsLocalScan() {
+  std::lock_guard guard(m_kext_scan_mutex);
+  if (!m_did_kext_scan) {
+CollectKextAndKernelDirectories();
+SearchForKextsAndKernelsRecursively();
+m_did_kext_scan = true;
+  }
+}
+
 Status PlatformDarwinKernel::GetSharedModule(
 const ModuleSpec &module_spec, Process *process, ModuleSP &module_sp,
 const FileSpecList *module_search_paths_ptr,
@@ -789,6 +795,7 @@
 llvm::SmallVectorImpl *old_modules, bool *did_create_ptr) {
   Status error;
   module_sp.reset();
+  UpdateKextandKernelsLocalScan();
 
   // First try all kernel binaries that have a dSYM next to them
   for (auto possible_kernel : m_kernel_binaries_with_dsyms) {


Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.h
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.h
+

[Lldb-commits] [PATCH] D150619: [lldb] Delay removal of persistent results

2023-05-15 Thread Dave Lee via Phabricator via lldb-commits
kastiglione added a comment.

This change is tested using the original tests from D144044 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150619

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


[Lldb-commits] [PATCH] D149914: [lldb] Refactor ObjCLanguage::MethodName

2023-05-15 Thread Alex Langford via Phabricator via lldb-commits
bulbazord added a comment.

Some more numbers:

  % hyperfine -w 3 -- "$lldb -x -b -o 'b main' -o 'r' -o 'c' -o 'b $Selector' 
-o 'b $NonExistentSelector' -o 'b $FullObjCName' $App"
  Benchmark 1: $lldb -x -b -o 'b main' -o 'r' -o 'c' -o 'b $Selector' -o 'b 
$NonExistentSelector' -o 'b $FullObjCName' $App
Time (mean ± σ):  6.323 s ±  0.069 s[User: 5.626 s, System: 0.301 s]
Range (min … max):6.224 s …  6.443 s10 runs
  
  % hyperfine -w 3 -- "$lldb -x -b -o 'b main' -o 'r' -o 'c' -o 'b $Selector' 
-o 'b $NonExistentSelector' -o 'b $FullObjCName' $App"
  Benchmark 1: $lldb -x -b -o 'b main' -o 'r' -o 'c' -o 'b $Selector' -o 'b 
$NonExistentSelector' -o 'b $FullObjCName' $App
Time (mean ± σ):  6.270 s ±  0.042 s[User: 5.557 s, System: 0.313 s]
Range (min … max):6.210 s …  6.338 s10 runs

I hope this is sufficient to show we're not regressing performance. I measured 
this a few times with and without my change and I observed that this is usually 
faster but they are usually within 100ms of each other.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149914

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


[Lldb-commits] [PATCH] D149914: [lldb] Refactor ObjCLanguage::MethodName

2023-05-15 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

LGTM, Adrian's comment is still outstanding however.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149914

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


[Lldb-commits] [PATCH] D149914: [lldb] Refactor ObjCLanguage::MethodName

2023-05-15 Thread Alex Langford via Phabricator via lldb-commits
bulbazord added a comment.

In D149914#4335858 , @aprantl wrote:

> I'm not opposed to using this implementation, but have you considered using 
> something like the stdlib regex library to do the heavy lifting?

I talked to Jonas and did a little research. It seems like `` is quite 
slow and many supported compilers have buggy implementations. If I were to use 
regular expressions to rewrite this, I'd likely use whatever LLVM has 
implemented. That being said, I'm not too keen on holding up this patch because 
of that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149914

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


[Lldb-commits] [PATCH] D150624: [lldb] Fix lua build after 27b6a4e63afe

2023-05-15 Thread Alex Langford via Phabricator via lldb-commits
bulbazord created this revision.
bulbazord added reviewers: kparzysz, JDevlieghere, mib.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This applies the same trick for Lua that I did for python in
27b6a4e63afe 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150624

Files:
  lldb/bindings/lua/lua-wrapper.swig
  lldb/include/lldb/API/SBBreakpointLocation.h
  lldb/include/lldb/API/SBFrame.h
  lldb/include/lldb/API/SBStructuredData.h
  lldb/include/lldb/API/SBWatchpoint.h
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/SWIGLuaBridge.h
  lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp

Index: lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
+++ lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
@@ -14,14 +14,16 @@
 
 extern "C" int luaopen_lldb(lua_State *L) { return 0; }
 
-llvm::Expected lldb_private::LLDBSwigLuaBreakpointCallbackFunction(
+llvm::Expected
+lldb_private::lua::SWIGBridge::LLDBSwigLuaBreakpointCallbackFunction(
 lua_State *L, lldb::StackFrameSP stop_frame_sp,
 lldb::BreakpointLocationSP bp_loc_sp,
 const StructuredDataImpl &extra_args_impl) {
   return false;
 }
 
-llvm::Expected lldb_private::LLDBSwigLuaWatchpointCallbackFunction(
+llvm::Expected
+lldb_private::lua::SWIGBridge::LLDBSwigLuaWatchpointCallbackFunction(
 lua_State *L, lldb::StackFrameSP stop_frame_sp, lldb::WatchpointSP wp_sp) {
   return false;
 }
Index: lldb/source/Plugins/ScriptInterpreter/Lua/SWIGLuaBridge.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/SWIGLuaBridge.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/SWIGLuaBridge.h
@@ -15,13 +15,20 @@
 
 namespace lldb_private {
 
-llvm::Expected LLDBSwigLuaBreakpointCallbackFunction(
-lua_State *L, lldb::StackFrameSP stop_frame_sp,
-lldb::BreakpointLocationSP bp_loc_sp,
-const StructuredDataImpl &extra_args_impl);
+namespace lua {
 
-llvm::Expected LLDBSwigLuaWatchpointCallbackFunction(
-lua_State *L, lldb::StackFrameSP stop_frame_sp, lldb::WatchpointSP wp_sp);
+class SWIGBridge {
+public:
+  static llvm::Expected LLDBSwigLuaBreakpointCallbackFunction(
+  lua_State *L, lldb::StackFrameSP stop_frame_sp,
+  lldb::BreakpointLocationSP bp_loc_sp,
+  const StructuredDataImpl &extra_args_impl);
+
+  static llvm::Expected LLDBSwigLuaWatchpointCallbackFunction(
+  lua_State *L, lldb::StackFrameSP stop_frame_sp, lldb::WatchpointSP wp_sp);
+};
+
+} // namespace lua
 
 } // namespace lldb_private
 
Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
@@ -83,8 +83,8 @@
   lua_pushlightuserdata(m_lua_state, baton);
   lua_gettable(m_lua_state, LUA_REGISTRYINDEX);
   StructuredDataImpl extra_args_impl(std::move(extra_args_sp));
-  return LLDBSwigLuaBreakpointCallbackFunction(m_lua_state, stop_frame_sp,
-   bp_loc_sp, extra_args_impl);
+  return lua::SWIGBridge::LLDBSwigLuaBreakpointCallbackFunction(
+  m_lua_state, stop_frame_sp, bp_loc_sp, extra_args_impl);
 }
 
 llvm::Error Lua::RegisterWatchpointCallback(void *baton, const char *body) {
@@ -109,8 +109,8 @@
 
   lua_pushlightuserdata(m_lua_state, baton);
   lua_gettable(m_lua_state, LUA_REGISTRYINDEX);
-  return LLDBSwigLuaWatchpointCallbackFunction(m_lua_state, stop_frame_sp,
-   wp_sp);
+  return lua::SWIGBridge::LLDBSwigLuaWatchpointCallbackFunction(
+  m_lua_state, stop_frame_sp, wp_sp);
 }
 
 llvm::Error Lua::CheckSyntax(llvm::StringRef buffer) {
Index: lldb/include/lldb/API/SBWatchpoint.h
===
--- lldb/include/lldb/API/SBWatchpoint.h
+++ lldb/include/lldb/API/SBWatchpoint.h
@@ -13,7 +13,10 @@
 #include "lldb/API/SBType.h"
 
 namespace lldb_private {
-namespace ptyhon {
+namespace python {
+class SWIGBridge;
+}
+namespace lua {
 class SWIGBridge;
 }
 } // namespace lldb_private
@@ -86,6 +89,7 @@
 
 protected:
   friend class lldb_private::python::SWIGBridge;
+  friend class lldb_private::lua::SWIGBridge;
 
   SBWatchpoint(const lldb::WatchpointSP &wp_sp);
 
Index: lldb/include/lldb/API/SBStructuredData.h
===
--- lldb/include/lldb/API/SBStructuredData.h
+++ lldb/include/lldb/API/SBStructuredData.h
@@ -16,6 +16,9 @@
 namespace python {
 class SWIGBridge;
 }
+namespace lua {
+class SWIGBridge;
+}
 } // namespace lldb_private
 
 namespace lldb {
@@ -104,6 +107,7 @@
   friend 

[Lldb-commits] [PATCH] D150157: [lldb] Mark most SBAPI methods involving private types as protected or private

2023-05-15 Thread Alex Langford via Phabricator via lldb-commits
bulbazord added a comment.

This broke Lua support, so I am fixing it in D150624 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150157

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


[Lldb-commits] [PATCH] D150624: [lldb] Fix lua build after 27b6a4e63afe

2023-05-15 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib requested changes to this revision.
mib added a comment.
This revision now requires changes to proceed.

We've already discussed that offline but I really think there should be a 
ScriptInterpreter `SWIGBridge` plugin and that would have all the method the 
Python and Lua SWIGBridge have in common, so the SB classes won't have to be 
specialized for every scripting language.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150624

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


[Lldb-commits] [PATCH] D150590: [lldb][DWARFASTParserClang][NFC] Extract condition for unnamed bitfield creation into helper function

2023-05-15 Thread Michael Buch via Phabricator via lldb-commits
Michael137 updated this revision to Diff 522382.
Michael137 added a comment.

- Add doxygen comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150590

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -236,6 +236,22 @@
 }
   };
 
+  /// Returns 'true' if we should create an unnamed bitfield
+  /// and add it to the parser's current AST.
+  ///
+  /// \param[in] last_field_info FieldInfo of the previous DW_TAG_member
+  ///we parsed.
+  /// \param[in] last_field_end Offset (in bits) where the last parsed field
+  ///ended.
+  /// \param[in] this_field_info FieldInfo of the current DW_TAG_member
+  ///being parsed.
+  /// \param[in] layout_info Layout information of all decls parsed by the
+  ///current parser.
+  bool ShouldCreateUnnamedBitfield(
+  FieldInfo const &last_field_info, uint64_t last_field_end,
+  FieldInfo const &this_field_info,
+  lldb_private::ClangASTImporter::LayoutInfo const &layout_info) const;
+
   /// Parses a DW_TAG_APPLE_property DIE and appends the parsed data to the
   /// list of delayed Objective-C properties.
   ///
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2893,18 +2893,8 @@
   last_field_end += word_width - (last_field_end % word_width);
   }
 
-  // If we have a gap between the last_field_end and the current
-  // field we have an unnamed bit-field.
-  // If we have a base class, we assume there is no unnamed
-  // bit-field if this is the first field since the gap can be
-  // attributed to the members from the base class. This assumption
-  // is not correct if the first field of the derived class is
-  // indeed an unnamed bit-field. We currently do not have the
-  // machinary to track the offset of the last field of classes we
-  // have seen before, so we are not handling this case.
-  if (this_field_info.bit_offset > last_field_end &&
-  !(last_field_info.bit_offset == 0 && last_field_info.bit_size == 0 &&
-layout_info.base_offsets.size() != 0)) {
+  if (ShouldCreateUnnamedBitfield(last_field_info, last_field_end,
+  this_field_info, layout_info)) {
 unnamed_field_info = FieldInfo{};
 unnamed_field_info->bit_size =
 this_field_info.bit_offset - last_field_end;
@@ -3696,3 +3686,29 @@
 
   return !failures.empty();
 }
+
+bool DWARFASTParserClang::ShouldCreateUnnamedBitfield(
+FieldInfo const &last_field_info, uint64_t last_field_end,
+FieldInfo const &this_field_info,
+lldb_private::ClangASTImporter::LayoutInfo const &layout_info) const {
+  // If we have a gap between the last_field_end and the current
+  // field we have an unnamed bit-field.
+  if (this_field_info.bit_offset <= last_field_end)
+return false;
+
+  // If we have a base class, we assume there is no unnamed
+  // bit-field if this is the first field since the gap can be
+  // attributed to the members from the base class. This assumption
+  // is not correct if the first field of the derived class is
+  // indeed an unnamed bit-field. We currently do not have the
+  // machinary to track the offset of the last field of classes we
+  // have seen before, so we are not handling this case.
+  const bool have_base = layout_info.base_offsets.size() != 0;
+  const bool this_is_first_field =
+  last_field_info.bit_offset == 0 && last_field_info.bit_size == 0;
+
+  if (have_base && this_is_first_field)
+return false;
+
+  return true;
+}


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -236,6 +236,22 @@
 }
   };
 
+  /// Returns 'true' if we should create an unnamed bitfield
+  /// and add it to the parser's current AST.
+  ///
+  /// \param[in] last_field_info FieldInfo of the previous DW_TAG_member
+  ///we parsed.
+  /// \param[in] last_field_end Offset (in bits) where the last parsed field
+  ///ended.
+  /// \param[in] this_field_info FieldInfo of the current DW_TAG_member
+  ///being parsed.
+  /// \param[in] layout_info Layout information of all decls parsed by the
+  ///   

[Lldb-commits] [PATCH] D150591: [lldb][DWARFASTParserClang] Don't create unnamed bitfields to account for vtable pointer

2023-05-15 Thread Michael Buch via Phabricator via lldb-commits
Michael137 updated this revision to Diff 522383.
Michael137 added a comment.

- Rephrase comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150591

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
  lldb/test/API/lang/cpp/bitfields/main.cpp

Index: lldb/test/API/lang/cpp/bitfields/main.cpp
===
--- lldb/test/API/lang/cpp/bitfields/main.cpp
+++ lldb/test/API/lang/cpp/bitfields/main.cpp
@@ -113,6 +113,12 @@
 };
 HasBaseWithVTable base_with_vtable;
 
+struct DerivedWithVTable : public Base {
+  virtual ~DerivedWithVTable() {}
+  unsigned a : 1;
+};
+DerivedWithVTable derived_with_vtable;
+
 int main(int argc, char const *argv[]) {
   lba.a = 2;
 
@@ -153,5 +159,8 @@
   base_with_vtable.b = 0;
   base_with_vtable.c = 5;
 
+  derived_with_vtable.b_a = 2;
+  derived_with_vtable.a = 1;
+
   return 0; // break here
 }
Index: lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
===
--- lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
+++ lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
@@ -168,3 +168,14 @@
  result_children=with_vtable_and_unnamed_children)
 self.expect_var_path("with_vtable_and_unnamed",
  children=with_vtable_and_unnamed_children)
+
+derived_with_vtable_children = [
+ValueCheck(name="Base", children=[
+  ValueCheck(name="b_a", value="2", type="uint32_t")
+]),
+ValueCheck(name="a", value="1", type="unsigned int:1")
+]
+self.expect_expr("derived_with_vtable",
+ result_children=derived_with_vtable_children)
+self.expect_var_path("derived_with_vtable",
+ children=derived_with_vtable_children)
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -223,12 +223,16 @@
 uint64_t bit_size = 0;
 uint64_t bit_offset = 0;
 bool is_bitfield = false;
+bool is_artificial = false;
 
 FieldInfo() = default;
 
 void SetIsBitfield(bool flag) { is_bitfield = flag; }
 bool IsBitfield() { return is_bitfield; }
 
+void SetIsArtificial(bool flag) { is_artificial = flag; }
+bool IsArtificial() const { return is_artificial; }
+
 bool NextBitfieldOffsetIsValid(const uint64_t next_bit_offset) const {
   // Any subsequent bitfields must not overlap and must be at a higher
   // bit offset than any previous bitfield + size.
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2935,8 +2935,10 @@
   // artificial member with (unnamed bitfield) padding.
   // FIXME: This check should verify that this is indeed an artificial member
   // we are supposed to ignore.
-  if (attrs.is_artificial)
+  if (attrs.is_artificial) {
+last_field_info.SetIsArtificial(true);
 return;
+  }
 
   if (!member_clang_type.IsCompleteType())
 member_clang_type.GetCompleteType();
@@ -3697,17 +3699,23 @@
 return false;
 
   // If we have a base class, we assume there is no unnamed
-  // bit-field if this is the first field since the gap can be
-  // attributed to the members from the base class. This assumption
-  // is not correct if the first field of the derived class is
-  // indeed an unnamed bit-field. We currently do not have the
-  // machinary to track the offset of the last field of classes we
-  // have seen before, so we are not handling this case.
+  // bit-field if either of the following is true:
+  // (a) this is the first field since the gap can be
+  // attributed to the members from the base class.
+  // FIXME: This assumption is not correct if the first field of
+  // the derived class is indeed an unnamed bit-field. We currently
+  // do not have the machinary to track the offset of the last field
+  // of classes we have seen before, so we are not handling this case.
+  // (b) Or, the first member of the derived class was a vtable pointer.
+  // In this case we don't want to create an unnamed bitfield either
+  // since those will be inserted by clang later.
   const bool have_base = layout_info.base_offsets.size() != 0;
   const bool this_is_first_field =
   last_field_info.bit_offset == 0 && last_field_info.bit_size == 0;
+  const bool first_field_is_vptr =
+  last_field_info.bit_offset == 0 && last_field_i

[Lldb-commits] [PATCH] D150619: [lldb] Delay removal of persistent results

2023-05-15 Thread Dave Lee via Phabricator via lldb-commits
kastiglione updated this revision to Diff 522384.
kastiglione added a comment.

check frame pointer before calling GuessLanguage


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150619

Files:
  lldb/source/Commands/CommandObjectDWIMPrint.cpp
  lldb/source/Commands/CommandObjectExpression.cpp
  lldb/source/Commands/CommandObjectExpression.h

Index: lldb/source/Commands/CommandObjectExpression.h
===
--- lldb/source/Commands/CommandObjectExpression.h
+++ lldb/source/Commands/CommandObjectExpression.h
@@ -41,6 +41,9 @@
 const Target &target,
 const OptionGroupValueObjectDisplay &display_opts);
 
+bool ShouldSuppressResult(
+const OptionGroupValueObjectDisplay &display_opts) const;
+
 bool top_level;
 bool unwind_on_error;
 bool ignore_breakpoints;
Index: lldb/source/Commands/CommandObjectExpression.cpp
===
--- lldb/source/Commands/CommandObjectExpression.cpp
+++ lldb/source/Commands/CommandObjectExpression.cpp
@@ -10,6 +10,7 @@
 
 #include "CommandObjectExpression.h"
 #include "lldb/Core/Debugger.h"
+#include "lldb/Expression/ExpressionVariable.h"
 #include "lldb/Expression/REPL.h"
 #include "lldb/Expression/UserExpression.h"
 #include "lldb/Host/OptionParser.h"
@@ -21,6 +22,7 @@
 #include "lldb/Target/Process.h"
 #include "lldb/Target/StackFrame.h"
 #include "lldb/Target/Target.h"
+#include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-private-enumerations.h"
 
 using namespace lldb;
@@ -200,13 +202,6 @@
 const Target &target, const OptionGroupValueObjectDisplay &display_opts) {
   EvaluateExpressionOptions options;
   options.SetCoerceToId(display_opts.use_objc);
-  // Explicitly disabling persistent results takes precedence over the
-  // m_verbosity/use_objc logic.
-  if (suppress_persistent_result != eLazyBoolCalculate)
-options.SetSuppressPersistentResult(suppress_persistent_result ==
-eLazyBoolYes);
-  else if (m_verbosity == eLanguageRuntimeDescriptionDisplayVerbosityCompact)
-options.SetSuppressPersistentResult(display_opts.use_objc);
   options.SetUnwindOnError(unwind_on_error);
   options.SetIgnoreBreakpoints(ignore_breakpoints);
   options.SetKeepInMemory(true);
@@ -242,6 +237,17 @@
   return options;
 }
 
+bool CommandObjectExpression::CommandOptions::ShouldSuppressResult(
+const OptionGroupValueObjectDisplay &display_opts) const {
+  // Explicitly disabling persistent results takes precedence over the
+  // m_verbosity/use_objc logic.
+  if (suppress_persistent_result != eLazyBoolCalculate)
+return suppress_persistent_result == eLazyBoolYes;
+
+  return display_opts.use_objc &&
+ m_verbosity == eLanguageRuntimeDescriptionDisplayVerbosityCompact;
+}
+
 CommandObjectExpression::CommandObjectExpression(
 CommandInterpreter &interpreter)
 : CommandObjectRaw(interpreter, "expression",
@@ -454,14 +460,27 @@
   }
 }
 
+bool suppress_result =
+m_command_options.ShouldSuppressResult(m_varobj_options);
+
 DumpValueObjectOptions options(m_varobj_options.GetAsDumpOptions(
 m_command_options.m_verbosity, format));
-options.SetHideRootName(eval_options.GetSuppressPersistentResult());
+options.SetHideRootName(suppress_result);
 options.SetVariableFormatDisplayLanguage(
 result_valobj_sp->GetPreferredDisplayLanguage());
 
 result_valobj_sp->Dump(output_stream, options);
 
+if (suppress_result)
+  if (auto result_var_sp =
+  target.GetPersistentVariable(result_valobj_sp->GetName())) {
+auto language = eval_options.GetLanguage();
+if (frame && language == lldb::eLanguageTypeUnknown)
+  language = frame->GuessLanguage();
+if (auto *persistent_state =
+target.GetPersistentExpressionStateForLanguage(language))
+  persistent_state->RemovePersistentVariable(result_var_sp);
+  }
 result.SetStatus(eReturnStatusSuccessFinishResult);
   }
 } else {
Index: lldb/source/Commands/CommandObjectDWIMPrint.cpp
===
--- lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -10,6 +10,7 @@
 
 #include "lldb/Core/ValueObject.h"
 #include "lldb/DataFormatters/DumpValueObjectOptions.h"
+#include "lldb/Expression/ExpressionVariable.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandObject.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
@@ -76,6 +77,7 @@
   // If the user has not specified, default to disabling persistent results.
   if (m_expr_options.suppress_persistent_result == eLazyBoolCalculate)
 m_expr_options.suppress

[Lldb-commits] [PATCH] D150619: [lldb] Delay removal of persistent results

2023-05-15 Thread Dave Lee via Phabricator via lldb-commits
kastiglione added a comment.

s/frame pointer/pointer to StackFrame/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150619

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


[Lldb-commits] [PATCH] D150621: lldb PlatformDarwinKernel, delay local filesystem scan until needed

2023-05-15 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere requested changes to this revision.
JDevlieghere added a comment.
This revision now requires changes to proceed.

You can do this simpler with a single `std::once_flag`.




Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp:710-715
+  std::lock_guard guard(m_kext_scan_mutex);
+  if (!m_did_kext_scan) {
+CollectKextAndKernelDirectories();
+SearchForKextsAndKernelsRecursively();
+m_did_kext_scan = true;
+  }





Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.h:199-200
 
+  std::mutex m_kext_scan_mutex;
+  bool m_did_kext_scan;
+




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150621

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


[Lldb-commits] [PATCH] D150624: [lldb] Fix lua build after 27b6a4e63afe

2023-05-15 Thread Alex Langford via Phabricator via lldb-commits
bulbazord requested review of this revision.
bulbazord added a comment.

So, I agree that we should definitely unify this or else we end up doing this 
for **every** scripting language that we add support for. That being said, I 
think this is actually better than what we had before because now we're not 
exposing lldb private details in the public interface. There is a TODO comment 
in `SWIGPythonBridge.h` already for cleaning this up when we have a moment... I 
don't want to block this patch unless we're ok reverting the previous one, and 
I'd preferably not revert that one...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150624

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


[Lldb-commits] [PATCH] D150624: [lldb] Fix lua build after 27b6a4e63afe

2023-05-15 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib accepted this revision.
mib added a comment.
This revision is now accepted and ready to land.

Let's fix the Lua build failures first


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150624

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


[Lldb-commits] [PATCH] D150624: [lldb] Fix lua build after 27b6a4e63afe

2023-05-15 Thread Alex Langford via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG692ae97ae71d: [lldb] Fix lua build after 27b6a4e63afe 
(authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150624

Files:
  lldb/bindings/lua/lua-wrapper.swig
  lldb/include/lldb/API/SBBreakpointLocation.h
  lldb/include/lldb/API/SBFrame.h
  lldb/include/lldb/API/SBStructuredData.h
  lldb/include/lldb/API/SBWatchpoint.h
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/SWIGLuaBridge.h
  lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp

Index: lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
+++ lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
@@ -14,14 +14,16 @@
 
 extern "C" int luaopen_lldb(lua_State *L) { return 0; }
 
-llvm::Expected lldb_private::LLDBSwigLuaBreakpointCallbackFunction(
+llvm::Expected
+lldb_private::lua::SWIGBridge::LLDBSwigLuaBreakpointCallbackFunction(
 lua_State *L, lldb::StackFrameSP stop_frame_sp,
 lldb::BreakpointLocationSP bp_loc_sp,
 const StructuredDataImpl &extra_args_impl) {
   return false;
 }
 
-llvm::Expected lldb_private::LLDBSwigLuaWatchpointCallbackFunction(
+llvm::Expected
+lldb_private::lua::SWIGBridge::LLDBSwigLuaWatchpointCallbackFunction(
 lua_State *L, lldb::StackFrameSP stop_frame_sp, lldb::WatchpointSP wp_sp) {
   return false;
 }
Index: lldb/source/Plugins/ScriptInterpreter/Lua/SWIGLuaBridge.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/SWIGLuaBridge.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/SWIGLuaBridge.h
@@ -15,13 +15,20 @@
 
 namespace lldb_private {
 
-llvm::Expected LLDBSwigLuaBreakpointCallbackFunction(
-lua_State *L, lldb::StackFrameSP stop_frame_sp,
-lldb::BreakpointLocationSP bp_loc_sp,
-const StructuredDataImpl &extra_args_impl);
+namespace lua {
 
-llvm::Expected LLDBSwigLuaWatchpointCallbackFunction(
-lua_State *L, lldb::StackFrameSP stop_frame_sp, lldb::WatchpointSP wp_sp);
+class SWIGBridge {
+public:
+  static llvm::Expected LLDBSwigLuaBreakpointCallbackFunction(
+  lua_State *L, lldb::StackFrameSP stop_frame_sp,
+  lldb::BreakpointLocationSP bp_loc_sp,
+  const StructuredDataImpl &extra_args_impl);
+
+  static llvm::Expected LLDBSwigLuaWatchpointCallbackFunction(
+  lua_State *L, lldb::StackFrameSP stop_frame_sp, lldb::WatchpointSP wp_sp);
+};
+
+} // namespace lua
 
 } // namespace lldb_private
 
Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
@@ -83,8 +83,8 @@
   lua_pushlightuserdata(m_lua_state, baton);
   lua_gettable(m_lua_state, LUA_REGISTRYINDEX);
   StructuredDataImpl extra_args_impl(std::move(extra_args_sp));
-  return LLDBSwigLuaBreakpointCallbackFunction(m_lua_state, stop_frame_sp,
-   bp_loc_sp, extra_args_impl);
+  return lua::SWIGBridge::LLDBSwigLuaBreakpointCallbackFunction(
+  m_lua_state, stop_frame_sp, bp_loc_sp, extra_args_impl);
 }
 
 llvm::Error Lua::RegisterWatchpointCallback(void *baton, const char *body) {
@@ -109,8 +109,8 @@
 
   lua_pushlightuserdata(m_lua_state, baton);
   lua_gettable(m_lua_state, LUA_REGISTRYINDEX);
-  return LLDBSwigLuaWatchpointCallbackFunction(m_lua_state, stop_frame_sp,
-   wp_sp);
+  return lua::SWIGBridge::LLDBSwigLuaWatchpointCallbackFunction(
+  m_lua_state, stop_frame_sp, wp_sp);
 }
 
 llvm::Error Lua::CheckSyntax(llvm::StringRef buffer) {
Index: lldb/include/lldb/API/SBWatchpoint.h
===
--- lldb/include/lldb/API/SBWatchpoint.h
+++ lldb/include/lldb/API/SBWatchpoint.h
@@ -13,7 +13,10 @@
 #include "lldb/API/SBType.h"
 
 namespace lldb_private {
-namespace ptyhon {
+namespace python {
+class SWIGBridge;
+}
+namespace lua {
 class SWIGBridge;
 }
 } // namespace lldb_private
@@ -86,6 +89,7 @@
 
 protected:
   friend class lldb_private::python::SWIGBridge;
+  friend class lldb_private::lua::SWIGBridge;
 
   SBWatchpoint(const lldb::WatchpointSP &wp_sp);
 
Index: lldb/include/lldb/API/SBStructuredData.h
===
--- lldb/include/lldb/API/SBStructuredData.h
+++ lldb/include/lldb/API/SBStructuredData.h
@@ -16,6 +16,9 @@
 namespace python {
 class SWIGBridge;
 }
+namespace lua {
+class SWIGBridge;
+}
 } // namespace lldb_private
 
 namespace lldb {
@@ -104,6 +107,7 @@
   friend class SBBreakpointName;
   friend class SBTrace;
   friend class lldb_private::python::SWIGBridge;
+  friend class lldb_private::lua::SWIGBrid

[Lldb-commits] [lldb] 692ae97 - [lldb] Fix lua build after 27b6a4e63afe

2023-05-15 Thread Alex Langford via lldb-commits

Author: Alex Langford
Date: 2023-05-15T17:24:00-07:00
New Revision: 692ae97ae71d62ce51d784681a0481fb54343fc1

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

LOG: [lldb] Fix lua build after 27b6a4e63afe

This applies the same trick for Lua that I did for python in
27b6a4e63afe.

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

Added: 


Modified: 
lldb/bindings/lua/lua-wrapper.swig
lldb/include/lldb/API/SBBreakpointLocation.h
lldb/include/lldb/API/SBFrame.h
lldb/include/lldb/API/SBStructuredData.h
lldb/include/lldb/API/SBWatchpoint.h
lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
lldb/source/Plugins/ScriptInterpreter/Lua/SWIGLuaBridge.h
lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp

Removed: 




diff  --git a/lldb/bindings/lua/lua-wrapper.swig 
b/lldb/bindings/lua/lua-wrapper.swig
index a36e69a6fc935..1409148873858 100644
--- a/lldb/bindings/lua/lua-wrapper.swig
+++ b/lldb/bindings/lua/lua-wrapper.swig
@@ -3,7 +3,8 @@
 template  void PushSBClass(lua_State * L, T * obj);
 
 // This function is called from Lua::CallBreakpointCallback
-llvm::Expected lldb_private::LLDBSwigLuaBreakpointCallbackFunction(
+llvm::Expected
+lldb_private::lua::SWIGBridge::LLDBSwigLuaBreakpointCallbackFunction(
 lua_State * L, lldb::StackFrameSP stop_frame_sp,
 lldb::BreakpointLocationSP bp_loc_sp,
 const StructuredDataImpl &extra_args_impl) {
@@ -41,7 +42,8 @@ llvm::Expected 
lldb_private::LLDBSwigLuaBreakpointCallbackFunction(
 }
 
 // This function is called from Lua::CallWatchpointCallback
-llvm::Expected lldb_private::LLDBSwigLuaWatchpointCallbackFunction(
+llvm::Expected
+lldb_private::lua::SWIGBridge::LLDBSwigLuaWatchpointCallbackFunction(
 lua_State * L, lldb::StackFrameSP stop_frame_sp, lldb::WatchpointSP wp_sp) 
{
   lldb::SBFrame sb_frame(stop_frame_sp);
   lldb::SBWatchpoint sb_wp(wp_sp);

diff  --git a/lldb/include/lldb/API/SBBreakpointLocation.h 
b/lldb/include/lldb/API/SBBreakpointLocation.h
index bc06aeeb6f1c2..fa823e2b518ac 100644
--- a/lldb/include/lldb/API/SBBreakpointLocation.h
+++ b/lldb/include/lldb/API/SBBreakpointLocation.h
@@ -16,6 +16,9 @@ namespace lldb_private {
 namespace python {
 class SWIGBridge;
 }
+namespace lua {
+class SWIGBridge;
+}
 } // namespace lldb_private
 
 namespace lldb {
@@ -98,6 +101,7 @@ class LLDB_API SBBreakpointLocation {
 
 protected:
   friend class lldb_private::python::SWIGBridge;
+  friend class lldb_private::lua::SWIGBridge;
   SBBreakpointLocation(const lldb::BreakpointLocationSP &break_loc_sp);
 
 private:

diff  --git a/lldb/include/lldb/API/SBFrame.h b/lldb/include/lldb/API/SBFrame.h
index 8fc4d510be4a7..7c4477f9125d1 100644
--- a/lldb/include/lldb/API/SBFrame.h
+++ b/lldb/include/lldb/API/SBFrame.h
@@ -16,6 +16,9 @@ namespace lldb_private {
 namespace python {
 class SWIGBridge;
 }
+namespace lua {
+class SWIGBridge;
+}
 } // namespace lldb_private
 
 namespace lldb {
@@ -198,6 +201,7 @@ class LLDB_API SBFrame {
   friend class SBValue;
 
   friend class lldb_private::python::SWIGBridge;
+  friend class lldb_private::lua::SWIGBridge;
 
   SBFrame(const lldb::StackFrameSP &lldb_object_sp);
 

diff  --git a/lldb/include/lldb/API/SBStructuredData.h 
b/lldb/include/lldb/API/SBStructuredData.h
index 3021069793bd1..75f8ebb7c9e75 100644
--- a/lldb/include/lldb/API/SBStructuredData.h
+++ b/lldb/include/lldb/API/SBStructuredData.h
@@ -16,6 +16,9 @@ namespace lldb_private {
 namespace python {
 class SWIGBridge;
 }
+namespace lua {
+class SWIGBridge;
+}
 } // namespace lldb_private
 
 namespace lldb {
@@ -104,6 +107,7 @@ class SBStructuredData {
   friend class SBBreakpointName;
   friend class SBTrace;
   friend class lldb_private::python::SWIGBridge;
+  friend class lldb_private::lua::SWIGBridge;
 
   SBStructuredData(const lldb_private::StructuredDataImpl &impl);
 

diff  --git a/lldb/include/lldb/API/SBWatchpoint.h 
b/lldb/include/lldb/API/SBWatchpoint.h
index de8e87f3a7322..dc613a840beb2 100644
--- a/lldb/include/lldb/API/SBWatchpoint.h
+++ b/lldb/include/lldb/API/SBWatchpoint.h
@@ -13,7 +13,10 @@
 #include "lldb/API/SBType.h"
 
 namespace lldb_private {
-namespace ptyhon {
+namespace python {
+class SWIGBridge;
+}
+namespace lua {
 class SWIGBridge;
 }
 } // namespace lldb_private
@@ -86,6 +89,7 @@ class LLDB_API SBWatchpoint {
 
 protected:
   friend class lldb_private::python::SWIGBridge;
+  friend class lldb_private::lua::SWIGBridge;
 
   SBWatchpoint(const lldb::WatchpointSP &wp_sp);
 

diff  --git a/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp 
b/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
index 9c2227cc3884e..8dad22d077be3 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
@@ -83,8 +83,8 @@ Lua::CallBre

[Lldb-commits] [PATCH] D150621: lldb PlatformDarwinKernel, delay local filesystem scan until needed

2023-05-15 Thread Alex Langford via Phabricator via lldb-commits
bulbazord added a comment.

In D150621#4344243 , @JDevlieghere 
wrote:

> You can do this simpler with a single `std::once_flag`.

+1

I like the idea of this patch a lot! LGTM otherwise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150621

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


[Lldb-commits] [PATCH] D150621: lldb PlatformDarwinKernel, delay local filesystem scan until needed

2023-05-15 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added a comment.

Ah, makes sense, will update.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150621

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


[Lldb-commits] [PATCH] D150630: [lldb][docs] Update SB API design document

2023-05-15 Thread Alex Langford via Phabricator via lldb-commits
bulbazord created this revision.
bulbazord added reviewers: JDevlieghere, jingham, mib, kastiglione.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

The documentation should have been updated in 662548c82683 
.
This updates it to be more accurate with the current design.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150630

Files:
  lldb/docs/design/sbapi.rst


Index: lldb/docs/design/sbapi.rst
===
--- lldb/docs/design/sbapi.rst
+++ lldb/docs/design/sbapi.rst
@@ -7,6 +7,9 @@
 incompatibilities that C++ is so susceptible to. We've established a few rules
 to ensure that this happens.
 
+Extending the SB API
+
+
 The classes in the SB API's are all called SB, where SomeName is in
 CamelCase starting with an upper case letter. The method names are all
 CamelCase with initial capital letter as well.
@@ -45,14 +48,29 @@
 
 Another piece of the SB API infrastructure is the Python (or other script
 interpreter) customization. SWIG allows you to add property access, iterators
-and documentation to classes, but to do that you have to use a Swig interface
-file in place of the .h file. Those files have a different format than a
-straight C++ header file. These files are called SB.i, and live in
-"scripts/interface". They are constructed by starting with the associated .h
-file, and adding documentation and the Python decorations, etc. We do this in a
-decidedly low-tech way, by maintaining the two files in parallel. That
-simplifies the build process, but it does mean that if you add a method to the
-C++ API's for an SB class, you have to copy the interface to the .i file.
+and documentation to classes. We place the property accessors and iterators in
+a file dedicated to extensions to existing SB classes at
+"bindings/interface/SBExtensions.i". The documentation is similarly
+located at "bindings/interface/SBDocstrings.i". These two files, in
+addition to the actual header SB.h, forms the interface that lldb
+exposes to users through the scripting languages.
+
+There are some situations where you may want to add functionality to the SB API
+only for use in C++. To prevent SWIG from generating bindings to these
+functions, you can use a C macro guard, like so:
+
+::
+
+  #ifndef SWIG
+  int GetResourceCPPOnly() const;
+  #endif
+
+In this case, ``GetResourceCPPOnly`` will not be generated for Python or other
+scripting languages. If you wanted to add a resource specifically only for the
+SWIG case, you can invert the condition and use ``#ifdef SWIG`` instead. When
+building the LLDB framework for macOS, the headers are processed with
+``unifdef`` prior to being copied into the framework bundle to remove macros
+involving SWIG.
 
 API Instrumentation
 ---


Index: lldb/docs/design/sbapi.rst
===
--- lldb/docs/design/sbapi.rst
+++ lldb/docs/design/sbapi.rst
@@ -7,6 +7,9 @@
 incompatibilities that C++ is so susceptible to. We've established a few rules
 to ensure that this happens.
 
+Extending the SB API
+
+
 The classes in the SB API's are all called SB, where SomeName is in
 CamelCase starting with an upper case letter. The method names are all
 CamelCase with initial capital letter as well.
@@ -45,14 +48,29 @@
 
 Another piece of the SB API infrastructure is the Python (or other script
 interpreter) customization. SWIG allows you to add property access, iterators
-and documentation to classes, but to do that you have to use a Swig interface
-file in place of the .h file. Those files have a different format than a
-straight C++ header file. These files are called SB.i, and live in
-"scripts/interface". They are constructed by starting with the associated .h
-file, and adding documentation and the Python decorations, etc. We do this in a
-decidedly low-tech way, by maintaining the two files in parallel. That
-simplifies the build process, but it does mean that if you add a method to the
-C++ API's for an SB class, you have to copy the interface to the .i file.
+and documentation to classes. We place the property accessors and iterators in
+a file dedicated to extensions to existing SB classes at
+"bindings/interface/SBExtensions.i". The documentation is similarly
+located at "bindings/interface/SBDocstrings.i". These two files, in
+addition to the actual header SB.h, forms the interface that lldb
+exposes to users through the scripting languages.
+
+There are some situations where you may want to add functionality to the SB API
+only for use in C++. To prevent SWIG from generating bindings to these
+functions, you can use a C macro guard, like so:
+
+::
+
+  #ifndef SWIG
+  int GetResourceCPPOnly() const;
+  #endif
+
+In this case, ``GetResourceCPPOnly`

[Lldb-commits] [lldb] 9adf60f - [lldb] Fix lldb_assert -> lldbassert in docs

2023-05-15 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2023-05-15T22:18:08-07:00
New Revision: 9adf60fc53fb105e331eec20e7fd2be71ee2a13c

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

LOG: [lldb] Fix lldb_assert -> lldbassert in docs

Update the documentation to reference lldbassert rather than
lldb_assert. The latter is the implementation, which shouldn't be used
directly. Instead, users should use lldbassert which is the macro that
expands to assert or lldb_assert depending on the build type.

Added: 


Modified: 
lldb/docs/resources/contributing.rst

Removed: 




diff  --git a/lldb/docs/resources/contributing.rst 
b/lldb/docs/resources/contributing.rst
index b2d4cf0e47181..5f4b24a8013e7 100644
--- a/lldb/docs/resources/contributing.rst
+++ b/lldb/docs/resources/contributing.rst
@@ -64,14 +64,14 @@ rules of thumb:
   errors cannot reasonably be surfaced to the end user, the error may
   be written to a topical log channel.
 
-* Soft assertions.  LLDB provides ``lldb_assert()`` as a soft
+* Soft assertions.  LLDB provides ``lldbassert()`` as a soft
   alternative to cover the middle ground of situations that indicate a
-  recoverable bug in LLDB.  In a Debug configuration ``lldb_assert()``
+  recoverable bug in LLDB.  In a Debug configuration ``lldbassert()``
   behaves like ``assert()``. In a Release configuration it will print a
   warning and encourage the user to file a bug report, similar to
   LLVM's crash handler, and then return execution. Use these sparingly
   and only if error handling is not otherwise feasible.  Specifically,
-  new code should not be using ``lldb_assert()`` and existing
+  new code should not be using ``lldbassert()`` and existing
   uses should be replaced by other means of error handling.
 
 * Fatal errors.  Aborting LLDB's process using



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


[Lldb-commits] [PATCH] D150624: [lldb] Fix lua build after 27b6a4e63afe

2023-05-15 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

I have enabled Lua testing on the incremental bot on GreenDragon 
(https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/). This has always 
been the intention (note how the job mentions "Python 3 and Lua 5.3") but 
wasn't enabled until earlier today.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150624

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


[Lldb-commits] [PATCH] D150639: [lldb] Define lldbassert based on NDEBUG instead of LLDB_CONFIGURATION_DEBUG

2023-05-15 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere created this revision.
JDevlieghere added reviewers: bulbazord, labath, mib.
Herald added a project: All.
JDevlieghere requested review of this revision.

Whether assertions are enabled or not is orthogonal to the build type
which could lead to surprising behavior for lldbassert. Previously, when
doing a debug build with assertions disabled, lldbassert would become a
NOOP, rather than printing an error like it does in a release build. By
definining lldbassert in terms of NDEBUG, it behaves like a regular
assert when assertions are enabled, and like a soft assert.


https://reviews.llvm.org/D150639

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


Index: lldb/source/Utility/LLDBAssert.cpp
===
--- lldb/source/Utility/LLDBAssert.cpp
+++ lldb/source/Utility/LLDBAssert.cpp
@@ -25,9 +25,6 @@
   if (LLVM_LIKELY(expression))
 return;
 
-  // If asserts are enabled abort here.
-  assert(false && "lldb_assert failed");
-
 #if LLVM_SUPPORT_XCODE_SIGNPOSTS
   if (__builtin_available(macos 10.12, iOS 10, tvOS 10, watchOS 3, *)) {
 os_log_fault(OS_LOG_DEFAULT,
@@ -36,9 +33,8 @@
   }
 #endif
 
-  // In a release configuration it will print a warning and encourage the user
-  // to file a bug report, similar to LLVM’s crash handler, and then return
-  // execution.
+  // Print a warning and encourage the user to file a bug report, similar to
+  // LLVM’s crash handler, and then return execution.
   errs() << format("Assertion failed: (%s), function %s, file %s, line %u\n",
expr_text, func, file, line);
   errs() << "backtrace leading to the failure:\n";
Index: lldb/include/lldb/Utility/LLDBAssert.h
===
--- lldb/include/lldb/Utility/LLDBAssert.h
+++ lldb/include/lldb/Utility/LLDBAssert.h
@@ -9,13 +9,19 @@
 #ifndef LLDB_UTILITY_LLDBASSERT_H
 #define LLDB_UTILITY_LLDBASSERT_H
 
-#ifdef LLDB_CONFIGURATION_DEBUG
+#ifndef NDEBUG
 #define lldbassert(x) assert(x)
 #else
+#if defined(__clang__)
+#define lldbassert(x)  
\
+  lldb_private::lldb_assert(static_cast(x), #x, __FUNCTION__,
\
+__FILE_NAME__, __LINE__)
+#else
 #define lldbassert(x)  
\
   lldb_private::lldb_assert(static_cast(x), #x, __FUNCTION__, __FILE__,  
\
 __LINE__)
 #endif
+#endif
 
 namespace lldb_private {
 void lldb_assert(bool expression, const char *expr_text, const char *func,


Index: lldb/source/Utility/LLDBAssert.cpp
===
--- lldb/source/Utility/LLDBAssert.cpp
+++ lldb/source/Utility/LLDBAssert.cpp
@@ -25,9 +25,6 @@
   if (LLVM_LIKELY(expression))
 return;
 
-  // If asserts are enabled abort here.
-  assert(false && "lldb_assert failed");
-
 #if LLVM_SUPPORT_XCODE_SIGNPOSTS
   if (__builtin_available(macos 10.12, iOS 10, tvOS 10, watchOS 3, *)) {
 os_log_fault(OS_LOG_DEFAULT,
@@ -36,9 +33,8 @@
   }
 #endif
 
-  // In a release configuration it will print a warning and encourage the user
-  // to file a bug report, similar to LLVM’s crash handler, and then return
-  // execution.
+  // Print a warning and encourage the user to file a bug report, similar to
+  // LLVM’s crash handler, and then return execution.
   errs() << format("Assertion failed: (%s), function %s, file %s, line %u\n",
expr_text, func, file, line);
   errs() << "backtrace leading to the failure:\n";
Index: lldb/include/lldb/Utility/LLDBAssert.h
===
--- lldb/include/lldb/Utility/LLDBAssert.h
+++ lldb/include/lldb/Utility/LLDBAssert.h
@@ -9,13 +9,19 @@
 #ifndef LLDB_UTILITY_LLDBASSERT_H
 #define LLDB_UTILITY_LLDBASSERT_H
 
-#ifdef LLDB_CONFIGURATION_DEBUG
+#ifndef NDEBUG
 #define lldbassert(x) assert(x)
 #else
+#if defined(__clang__)
+#define lldbassert(x)  \
+  lldb_private::lldb_assert(static_cast(x), #x, __FUNCTION__,\
+__FILE_NAME__, __LINE__)
+#else
 #define lldbassert(x)  \
   lldb_private::lldb_assert(static_cast(x), #x, __FUNCTION__, __FILE__,  \
 __LINE__)
 #endif
+#endif
 
 namespace lldb_private {
 void lldb_assert(bool expression, const char *expr_text, const char *func,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D150639: [lldb] Define lldbassert based on NDEBUG instead of LLDB_CONFIGURATION_DEBUG

2023-05-15 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

This patch also uses `__FILE_NAME__` (Clang-specific extension that functions 
similar to __FILE__ but only renders the last path component (the filename) 
instead of an invocation dependent full path to that file.) when building with 
clang.


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

https://reviews.llvm.org/D150639

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


[Lldb-commits] [lldb] 617c31c - [LLDB] Fix typo in TestDataFormatterSynthVal.py

2023-05-15 Thread Muhammad Omair Javaid via lldb-commits

Author: Muhammad Omair Javaid
Date: 2023-05-16T10:13:06+04:00
New Revision: 617c31c6a1ddd57c6b750b3882c177175067a735

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

LOG: [LLDB] Fix typo in TestDataFormatterSynthVal.py

This is follow up to 039b28e14e6d to fix a typo to make sure skipped
part of test is only skipped for AArch64 Windows platform.

Added: 


Modified: 

lldb/test/API/functionalities/data-formatter/data-formatter-synthval/TestDataFormatterSynthVal.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-synthval/TestDataFormatterSynthVal.py
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-synthval/TestDataFormatterSynthVal.py
index 67619287ef65..6b0397626690 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-synthval/TestDataFormatterSynthVal.py
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-synthval/TestDataFormatterSynthVal.py
@@ -96,7 +96,7 @@ def cleanup():
 
 # check that an aptly defined synthetic provider does not affect
 # one-lining
-if self.isAArch64Windows():
+if not self.isAArch64Windows():
 self.expect(
 "expression struct Struct { myInt theInt{12}; }; Struct()",
 substrs=['(theInt = 12)'])



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


[Lldb-commits] [PATCH] D150043: [InferAddressSpaces] Handle vector of pointers type & Support intrinsic masked gather/scatter

2023-05-15 Thread CaprYang via Phabricator via lldb-commits
CaprYang added inline comments.



Comment at: llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp:289
+
+static bool hasSameElementOfPtrOrVecPtrs(Type *Ty1, Type *Ty2) {
+  assert(isPtrOrVecOfPtrsType(Ty1) && isPtrOrVecOfPtrsType(Ty2));

CaprYang wrote:
> arsenm wrote:
> > arsenm wrote:
> > > Ditto, only opaque pointers matter now
> > You don't need to bother using getWithSamePointeeType. You can use 
> > Type::getWithNewType
> Does it mean this? do't check non-opaque types.
> 
> ```
> static Type *getPtrOrVecOfPtrsWithNewAS(Type *Ty, unsigned NewAddrSpace) {
>   assert(Ty->isPtrOrPtrVectorTy());
>   PointerType *NPT = PointerType::get(Ty->getContext(), NewAddrSpace);
>   return Ty->getWithNewType(NPT);
> }
> ```
@arsenm Excuse me... can you help me review again?


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

https://reviews.llvm.org/D150043

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