date:20231208


https://github.com/clayborg edited 
https://github.com/llvm/llvm-project/pull/74773
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [SymbolFileDWARF][NFC] Remove duplicated code checking for type tags (PR #74773)



@@ -130,6 +130,10 @@ class PluginProperties : public Properties {
   }
 };
 
+bool IsTypeTag(llvm::dwarf::Tag Tag) {

clayborg wrote:

Make static and rename to `IsClassOrStructType`. There are many other type tags 
in DWARF, so this doesn't make it clear that this funciton is only looking for 
`DW_TAG_class_type` or `DW_TAG_structure_type`.

https://github.com/llvm/llvm-project/pull/74773
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [SymbolFileDWARF][NFC] Remove duplicated code checking for type tags (PR #74773)



@@ -3128,36 +3121,11 @@ 
SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext(const DWARFDIE &die) {
   if (type_system &&
   !type_system->SupportsLanguage(GetLanguage(*type_die.GetCU(
 return true;
-  bool try_resolving_type = false;
 
-  // Don't try and resolve the DIE we are looking for with the DIE
-  // itself!
   const dw_tag_t type_tag = type_die.Tag();
-  // Make sure the tags match
-  if (type_tag == tag) {
-// The tags match, lets try resolving this type
-try_resolving_type = true;
-  } else {
-// The tags don't match, but we need to watch our for a forward
-// declaration for a struct and ("struct foo") ends up being a
-// class ("class foo { ... };") or vice versa.
-switch (type_tag) {
-case DW_TAG_class_type:
-  // We had a "class foo", see if we ended up with a "struct foo
-  // { ... };"
-  try_resolving_type = (tag == DW_TAG_structure_type);
-  break;
-case DW_TAG_structure_type:
-  // We had a "struct foo", see if we ended up with a "class foo
-  // { ... };"
-  try_resolving_type = (tag == DW_TAG_class_type);
-  break;
-default:
-  // Tags don't match, don't event try to resolve using this type
-  // whose name matches
-  break;
-}
-  }
+  // Resolve the type if both have the same tag or {class, struct} tags.
+  const bool try_resolving_type =
+  type_tag == tag || (IsTypeTag(type_tag) && IsTypeTag(tag));

clayborg wrote:

The second call to `IsTypeTag(tag)` is redundant since you already checked for 
equality right before it.

https://github.com/llvm/llvm-project/pull/74773
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [SymbolFileDWARF][NFC] Remove duplicated code checking for type tags (PR #74773)


https://github.com/clayborg requested changes to this pull request.


https://github.com/llvm/llvm-project/pull/74773
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [clang] fixing issue #64441 (PR #74814)



@@ -56,10 +56,10 @@ namespace Foo = A::B;   // namespace alias
 
 using Foo::myfunc;  // using declaration
 
-using namespace Foo;// using directive
+//removing namespace foo; for quality naming 

Michael137 wrote:

The `using` directive was used here to make sure LLDB does the right thing when 
doing qualified lookup in the presence of various `DW_TAG_imported_module` and 
name shadowing. I don't think we should change this because it risks subtly 
changing the test coverage.

https://github.com/llvm/llvm-project/pull/74814
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [clang] fixing issue #64441 (PR #74814)

2023-12-08 Thread Jeevan Ghimire via lldb-commits



@@ -56,10 +56,10 @@ namespace Foo = A::B;   // namespace alias
 
 using Foo::myfunc;  // using declaration
 
-using namespace Foo;// using directive
+//removing namespace foo; for quality naming 

jeevanghimire wrote:

but it can create confusion if we just name the qualified lookup with standard 
naming it will make more sense

https://github.com/llvm/llvm-project/pull/74814
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][progress] Add discrete boolean flag to progress reports (PR #69516)



@@ -2225,7 +2225,8 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
   const char *file_name = file.GetFilename().AsCString("");
   LLDB_SCOPED_TIMERF("ObjectFileMachO::ParseSymtab () module = %s", file_name);
   LLDB_LOG(log, "Parsing symbol table for {0}", file_name);
-  Progress progress(llvm::formatv("Parsing symbol table for {0}", file_name));
+  Progress progress(llvm::formatv("Parsing symbol table for {0}", file_name),
+Progress::ProgressReportType::eAggregateProgressReport);

clayborg wrote:

I totally understand about timeframes and getting this in ASAP.

I am just trying to understand what the goal is here as what this change 
essentially does is groups all the progress notifications for all progress 
reports into the `is_aggregate == true` except for the SymbolLocationDefault's 
progress for downloading dSYM files. So essentially all progress notifications 
are considered spammy and something that won't get displayed? An enum means 
anytime someone adds a new progress, they need to make a category enum for it. 
I still think either a category string still makes the most sense. Then an IDE 
can watch for progress notifications, and if many come in from the same 
category, it can show a spinning progress with the category name "Parsing 
symbol tables" and any incoming progress updates could keep this progress UI 
alive. If a user opens up this top level "Parsing symbol tables" category, the 
individual progresses that are alive can be shown in detail. This allows an IDE 
to watch the progress notifications and if it gets too many too quickly from a 
specific category, it can group them all under the category name and not have 
to show specific individual entries that come and go too quickly.

The code from `SymbolFileDWARF::InitializeObject()` is what I am guessing can 
get really spammy for both the apple tables and DWARF5 tables. There is no work 
to be done when loading these tables, so I am guessing the progress is really 
quick for those and not that useful. We could remove these if they are some of 
the offending progress notifications that are causing issues.

Manual DWARF parsing and symbol table parsing, those do take some time, so they 
tend to be very useful as they really slow down debugger startup when you have 
large files and when users are sitting there watching a blank IDE window and no 
visible progress is made by the debugger, it can lead them to kill the debug 
session if there is no actual feedback.

https://github.com/llvm/llvm-project/pull/69516
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][progress] Add discrete boolean flag to progress reports (PR #69516)



@@ -2225,7 +2225,8 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
   const char *file_name = file.GetFilename().AsCString("");
   LLDB_SCOPED_TIMERF("ObjectFileMachO::ParseSymtab () module = %s", file_name);
   LLDB_LOG(log, "Parsing symbol table for {0}", file_name);
-  Progress progress(llvm::formatv("Parsing symbol table for {0}", file_name));
+  Progress progress(llvm::formatv("Parsing symbol table for {0}", file_name),
+Progress::ProgressReportType::eAggregateProgressReport);

clayborg wrote:

If the goal is to get rid of super spammy notifications (ones that are too 
quick), that can actually be done client side when the progress notification 
events are received by setting a timeout once the start progress has been 
received, and if a certain timeout happens (more than 1 second), then just 
don't show the progress, but if it does last then pop the progress dialog up. 
We tried this in lldb-dap.cpp. See the function named `void 
ProgressEventThreadFunction()` which is:

```
void DAP::SendProgressEvent(uint64_t progress_id, const char *message,
uint64_t completed, uint64_t total) {
  progress_event_reporter.Push(progress_id, message, completed, total);
}
```
We have a class called `ProgressEventReporter`:
```
/// Class that filters out progress event messages that shouldn't be reported
/// to the IDE, because they are invalid, they carry no new information, or they
/// don't last long enough.
///
/// We need to limit the amount of events that are sent to the IDE, as they slow
/// the render thread of the UI user, and they end up spamming the DAP
/// connection, which also takes some processing time out of the IDE.
class ProgressEventReporter {
```
The problem we ran into with sending tons of notifications through the debug 
adaptor protocol was it was too many packets, so we trimmed them down. The main 
issue with this approach is if you get 1000 progress notifications and each 
progress takes less than the minimum report time, like 1 second, and if each 
progress takes just under the timeout of 1 second, you have a long delay with 
no feedback.

For progress notifications like the DWARF manual index, it has a total count of 
the number of compile units and each time a compile unit is indexed, the 
progress gets advanced and can cause a ton of update packets to be sent, this 
class helps reduce these. 

So the `ProgressEventReporter` class will:
- not report events if they are too quick, but if they exceed a minimum 
threshold, they get report, albeit a bit late
- if there is a progress that is really spammy (1 of 1, 2 of 1, 3 of 
1, etc) it will report progress only every so often. This really reduces 
the number of progress notifications.

https://github.com/llvm/llvm-project/pull/69516
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test] TestLocationListLookup.py: skip expr check on unsupported platforms (PR #74818)


https://github.com/Michael137 created 
https://github.com/llvm/llvm-project/pull/74818

The `expect_expr` check was introduced in 
https://github.com/llvm/llvm-project/pull/74772. It is failing on Linux and 
Windows, so skip this test to unblock the bots

>From 32b8d4102f73f7b42792f9c817742d1eba629351 Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Fri, 8 Dec 2023 08:51:32 +
Subject: [PATCH] [lldb][test] TestLocationListLookup.py: skip expr check on
 unsupported platforms

---
 .../TestLocationListLookup.py | 24 ++-
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git 
a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py 
b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
index 07f306a6ed78b6..78fdaf939af1d4 100644
--- 
a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
+++ 
b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
@@ -7,10 +7,7 @@
 
 
 class LocationListLookupTestCase(TestBase):
-@skipIf(oslist=["linux"], archs=["arm"])
-def test_loclist(self):
-self.build()
-
+def launch(self) -> lldb.SBProcess:
 exe = self.getBuildArtifact("a.out")
 target = self.dbg.CreateTarget(exe)
 self.assertTrue(target, VALID_TARGET)
@@ -22,6 +19,9 @@ def test_loclist(self):
 self.assertTrue(process.IsValid())
 self.assertTrue(process.is_stopped)
 
+return process
+
+def check_local_vars(self, process: lldb.SBProcess, check_expr: bool):
 # Find `bar` on the stack, then
 # make sure we can read out the local
 # variables (with both `frame var` and `expr`)
@@ -32,5 +32,17 @@ def test_loclist(self):
 argv.GetDescription(strm)
 self.assertNotEqual(strm.GetData().find("a.out"), -1)
 
-process.GetSelectedThread().SetSelectedFrame(f.idx)
-self.expect_expr("this", result_type="Foo *")
+if check_expr:
+process.GetSelectedThread().SetSelectedFrame(f.idx)
+self.expect_expr("this", result_type="Foo *")
+
+@skipIf(oslist=["linux"], archs=["arm"])
+@skipIfDarwin
+def test_loclist_frame_var(self):
+self.build()
+self.check_local_vars(self.launch(), check_expr=False)
+
+@skipUnlessDarwin
+def test_loclist_expr(self):
+self.build()
+self.check_local_vars(self.launch(), check_expr=True)

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

[Lldb-commits] [lldb] [lldb][test] TestLocationListLookup.py: skip expr check on unsupported platforms (PR #74818)


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)


Changes

The `expect_expr` check was introduced in 
https://github.com/llvm/llvm-project/pull/74772. It is failing on Linux and 
Windows, so skip this test to unblock the bots

---
Full diff: https://github.com/llvm/llvm-project/pull/74818.diff


1 Files Affected:

- (modified) 
lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py 
(+18-6) 


``diff
diff --git 
a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py 
b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
index 07f306a6ed78b..78fdaf939af1d 100644
--- 
a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
+++ 
b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
@@ -7,10 +7,7 @@
 
 
 class LocationListLookupTestCase(TestBase):
-@skipIf(oslist=["linux"], archs=["arm"])
-def test_loclist(self):
-self.build()
-
+def launch(self) -> lldb.SBProcess:
 exe = self.getBuildArtifact("a.out")
 target = self.dbg.CreateTarget(exe)
 self.assertTrue(target, VALID_TARGET)
@@ -22,6 +19,9 @@ def test_loclist(self):
 self.assertTrue(process.IsValid())
 self.assertTrue(process.is_stopped)
 
+return process
+
+def check_local_vars(self, process: lldb.SBProcess, check_expr: bool):
 # Find `bar` on the stack, then
 # make sure we can read out the local
 # variables (with both `frame var` and `expr`)
@@ -32,5 +32,17 @@ def test_loclist(self):
 argv.GetDescription(strm)
 self.assertNotEqual(strm.GetData().find("a.out"), -1)
 
-process.GetSelectedThread().SetSelectedFrame(f.idx)
-self.expect_expr("this", result_type="Foo *")
+if check_expr:
+process.GetSelectedThread().SetSelectedFrame(f.idx)
+self.expect_expr("this", result_type="Foo *")
+
+@skipIf(oslist=["linux"], archs=["arm"])
+@skipIfDarwin
+def test_loclist_frame_var(self):
+self.build()
+self.check_local_vars(self.launch(), check_expr=False)
+
+@skipUnlessDarwin
+def test_loclist_expr(self):
+self.build()
+self.check_local_vars(self.launch(), check_expr=True)

``




https://github.com/llvm/llvm-project/pull/74818
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test] TestLocationListLookup.py: skip expr check on unsupported platforms (PR #74818)


https://github.com/Michael137 updated 
https://github.com/llvm/llvm-project/pull/74818

>From 32b8d4102f73f7b42792f9c817742d1eba629351 Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Fri, 8 Dec 2023 08:51:32 +
Subject: [PATCH 1/2] [lldb][test] TestLocationListLookup.py: skip expr check
 on unsupported platforms

---
 .../TestLocationListLookup.py | 24 ++-
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git 
a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py 
b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
index 07f306a6ed78b6..78fdaf939af1d4 100644
--- 
a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
+++ 
b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
@@ -7,10 +7,7 @@
 
 
 class LocationListLookupTestCase(TestBase):
-@skipIf(oslist=["linux"], archs=["arm"])
-def test_loclist(self):
-self.build()
-
+def launch(self) -> lldb.SBProcess:
 exe = self.getBuildArtifact("a.out")
 target = self.dbg.CreateTarget(exe)
 self.assertTrue(target, VALID_TARGET)
@@ -22,6 +19,9 @@ def test_loclist(self):
 self.assertTrue(process.IsValid())
 self.assertTrue(process.is_stopped)
 
+return process
+
+def check_local_vars(self, process: lldb.SBProcess, check_expr: bool):
 # Find `bar` on the stack, then
 # make sure we can read out the local
 # variables (with both `frame var` and `expr`)
@@ -32,5 +32,17 @@ def test_loclist(self):
 argv.GetDescription(strm)
 self.assertNotEqual(strm.GetData().find("a.out"), -1)
 
-process.GetSelectedThread().SetSelectedFrame(f.idx)
-self.expect_expr("this", result_type="Foo *")
+if check_expr:
+process.GetSelectedThread().SetSelectedFrame(f.idx)
+self.expect_expr("this", result_type="Foo *")
+
+@skipIf(oslist=["linux"], archs=["arm"])
+@skipIfDarwin
+def test_loclist_frame_var(self):
+self.build()
+self.check_local_vars(self.launch(), check_expr=False)
+
+@skipUnlessDarwin
+def test_loclist_expr(self):
+self.build()
+self.check_local_vars(self.launch(), check_expr=True)

>From 9f71e25c3729962da0ad9697ae077adb04e2b7f9 Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Fri, 8 Dec 2023 08:56:41 +
Subject: [PATCH 2/2] fixup! skip on windows

---
 .../location-list-lookup/TestLocationListLookup.py   | 1 +
 1 file changed, 1 insertion(+)

diff --git 
a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py 
b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
index 78fdaf939af1d4..7ff8934228e2ce 100644
--- 
a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
+++ 
b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
@@ -38,6 +38,7 @@ def check_local_vars(self, process: lldb.SBProcess, 
check_expr: bool):
 
 @skipIf(oslist=["linux"], archs=["arm"])
 @skipIfDarwin
+@skipIfWindows # GetDisplayFunctionName returns None
 def test_loclist_frame_var(self):
 self.build()
 self.check_local_vars(self.launch(), check_expr=False)

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

[Lldb-commits] [lldb] [lldb][test] TestLocationListLookup.py: skip expr check on unsupported platforms (PR #74818)


https://github.com/Michael137 updated 
https://github.com/llvm/llvm-project/pull/74818

>From 32b8d4102f73f7b42792f9c817742d1eba629351 Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Fri, 8 Dec 2023 08:51:32 +
Subject: [PATCH 1/3] [lldb][test] TestLocationListLookup.py: skip expr check
 on unsupported platforms

---
 .../TestLocationListLookup.py | 24 ++-
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git 
a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py 
b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
index 07f306a6ed78b..78fdaf939af1d 100644
--- 
a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
+++ 
b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
@@ -7,10 +7,7 @@
 
 
 class LocationListLookupTestCase(TestBase):
-@skipIf(oslist=["linux"], archs=["arm"])
-def test_loclist(self):
-self.build()
-
+def launch(self) -> lldb.SBProcess:
 exe = self.getBuildArtifact("a.out")
 target = self.dbg.CreateTarget(exe)
 self.assertTrue(target, VALID_TARGET)
@@ -22,6 +19,9 @@ def test_loclist(self):
 self.assertTrue(process.IsValid())
 self.assertTrue(process.is_stopped)
 
+return process
+
+def check_local_vars(self, process: lldb.SBProcess, check_expr: bool):
 # Find `bar` on the stack, then
 # make sure we can read out the local
 # variables (with both `frame var` and `expr`)
@@ -32,5 +32,17 @@ def test_loclist(self):
 argv.GetDescription(strm)
 self.assertNotEqual(strm.GetData().find("a.out"), -1)
 
-process.GetSelectedThread().SetSelectedFrame(f.idx)
-self.expect_expr("this", result_type="Foo *")
+if check_expr:
+process.GetSelectedThread().SetSelectedFrame(f.idx)
+self.expect_expr("this", result_type="Foo *")
+
+@skipIf(oslist=["linux"], archs=["arm"])
+@skipIfDarwin
+def test_loclist_frame_var(self):
+self.build()
+self.check_local_vars(self.launch(), check_expr=False)
+
+@skipUnlessDarwin
+def test_loclist_expr(self):
+self.build()
+self.check_local_vars(self.launch(), check_expr=True)

>From 9f71e25c3729962da0ad9697ae077adb04e2b7f9 Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Fri, 8 Dec 2023 08:56:41 +
Subject: [PATCH 2/3] fixup! skip on windows

---
 .../location-list-lookup/TestLocationListLookup.py   | 1 +
 1 file changed, 1 insertion(+)

diff --git 
a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py 
b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
index 78fdaf939af1d..7ff8934228e2c 100644
--- 
a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
+++ 
b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
@@ -38,6 +38,7 @@ def check_local_vars(self, process: lldb.SBProcess, 
check_expr: bool):
 
 @skipIf(oslist=["linux"], archs=["arm"])
 @skipIfDarwin
+@skipIfWindows # GetDisplayFunctionName returns None
 def test_loclist_frame_var(self):
 self.build()
 self.check_local_vars(self.launch(), check_expr=False)

>From 7e8fb51e25190a77ffcc5cb29e5aed9e0669d0fd Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Fri, 8 Dec 2023 08:57:10 +
Subject: [PATCH 3/3] fixup! format

---
 .../location-list-lookup/TestLocationListLookup.py  | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py 
b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
index 7ff8934228e2c..7eb790e5a3066 100644
--- 
a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
+++ 
b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
@@ -38,7 +38,7 @@ def check_local_vars(self, process: lldb.SBProcess, 
check_expr: bool):
 
 @skipIf(oslist=["linux"], archs=["arm"])
 @skipIfDarwin
-@skipIfWindows # GetDisplayFunctionName returns None
+@skipIfWindows  # GetDisplayFunctionName returns None
 def test_loclist_frame_var(self):
 self.build()
 self.check_local_vars(self.launch(), check_expr=False)

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

[Lldb-commits] [lldb] [lldb][test] TestLocationListLookup.py: skip expr check on unsupported platforms (PR #74818)


github-actions[bot] wrote:




:warning: Python code formatter, darker found issues in your code. :warning:



You can test this locally with the following command:


``bash
darker --check --diff -r 
c340cf0a353cd6d1090297cf84caf2720d1c7d90...9f71e25c3729962da0ad9697ae077adb04e2b7f9
 lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
``





View the diff from darker here.


``diff
--- TestLocationListLookup.py   2023-12-08 08:56:41.00 +
+++ TestLocationListLookup.py   2023-12-08 08:58:56.493426 +
@@ -36,11 +36,11 @@
 process.GetSelectedThread().SetSelectedFrame(f.idx)
 self.expect_expr("this", result_type="Foo *")
 
 @skipIf(oslist=["linux"], archs=["arm"])
 @skipIfDarwin
-@skipIfWindows # GetDisplayFunctionName returns None
+@skipIfWindows  # GetDisplayFunctionName returns None
 def test_loclist_frame_var(self):
 self.build()
 self.check_local_vars(self.launch(), check_expr=False)
 
 @skipUnlessDarwin

``




https://github.com/llvm/llvm-project/pull/74818
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test] TestLocationListLookup.py: skip expr check on unsupported platforms (PR #74818)


https://github.com/Michael137 updated 
https://github.com/llvm/llvm-project/pull/74818

>From 32b8d4102f73f7b42792f9c817742d1eba629351 Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Fri, 8 Dec 2023 08:51:32 +
Subject: [PATCH 1/4] [lldb][test] TestLocationListLookup.py: skip expr check
 on unsupported platforms

---
 .../TestLocationListLookup.py | 24 ++-
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git 
a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py 
b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
index 07f306a6ed78b..78fdaf939af1d 100644
--- 
a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
+++ 
b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
@@ -7,10 +7,7 @@
 
 
 class LocationListLookupTestCase(TestBase):
-@skipIf(oslist=["linux"], archs=["arm"])
-def test_loclist(self):
-self.build()
-
+def launch(self) -> lldb.SBProcess:
 exe = self.getBuildArtifact("a.out")
 target = self.dbg.CreateTarget(exe)
 self.assertTrue(target, VALID_TARGET)
@@ -22,6 +19,9 @@ def test_loclist(self):
 self.assertTrue(process.IsValid())
 self.assertTrue(process.is_stopped)
 
+return process
+
+def check_local_vars(self, process: lldb.SBProcess, check_expr: bool):
 # Find `bar` on the stack, then
 # make sure we can read out the local
 # variables (with both `frame var` and `expr`)
@@ -32,5 +32,17 @@ def test_loclist(self):
 argv.GetDescription(strm)
 self.assertNotEqual(strm.GetData().find("a.out"), -1)
 
-process.GetSelectedThread().SetSelectedFrame(f.idx)
-self.expect_expr("this", result_type="Foo *")
+if check_expr:
+process.GetSelectedThread().SetSelectedFrame(f.idx)
+self.expect_expr("this", result_type="Foo *")
+
+@skipIf(oslist=["linux"], archs=["arm"])
+@skipIfDarwin
+def test_loclist_frame_var(self):
+self.build()
+self.check_local_vars(self.launch(), check_expr=False)
+
+@skipUnlessDarwin
+def test_loclist_expr(self):
+self.build()
+self.check_local_vars(self.launch(), check_expr=True)

>From 9f71e25c3729962da0ad9697ae077adb04e2b7f9 Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Fri, 8 Dec 2023 08:56:41 +
Subject: [PATCH 2/4] fixup! skip on windows

---
 .../location-list-lookup/TestLocationListLookup.py   | 1 +
 1 file changed, 1 insertion(+)

diff --git 
a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py 
b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
index 78fdaf939af1d..7ff8934228e2c 100644
--- 
a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
+++ 
b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
@@ -38,6 +38,7 @@ def check_local_vars(self, process: lldb.SBProcess, 
check_expr: bool):
 
 @skipIf(oslist=["linux"], archs=["arm"])
 @skipIfDarwin
+@skipIfWindows # GetDisplayFunctionName returns None
 def test_loclist_frame_var(self):
 self.build()
 self.check_local_vars(self.launch(), check_expr=False)

>From 7e8fb51e25190a77ffcc5cb29e5aed9e0669d0fd Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Fri, 8 Dec 2023 08:57:10 +
Subject: [PATCH 3/4] fixup! format

---
 .../location-list-lookup/TestLocationListLookup.py  | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py 
b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
index 7ff8934228e2c..7eb790e5a3066 100644
--- 
a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
+++ 
b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
@@ -38,7 +38,7 @@ def check_local_vars(self, process: lldb.SBProcess, 
check_expr: bool):
 
 @skipIf(oslist=["linux"], archs=["arm"])
 @skipIfDarwin
-@skipIfWindows # GetDisplayFunctionName returns None
+@skipIfWindows  # GetDisplayFunctionName returns None
 def test_loclist_frame_var(self):
 self.build()
 self.check_local_vars(self.launch(), check_expr=False)

>From 9b2ec1eb3280a52f5a0e975efbbb5fcb4daef554 Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Fri, 8 Dec 2023 08:59:37 +
Subject: [PATCH 4/4] fixup! unskip on windows and check frame_name

---
 .../location-list-lookup/TestLocationListLookup.py| 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git 
a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py 
b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
index 7eb790e5a3066..ccee3bfde3f5d 100644
--- 
a/lldb/test/API/functionalitie

[Lldb-commits] [lldb] [lldb] Return index of element in ValueObject path instead of the element's value (PR #74413)


DavidSpickett wrote:

> doesn't appear to be anything that calls these methods

Candidate for removal then? If so it still makes sense to fix them, then remove 
them. Saves fixing them again if they are brought back.

Either way, I'm not going to block this by requiring a test case.

https://github.com/llvm/llvm-project/pull/74413
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] colorize symbols in image lookup with a regex pattern (PR #69422)

=?utf-8?q?José?= L. Junior ,taalhaataahir0102
 <23100...@lums.edu.pk>,taalhaataahir0102 
<23100...@lums.edu.pk>,taalhaataahir0102
 <23100...@lums.edu.pk>,taalhaataahir0102 <23100...@lums.edu.pk>,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior 
Message-ID:
In-Reply-To: 



@@ -1618,12 +1621,19 @@ static uint32_t LookupSymbolInModule(CommandInterpreter 
&interpreter,
 if (symbol->ValueIsAddress()) {
   DumpAddress(
   interpreter.GetExecutionContext().GetBestExecutionContextScope(),
-  symbol->GetAddressRef(), verbose, all_ranges, strm);
+  symbol->GetAddressRef(), verbose, all_ranges, strm,
+  use_color && name_is_regex ? name : nullptr);
   strm.EOL();
 } else {
   strm.IndentMore();
   strm.Indent("Name: ");
-  strm.PutCString(symbol->GetDisplayName().GetStringRef());
+  llvm::StringRef ansi_prefix =
+  interpreter.GetDebugger().GetRegexMatchAnsiPrefix();
+  llvm::StringRef ansi_suffix =
+  interpreter.GetDebugger().GetRegexMatchAnsiSuffix();

DavidSpickett wrote:

I think you might be mistaking putting the calls inside 
`PutCStringColorHighlighted` vs. putting the calls inside *the call* to 
`PutCStringColorHighlighted`.

I'm suggesting:
```
PutCStringColorHighlighted(<...>, 
interpreter.GetDebugger().GetRegexMatchAnsiPrefix(), 
interpreter.GetDebugger().GetRegexMatchAnsiSuffix())
```

(should have included that example in my first comment tbh :) )

https://github.com/llvm/llvm-project/pull/69422
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test] TestLocationListLookup.py: skip expr check on unsupported platforms (PR #74818)


https://github.com/DavidSpickett approved this pull request.

Not 100% that the skips are going to work for AArch64 but I'll fix that if it 
is the case.

https://github.com/llvm/llvm-project/pull/74818
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 11a7e57 - [lldb][test] TestLocationListLookup.py: skip expr check on unsupported platforms (#74818)


Author: Michael Buch
Date: 2023-12-08T10:09:38Z
New Revision: 11a7e5781c6363ca3061f57f3aa7e49164673821

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

LOG: [lldb][test] TestLocationListLookup.py: skip expr check on unsupported 
platforms (#74818)

The `expect_expr` check was introduced in
https://github.com/llvm/llvm-project/pull/74772. It is failing on Linux
and Windows, so skip this test to unblock the bots

Added: 


Modified: 
lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py 
b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
index 07f306a6ed78b..ccee3bfde3f5d 100644
--- 
a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
+++ 
b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
@@ -7,10 +7,7 @@
 
 
 class LocationListLookupTestCase(TestBase):
-@skipIf(oslist=["linux"], archs=["arm"])
-def test_loclist(self):
-self.build()
-
+def launch(self) -> lldb.SBProcess:
 exe = self.getBuildArtifact("a.out")
 target = self.dbg.CreateTarget(exe)
 self.assertTrue(target, VALID_TARGET)
@@ -22,15 +19,31 @@ def test_loclist(self):
 self.assertTrue(process.IsValid())
 self.assertTrue(process.is_stopped)
 
+return process
+
+def check_local_vars(self, process: lldb.SBProcess, check_expr: bool):
 # Find `bar` on the stack, then
 # make sure we can read out the local
 # variables (with both `frame var` and `expr`)
 for f in process.GetSelectedThread().frames:
-if f.GetDisplayFunctionName().startswith("Foo::bar"):
+frame_name = f.GetDisplayFunctionName()
+if frame_name is not None and frame_name.startswith("Foo::bar"):
 argv = f.GetValueForVariablePath("argv").GetChildAtIndex(0)
 strm = lldb.SBStream()
 argv.GetDescription(strm)
 self.assertNotEqual(strm.GetData().find("a.out"), -1)
 
-process.GetSelectedThread().SetSelectedFrame(f.idx)
-self.expect_expr("this", result_type="Foo *")
+if check_expr:
+process.GetSelectedThread().SetSelectedFrame(f.idx)
+self.expect_expr("this", result_type="Foo *")
+
+@skipIf(oslist=["linux"], archs=["arm"])
+@skipIfDarwin
+def test_loclist_frame_var(self):
+self.build()
+self.check_local_vars(self.launch(), check_expr=False)
+
+@skipUnlessDarwin
+def test_loclist_expr(self):
+self.build()
+self.check_local_vars(self.launch(), check_expr=True)



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

[Lldb-commits] [lldb] [lldb][test] TestLocationListLookup.py: skip expr check on unsupported platforms (PR #74818)


https://github.com/DavidSpickett closed 
https://github.com/llvm/llvm-project/pull/74818
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [lldb] Rename lldb-vscode to lldb-dap (PR #69264)


DavidSpickett wrote:

My intent when asking for this link was that links to the README of lldb-vscode 
would end up going to the README of lldb-dap (maybe the wrong section, but at 
least you'd know the name had changed). The audience for it is small but I 
didn't want to make it even smaller by giving them no signal of the rename.

We could narrow the link to that one file, or remove it once 18 has branched? 
There is a symlink too in `/bin` I think? Maybe both can go once 18 has 
branched.

https://github.com/llvm/llvm-project/pull/69264
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] e33302f - Revert "[lldb][test] TestLocationListLookup.py: skip expr check on unsupported platforms (#74818)"

2023-12-08 Thread Mikhail Goncharov via lldb-commits


Author: Mikhail Goncharov
Date: 2023-12-08T11:39:10+01:00
New Revision: e33302fa1279d0a15aac18eca3f0311669bfe328

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

LOG: Revert "[lldb][test] TestLocationListLookup.py: skip expr check on 
unsupported platforms (#74818)"

This reverts commit 11a7e5781c6363ca3061f57f3aa7e49164673821.

Test fails: https://lab.llvm.org/buildbot/#/builders/219/builds/7416 and
others.

Added: 


Modified: 
lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py 
b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
index ccee3bfde3f5de..07f306a6ed78b6 100644
--- 
a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
+++ 
b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
@@ -7,7 +7,10 @@
 
 
 class LocationListLookupTestCase(TestBase):
-def launch(self) -> lldb.SBProcess:
+@skipIf(oslist=["linux"], archs=["arm"])
+def test_loclist(self):
+self.build()
+
 exe = self.getBuildArtifact("a.out")
 target = self.dbg.CreateTarget(exe)
 self.assertTrue(target, VALID_TARGET)
@@ -19,31 +22,15 @@ def launch(self) -> lldb.SBProcess:
 self.assertTrue(process.IsValid())
 self.assertTrue(process.is_stopped)
 
-return process
-
-def check_local_vars(self, process: lldb.SBProcess, check_expr: bool):
 # Find `bar` on the stack, then
 # make sure we can read out the local
 # variables (with both `frame var` and `expr`)
 for f in process.GetSelectedThread().frames:
-frame_name = f.GetDisplayFunctionName()
-if frame_name is not None and frame_name.startswith("Foo::bar"):
+if f.GetDisplayFunctionName().startswith("Foo::bar"):
 argv = f.GetValueForVariablePath("argv").GetChildAtIndex(0)
 strm = lldb.SBStream()
 argv.GetDescription(strm)
 self.assertNotEqual(strm.GetData().find("a.out"), -1)
 
-if check_expr:
-process.GetSelectedThread().SetSelectedFrame(f.idx)
-self.expect_expr("this", result_type="Foo *")
-
-@skipIf(oslist=["linux"], archs=["arm"])
-@skipIfDarwin
-def test_loclist_frame_var(self):
-self.build()
-self.check_local_vars(self.launch(), check_expr=False)
-
-@skipUnlessDarwin
-def test_loclist_expr(self):
-self.build()
-self.check_local_vars(self.launch(), check_expr=True)
+process.GetSelectedThread().SetSelectedFrame(f.idx)
+self.expect_expr("this", result_type="Foo *")



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

[Lldb-commits] [lldb] b43ab18 - Reapply "[lldb][test] TestLocationListLookup.py: skip expr check on unsupported platforms (#74818)"

2023-12-08 Thread Mikhail Goncharov via lldb-commits


Author: Mikhail Goncharov
Date: 2023-12-08T11:43:14+01:00
New Revision: b43ab182040f7c3b43e37ade7af600af1c9b3dfd

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

LOG: Reapply "[lldb][test] TestLocationListLookup.py: skip expr check on 
unsupported platforms (#74818)"

This reverts commit e33302fa1279d0a15aac18eca3f0311669bfe328.

there is a fix already, sorry for the noise

Added: 


Modified: 
lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py 
b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
index 07f306a6ed78b..ccee3bfde3f5d 100644
--- 
a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
+++ 
b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
@@ -7,10 +7,7 @@
 
 
 class LocationListLookupTestCase(TestBase):
-@skipIf(oslist=["linux"], archs=["arm"])
-def test_loclist(self):
-self.build()
-
+def launch(self) -> lldb.SBProcess:
 exe = self.getBuildArtifact("a.out")
 target = self.dbg.CreateTarget(exe)
 self.assertTrue(target, VALID_TARGET)
@@ -22,15 +19,31 @@ def test_loclist(self):
 self.assertTrue(process.IsValid())
 self.assertTrue(process.is_stopped)
 
+return process
+
+def check_local_vars(self, process: lldb.SBProcess, check_expr: bool):
 # Find `bar` on the stack, then
 # make sure we can read out the local
 # variables (with both `frame var` and `expr`)
 for f in process.GetSelectedThread().frames:
-if f.GetDisplayFunctionName().startswith("Foo::bar"):
+frame_name = f.GetDisplayFunctionName()
+if frame_name is not None and frame_name.startswith("Foo::bar"):
 argv = f.GetValueForVariablePath("argv").GetChildAtIndex(0)
 strm = lldb.SBStream()
 argv.GetDescription(strm)
 self.assertNotEqual(strm.GetData().find("a.out"), -1)
 
-process.GetSelectedThread().SetSelectedFrame(f.idx)
-self.expect_expr("this", result_type="Foo *")
+if check_expr:
+process.GetSelectedThread().SetSelectedFrame(f.idx)
+self.expect_expr("this", result_type="Foo *")
+
+@skipIf(oslist=["linux"], archs=["arm"])
+@skipIfDarwin
+def test_loclist_frame_var(self):
+self.build()
+self.check_local_vars(self.launch(), check_expr=False)
+
+@skipUnlessDarwin
+def test_loclist_expr(self):
+self.build()
+self.check_local_vars(self.launch(), check_expr=True)



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

[Lldb-commits] [lld] [polly] [openmp] [llvm] [flang] [clang] [mlir] [compiler-rt] [lldb] [VPlan] Compute scalable VF in preheader for induction increment. (PR #74762)



@@ -340,8 +340,13 @@ Value *VPInstruction::generateInstruction(VPTransformState 
&State,
   auto *Phi = State.get(getOperand(0), 0);
   // The loop step is equal to the vectorization factor (num of SIMD
   // elements) times the unroll factor (num of SIMD instructions).
-  Value *Step =
-  createStepForVF(Builder, Phi->getType(), State.VF, State.UF);
+  Value *Step;
+  {
+BasicBlock *VectorPH = State.CFG.getPreheaderBBFor(this);
+IRBuilder<>::InsertPointGuard Guard(Builder);
+Builder.SetInsertPoint(VectorPH->getTerminator());

fhahn wrote:

Adjusted in the latest version, thanks!

https://github.com/llvm/llvm-project/pull/74762
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test] TestLocationListLookup.py: skip expr check on unsupported platforms (PR #74818)


DavidSpickett wrote:

It was not the case, all green again!

https://github.com/llvm/llvm-project/pull/74818
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] c90cb6e - [lldb] colorize symbols in image lookup with a regex pattern (#69422)


Author: taalhaataahir0102
Date: 2023-12-08T11:09:04Z
New Revision: c90cb6eee8296953c097fcc9fc6e61f739c0dad3

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

LOG: [lldb] colorize symbols in image lookup with a regex pattern (#69422)

Fixes https://github.com/llvm/llvm-project/issues/57372

Previously some work has already been done on this. A PR was generated
but it remained in review:
https://reviews.llvm.org/D136462

In short previous approach was following:
Changing the symbol names (making the searched part colorized) ->
printing them -> restoring the symbol names back in their original form.

The reviewers suggested that instead of changing the symbol table, this
colorization should be done in the dump functions itself. Our strategy
involves passing the searched regex pattern to the existing dump
functions responsible for printing information about the searched
symbol. This pattern is propagated until it reaches the line in the dump
functions responsible for displaying symbol information on screen.

At this point, we've introduced a new function called
"PutCStringColorHighlighted," which takes the searched pattern, a prefix and 
suffix,
and the text and applies colorization to highlight the pattern in the
output. This approach aims to streamline the symbol search process to
improve readability of search results.

Co-authored-by: José L. Junior 

Added: 
lldb/test/Shell/Commands/command-image-lookup-color.test

Modified: 
lldb/include/lldb/Core/Address.h
lldb/include/lldb/Core/Debugger.h
lldb/include/lldb/Symbol/Symbol.h
lldb/include/lldb/Symbol/SymbolContext.h
lldb/include/lldb/Utility/Stream.h
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Core/Address.cpp
lldb/source/Core/CoreProperties.td
lldb/source/Core/Debugger.cpp
lldb/source/Symbol/Symbol.cpp
lldb/source/Symbol/SymbolContext.cpp
lldb/source/Utility/Stream.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/Address.h 
b/lldb/include/lldb/Core/Address.h
index b19e694427546f..725b5d9f91d3d5 100644
--- a/lldb/include/lldb/Core/Address.h
+++ b/lldb/include/lldb/Core/Address.h
@@ -14,6 +14,8 @@
 #include "lldb/lldb-private-enumerations.h"
 #include "lldb/lldb-types.h"
 
+#include "llvm/ADT/StringRef.h"
+
 #include 
 #include 
 
@@ -237,6 +239,12 @@ class Address {
   /// contains the address, otherwise dumping the range that contains the
   /// address.
   ///
+  /// \param[in] pattern
+  /// An optional regex pattern to match against the description. If
+  /// specified, parts of the description matching this pattern may be
+  /// highlighted or processed 
diff erently. If this parameter is an empty
+  /// string or not provided, no highlighting is applied.
+  ///
   /// \return
   /// Returns \b true if the address was able to be displayed.
   /// File and load addresses may be unresolved and it may not be
@@ -246,8 +254,8 @@ class Address {
   /// \see Address::DumpStyle
   bool Dump(Stream *s, ExecutionContextScope *exe_scope, DumpStyle style,
 DumpStyle fallback_style = DumpStyleInvalid,
-uint32_t addr_byte_size = UINT32_MAX,
-bool all_ranges = false) const;
+uint32_t addr_byte_size = UINT32_MAX, bool all_ranges = false,
+llvm::StringRef pattern = "") const;
 
   AddressClass GetAddressClass() const;
 

diff  --git a/lldb/include/lldb/Core/Debugger.h 
b/lldb/include/lldb/Core/Debugger.h
index e4ee94809cf1a0..c6d603ca5dcde0 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -321,6 +321,10 @@ class Debugger : public 
std::enable_shared_from_this,
 
   llvm::StringRef GetAutosuggestionAnsiSuffix() const;
 
+  llvm::StringRef GetRegexMatchAnsiPrefix() const;
+
+  llvm::StringRef GetRegexMatchAnsiSuffix() const;
+
   bool GetShowDontUsePoHint() const;
 
   bool GetUseSourceCache() const;

diff  --git a/lldb/include/lldb/Symbol/Symbol.h 
b/lldb/include/lldb/Symbol/Symbol.h
index 44a2d560010fe4..e6c0b495bcf28c 100644
--- a/lldb/include/lldb/Symbol/Symbol.h
+++ b/lldb/include/lldb/Symbol/Symbol.h
@@ -174,8 +174,8 @@ class Symbol : public SymbolContextScope {
 
   void SetFlags(uint32_t flags) { m_flags = flags; }
 
-  void GetDescription(Stream *s, lldb::DescriptionLevel level,
-  Target *target) const;
+  void GetDescription(Stream *s, lldb::DescriptionLevel level, Target *target,
+  llvm::StringRef pattern = "") const;
 
   bool IsSynthetic() const { return m_is_synthetic; }
 

diff  --git a/lldb/include/lldb/Symbol/SymbolContext.h 
b/lldb/include/lldb/Symbol/SymbolContext.h
index b0f5ffead2a165..26f3bac09a9626 100644
--- a/lldb/include/lldb/Symbol/SymbolContext.h
+++ b/lldb/include/lldb/Symbol/Sy

[Lldb-commits] [lldb] [lldb] colorize symbols in image lookup with a regex pattern (PR #69422)

=?utf-8?q?José?= L. Junior ,taalhaataahir0102
 <23100...@lums.edu.pk>,taalhaataahir0102 
<23100...@lums.edu.pk>,taalhaataahir0102
 <23100...@lums.edu.pk>,taalhaataahir0102 <23100...@lums.edu.pk>,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior 
Message-ID:
In-Reply-To: 


https://github.com/DavidSpickett closed 
https://github.com/llvm/llvm-project/pull/69422
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] colorize symbols in image lookup with a regex pattern (PR #69422)

=?utf-8?q?José?= L. Junior ,taalhaataahir0102
 <23100...@lums.edu.pk>,taalhaataahir0102 
<23100...@lums.edu.pk>,taalhaataahir0102
 <23100...@lums.edu.pk>,taalhaataahir0102 <23100...@lums.edu.pk>,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior 
Message-ID:
In-Reply-To: 


DavidSpickett wrote:

This is failing tests but I'm pretty sure it's a missing nullptr check. I'll 
fix it.

https://github.com/llvm/llvm-project/pull/69422
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] ffd61c1 - [lldb] Add missing nullptr checks when colouring symbol output


Author: David Spickett
Date: 2023-12-08T11:24:07Z
New Revision: ffd61c1e96e9c8a472f305585930b45be0d639d3

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

LOG: [lldb] Add missing nullptr checks when colouring symbol output

This adds some checks missed by c90cb6eee8296953c097fcc9fc6e61f739c0dad3,
probably because some tests only run on certain platforms.

Added: 


Modified: 
lldb/source/Core/Address.cpp
lldb/source/Symbol/Symbol.cpp

Removed: 




diff  --git a/lldb/source/Core/Address.cpp b/lldb/source/Core/Address.cpp
index d86fb1520a896..19d34db44ea55 100644
--- a/lldb/source/Core/Address.cpp
+++ b/lldb/source/Core/Address.cpp
@@ -516,10 +516,14 @@ bool Address::Dump(Stream *s, ExecutionContextScope 
*exe_scope, DumpStyle style,
   if (symbol) {
 const char *symbol_name = symbol->GetName().AsCString();
 if (symbol_name) {
-  llvm::StringRef ansi_prefix =
-  target->GetDebugger().GetRegexMatchAnsiPrefix();
-  llvm::StringRef ansi_suffix =
-  target->GetDebugger().GetRegexMatchAnsiSuffix();
+  llvm::StringRef ansi_prefix;
+  llvm::StringRef ansi_suffix;
+  if (target) {
+ansi_prefix =
+target->GetDebugger().GetRegexMatchAnsiPrefix();
+ansi_suffix =
+target->GetDebugger().GetRegexMatchAnsiSuffix();
+  }
   s->PutCStringColorHighlighted(symbol_name, pattern,
 ansi_prefix, ansi_suffix);
   addr_t delta =

diff  --git a/lldb/source/Symbol/Symbol.cpp b/lldb/source/Symbol/Symbol.cpp
index 0d20d11b8c4ff..fcc45f861c225 100644
--- a/lldb/source/Symbol/Symbol.cpp
+++ b/lldb/source/Symbol/Symbol.cpp
@@ -254,8 +254,12 @@ void Symbol::GetDescription(Stream *s, 
lldb::DescriptionLevel level,
   s->Printf(", value = 0x%16.16" PRIx64,
 m_addr_range.GetBaseAddress().GetOffset());
   }
-  llvm::StringRef ansi_prefix = 
target->GetDebugger().GetRegexMatchAnsiPrefix();
-  llvm::StringRef ansi_suffix = 
target->GetDebugger().GetRegexMatchAnsiSuffix();
+  llvm::StringRef ansi_prefix;
+  llvm::StringRef ansi_suffix;
+  if (target) {
+ansi_prefix = target->GetDebugger().GetRegexMatchAnsiPrefix();
+ansi_suffix = target->GetDebugger().GetRegexMatchAnsiSuffix();
+  }
   if (ConstString demangled = m_mangled.GetDemangledName()) {
 s->PutCString(", name=\"");
 s->PutCStringColorHighlighted(demangled.GetStringRef(), pattern,



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

[Lldb-commits] [lldb] [lldb] colorize symbols in image lookup with a regex pattern (PR #69422)

=?utf-8?q?José?= L. Junior ,taalhaataahir0102
 <23100...@lums.edu.pk>,taalhaataahir0102 
<23100...@lums.edu.pk>,taalhaataahir0102
 <23100...@lums.edu.pk>,taalhaataahir0102 <23100...@lums.edu.pk>,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior 
Message-ID:
In-Reply-To: 


DavidSpickett wrote:

https://github.com/llvm/llvm-project/commit/ffd61c1e96e9c8a472f305585930b45be0d639d3

https://github.com/llvm/llvm-project/pull/69422
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [flang] [lldb] [mlir] [polly] [openmp] [compiler-rt] [lld] [clang] [llvm] [VPlan] Compute scalable VF in preheader for induction increment. (PR #74762)


https://github.com/fhahn closed https://github.com/llvm/llvm-project/pull/74762
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] colorize symbols in image lookup with a regex pattern (PR #69422)

=?utf-8?q?José?= L. Junior ,taalhaataahir0102
<23100...@lums.edu.pk>,taalhaataahir0102
<23100...@lums.edu.pk>,taalhaataahir0102
<23100...@lums.edu.pk>,taalhaataahir0102 <23100...@lums.edu.pk>,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior
Message-ID:
In-Reply-To:

DavidSpickett wrote:

Ok bots look good, so congratulations on landing this change! :partying_face:

(you will have some email from build bots, it's a good idea to go look at the
failures just to see what they look like, but I have fixed them)

Sounds like you already have ideas for follow ups. You should also add a
release note
(https://github.com/llvm/llvm-project/blob/main/llvm/docs/ReleaseNotes.rst#changes-to-lldb)
at some point when you're happy with the feature overall. Or in other words,
when it can be described in 1 or 2 sentences without a lot of caveats.

I'm trying the feature myself and noticing the thing you said about the symbol
context. Even when looking up symbols on a running process, when we print a
`Summary: ` line, `target_sp` is always nullptr. Surprising, and a bit annoying
as a majority of the symbols in my searches didn't have a `Name: ` line.

(though one can work around this by adding `-v`)

Could be that there's never been a need for target here, and as you said, what
you really want is the debugger settings. So perhaps the better route is to add
a link back to those.

https://github.com/llvm/llvm-project/pull/69422
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [llvm] [flang] [lld] [polly] [lldb] [openmp] [clang] [compiler-rt] [mlir] [VPlan] Initial modeling of VF * UF as VPValue. (PR #74761)


https://github.com/fhahn edited https://github.com/llvm/llvm-project/pull/74761
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] ce3c7c0 - [lldb][test] Don't check line number in image lookup colour test

2023-12-08 Thread Felipe de Azevedo Piovezan via lldb-commits


Author: David Spickett
Date: 2023-12-08T13:34:07Z
New Revision: ce3c7c09100803608177459b4d923f17742885f9

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

LOG: [lldb][test] Don't check line number in image lookup colour test

We can assume the correct symbol is found, so putting the line
number here is just going to confuse anyone extending these tests.

Added: 


Modified: 
lldb/test/Shell/Commands/command-image-lookup-color.test

Removed: 




diff  --git a/lldb/test/Shell/Commands/command-image-lookup-color.test 
b/lldb/test/Shell/Commands/command-image-lookup-color.test
index 5082f89113281..2b4452483acdf 100644
--- a/lldb/test/Shell/Commands/command-image-lookup-color.test
+++ b/lldb/test/Shell/Commands/command-image-lookup-color.test
@@ -26,7 +26,7 @@
 # Checking no colorization without regex search
 
 # RUN: %lldb %t -b -o 'settings set use-color true' -o 'image lookup -s main' 
| FileCheck %s --check-prefix CHECK6
-# CHECK6:Summary: {{.+}}`main at main.c:2
+# CHECK6:Summary: {{.+}}`main at main.c:
 
 # Checking no colorization when use-color is false
 



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

[Lldb-commits] [lldb] [SymbolFileDWARF][NFC] Remove duplicated code checking for type tags (PR #74773)



@@ -3128,36 +3121,11 @@ 
SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext(const DWARFDIE &die) {
   if (type_system &&
   !type_system->SupportsLanguage(GetLanguage(*type_die.GetCU(
 return true;
-  bool try_resolving_type = false;
 
-  // Don't try and resolve the DIE we are looking for with the DIE
-  // itself!
   const dw_tag_t type_tag = type_die.Tag();
-  // Make sure the tags match
-  if (type_tag == tag) {
-// The tags match, lets try resolving this type
-try_resolving_type = true;
-  } else {
-// The tags don't match, but we need to watch our for a forward
-// declaration for a struct and ("struct foo") ends up being a
-// class ("class foo { ... };") or vice versa.
-switch (type_tag) {
-case DW_TAG_class_type:
-  // We had a "class foo", see if we ended up with a "struct foo
-  // { ... };"
-  try_resolving_type = (tag == DW_TAG_structure_type);
-  break;
-case DW_TAG_structure_type:
-  // We had a "struct foo", see if we ended up with a "class foo
-  // { ... };"
-  try_resolving_type = (tag == DW_TAG_class_type);
-  break;
-default:
-  // Tags don't match, don't event try to resolve using this type
-  // whose name matches
-  break;
-}
-  }
+  // Resolve the type if both have the same tag or {class, struct} tags.
+  const bool try_resolving_type =
+  type_tag == tag || (IsTypeTag(type_tag) && IsTypeTag(tag));

felipepiovezan wrote:

I'm not sure I follow, the logic of the opposite: equality failed, so we have 
to test whether both are "clasd-ish" tags. If one is a class but the other is a 
namespace, we need this expression to be false

https://github.com/llvm/llvm-project/pull/74773
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [SymbolFileDWARF][NFC] Remove duplicated code checking for type tags (PR #74773)

2023-12-08 Thread Felipe de Azevedo Piovezan via lldb-commits



@@ -130,6 +130,10 @@ class PluginProperties : public Properties {
   }
 };
 
+bool IsTypeTag(llvm::dwarf::Tag Tag) {

felipepiovezan wrote:

I'll rename it.

Regarding static, this function as well as most functions defined above it, are 
in an anonymous namespace

https://github.com/llvm/llvm-project/pull/74773
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [SymbolFileDWARF][NFC] Remove duplicated code checking for type tags (PR #74773)

2023-12-08 Thread Felipe de Azevedo Piovezan via lldb-commits


https://github.com/felipepiovezan edited 
https://github.com/llvm/llvm-project/pull/74773
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [SymbolFileDWARF][NFC] Remove duplicated code checking for type tags (PR #74773)

2023-12-08 Thread Felipe de Azevedo Piovezan via lldb-commits


https://github.com/felipepiovezan edited 
https://github.com/llvm/llvm-project/pull/74773
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 810d09f - [lldb][test] Disable image lookup colour test on Windows


Author: David Spickett
Date: 2023-12-08T13:36:41Z
New Revision: 810d09faf89af53025205c540ef9980e2286e687

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

LOG: [lldb][test] Disable image lookup colour test on Windows

On Linux `main.c` shows up in the symbol search but this is not the
case on Windows according to:
https://lab.llvm.org/buildbot/#/builders/219/builds/7422/steps/6/logs/stdio

It's possible we could make this test work there once function
search highlighting is implemented.

Added: 


Modified: 
lldb/test/Shell/Commands/command-image-lookup-color.test

Removed: 




diff  --git a/lldb/test/Shell/Commands/command-image-lookup-color.test 
b/lldb/test/Shell/Commands/command-image-lookup-color.test
index 2b4452483acdf..4a3169f269f2b 100644
--- a/lldb/test/Shell/Commands/command-image-lookup-color.test
+++ b/lldb/test/Shell/Commands/command-image-lookup-color.test
@@ -1,5 +1,8 @@
 # RUN: %clang_host -g %S/Inputs/main.c -o %t
 
+# The file main.c is does not show up in search on Windows.
+# UNSUPPORTED: system-windows
+
 # Checking simple regex search
 
 # RUN: %lldb %t -b -o 'settings set use-color true' -o 'image lookup -r -s ma' 
| FileCheck %s --check-prefix CHECK1



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

[Lldb-commits] [lldb] 61f1825 - [lldb][test] Disable image lookup colour test on Mac OS


Author: David Spickett
Date: 2023-12-08T13:44:06Z
New Revision: 61f18255fab3c404dc43a59091a750c22e5d0ccb

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

LOG: [lldb][test] Disable image lookup colour test on Mac OS

I think it can work there but we need to correct the CHECK lines.

```
command-image-lookup-color.test:34:11: error: CHECK7: expected string not found 
in input
  ^
```
https://green.lab.llvm.org/green/view/LLDB/job/as-lldb-cmake/10880/testReport/

I don't have a way to see the full output.

Added: 


Modified: 
lldb/test/Shell/Commands/command-image-lookup-color.test

Removed: 




diff  --git a/lldb/test/Shell/Commands/command-image-lookup-color.test 
b/lldb/test/Shell/Commands/command-image-lookup-color.test
index 4a3169f269f2b..186526b7efaee 100644
--- a/lldb/test/Shell/Commands/command-image-lookup-color.test
+++ b/lldb/test/Shell/Commands/command-image-lookup-color.test
@@ -3,6 +3,9 @@
 # The file main.c is does not show up in search on Windows.
 # UNSUPPORTED: system-windows
 
+# Until we figure out the correct CHECK lines.
+# UNSUPPORTED: system-darwin
+
 # Checking simple regex search
 
 # RUN: %lldb %t -b -o 'settings set use-color true' -o 'image lookup -r -s ma' 
| FileCheck %s --check-prefix CHECK1



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

[Lldb-commits] [lldb] [lldb] colorize symbols in image lookup with a regex pattern (PR #69422)

2023-12-08 Thread Felipe de Azevedo Piovezan via lldb-commits

=?utf-8?q?José?= L. Junior ,taalhaataahir0102
 <23100...@lums.edu.pk>,taalhaataahir0102 
<23100...@lums.edu.pk>,taalhaataahir0102
 <23100...@lums.edu.pk>,taalhaataahir0102 <23100...@lums.edu.pk>,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior 
Message-ID:
In-Reply-To: 


DavidSpickett wrote:

Spoke too soon, had to disable the test on Windows: 
https://github.com/llvm/llvm-project/commit/810d09faf89af53025205c540ef9980e2286e687

Making it work there may require making function lookup work, not sure.

And Mac OS: 
https://github.com/llvm/llvm-project/commit/61f18255fab3c404dc43a59091a750c22e5d0ccb

Which I think is a matter of finding the right regex for the CHECK. If either 
of you have access to Windows/Mac, please give it a go.

Otherwise, @JDevlieghere could you (/one of your colleagues) try the test on 
Mac and see what can be done?

https://github.com/llvm/llvm-project/pull/69422
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [SymbolFileDWARF][NFC] Remove duplicated code checking for type tags (PR #74773)



@@ -130,6 +130,10 @@ class PluginProperties : public Properties {
   }
 };
 
+bool IsTypeTag(llvm::dwarf::Tag Tag) {

felipepiovezan wrote:

Do you prefer if I take it out of the namespace and make it static? In 
llvm-project/llvm I would have done because of the programming guidelines

https://github.com/llvm/llvm-project/pull/74773
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [SymbolFileDWARF][NFC] Remove duplicated code checking for type tags (PR #74773)

2023-12-08 Thread Felipe de Azevedo Piovezan via lldb-commits


https://github.com/felipepiovezan edited 
https://github.com/llvm/llvm-project/pull/74773
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [lldb] fixing issue #64441 (PR #74814)

2023-12-08 Thread Felipe de Azevedo Piovezan via lldb-commits



@@ -56,10 +56,10 @@ namespace Foo = A::B;   // namespace alias
 
 using Foo::myfunc;  // using declaration
 
-using namespace Foo;// using directive
+//removing namespace foo; for quality naming 

felipepiovezan wrote:

Hi @jeevanghimire, please note that this is a test, so we must ensure that LLDB 
does the right thing regardless of the input it receives, even if said input is 
not considered best practices. The test was likely created to either ensure 
LLDB works in the case, or because it didn't use to work and  someone created a 
test at the same time the fix was done.

 As such, I don't think we should change this.


https://github.com/llvm/llvm-project/pull/74814
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [lldb] fixing issue #64441 (PR #74814)

2023-12-08 Thread Felipe de Azevedo Piovezan via lldb-commits



@@ -34,7 +34,7 @@ namespace A {
 int myfunc (int a);
 int myfunc2(int a)
 {
- return a + 2;
+return a + 2; //just changing tab not much

felipepiovezan wrote:

In general, we don't add diffs that are unrelated to the problem being solved 
by the PR.
On a similar note, this comment seems to be trying to communicate with 
reviewers, not future readers of this test file. As such, this is the kind of 
comment that should be part of the commit message / PR description, and not 
part of the test itself

https://github.com/llvm/llvm-project/pull/74814
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [clang] fixing issue #64441 (PR #74814)

2023-12-08 Thread Felipe de Azevedo Piovezan via lldb-commits


https://github.com/felipepiovezan requested changes to this pull request.

Hi @jeevanghimire, thank you for the PR! I believe the original issue was not 
about changing LLVM test inputs, but rather about changing clang-tidy to warn 
about this kind of patterns. 
Please see my rationale in a previous comment for why we should not be changing 
tests.

For future contributions, I'd also recommend using a more descriptive commit 
title and message. If you inspect the git log of the project, we generally 
follow the pattern:

[some topic]  




For example, this commit message could have been:

[clang-tidy] Implement "using namespace" check


https://github.com/llvm/llvm-project/pull/74814
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [compiler-rt] [flang] [lldb] [lld] [clang] [llvm] [libcxxabi] [libcxx] [clang-tools-extra] [AMDGPU] GFX12: select @llvm.prefetch intrinsic (PR #74576)

2023-12-08 Thread Jay Foad via lldb-commits



@@ -959,6 +967,32 @@ def : GCNPat <
 }
 } // let OtherPredicates = [HasShaderCyclesRegister]
 
+def SIMM24bitPtr : ImmLeaf (Imm);}]
+>;
+
+multiclass SMPrefetchPat {
+  def : GCNPat <
+(smrd_prefetch (SMRDImm i64:$sbase, i32:$offset), timm, timm, (i32 
cache_type)),
+(!cast("S_PREFETCH_"#type) $sbase, $offset, (i32 
SGPR_NULL), (i8 0))
+  >;
+
+  def : GCNPat <
+(smrd_prefetch (i64 SReg_64:$sbase), timm, timm, (i32 cache_type)),
+(!cast("S_PREFETCH_"#type) $sbase, 0, (i32 SGPR_NULL), 
(i8 0))
+  >;
+
+  def : GCNPat <
+(prefetch SIMM24bitPtr:$offset, timm, timm, (i32 cache_type)),
+(!cast("S_PREFETCH_"#type#"_PC_REL") (as_i32timm 
$offset), (i32 SGPR_NULL), (i8 0))
+  > {
+let AddedComplexity = 10;
+  }

jayfoad wrote:

I really don't know. What would the use cases look like? Maybe it could be a 
generic intrinsic, if there is consensus that it is useful.

For the existing llvm.prefetch intrinsic, the only useful case I think of for 
instruction prefetching is:
```
define @f0() {
  call @llvm.prefetch(@f1, ...) 
  ...
  call @f1()
}
define @f1() { ... }
```
to prefetch the code at the start of a function you are going to call. We could 
codegen that case using the _pc_rel form of the instruction.

https://github.com/llvm/llvm-project/pull/74576
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [SymbolFileDWARF][NFC] Remove duplicated code checking for type tags (PR #74773)

2023-12-08 Thread Felipe de Azevedo Piovezan via lldb-commits


https://github.com/felipepiovezan edited 
https://github.com/llvm/llvm-project/pull/74773
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [SymbolFileDWARF][NFC] Remove duplicated code checking for type tags (PR #74773)

2023-12-08 Thread Felipe de Azevedo Piovezan via lldb-commits


https://github.com/felipepiovezan updated 
https://github.com/llvm/llvm-project/pull/74773

>From 9f38564875620a44a982a50492d87ee431baffcd Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan 
Date: Thu, 7 Dec 2023 13:41:44 -0800
Subject: [PATCH 1/3] [SymbolFileDWARF][NFC] Remove duplicated code checking
 for type tags

There was duplicated (and complex) code querying whether tags were type-like
tags (i.e. class or struct); this has been factored out into a helper function.

There was also a comment about not comparing identical DIEs without ever
performing that check; this comment has been removed. It was likely a result of
copy paste from another function in this same file which actually does that
check.
---
 .../SymbolFile/DWARF/SymbolFileDWARF.cpp  | 58 +--
 1 file changed, 13 insertions(+), 45 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index d4cc26a3c329b..541d8d75f2187 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -130,6 +130,10 @@ class PluginProperties : public Properties {
   }
 };
 
+bool IsTypeTag(llvm::dwarf::Tag Tag) {
+  return Tag == llvm::dwarf::Tag::DW_TAG_class_type ||
+ Tag == llvm::dwarf::Tag::DW_TAG_structure_type;
+}
 } // namespace
 
 static PluginProperties &GetGlobalPluginProperties() {
@@ -2947,29 +2951,18 @@ TypeSP 
SymbolFileDWARF::FindCompleteObjCDefinitionTypeForDIE(
 
   m_index->GetCompleteObjCClass(
   type_name, must_be_implementation, [&](DWARFDIE type_die) {
-bool try_resolving_type = false;
-
 // Don't try and resolve the DIE we are looking for with the DIE
 // itself!
-if (type_die != die) {
-  switch (type_die.Tag()) {
-  case DW_TAG_class_type:
-  case DW_TAG_structure_type:
-try_resolving_type = true;
-break;
-  default:
-break;
-  }
-}
-if (!try_resolving_type)
+if (type_die == die || !IsTypeTag(type_die.Tag()))
   return true;
 
 if (must_be_implementation &&
-type_die.Supports_DW_AT_APPLE_objc_complete_type())
-  try_resolving_type = type_die.GetAttributeValueAsUnsigned(
+type_die.Supports_DW_AT_APPLE_objc_complete_type()) {
+  bool try_resolving_type = type_die.GetAttributeValueAsUnsigned(
   DW_AT_APPLE_objc_complete_type, 0);
-if (!try_resolving_type)
-  return true;
+  if (!try_resolving_type)
+return true;
+}
 
 Type *resolved_type = ResolveType(type_die, false, true);
 if (!resolved_type || resolved_type == DIE_IS_BEING_PARSED)
@@ -3128,36 +3121,11 @@ 
SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext(const DWARFDIE &die) {
   if (type_system &&
   !type_system->SupportsLanguage(GetLanguage(*type_die.GetCU(
 return true;
-  bool try_resolving_type = false;
 
-  // Don't try and resolve the DIE we are looking for with the DIE
-  // itself!
   const dw_tag_t type_tag = type_die.Tag();
-  // Make sure the tags match
-  if (type_tag == tag) {
-// The tags match, lets try resolving this type
-try_resolving_type = true;
-  } else {
-// The tags don't match, but we need to watch our for a forward
-// declaration for a struct and ("struct foo") ends up being a
-// class ("class foo { ... };") or vice versa.
-switch (type_tag) {
-case DW_TAG_class_type:
-  // We had a "class foo", see if we ended up with a "struct foo
-  // { ... };"
-  try_resolving_type = (tag == DW_TAG_structure_type);
-  break;
-case DW_TAG_structure_type:
-  // We had a "struct foo", see if we ended up with a "class foo
-  // { ... };"
-  try_resolving_type = (tag == DW_TAG_class_type);
-  break;
-default:
-  // Tags don't match, don't event try to resolve using this type
-  // whose name matches
-  break;
-}
-  }
+  // Resolve the type if both have the same tag or {class, struct} tags.
+  bool try_resolving_type =
+  type_tag == tag || (IsTypeTag(type_tag) && IsTypeTag(tag));
 
   if (!try_resolving_type) {
 if (log) {

>From 0225efc828696fe2dabae519a09f5a7299ef2aed Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan 
Date: Thu, 7 Dec 2023 14:39:24 -0800
Subject: [PATCH 2/3] fixup! [SymbolFileDWARF][NFC] Remove duplicated code
 checking for type tags

---
 lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 541d8d75f2187..a94bb93fc2f7f 100644
--- a/lldb/so

[Lldb-commits] [lldb] [SymbolFileDWARF][NFC] Remove duplicated code checking for type tags (PR #74773)

2023-12-08 Thread Felipe de Azevedo Piovezan via lldb-commits



@@ -130,6 +130,10 @@ class PluginProperties : public Properties {
   }
 };
 
+bool IsTypeTag(llvm::dwarf::Tag Tag) {

felipepiovezan wrote:

In the interest of expediency, I've moved it out of the anonymous namespace and 
made it static

https://github.com/llvm/llvm-project/pull/74773
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] colorize symbols in image lookup with a regex pattern (PR #69422)

2023-12-08 Thread Felipe de Azevedo Piovezan via lldb-commits

=?utf-8?q?José?= L. Junior ,taalhaataahir0102
 <23100...@lums.edu.pk>,taalhaataahir0102 
<23100...@lums.edu.pk>,taalhaataahir0102
 <23100...@lums.edu.pk>,taalhaataahir0102 <23100...@lums.edu.pk>,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior ,
=?utf-8?q?José?= L. Junior 
Message-ID:
In-Reply-To: 


taalhaataahir0102 wrote:

Sure will try to make it work for windows. Hoping it will be as easier to build 
lldb on windows as it's for linux 🙂

https://github.com/llvm/llvm-project/pull/69422
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Make only one function that needs to be implemented when searching for types (PR #74786)


felipepiovezan wrote:

I am really struggling to handle the scope of the changes here, IIUC there are 
a handful of changes that could be split into separate commits and merged 
independently of each other: there is some code being moved around, new 
GetCompilerContext APIs, the new query, the replacing of the old queries with 
the new queries. All of these could be different changes? It would also making 
reverting things a lot easier if we end up breaking bots down the line, and 
some of these changes are just "NFC goodness" that would never need to be 
reverted

https://github.com/llvm/llvm-project/pull/74786
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb-dap] Implement quiet commands (PR #74808)

2023-12-08 Thread Walter Erquinigo via lldb-commits


https://github.com/walter-erquinigo updated 
https://github.com/llvm/llvm-project/pull/74808

>From d544ba0cc6836c0b5c2086edccfc2b41080174ef Mon Sep 17 00:00:00 2001
From: walter erquinigo 
Date: Fri, 8 Dec 2023 00:11:19 -0500
Subject: [PATCH] [lldb-dap] Implement quiet commands

This adds support for optionally prefixing any command with `?`, which 
effectively prevents the output of these commands to be printed to the console 
unless they fail. This comes handy when programmaticaly running commands on 
behalf of the user without wanting them to know unless they fail.

In a later PR I plan to implement the `!` prefix for commands that abort 
lldb-dap upon errors.
---
 .../test/API/tools/lldb-dap/commands/Makefile |  3 +
 .../lldb-dap/commands/TestDAP_commands.py | 44 ++
 .../test/API/tools/lldb-dap/commands/main.cpp |  1 +
 lldb/tools/lldb-dap/LLDBUtils.cpp | 59 ---
 lldb/tools/lldb-dap/LLDBUtils.h   | 20 ++-
 lldb/tools/lldb-dap/lldb-dap.cpp  |  3 +-
 lldb/tools/lldb-dap/package.json  | 24 
 7 files changed, 130 insertions(+), 24 deletions(-)
 create mode 100644 lldb/test/API/tools/lldb-dap/commands/Makefile
 create mode 100644 lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py
 create mode 100644 lldb/test/API/tools/lldb-dap/commands/main.cpp

diff --git a/lldb/test/API/tools/lldb-dap/commands/Makefile 
b/lldb/test/API/tools/lldb-dap/commands/Makefile
new file mode 100644
index 0..8b20bcb05
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/commands/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git a/lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py 
b/lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py
new file mode 100644
index 0..aa0c76b056a46
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py
@@ -0,0 +1,44 @@
+import os
+
+import dap_server
+import lldbdap_testcase
+from lldbsuite.test import lldbtest, lldbutil
+from lldbsuite.test.decorators import *
+
+
+class TestDAP_commands(lldbdap_testcase.DAPTestCaseBase):
+def test_command_directive_quiet_on_success(self):
+program = self.getBuildArtifact("a.out")
+command_quiet = (
+"settings set target.show-hex-variable-values-with-leading-zeroes 
false"
+)
+command_not_quiet = (
+"settings set target.show-hex-variable-values-with-leading-zeroes 
true"
+)
+self.build_and_launch(
+program,
+initCommands=["?" + command_quiet, command_not_quiet],
+)
+full_output = self.collect_console(duration=1.0)
+self.assertNotIn(command_quiet, full_output)
+self.assertIn(command_not_quiet, full_output)
+
+def test_command_directive_abort_on_error(self):
+program = self.getBuildArtifact("a.out")
+command_quiet = (
+"settings set target.show-hex-variable-values-with-leading-zeroes 
false"
+)
+command_abort_on_error = "settings set foo bar"
+try:
+# We use try/except because the client raises after the adapter
+# abruptly terminates.
+self.build_and_launch(
+program,
+initCommands=["?!" + command_quiet, "!" + 
command_abort_on_error],
+)
+except:
+pass
+full_output = self.collect_console(duration=1.0)
+self.assertNotIn(command_quiet, full_output)
+self.assertIn(command_abort_on_error, full_output)
+self.assertIn("Terminating", full_output)
diff --git a/lldb/test/API/tools/lldb-dap/commands/main.cpp 
b/lldb/test/API/tools/lldb-dap/commands/main.cpp
new file mode 100644
index 0..76e8197013aab
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/commands/main.cpp
@@ -0,0 +1 @@
+int main() { return 0; }
diff --git a/lldb/tools/lldb-dap/LLDBUtils.cpp 
b/lldb/tools/lldb-dap/LLDBUtils.cpp
index 955c11f636895..2c127f162dc56 100644
--- a/lldb/tools/lldb-dap/LLDBUtils.cpp
+++ b/lldb/tools/lldb-dap/LLDBUtils.cpp
@@ -13,16 +13,22 @@ namespace lldb_dap {
 
 void RunLLDBCommands(llvm::StringRef prefix,
  const llvm::ArrayRef &commands,
- llvm::raw_ostream &strm) {
+ llvm::raw_ostream &strm, bool parse_command_directives) {
   if (commands.empty())
 return;
-  lldb::SBCommandInterpreter interp = g_dap.debugger.GetCommandInterpreter();
-  if (!prefix.empty())
-strm << prefix << "\n";
-  for (const auto &command : commands) {
-lldb::SBCommandReturnObject result;
+
+  bool did_print_prefix = false;
+
+  auto print_result = [&](llvm::StringRef command,
+  lldb::SBCommandReturnObject &result,
+  llvm::raw_ostream &strm) {
+// We only print the prefix if we are effectively printing at least one
+// command.
+if (!did_print_prefix && !prefix.emp

[Lldb-commits] [lldb] [lldb-dap] Implement command directives (PR #74808)

2023-12-08 Thread Walter Erquinigo via lldb-commits


https://github.com/walter-erquinigo edited 
https://github.com/llvm/llvm-project/pull/74808
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb-dap] Implement command directives (PR #74808)

2023-12-08 Thread Walter Erquinigo via lldb-commits


https://github.com/walter-erquinigo edited 
https://github.com/llvm/llvm-project/pull/74808
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb-dap] Include [opt] in the frame name only if a custom frame format is not specified. (PR #74861)

2023-12-08 Thread Walter Erquinigo via lldb-commits


https://github.com/walter-erquinigo created 
https://github.com/llvm/llvm-project/pull/74861

Currently there's an include in which `[opt]` might be emitted twice if the 
frame format also asks for it. As a trivial fix, we should manually emit 
`[opt]` only if a custom frame format is not specified.


>From 349cd0694eccdaa4e403cfc6ba9389ed61b1d232 Mon Sep 17 00:00:00 2001
From: walter erquinigo 
Date: Wed, 29 Nov 2023 13:46:54 -0500
Subject: [PATCH] [lldb-dap] Include [opt] in the frame name only if a custom
 frame format is not specified.

Currently there's an include in which `[opt]` might be emitted twice if the 
frame format also asks for it. As a trivial fix, we should manually emit 
`[opt]` only if a custom frame format is not specified.
---
 lldb/tools/lldb-dap/JSONUtils.cpp | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp 
b/lldb/tools/lldb-dap/JSONUtils.cpp
index 03a43f9da87f24..e55a69ecd0e9f2 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -803,9 +803,11 @@ llvm::json::Value CreateStackFrame(lldb::SBFrame &frame) {
 llvm::raw_string_ostream os(frame_name);
 os << llvm::format_hex(frame.GetPC(), 18);
   }
-  bool is_optimized = frame.GetFunction().GetIsOptimized();
-  if (is_optimized)
+
+  // We only include `[opt]` if a custom frame format is not specified.
+  if (!g_dap.frame_format && frame.GetFunction().GetIsOptimized())
 frame_name += " [opt]";
+
   EmplaceSafeString(object, "name", frame_name);
 
   auto source = CreateSource(frame);

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

[Lldb-commits] [lldb] [lldb-dap] Include [opt] in the frame name only if a custom frame format is not specified. (PR #74861)


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Walter Erquinigo (walter-erquinigo)


Changes

Currently there's an include in which `[opt]` might be emitted twice if the 
frame format also asks for it. As a trivial fix, we should manually emit 
`[opt]` only if a custom frame format is not specified.


---
Full diff: https://github.com/llvm/llvm-project/pull/74861.diff


1 Files Affected:

- (modified) lldb/tools/lldb-dap/JSONUtils.cpp (+4-2) 


``diff
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp 
b/lldb/tools/lldb-dap/JSONUtils.cpp
index 03a43f9da87f24..e55a69ecd0e9f2 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -803,9 +803,11 @@ llvm::json::Value CreateStackFrame(lldb::SBFrame &frame) {
 llvm::raw_string_ostream os(frame_name);
 os << llvm::format_hex(frame.GetPC(), 18);
   }
-  bool is_optimized = frame.GetFunction().GetIsOptimized();
-  if (is_optimized)
+
+  // We only include `[opt]` if a custom frame format is not specified.
+  if (!g_dap.frame_format && frame.GetFunction().GetIsOptimized())
 frame_name += " [opt]";
+
   EmplaceSafeString(object, "name", frame_name);
 
   auto source = CreateSource(frame);

``




https://github.com/llvm/llvm-project/pull/74861
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Return index of element in ValueObject path instead of the element's value (PR #74413)

2023-12-08 Thread Adrian Prantl via lldb-commits


adrian-prantl wrote:

> > doesn't appear to be anything that calls these methods
> 
> Candidate for removal then? If so it still makes sense to fix them, then 
> remove them. Saves fixing them again if they are brought back.
> 
> Either way, I'm not going to block this by requiring a test case.

I think that makes sense. @PortalPete why don't you land this without a test 
and then create a PR to remove the unused methods?

https://github.com/llvm/llvm-project/pull/74413
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb-dap] Emit declarations along with variables (PR #74865)

2023-12-08 Thread Walter Erquinigo via lldb-commits


https://github.com/walter-erquinigo created 
https://github.com/llvm/llvm-project/pull/74865

This is an extension to the protocol that emits the declaration information 
along with the metadata of each variable. This can be used by vscode extensions 
to implement, for example, a "goToDefinition" action in the debug tab, or for 
showing the value of a variable right next to where it's declared during a 
debug session.
As this is cheap, I'm not gating this information under any setting.


>From 534d019a744a4421f5ddef01635b62dfa39fe968 Mon Sep 17 00:00:00 2001
From: walter erquinigo 
Date: Fri, 8 Dec 2023 11:53:15 -0500
Subject: [PATCH] [lldb-dap] Emit declarations along with variables

This is an extension to the protocol that emits the declaration information 
along with the metadata of each variable. This can be used by vscode extensions 
to implement, for example, a "goToDefinition" action in the debug tab, or for 
showing the value of a variable right next to where it's declared during a 
debug session.
As this is cheap, I'm not gating this information under any setting.
---
 .../lldb-dap/variables/TestDAP_variables.py   | 10 -
 lldb/tools/lldb-dap/JSONUtils.cpp | 41 +++
 2 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py 
b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
index d2a21ad3cd1d4..9b0755eea7d3e 100644
--- a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
+++ b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
@@ -4,8 +4,8 @@
 
 import os
 
-import lldbdap_testcase
 import dap_server
+import lldbdap_testcase
 from lldbsuite.test import lldbutil
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
@@ -152,7 +152,13 @@ def do_test_scopes_variables_setVariable_evaluate(
 globals = self.dap_server.get_global_variables()
 buffer_children = make_buffer_verify_dict(0, 32)
 verify_locals = {
-"argc": {"equals": {"type": "int", "value": "1"}},
+"argc": {
+"equals": {"type": "int", "value": "1"},
+"declaration": {
+"equals": {"line": 12, "column": 14},
+"contains": {"path": ["lldb-dap", "variables", 
"main.cpp"]},
+},
+},
 "argv": {
 "equals": {"type": "const char **"},
 "startswith": {"value": "0x"},
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp 
b/lldb/tools/lldb-dap/JSONUtils.cpp
index 03a43f9da87f2..d8b5636fd03a9 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -1101,6 +1101,29 @@ std::string 
CreateUniqueVariableNameForDisplay(lldb::SBValue v,
 //   can use this optional information to present the
 //   children in a paged UI and fetch them in chunks."
 // }
+// "declaration": {
+//   "type": "object | undefined",
+//   "description": "Extension to the protocol that indicates the source
+//   location where the variable was declared. This value
+//   might not be present if no declaration is available.",
+//   "properties": {
+// "path": {
+//   "type": "string | undefined",
+//   "description": "The source file path where the variable was
+//   declared."
+// },
+// "line": {
+//   "type": "number | undefined",
+//   "description": "The 1-indexed source line where the variable was
+//  declared."
+// },
+// "column": {
+//   "type": "number | undefined",
+//   "description": "The 1-indexed source column where the variable was
+//  declared."
+// }
+//   }
+// }
 //   },
 //   "required": [ "name", "value", "variablesReference" ]
 // }
@@ -1165,6 +1188,24 @@ llvm::json::Value CreateVariable(lldb::SBValue v, 
int64_t variablesReference,
   const char *evaluateName = evaluateStream.GetData();
   if (evaluateName && evaluateName[0])
 EmplaceSafeString(object, "evaluateName", std::string(evaluateName));
+
+  if (lldb::SBDeclaration decl = v.GetDeclaration(); decl.IsValid()) {
+llvm::json::Object decl_obj;
+if (lldb::SBFileSpec file = decl.GetFileSpec(); file.IsValid()) {
+  char path[PATH_MAX] = "";
+  if (file.GetPath(path, sizeof(path)) &&
+  lldb::SBFileSpec::ResolvePath(path, path, PATH_MAX)) {
+decl_obj.try_emplace("path", std::string(path));
+  }
+}
+
+if (int line = decl.GetLine())
+  decl_obj.try_emplace("line", line);
+if (int column = decl.GetColumn())
+  decl_obj.try_emplace("column", column);
+
+object.try_emplace("declaration", std::move(decl_obj));
+  }
   return llvm::json::Value(std::move(object));
 }
 

___
lldb-co

[Lldb-commits] [lldb] [lldb-dap] Emit declarations along with variables (PR #74865)


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Walter Erquinigo (walter-erquinigo)


Changes

This is an extension to the protocol that emits the declaration information 
along with the metadata of each variable. This can be used by vscode extensions 
to implement, for example, a "goToDefinition" action in the debug tab, or for 
showing the value of a variable right next to where it's declared during a 
debug session.
As this is cheap, I'm not gating this information under any setting.


---
Full diff: https://github.com/llvm/llvm-project/pull/74865.diff


2 Files Affected:

- (modified) lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py (+8-2) 
- (modified) lldb/tools/lldb-dap/JSONUtils.cpp (+41) 


``diff
diff --git a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py 
b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
index d2a21ad3cd1d4..9b0755eea7d3e 100644
--- a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
+++ b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
@@ -4,8 +4,8 @@
 
 import os
 
-import lldbdap_testcase
 import dap_server
+import lldbdap_testcase
 from lldbsuite.test import lldbutil
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
@@ -152,7 +152,13 @@ def do_test_scopes_variables_setVariable_evaluate(
 globals = self.dap_server.get_global_variables()
 buffer_children = make_buffer_verify_dict(0, 32)
 verify_locals = {
-"argc": {"equals": {"type": "int", "value": "1"}},
+"argc": {
+"equals": {"type": "int", "value": "1"},
+"declaration": {
+"equals": {"line": 12, "column": 14},
+"contains": {"path": ["lldb-dap", "variables", 
"main.cpp"]},
+},
+},
 "argv": {
 "equals": {"type": "const char **"},
 "startswith": {"value": "0x"},
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp 
b/lldb/tools/lldb-dap/JSONUtils.cpp
index 03a43f9da87f2..d8b5636fd03a9 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -1101,6 +1101,29 @@ std::string 
CreateUniqueVariableNameForDisplay(lldb::SBValue v,
 //   can use this optional information to present the
 //   children in a paged UI and fetch them in chunks."
 // }
+// "declaration": {
+//   "type": "object | undefined",
+//   "description": "Extension to the protocol that indicates the source
+//   location where the variable was declared. This value
+//   might not be present if no declaration is available.",
+//   "properties": {
+// "path": {
+//   "type": "string | undefined",
+//   "description": "The source file path where the variable was
+//   declared."
+// },
+// "line": {
+//   "type": "number | undefined",
+//   "description": "The 1-indexed source line where the variable was
+//  declared."
+// },
+// "column": {
+//   "type": "number | undefined",
+//   "description": "The 1-indexed source column where the variable was
+//  declared."
+// }
+//   }
+// }
 //   },
 //   "required": [ "name", "value", "variablesReference" ]
 // }
@@ -1165,6 +1188,24 @@ llvm::json::Value CreateVariable(lldb::SBValue v, 
int64_t variablesReference,
   const char *evaluateName = evaluateStream.GetData();
   if (evaluateName && evaluateName[0])
 EmplaceSafeString(object, "evaluateName", std::string(evaluateName));
+
+  if (lldb::SBDeclaration decl = v.GetDeclaration(); decl.IsValid()) {
+llvm::json::Object decl_obj;
+if (lldb::SBFileSpec file = decl.GetFileSpec(); file.IsValid()) {
+  char path[PATH_MAX] = "";
+  if (file.GetPath(path, sizeof(path)) &&
+  lldb::SBFileSpec::ResolvePath(path, path, PATH_MAX)) {
+decl_obj.try_emplace("path", std::string(path));
+  }
+}
+
+if (int line = decl.GetLine())
+  decl_obj.try_emplace("line", line);
+if (int column = decl.GetColumn())
+  decl_obj.try_emplace("column", column);
+
+object.try_emplace("declaration", std::move(decl_obj));
+  }
   return llvm::json::Value(std::move(object));
 }
 

``




https://github.com/llvm/llvm-project/pull/74865
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [SymbolFileDWARF][NFC] Remove duplicated code checking for type tags (PR #74773)


https://github.com/clayborg approved this pull request.

Thanks for the quick change, and I need to check sources for the anonymous 
namespace before I comment next time!

https://github.com/llvm/llvm-project/pull/74773
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Make only one function that needs to be implemented when searching for types (PR #74786)

2023-12-08 Thread Stanislav Mekhanoshin via lldb-commits

clayborg wrote:

> I am really struggling to handle the scope of the changes here, IIUC there 
> are a handful of changes that could be split into separate commits and merged 
> independently of each other: there is some code being moved around, new 
> GetCompilerContext APIs, the new query, the replacing of the old queries with 
> the new queries. All of these could be different changes? It would also 
> making reverting things a lot easier if we end up breaking bots down the 
> line, and some of these changes are just "NFC goodness" that would never need 
> to be reverted

I would prefer to keep this all in one as I spent a month last year making all 
of the changes needed for this patch and it was accepted in Phabricator by 
Adrian after many iterations. While some of the changes are "NFC goodness", 
they don't make sense without this patch and we usually don't commit changes 
that aren't used (the GetCompilerContext APIs). The only code that was moved 
was the Language set.  And if we want to revert this, I would say this is 
easier to revert as a single commit.

@adrian-prantl thoughts?

https://github.com/llvm/llvm-project/pull/74786
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [compiler-rt] [libcxx] [lldb] [libcxxabi] [clang-tools-extra] [lld] [llvm] [clang] [flang] [AMDGPU] GFX12: select @llvm.prefetch intrinsic (PR #74576)



@@ -959,6 +967,32 @@ def : GCNPat <
 }
 } // let OtherPredicates = [HasShaderCyclesRegister]
 
+def SIMM24bitPtr : ImmLeaf (Imm);}]
+>;
+
+multiclass SMPrefetchPat {
+  def : GCNPat <
+(smrd_prefetch (SMRDImm i64:$sbase, i32:$offset), timm, timm, (i32 
cache_type)),
+(!cast("S_PREFETCH_"#type) $sbase, $offset, (i32 
SGPR_NULL), (i8 0))
+  >;
+
+  def : GCNPat <
+(smrd_prefetch (i64 SReg_64:$sbase), timm, timm, (i32 cache_type)),
+(!cast("S_PREFETCH_"#type) $sbase, 0, (i32 SGPR_NULL), 
(i8 0))
+  >;
+
+  def : GCNPat <
+(prefetch SIMM24bitPtr:$offset, timm, timm, (i32 cache_type)),
+(!cast("S_PREFETCH_"#type#"_PC_REL") (as_i32timm 
$offset), (i32 SGPR_NULL), (i8 0))
+  > {
+let AddedComplexity = 10;
+  }

rampitec wrote:

I do not think we need to use PC_REL form to prefetch on a function's address. 
The instruction can take full 64-bit address, so one can just use this address. 
 My understanding that PC_REL form can be useful if you expect something like a 
huge loop or a local branch and want to prefetch something like 1K from the PC. 
I am not sure though how useful this can be at a high language level or even in 
IR.

https://github.com/llvm/llvm-project/pull/74576
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [compiler-rt] [openmp] [lldb] [polly] [mlir] [lld] [llvm] [clang] [flang] [VPlan] Initial modeling of VF * UF as VPValue. (PR #74761)


https://github.com/fhahn closed https://github.com/llvm/llvm-project/pull/74761
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [flang] [compiler-rt] [lldb] [libc] [openmp] [libcxx] [llvm] [libcxxabi] [clang-tools-extra] [mlir] [lld] [clang] [libc++] Fix `take_view::__sentinel`'s `operator==` (PR #74655)

2023-12-08 Thread Hristo Hristov via lldb-commits



@@ -8,10 +8,7 @@
 
 // UNSUPPORTED: c++03, c++11, c++14, c++17
 
-// sentinel() = default;
-// constexpr explicit sentinel(sentinel_t end);
-// constexpr sentinel(sentinel s)
-//   requires Const && convertible_to, sentinel_t>;
+// constexpr sentinel_t base() const;

Zingam wrote:

Maybe that's already encoded in the parent directory as all other tests use 
just "sentinel" and "iterator"

https://github.com/llvm/llvm-project/pull/74655
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] lldb: add support for thread names on Windows (PR #74731)


https://github.com/oltolm updated 
https://github.com/llvm/llvm-project/pull/74731

>From 9383b8b92849c71a96b4b4e7e55f615d8f2efedb Mon Sep 17 00:00:00 2001
From: oltolm 
Date: Fri, 1 Dec 2023 16:49:13 +0100
Subject: [PATCH 1/2] lldb: add support for thread names on Windows

---
 .../Windows/Common/TargetThreadWindows.cpp| 31 +++
 .../Windows/Common/TargetThreadWindows.h  |  2 ++
 2 files changed, 33 insertions(+)

diff --git a/lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp 
b/lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp
index 37dc8f6d6d14a..047019ac30f49 100644
--- a/lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp
+++ b/lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp
@@ -15,6 +15,7 @@
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/State.h"
+#include "llvm/Support/ConvertUTF.h"
 
 #include "ProcessWindows.h"
 #include "ProcessWindowsLog.h"
@@ -33,6 +34,9 @@
 using namespace lldb;
 using namespace lldb_private;
 
+using GetThreadDescriptionFunctionPtr = HRESULT
+WINAPI (*)(HANDLE hThread, PWSTR *ppszThreadDescription);
+
 TargetThreadWindows::TargetThreadWindows(ProcessWindows &process,
  const HostThread &thread)
 : Thread(process, thread.GetNativeThread().GetThreadId()),
@@ -175,3 +179,30 @@ Status TargetThreadWindows::DoResume() {
 
   return Status();
 }
+
+const char *TargetThreadWindows::GetName() {
+  Log *log = GetLog(LLDBLog::Thread);
+  HMODULE hModule = ::LoadLibraryW(L"Kernel32.dll");
+  if (hModule) {
+auto GetThreadDescription =
+reinterpret_cast(
+::GetProcAddress(hModule, "GetThreadDescription"));
+LLDB_LOGF(log, "GetProcAddress: %p",
+  reinterpret_cast(GetThreadDescription));
+if (GetThreadDescription) {
+  PWSTR pszThreadName;
+  if (SUCCEEDED(GetThreadDescription(
+  m_host_thread.GetNativeThread().GetSystemHandle(),
+  &pszThreadName))) {
+LLDB_LOGF(log, "GetThreadDescription: %ls", pszThreadName);
+llvm::convertUTF16ToUTF8String(
+llvm::ArrayRef(reinterpret_cast(pszThreadName),
+   wcslen(pszThreadName) * sizeof(wchar_t)),
+m_name);
+::LocalFree(pszThreadName);
+  }
+}
+  }
+
+  return m_name.c_str();
+}
diff --git a/lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.h 
b/lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.h
index 2845847738f60..07e1db464ad59 100644
--- a/lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.h
+++ b/lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.h
@@ -34,6 +34,7 @@ class TargetThreadWindows : public lldb_private::Thread {
   lldb::RegisterContextSP
   CreateRegisterContextForFrame(StackFrame *frame) override;
   bool CalculateStopInfo() override;
+  const char *GetName() override;
 
   Status DoResume();
 
@@ -42,6 +43,7 @@ class TargetThreadWindows : public lldb_private::Thread {
 private:
   lldb::RegisterContextSP m_thread_reg_ctx_sp;
   HostThread m_host_thread;
+  std::string m_name;
 };
 } // namespace lldb_private
 

>From 89ea63ca021da9e350bbb3016da601d7543d7e50 Mon Sep 17 00:00:00 2001
From: oltolm 
Date: Fri, 8 Dec 2023 19:42:48 +0100
Subject: [PATCH 2/2] [lldb] add unit test for Windows thread names

---
 lldb/unittests/Thread/CMakeLists.txt |  2 +
 lldb/unittests/Thread/ThreadTest.cpp | 71 
 2 files changed, 73 insertions(+)

diff --git a/lldb/unittests/Thread/CMakeLists.txt 
b/lldb/unittests/Thread/CMakeLists.txt
index d6e365adac5dd..f6c8795f349a5 100644
--- a/lldb/unittests/Thread/CMakeLists.txt
+++ b/lldb/unittests/Thread/CMakeLists.txt
@@ -11,5 +11,7 @@ add_lldb_unittest(ThreadTests
   lldbInterpreter
   lldbBreakpoint
   lldbPluginPlatformLinux
+  lldbPluginPlatformWindows
+  lldbPluginProcessWindowsCommon
   )
 
diff --git a/lldb/unittests/Thread/ThreadTest.cpp 
b/lldb/unittests/Thread/ThreadTest.cpp
index bd8cdce99f172..4c660e9815c3e 100644
--- a/lldb/unittests/Thread/ThreadTest.cpp
+++ b/lldb/unittests/Thread/ThreadTest.cpp
@@ -8,9 +8,20 @@
 
 #include "lldb/Target/Thread.h"
 #include "Plugins/Platform/Linux/PlatformLinux.h"
+#include 
+#ifdef _WIN32
+#include "lldb/Host/windows/HostThreadWindows.h"
+#include "lldb/Host/windows/windows.h"
+
+#include "Plugins/Platform/Windows/PlatformWindows.h"
+#include "Plugins/Process/Windows/Common/LocalDebugDelegate.h"
+#include "Plugins/Process/Windows/Common/ProcessWindows.h"
+#include "Plugins/Process/Windows/Common/TargetThreadWindows.h"
+#endif
 #include "lldb/Core/Debugger.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/HostInfo.h"
+#include "lldb/Host/HostThread.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/StopInfo.h"
 #include "lldb/Utility/ArchSpec.h"
@@ -21,14 +32,33 @@ using namespace lldb_private::repro;
 using namespace l

[Lldb-commits] [lldb] [lldb] Return index of element in ValueObject path instead of the element's value (PR #74413)


PortalPete wrote:

> > > doesn't appear to be anything that calls these methods
> > 
> > 
> > Candidate for removal then? If so it still makes sense to fix them, then 
> > remove them. Saves fixing them again if they are brought back.
> > Either way, I'm not going to block this by requiring a test case.
> 
> I think that makes sense. @PortalPete why don't you land this without a test 
> and then create a PR to remove the unused methods?

That works for me, @adrian-prantl.
I'll remove these methods wholesale after someone approves/merges this PR on my 
behalf.

https://github.com/llvm/llvm-project/pull/74413
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] lldb: add support for thread names on Windows (PR #74731)

2023-12-08 Thread Felipe de Azevedo Piovezan via lldb-commits


oltolm wrote:

I have added a test.

https://github.com/llvm/llvm-project/pull/74731
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Return index of element in ValueObject path instead of the element's value (PR #74413)


https://github.com/felipepiovezan approved this pull request.


https://github.com/llvm/llvm-project/pull/74413
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][progress] Add discrete boolean flag to progress reports (PR #69516)

2023-12-08 Thread Chelsea Cassanova via lldb-commits



@@ -2225,7 +2225,8 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
   const char *file_name = file.GetFilename().AsCString("");
   LLDB_SCOPED_TIMERF("ObjectFileMachO::ParseSymtab () module = %s", file_name);
   LLDB_LOG(log, "Parsing symbol table for {0}", file_name);
-  Progress progress(llvm::formatv("Parsing symbol table for {0}", file_name));
+  Progress progress(llvm::formatv("Parsing symbol table for {0}", file_name),
+Progress::ProgressReportType::eAggregateProgressReport);

chelcassanova wrote:

When we looked at how to group progress notifications by `is_aggregate` we 
found that although the symbol table/DWARF progress notifications would be 
grouped under `is_aggregate == true`, long-running operations related to Swift 
(importing/loading Swift modules etc.) could actually be grouped under 
`is_aggregate == false` which allows the IDE team to display and group those 
notifications. (https://github.com/apple/llvm-project/pull/7769). We want to 
integrate this enum flag into the Swift progress reports once landed. 

That being said, this does group long-running LLDB notifications as all being 
too spammy to show since we couldn't group them together under umbrella 
progress reports. We don't want to keep it this way forever as ultimately we'd 
like to group all categories of progress reports under their own umbrella 
`Progress` objects instead of sending several individual progress reports, but 
this is what we're doing right now to help the IDE have more streamlined UI 
progress reports.

We also thought about going with the timeout approach that you suggested but 
ran into similar issues as you mentioned wherein if many reports were too short 
then they wouldn't get displayed.

https://github.com/llvm/llvm-project/pull/69516
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][progress] Add discrete boolean flag to progress reports (PR #69516)



@@ -2225,7 +2225,8 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
   const char *file_name = file.GetFilename().AsCString("");
   LLDB_SCOPED_TIMERF("ObjectFileMachO::ParseSymtab () module = %s", file_name);
   LLDB_LOG(log, "Parsing symbol table for {0}", file_name);
-  Progress progress(llvm::formatv("Parsing symbol table for {0}", file_name));
+  Progress progress(llvm::formatv("Parsing symbol table for {0}", file_name),
+Progress::ProgressReportType::eAggregateProgressReport);

clayborg wrote:

So we do want all of the reports to be sent, this seems like a GUI / display 
issue that can be solved with category names. For you guys the DWARF isn't a 
big contributor and we want to always see those since on linux a lot of DWARF 
is not indexed and that is a major contributor to our startup times. Symbol 
table parsing takes time on mac and linux as well. So everyone is going to 
classify what progress is interesting in their own way. With a single enum for 
what Apple users might consider "aggregate" doesn't match up with what Linux or 
other users consider aggregate and we are now going to be hard coded this into 
the progress objectsd and that is what is worrying me about the current 
approach. 

The user interface is what needs to figure out how to display these things 
efficiently and the category string seems to allow user interfaces to do more 
grouping in more efficient ways. The boolean of is_aggregate is a bit too 
coarse. With the category strings like "Parsing symbol tables" or "Manually 
Indexing DWARF" or "Loading DWARF accelerator tables", it would allow spammy 
stuff to be noticed on the, and then instead of displaying individual entries, 
it would display a single entry with the category name and a spinning progress 
GUI. Of course you could hard code things based on the category name all the 
time in Xcode so that "Parsing symbol tables" and "Loading DWARF accelerator 
tables" would always be grouped if desired to keep the noise down.

If no one else agrees with me here, please let me know

https://github.com/llvm/llvm-project/pull/69516
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [flang] [llvm] [lld] [libcxxabi] [libc] [lldb] [openmp] [compiler-rt] [libcxx] [clang-tools-extra] [mlir] [llvm] Support IFuncs on Darwin platforms (PR #73686)


https://github.com/jroelofs updated 
https://github.com/llvm/llvm-project/pull/73686

>From bc152095691b32d1ad8539dfd60f5089df5eed8d Mon Sep 17 00:00:00 2001
From: Jon Roelofs 
Date: Tue, 28 Nov 2023 10:39:44 -0800
Subject: [PATCH 01/10] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20?=
 =?UTF-8?q?initial=20version?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Created using spr 1.3.4
---
 llvm/docs/LangRef.rst |   7 +-
 llvm/include/llvm/CodeGen/AsmPrinter.h|   6 +-
 llvm/lib/CodeGen/GlobalISel/CallLowering.cpp  |   7 +-
 llvm/lib/IR/Verifier.cpp  |  12 +-
 llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp | 308 ++
 llvm/lib/Target/X86/X86AsmPrinter.cpp |  28 ++
 llvm/lib/Target/X86/X86AsmPrinter.h   |   1 +
 .../AArch64/GlobalISel/call-lowering-ifunc.ll |  37 +++
 llvm/test/CodeGen/AArch64/addrsig-macho.ll|   4 +-
 llvm/test/CodeGen/AArch64/ifunc-asm.ll|  82 +
 llvm/test/CodeGen/X86/ifunc-asm.ll|  28 +-
 llvm/test/Verifier/ifunc-macho.ll |  42 +++
 12 files changed, 539 insertions(+), 23 deletions(-)
 create mode 100644 llvm/test/CodeGen/AArch64/GlobalISel/call-lowering-ifunc.ll
 create mode 100644 llvm/test/CodeGen/AArch64/ifunc-asm.ll
 create mode 100644 llvm/test/Verifier/ifunc-macho.ll

diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index e448c5ed5c5d9..cb222e979db29 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -934,10 +934,11 @@ IFuncs
 ---
 
 IFuncs, like as aliases, don't create any new data or func. They are just a new
-symbol that dynamic linker resolves at runtime by calling a resolver function.
+symbol that is resolved at runtime by calling a resolver function.
 
-IFuncs have a name and a resolver that is a function called by dynamic linker
-that returns address of another function associated with the name.
+On ELF platforms, IFuncs are resolved by the dynamic linker at load time. On
+MachO platforms, they are lowered in terms of ``.symbol_resolver``s, which
+lazily resolve the callee the first time they are called.
 
 IFunc may have an optional :ref:`linkage type ` and an optional
 :ref:`visibility style `.
diff --git a/llvm/include/llvm/CodeGen/AsmPrinter.h 
b/llvm/include/llvm/CodeGen/AsmPrinter.h
index 2731ef452c79c..48fa6c478464c 100644
--- a/llvm/include/llvm/CodeGen/AsmPrinter.h
+++ b/llvm/include/llvm/CodeGen/AsmPrinter.h
@@ -882,7 +882,11 @@ class AsmPrinter : public MachineFunctionPass {
 
   GCMetadataPrinter *getOrCreateGCPrinter(GCStrategy &S);
   void emitGlobalAlias(Module &M, const GlobalAlias &GA);
-  void emitGlobalIFunc(Module &M, const GlobalIFunc &GI);
+
+protected:
+  virtual void emitGlobalIFunc(Module &M, const GlobalIFunc &GI);
+
+private:
 
   /// This method decides whether the specified basic block requires a label.
   bool shouldEmitLabelForBasicBlock(const MachineBasicBlock &MBB) const;
diff --git a/llvm/lib/CodeGen/GlobalISel/CallLowering.cpp 
b/llvm/lib/CodeGen/GlobalISel/CallLowering.cpp
index 2527b14312896..e0080b145d4f9 100644
--- a/llvm/lib/CodeGen/GlobalISel/CallLowering.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/CallLowering.cpp
@@ -144,7 +144,12 @@ bool CallLowering::lowerCall(MachineIRBuilder &MIRBuilder, 
const CallBase &CB,
   // Try looking through a bitcast from one function type to another.
   // Commonly happens with calls to objc_msgSend().
   const Value *CalleeV = CB.getCalledOperand()->stripPointerCasts();
-  if (const Function *F = dyn_cast(CalleeV))
+  if (const GlobalIFunc *IF = dyn_cast(CalleeV);
+  IF && MF.getTarget().getTargetTriple().isOSBinFormatMachO()) {
+// ld64 requires that .symbol_resolvers to be called via a stub, so these
+// must always be a diret call.
+Info.Callee = MachineOperand::CreateGA(IF, 0);
+  } else if (const Function *F = dyn_cast(CalleeV))
 Info.Callee = MachineOperand::CreateGA(F, 0);
   else
 Info.Callee = MachineOperand::CreateReg(GetCalleeReg(), false);
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 5560c037aa3ee..94e76a43bf38d 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -959,6 +959,7 @@ void Verifier::visitGlobalIFunc(const GlobalIFunc &GI) {
   GlobalIFunc::getResolverFunctionType(GI.getValueType());
   Check(ResolverTy == ResolverFuncTy->getPointerTo(GI.getAddressSpace()),
 "IFunc resolver has incorrect type", &GI);
+
 }
 
 void Verifier::visitNamedMDNode(const NamedMDNode &NMD) {
@@ -2239,13 +2240,10 @@ void Verifier::verifyFunctionAttrs(FunctionType *FT, 
AttributeList Attrs,
   }
 
   // Check EVEX512 feature.
-  if (MaxParameterWidth >= 512 && Attrs.hasFnAttr("target-features")) {
-Triple T(M.getTargetTriple());
-if (T.isX86()) {
-  StringRef TF = Attrs.getFnAttr("target-features").getValueAsString();
-  Check(!TF.contains("+avx512f") || !TF.contains("-evex512"),
-"512-bit vector arguments

[Lldb-commits] [clang-tools-extra] [compiler-rt] [clang] [flang] [libc] [lld] [openmp] [mlir] [lldb] [libcxxabi] [llvm] [libcxx] [llvm] Support IFuncs on Darwin platforms (PR #73686)


https://github.com/jroelofs updated 
https://github.com/llvm/llvm-project/pull/73686

>From bc152095691b32d1ad8539dfd60f5089df5eed8d Mon Sep 17 00:00:00 2001
From: Jon Roelofs 
Date: Tue, 28 Nov 2023 10:39:44 -0800
Subject: [PATCH 01/10] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20?=
 =?UTF-8?q?initial=20version?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Created using spr 1.3.4
---
 llvm/docs/LangRef.rst |   7 +-
 llvm/include/llvm/CodeGen/AsmPrinter.h|   6 +-
 llvm/lib/CodeGen/GlobalISel/CallLowering.cpp  |   7 +-
 llvm/lib/IR/Verifier.cpp  |  12 +-
 llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp | 308 ++
 llvm/lib/Target/X86/X86AsmPrinter.cpp |  28 ++
 llvm/lib/Target/X86/X86AsmPrinter.h   |   1 +
 .../AArch64/GlobalISel/call-lowering-ifunc.ll |  37 +++
 llvm/test/CodeGen/AArch64/addrsig-macho.ll|   4 +-
 llvm/test/CodeGen/AArch64/ifunc-asm.ll|  82 +
 llvm/test/CodeGen/X86/ifunc-asm.ll|  28 +-
 llvm/test/Verifier/ifunc-macho.ll |  42 +++
 12 files changed, 539 insertions(+), 23 deletions(-)
 create mode 100644 llvm/test/CodeGen/AArch64/GlobalISel/call-lowering-ifunc.ll
 create mode 100644 llvm/test/CodeGen/AArch64/ifunc-asm.ll
 create mode 100644 llvm/test/Verifier/ifunc-macho.ll

diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index e448c5ed5c5d9..cb222e979db29 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -934,10 +934,11 @@ IFuncs
 ---
 
 IFuncs, like as aliases, don't create any new data or func. They are just a new
-symbol that dynamic linker resolves at runtime by calling a resolver function.
+symbol that is resolved at runtime by calling a resolver function.
 
-IFuncs have a name and a resolver that is a function called by dynamic linker
-that returns address of another function associated with the name.
+On ELF platforms, IFuncs are resolved by the dynamic linker at load time. On
+MachO platforms, they are lowered in terms of ``.symbol_resolver``s, which
+lazily resolve the callee the first time they are called.
 
 IFunc may have an optional :ref:`linkage type ` and an optional
 :ref:`visibility style `.
diff --git a/llvm/include/llvm/CodeGen/AsmPrinter.h 
b/llvm/include/llvm/CodeGen/AsmPrinter.h
index 2731ef452c79c..48fa6c478464c 100644
--- a/llvm/include/llvm/CodeGen/AsmPrinter.h
+++ b/llvm/include/llvm/CodeGen/AsmPrinter.h
@@ -882,7 +882,11 @@ class AsmPrinter : public MachineFunctionPass {
 
   GCMetadataPrinter *getOrCreateGCPrinter(GCStrategy &S);
   void emitGlobalAlias(Module &M, const GlobalAlias &GA);
-  void emitGlobalIFunc(Module &M, const GlobalIFunc &GI);
+
+protected:
+  virtual void emitGlobalIFunc(Module &M, const GlobalIFunc &GI);
+
+private:
 
   /// This method decides whether the specified basic block requires a label.
   bool shouldEmitLabelForBasicBlock(const MachineBasicBlock &MBB) const;
diff --git a/llvm/lib/CodeGen/GlobalISel/CallLowering.cpp 
b/llvm/lib/CodeGen/GlobalISel/CallLowering.cpp
index 2527b14312896..e0080b145d4f9 100644
--- a/llvm/lib/CodeGen/GlobalISel/CallLowering.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/CallLowering.cpp
@@ -144,7 +144,12 @@ bool CallLowering::lowerCall(MachineIRBuilder &MIRBuilder, 
const CallBase &CB,
   // Try looking through a bitcast from one function type to another.
   // Commonly happens with calls to objc_msgSend().
   const Value *CalleeV = CB.getCalledOperand()->stripPointerCasts();
-  if (const Function *F = dyn_cast(CalleeV))
+  if (const GlobalIFunc *IF = dyn_cast(CalleeV);
+  IF && MF.getTarget().getTargetTriple().isOSBinFormatMachO()) {
+// ld64 requires that .symbol_resolvers to be called via a stub, so these
+// must always be a diret call.
+Info.Callee = MachineOperand::CreateGA(IF, 0);
+  } else if (const Function *F = dyn_cast(CalleeV))
 Info.Callee = MachineOperand::CreateGA(F, 0);
   else
 Info.Callee = MachineOperand::CreateReg(GetCalleeReg(), false);
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 5560c037aa3ee..94e76a43bf38d 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -959,6 +959,7 @@ void Verifier::visitGlobalIFunc(const GlobalIFunc &GI) {
   GlobalIFunc::getResolverFunctionType(GI.getValueType());
   Check(ResolverTy == ResolverFuncTy->getPointerTo(GI.getAddressSpace()),
 "IFunc resolver has incorrect type", &GI);
+
 }
 
 void Verifier::visitNamedMDNode(const NamedMDNode &NMD) {
@@ -2239,13 +2240,10 @@ void Verifier::verifyFunctionAttrs(FunctionType *FT, 
AttributeList Attrs,
   }
 
   // Check EVEX512 feature.
-  if (MaxParameterWidth >= 512 && Attrs.hasFnAttr("target-features")) {
-Triple T(M.getTargetTriple());
-if (T.isX86()) {
-  StringRef TF = Attrs.getFnAttr("target-features").getValueAsString();
-  Check(!TF.contains("+avx512f") || !TF.contains("-evex512"),
-"512-bit vector arguments

[Lldb-commits] [lldb] Make only one function that needs to be implemented when searching for types (PR #74786)

2023-12-08 Thread Adrian Prantl via lldb-commits


adrian-prantl wrote:

Sorry, it's been a while since I last reviewed this and I must admit I forgot 
about this in the mean time. The only thing I'd like to check before we can 
land this is to hear from @Michael137 who has been actively working on 
FindTypes performance recently whether this aligns or clashes with what he's 
been working on so we don't get a mid-air collision between patches.

https://github.com/llvm/llvm-project/pull/74786
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lld] [llvm] [mlir] [lldb] [flang] [clang] [openmp] [clang-tools-extra] [libcxx] [libcxxabi] [compiler-rt] [libc] [llvm] Support IFuncs on Darwin platforms (PR #73686)


https://github.com/jroelofs updated 
https://github.com/llvm/llvm-project/pull/73686

>From bc152095691b32d1ad8539dfd60f5089df5eed8d Mon Sep 17 00:00:00 2001
From: Jon Roelofs 
Date: Tue, 28 Nov 2023 10:39:44 -0800
Subject: [PATCH 01/11] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20?=
 =?UTF-8?q?initial=20version?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Created using spr 1.3.4
---
 llvm/docs/LangRef.rst |   7 +-
 llvm/include/llvm/CodeGen/AsmPrinter.h|   6 +-
 llvm/lib/CodeGen/GlobalISel/CallLowering.cpp  |   7 +-
 llvm/lib/IR/Verifier.cpp  |  12 +-
 llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp | 308 ++
 llvm/lib/Target/X86/X86AsmPrinter.cpp |  28 ++
 llvm/lib/Target/X86/X86AsmPrinter.h   |   1 +
 .../AArch64/GlobalISel/call-lowering-ifunc.ll |  37 +++
 llvm/test/CodeGen/AArch64/addrsig-macho.ll|   4 +-
 llvm/test/CodeGen/AArch64/ifunc-asm.ll|  82 +
 llvm/test/CodeGen/X86/ifunc-asm.ll|  28 +-
 llvm/test/Verifier/ifunc-macho.ll |  42 +++
 12 files changed, 539 insertions(+), 23 deletions(-)
 create mode 100644 llvm/test/CodeGen/AArch64/GlobalISel/call-lowering-ifunc.ll
 create mode 100644 llvm/test/CodeGen/AArch64/ifunc-asm.ll
 create mode 100644 llvm/test/Verifier/ifunc-macho.ll

diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index e448c5ed5c5d94..cb222e979db29d 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -934,10 +934,11 @@ IFuncs
 ---
 
 IFuncs, like as aliases, don't create any new data or func. They are just a new
-symbol that dynamic linker resolves at runtime by calling a resolver function.
+symbol that is resolved at runtime by calling a resolver function.
 
-IFuncs have a name and a resolver that is a function called by dynamic linker
-that returns address of another function associated with the name.
+On ELF platforms, IFuncs are resolved by the dynamic linker at load time. On
+MachO platforms, they are lowered in terms of ``.symbol_resolver``s, which
+lazily resolve the callee the first time they are called.
 
 IFunc may have an optional :ref:`linkage type ` and an optional
 :ref:`visibility style `.
diff --git a/llvm/include/llvm/CodeGen/AsmPrinter.h 
b/llvm/include/llvm/CodeGen/AsmPrinter.h
index 2731ef452c79cb..48fa6c478464c7 100644
--- a/llvm/include/llvm/CodeGen/AsmPrinter.h
+++ b/llvm/include/llvm/CodeGen/AsmPrinter.h
@@ -882,7 +882,11 @@ class AsmPrinter : public MachineFunctionPass {
 
   GCMetadataPrinter *getOrCreateGCPrinter(GCStrategy &S);
   void emitGlobalAlias(Module &M, const GlobalAlias &GA);
-  void emitGlobalIFunc(Module &M, const GlobalIFunc &GI);
+
+protected:
+  virtual void emitGlobalIFunc(Module &M, const GlobalIFunc &GI);
+
+private:
 
   /// This method decides whether the specified basic block requires a label.
   bool shouldEmitLabelForBasicBlock(const MachineBasicBlock &MBB) const;
diff --git a/llvm/lib/CodeGen/GlobalISel/CallLowering.cpp 
b/llvm/lib/CodeGen/GlobalISel/CallLowering.cpp
index 2527b143128967..e0080b145d4f99 100644
--- a/llvm/lib/CodeGen/GlobalISel/CallLowering.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/CallLowering.cpp
@@ -144,7 +144,12 @@ bool CallLowering::lowerCall(MachineIRBuilder &MIRBuilder, 
const CallBase &CB,
   // Try looking through a bitcast from one function type to another.
   // Commonly happens with calls to objc_msgSend().
   const Value *CalleeV = CB.getCalledOperand()->stripPointerCasts();
-  if (const Function *F = dyn_cast(CalleeV))
+  if (const GlobalIFunc *IF = dyn_cast(CalleeV);
+  IF && MF.getTarget().getTargetTriple().isOSBinFormatMachO()) {
+// ld64 requires that .symbol_resolvers to be called via a stub, so these
+// must always be a diret call.
+Info.Callee = MachineOperand::CreateGA(IF, 0);
+  } else if (const Function *F = dyn_cast(CalleeV))
 Info.Callee = MachineOperand::CreateGA(F, 0);
   else
 Info.Callee = MachineOperand::CreateReg(GetCalleeReg(), false);
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 5560c037aa3ee6..94e76a43bf38d6 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -959,6 +959,7 @@ void Verifier::visitGlobalIFunc(const GlobalIFunc &GI) {
   GlobalIFunc::getResolverFunctionType(GI.getValueType());
   Check(ResolverTy == ResolverFuncTy->getPointerTo(GI.getAddressSpace()),
 "IFunc resolver has incorrect type", &GI);
+
 }
 
 void Verifier::visitNamedMDNode(const NamedMDNode &NMD) {
@@ -2239,13 +2240,10 @@ void Verifier::verifyFunctionAttrs(FunctionType *FT, 
AttributeList Attrs,
   }
 
   // Check EVEX512 feature.
-  if (MaxParameterWidth >= 512 && Attrs.hasFnAttr("target-features")) {
-Triple T(M.getTargetTriple());
-if (T.isX86()) {
-  StringRef TF = Attrs.getFnAttr("target-features").getValueAsString();
-  Check(!TF.contains("+avx512f") || !TF.contains("-evex512"),
-"512-bit vector ar

[Lldb-commits] [lldb] Add a test for evicting unreachable modules from the global module cache (PR #74894)


https://github.com/jimingham created 
https://github.com/llvm/llvm-project/pull/74894

When you debug a binary and the change & rebuild and then rerun in lldb w/o 
quitting lldb, the Modules in the Global Module Cache for the old binary & .o 
files if used are now "unreachable".  Nothing in lldb is holding them alive, 
and they've already been unlinked.  lldb will properly discard them if there's 
not another Target referencing them.

However, this only works in simple cases at present.  If you have several 
Targets that reference the same modules, it's pretty easy to end up stranding 
Modules that are no longer reachable, and if you use a sequence of SBDebuggers 
unreachable modules can also get stranded.  If you run a long-lived lldb 
process and are iteratively developing on a large code base, lldb's memory gets 
filled with useless Modules.

This patch adds a test for the mode that currently works:

(lldb) target create foo
(lldb) run

(lldb) run

In that case, we do delete the unreachable Modules.

The next step will be to add tests for the cases where we fail to do this, then 
see how to safely/efficiently evict unreachable modules in those cases as well.

>From 438d35a7a7fca454718062583f91776ca018b2b1 Mon Sep 17 00:00:00 2001
From: Jim Ingham 
Date: Fri, 8 Dec 2023 14:43:14 -0800
Subject: [PATCH] Add a test for evicting unreachable modules from the global
 module cache.

---
 .../python_api/global_module_cache/Makefile   |   1 +
 .../TestGlobalModuleCache.py  | 110 ++
 .../global_module_cache/one-print.c   |   8 ++
 .../global_module_cache/two-print.c   |   9 ++
 4 files changed, 128 insertions(+)
 create mode 100644 lldb/test/API/python_api/global_module_cache/Makefile
 create mode 100644 
lldb/test/API/python_api/global_module_cache/TestGlobalModuleCache.py
 create mode 100644 lldb/test/API/python_api/global_module_cache/one-print.c
 create mode 100644 lldb/test/API/python_api/global_module_cache/two-print.c

diff --git a/lldb/test/API/python_api/global_module_cache/Makefile 
b/lldb/test/API/python_api/global_module_cache/Makefile
new file mode 100644
index 00..22f1051530f871
--- /dev/null
+++ b/lldb/test/API/python_api/global_module_cache/Makefile
@@ -0,0 +1 @@
+include Makefile.rules
diff --git 
a/lldb/test/API/python_api/global_module_cache/TestGlobalModuleCache.py 
b/lldb/test/API/python_api/global_module_cache/TestGlobalModuleCache.py
new file mode 100644
index 00..ff74d09a128183
--- /dev/null
+++ b/lldb/test/API/python_api/global_module_cache/TestGlobalModuleCache.py
@@ -0,0 +1,110 @@
+"""
+Test the use of the global module cache in lldb
+"""
+
+import lldb
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+import os
+import shutil
+from pathlib import Path
+import time
+
+
+class GlobalModuleCacheTestCase(TestBase):
+# NO_DEBUG_INFO_TESTCASE = True
+
+def check_counter_var(self, thread, value):
+frame = thread.frames[0]
+var = frame.FindVariable("counter")
+self.assertTrue(var.GetError().Success(), "Got counter variable")
+self.assertEqual(var.GetValueAsUnsigned(), value, "This was one-print")
+
+def copy_to_main(self, src, dst):
+# We are relying on the source file being newer than the .o file from
+# a previous build, so sleep a bit here to ensure that the touch is 
later.
+time.sleep(2)
+try:
+shutil.copy(src, dst)
+except:
+self.fail(f"Could not copy {src} to {dst}")
+Path(dst).touch()
+
+# The rerun tests indicate rerunning on Windows doesn't really work, so
+# this one won't either.
+@skipIfWindows
+def test_OneTargetOneDebugger(self):
+# Make sure that if we have one target, and we run, then
+# change the binary and rerun, the binary (and any .o files
+# if using dwarf in .o file debugging) get removed from the
+# shared module cache.  They are no longer reachable.
+debug_style = self.getDebugInfo()
+
+# Before we do anything, clear the global module cache so we don't
+# see objects from other runs:
+lldb.SBDebugger.MemoryPressureDetected()
+
+# Set up the paths for our two versions of main.c:
+main_c_path = os.path.join(self.getBuildDir(), "main.c")
+one_print_path = os.path.join(self.getSourceDir(), "one-print.c")
+two_print_path = os.path.join(self.getSourceDir(), "two-print.c")
+main_filespec = lldb.SBFileSpec(main_c_path)
+
+# First copy the one-print.c to main.c in the build folder and
+# build our a.out from there:
+self.copy_to_main(one_print_path, main_c_path)
+self.build(dictionary={"C_SOURCES": main_c_path, "EXE": "a.out"})
+
+(target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+self, "return counter;", main_filespec
+)
+
+

[Lldb-commits] [lldb] Add a test for evicting unreachable modules from the global module cache (PR #74894)


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: None (jimingham)


Changes

When you debug a binary and the change & rebuild and then rerun in lldb w/o 
quitting lldb, the Modules in the Global Module Cache for the old binary & 
.o files if used are now "unreachable".  Nothing in lldb is holding them alive, 
and they've already been unlinked.  lldb will properly discard them if there's 
not another Target referencing them.

However, this only works in simple cases at present.  If you have several 
Targets that reference the same modules, it's pretty easy to end up stranding 
Modules that are no longer reachable, and if you use a sequence of SBDebuggers 
unreachable modules can also get stranded.  If you run a long-lived lldb 
process and are iteratively developing on a large code base, lldb's memory gets 
filled with useless Modules.

This patch adds a test for the mode that currently works:

(lldb) target create foo
(lldb) run

(lldb) run

In that case, we do delete the unreachable Modules.

The next step will be to add tests for the cases where we fail to do this, then 
see how to safely/efficiently evict unreachable modules in those cases as well.

---
Full diff: https://github.com/llvm/llvm-project/pull/74894.diff


4 Files Affected:

- (added) lldb/test/API/python_api/global_module_cache/Makefile (+1) 
- (added) lldb/test/API/python_api/global_module_cache/TestGlobalModuleCache.py 
(+110) 
- (added) lldb/test/API/python_api/global_module_cache/one-print.c (+8) 
- (added) lldb/test/API/python_api/global_module_cache/two-print.c (+9) 


``diff
diff --git a/lldb/test/API/python_api/global_module_cache/Makefile 
b/lldb/test/API/python_api/global_module_cache/Makefile
new file mode 100644
index 0..22f1051530f87
--- /dev/null
+++ b/lldb/test/API/python_api/global_module_cache/Makefile
@@ -0,0 +1 @@
+include Makefile.rules
diff --git 
a/lldb/test/API/python_api/global_module_cache/TestGlobalModuleCache.py 
b/lldb/test/API/python_api/global_module_cache/TestGlobalModuleCache.py
new file mode 100644
index 0..ff74d09a12818
--- /dev/null
+++ b/lldb/test/API/python_api/global_module_cache/TestGlobalModuleCache.py
@@ -0,0 +1,110 @@
+"""
+Test the use of the global module cache in lldb
+"""
+
+import lldb
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+import os
+import shutil
+from pathlib import Path
+import time
+
+
+class GlobalModuleCacheTestCase(TestBase):
+# NO_DEBUG_INFO_TESTCASE = True
+
+def check_counter_var(self, thread, value):
+frame = thread.frames[0]
+var = frame.FindVariable("counter")
+self.assertTrue(var.GetError().Success(), "Got counter variable")
+self.assertEqual(var.GetValueAsUnsigned(), value, "This was one-print")
+
+def copy_to_main(self, src, dst):
+# We are relying on the source file being newer than the .o file from
+# a previous build, so sleep a bit here to ensure that the touch is 
later.
+time.sleep(2)
+try:
+shutil.copy(src, dst)
+except:
+self.fail(f"Could not copy {src} to {dst}")
+Path(dst).touch()
+
+# The rerun tests indicate rerunning on Windows doesn't really work, so
+# this one won't either.
+@skipIfWindows
+def test_OneTargetOneDebugger(self):
+# Make sure that if we have one target, and we run, then
+# change the binary and rerun, the binary (and any .o files
+# if using dwarf in .o file debugging) get removed from the
+# shared module cache.  They are no longer reachable.
+debug_style = self.getDebugInfo()
+
+# Before we do anything, clear the global module cache so we don't
+# see objects from other runs:
+lldb.SBDebugger.MemoryPressureDetected()
+
+# Set up the paths for our two versions of main.c:
+main_c_path = os.path.join(self.getBuildDir(), "main.c")
+one_print_path = os.path.join(self.getSourceDir(), "one-print.c")
+two_print_path = os.path.join(self.getSourceDir(), "two-print.c")
+main_filespec = lldb.SBFileSpec(main_c_path)
+
+# First copy the one-print.c to main.c in the build folder and
+# build our a.out from there:
+self.copy_to_main(one_print_path, main_c_path)
+self.build(dictionary={"C_SOURCES": main_c_path, "EXE": "a.out"})
+
+(target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+self, "return counter;", main_filespec
+)
+
+# Make sure we ran the version we intended here:
+self.check_counter_var(thread, 1)
+process.Kill()
+
+# Now copy two-print.c over main.c, rebuild, and rerun:
+# os.unlink(target.GetExecutable().fullpath)
+self.copy_to_main(two_print_path, main_c_path)
+
+self.build(dictionary={"C_SOURCES": main_c_path, "EXE": "a.out"})
+error =

[Lldb-commits] [lldb] Add a test for evicting unreachable modules from the global module cache (PR #74894)


github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff 44dc1e0baae7c4b8a02ba06dcf396d3d452aa873 
438d35a7a7fca454718062583f91776ca018b2b1 -- 
lldb/test/API/python_api/global_module_cache/one-print.c 
lldb/test/API/python_api/global_module_cache/two-print.c
``





View the diff from clang-format here.


``diff
diff --git a/lldb/test/API/python_api/global_module_cache/one-print.c 
b/lldb/test/API/python_api/global_module_cache/one-print.c
index 5a572ca7c6..f008f36c25 100644
--- a/lldb/test/API/python_api/global_module_cache/one-print.c
+++ b/lldb/test/API/python_api/global_module_cache/one-print.c
@@ -1,7 +1,6 @@
 #include 
 
-int
-main() {
+int main() {
   int counter = 0;
   printf("I only print one time: %d.\n", counter++);
   return counter;
diff --git a/lldb/test/API/python_api/global_module_cache/two-print.c 
b/lldb/test/API/python_api/global_module_cache/two-print.c
index ce6cb84c5c..96f68cbed8 100644
--- a/lldb/test/API/python_api/global_module_cache/two-print.c
+++ b/lldb/test/API/python_api/global_module_cache/two-print.c
@@ -1,7 +1,6 @@
 #include 
 
-int
-main() {
+int main() {
   int counter = 0;
   printf("I print one time: %d.\n", counter++);
   printf("I print two times: %d.\n", counter++);

``




https://github.com/llvm/llvm-project/pull/74894
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [compiler-rt] [mlir] [clang] [lldb] [llvm] [flang] [openmp] [polly] [lld] [VPlan] Compute scalable VF in preheader for induction increment. (PR #74762)


https://github.com/ayalz edited https://github.com/llvm/llvm-project/pull/74762
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lld] [flang] [lldb] [compiler-rt] [mlir] [polly] [clang] [llvm] [openmp] [VPlan] Compute scalable VF in preheader for induction increment. (PR #74762)



@@ -340,8 +340,14 @@ Value *VPInstruction::generateInstruction(VPTransformState 
&State,
   auto *Phi = State.get(getOperand(0), 0);
   // The loop step is equal to the vectorization factor (num of SIMD
   // elements) times the unroll factor (num of SIMD instructions).
-  Value *Step =
-  createStepForVF(Builder, Phi->getType(), State.VF, State.UF);
+  Value *Step;
+  {
+BasicBlock *VectorPH = State.CFG.getPreheaderBBFor(this);
+IRBuilder<> PHBuilder(VectorPH->getTerminator());
+// Step is loop-invariant, calls to vscale will be placed in the
+// preheader.
+Step = createStepForVF(PHBuilder, Phi->getType(), State.VF, State.UF);
+  }

ayalz wrote:

```suggestion
  // Step is loop-invariant, calls to vscale will be placed in the 
preheader.
  BasicBlock *VectorPH = State.CFG.getPreheaderBBFor(this);
  IRBuilder<> PHBuilder(VectorPH->getTerminator());
  Value *Step = createStepForVF(PHBuilder, Phi->getType(), State.VF, 
State.UF);
```

(no need for the bracketed block, which was needed for the point guard.)

https://github.com/llvm/llvm-project/pull/74762
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [polly] [mlir] [llvm] [openmp] [clang] [flang] [lld] [compiler-rt] [VPlan] Compute scalable VF in preheader for induction increment. (PR #74762)


https://github.com/ayalz commented:

post-commit nit

https://github.com/llvm/llvm-project/pull/74762
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Add a test for evicting unreachable modules from the global module cache (PR #74894)


jimingham wrote:

Note, I thought about adding an SBDebugger::GetSharedModules or something, but 
I don't actually think that's a good thing to give external access to.  Some 
day we should make an SBTestAPI with some useful for testing but not for the SB 
API library so we can make this sort of testing easier, but for this test 
grubbing the command output is not all that bad.

https://github.com/llvm/llvm-project/pull/74894
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][NFC] Simplify DWARRFDeclContext::GetQualifiedName (PR #74788)


https://github.com/bulbazord approved this pull request.

TIL about this overload of `llvm::interleave`!!!

https://github.com/llvm/llvm-project/pull/74788
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb-dap] Implement command directives (PR #74808)



@@ -33,14 +39,49 @@ void RunLLDBCommands(llvm::StringRef prefix,
   const char *error = result.GetError();
   strm << error;
 }
+  };
+
+  lldb::SBCommandInterpreter interp = g_dap.debugger.GetCommandInterpreter();
+  for (llvm::StringRef command : commands) {
+lldb::SBCommandReturnObject result;
+bool quiet_on_success = false;
+bool abort_on_error = false;
+
+while (parse_command_directives) {
+  if (command.starts_with("?")) {
+command = command.drop_front();
+quiet_on_success = true;
+  } else if (command.starts_with("!")) {
+command = command.drop_front();
+abort_on_error = true;
+  } else {
+break;
+  }
+}
+
+interp.HandleCommand(command.str().c_str(), result);

bulbazord wrote:

If `parse_command_directives` is false and the commands start with `?` and `!` 
they'll fail right? Would you want the commands to run (ignoring the `?` and 
`!`) even though `parse_command_directives` is off?

https://github.com/llvm/llvm-project/pull/74808
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb-dap] Include [opt] in the frame name only if a custom frame format is not specified. (PR #74861)


https://github.com/bulbazord commented:

I think conceptually this makes sense, but I somewhat wonder if folks would get 
confused when they have a custom frame format and don't see the [opt] in there? 
The current behavior is that [opt] is always there so folks know they don't 
have to put it in their custom frame format. When it's missing after this 
change, I wonder if they'll notice.

https://github.com/llvm/llvm-project/pull/74861
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb-dap] Emit declarations along with variables (PR #74865)



@@ -1101,6 +1101,29 @@ std::string 
CreateUniqueVariableNameForDisplay(lldb::SBValue v,
 //   can use this optional information to present the
 //   children in a paged UI and fetch them in chunks."
 // }
+// "declaration": {
+//   "type": "object | undefined",

bulbazord wrote:

What is "type"? I also don't see you setting it in the diff below this one.

https://github.com/llvm/llvm-project/pull/74865
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [llvm] [clang-tools-extra] [libcxx] [flang] [lld] [openmp] [lldb] [libcxxabi] [compiler-rt] [mlir] [clang] [libc] [llvm] Support IFuncs on Darwin platforms (PR #73686)


https://github.com/jroelofs updated 
https://github.com/llvm/llvm-project/pull/73686

>From bc152095691b32d1ad8539dfd60f5089df5eed8d Mon Sep 17 00:00:00 2001
From: Jon Roelofs 
Date: Tue, 28 Nov 2023 10:39:44 -0800
Subject: [PATCH 01/11] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20?=
 =?UTF-8?q?initial=20version?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Created using spr 1.3.4
---
 llvm/docs/LangRef.rst |   7 +-
 llvm/include/llvm/CodeGen/AsmPrinter.h|   6 +-
 llvm/lib/CodeGen/GlobalISel/CallLowering.cpp  |   7 +-
 llvm/lib/IR/Verifier.cpp  |  12 +-
 llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp | 308 ++
 llvm/lib/Target/X86/X86AsmPrinter.cpp |  28 ++
 llvm/lib/Target/X86/X86AsmPrinter.h   |   1 +
 .../AArch64/GlobalISel/call-lowering-ifunc.ll |  37 +++
 llvm/test/CodeGen/AArch64/addrsig-macho.ll|   4 +-
 llvm/test/CodeGen/AArch64/ifunc-asm.ll|  82 +
 llvm/test/CodeGen/X86/ifunc-asm.ll|  28 +-
 llvm/test/Verifier/ifunc-macho.ll |  42 +++
 12 files changed, 539 insertions(+), 23 deletions(-)
 create mode 100644 llvm/test/CodeGen/AArch64/GlobalISel/call-lowering-ifunc.ll
 create mode 100644 llvm/test/CodeGen/AArch64/ifunc-asm.ll
 create mode 100644 llvm/test/Verifier/ifunc-macho.ll

diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index e448c5ed5c5d94..cb222e979db29d 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -934,10 +934,11 @@ IFuncs
 ---
 
 IFuncs, like as aliases, don't create any new data or func. They are just a new
-symbol that dynamic linker resolves at runtime by calling a resolver function.
+symbol that is resolved at runtime by calling a resolver function.
 
-IFuncs have a name and a resolver that is a function called by dynamic linker
-that returns address of another function associated with the name.
+On ELF platforms, IFuncs are resolved by the dynamic linker at load time. On
+MachO platforms, they are lowered in terms of ``.symbol_resolver``s, which
+lazily resolve the callee the first time they are called.
 
 IFunc may have an optional :ref:`linkage type ` and an optional
 :ref:`visibility style `.
diff --git a/llvm/include/llvm/CodeGen/AsmPrinter.h 
b/llvm/include/llvm/CodeGen/AsmPrinter.h
index 2731ef452c79cb..48fa6c478464c7 100644
--- a/llvm/include/llvm/CodeGen/AsmPrinter.h
+++ b/llvm/include/llvm/CodeGen/AsmPrinter.h
@@ -882,7 +882,11 @@ class AsmPrinter : public MachineFunctionPass {
 
   GCMetadataPrinter *getOrCreateGCPrinter(GCStrategy &S);
   void emitGlobalAlias(Module &M, const GlobalAlias &GA);
-  void emitGlobalIFunc(Module &M, const GlobalIFunc &GI);
+
+protected:
+  virtual void emitGlobalIFunc(Module &M, const GlobalIFunc &GI);
+
+private:
 
   /// This method decides whether the specified basic block requires a label.
   bool shouldEmitLabelForBasicBlock(const MachineBasicBlock &MBB) const;
diff --git a/llvm/lib/CodeGen/GlobalISel/CallLowering.cpp 
b/llvm/lib/CodeGen/GlobalISel/CallLowering.cpp
index 2527b143128967..e0080b145d4f99 100644
--- a/llvm/lib/CodeGen/GlobalISel/CallLowering.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/CallLowering.cpp
@@ -144,7 +144,12 @@ bool CallLowering::lowerCall(MachineIRBuilder &MIRBuilder, 
const CallBase &CB,
   // Try looking through a bitcast from one function type to another.
   // Commonly happens with calls to objc_msgSend().
   const Value *CalleeV = CB.getCalledOperand()->stripPointerCasts();
-  if (const Function *F = dyn_cast(CalleeV))
+  if (const GlobalIFunc *IF = dyn_cast(CalleeV);
+  IF && MF.getTarget().getTargetTriple().isOSBinFormatMachO()) {
+// ld64 requires that .symbol_resolvers to be called via a stub, so these
+// must always be a diret call.
+Info.Callee = MachineOperand::CreateGA(IF, 0);
+  } else if (const Function *F = dyn_cast(CalleeV))
 Info.Callee = MachineOperand::CreateGA(F, 0);
   else
 Info.Callee = MachineOperand::CreateReg(GetCalleeReg(), false);
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 5560c037aa3ee6..94e76a43bf38d6 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -959,6 +959,7 @@ void Verifier::visitGlobalIFunc(const GlobalIFunc &GI) {
   GlobalIFunc::getResolverFunctionType(GI.getValueType());
   Check(ResolverTy == ResolverFuncTy->getPointerTo(GI.getAddressSpace()),
 "IFunc resolver has incorrect type", &GI);
+
 }
 
 void Verifier::visitNamedMDNode(const NamedMDNode &NMD) {
@@ -2239,13 +2240,10 @@ void Verifier::verifyFunctionAttrs(FunctionType *FT, 
AttributeList Attrs,
   }
 
   // Check EVEX512 feature.
-  if (MaxParameterWidth >= 512 && Attrs.hasFnAttr("target-features")) {
-Triple T(M.getTargetTriple());
-if (T.isX86()) {
-  StringRef TF = Attrs.getFnAttr("target-features").getValueAsString();
-  Check(!TF.contains("+avx512f") || !TF.contains("-evex512"),
-"512-bit vector ar

[Lldb-commits] [lldb] [lldb-dap] Include [opt] in the frame name only if a custom frame format is not specified. (PR #74861)


jimingham wrote:

Would it make sense to have opt be the result of a frame-format token, which we 
could put in the default format (function.optimization?) and people could add 
or not in custom formats?

Jim

> On Dec 8, 2023, at 3:59 PM, Alex Langford ***@***.***> wrote:
> 
> 
> @bulbazord commented on this pull request.
> 
> I think conceptually this makes sense, but I somewhat wonder if folks would 
> get confused when they have a custom frame format and don't see the [opt] in 
> there? The current behavior is that [opt] is always there so folks know they 
> don't have to put it in their custom frame format. When it's missing after 
> this change, I wonder if they'll notice.
> 
> —
> Reply to this email directly, view it on GitHub 
> ,
>  or unsubscribe 
> .
> You are receiving this because you are on a team that was mentioned.
> 



https://github.com/llvm/llvm-project/pull/74861
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb-dap] Implement command directives (PR #74808)

2023-12-08 Thread Walter Erquinigo via lldb-commits



@@ -33,14 +39,49 @@ void RunLLDBCommands(llvm::StringRef prefix,
   const char *error = result.GetError();
   strm << error;
 }
+  };
+
+  lldb::SBCommandInterpreter interp = g_dap.debugger.GetCommandInterpreter();
+  for (llvm::StringRef command : commands) {
+lldb::SBCommandReturnObject result;
+bool quiet_on_success = false;
+bool abort_on_error = false;
+
+while (parse_command_directives) {
+  if (command.starts_with("?")) {
+command = command.drop_front();
+quiet_on_success = true;
+  } else if (command.starts_with("!")) {
+command = command.drop_front();
+abort_on_error = true;
+  } else {
+break;
+  }
+}
+
+interp.HandleCommand(command.str().c_str(), result);

walter-erquinigo wrote:

yes, that's to prevent someone unintentionally sending a command that starts 
with ! using the debug console and then having their debug session terminate

https://github.com/llvm/llvm-project/pull/74808
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb-dap] Emit declarations along with variables (PR #74865)

2023-12-08 Thread Walter Erquinigo via lldb-commits



@@ -1101,6 +1101,29 @@ std::string 
CreateUniqueVariableNameForDisplay(lldb::SBValue v,
 //   can use this optional information to present the
 //   children in a paged UI and fetch them in chunks."
 // }
+// "declaration": {
+//   "type": "object | undefined",

walter-erquinigo wrote:

this "type" is a JSON schema attribute, not an actual thing being returned

https://github.com/llvm/llvm-project/pull/74865
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] All ValueObjectSP instances are now valid (non-null) but have an error state (PR #74912)


https://github.com/PortalPete created 
https://github.com/llvm/llvm-project/pull/74912



Many methods return `nullptr` or `ValueObjectSP()` to represent an invalid 
result.

For methods that have a `Status &error` parameter and return an 
`ValueObjectSP`, the return value *becomes* the container for the error state, 
which eliminate the need for a parameter.

For all other methods that return a `ValueObjectSP` but don't have a `Status` 
parameter, they now return `std::optional`.

The implication here is that that callers of the former methods need to call 
one of these methods:
* `return_value->GetError.Success()`
* `return_value->GetError.Fail()`

Callers of latter methods need only check whether the optional has a value or 
not, and if it does, to call its `value()` method to get the `ValueObjectSP` 
within.

>From 2cdc04fa607f62df2dfdf96e3322cdf34bcd15db Mon Sep 17 00:00:00 2001
From: Pete Lawrence 
Date: Thu, 7 Dec 2023 12:14:01 -1000
Subject: [PATCH] [lldb] All ValueObjectSP instances are now valid (non-null)
 but may be in an error state

Many methods return `nullptr` or `ValueObjectSP()` to represent an invalid 
result.

For methods that have a `Status &error` parameter and return an 
`ValueObjectSP`, the return value *becomes* the container for the error state, 
which eliminate the need for a parameter.

For all other methods that return a `ValueObjectSP` but don't have a `Status` 
parameter, they now return `std::optional`.

The implication here is that that callers of the former methods need to call 
one of these methods:
* `return_value->GetError.Success()`
* `return_value->GetError.Fail()`

Callers of latter methods need only check whether the optional has a value or 
not,
and if it does, to call its `value()` method to get the `ValueObjectSP` within.
---
 lldb/include/lldb/Core/ValueObject.h  |  77 +-
 .../lldb/Core/ValueObjectConstResult.h|   2 +-
 .../lldb/Core/ValueObjectConstResultImpl.h|   2 +-
 lldb/include/lldb/Core/ValueObjectList.h  |  12 +-
 lldb/include/lldb/Core/ValueObjectRegister.h  |   4 +-
 .../lldb/Core/ValueObjectSyntheticFilter.h|  10 +-
 lldb/include/lldb/Core/ValueObjectUpdater.h   |   6 +-
 .../lldb/DataFormatters/TypeSynthetic.h   |  18 +-
 lldb/include/lldb/Expression/UserExpression.h |   2 +-
 .../lldb/Interpreter/ScriptInterpreter.h  |   8 +-
 lldb/include/lldb/Target/Target.h |   2 +-
 lldb/include/lldb/lldb-forward.h  |  20 +-
 lldb/source/Core/ValueObject.cpp  | 832 +-
 lldb/source/Core/ValueObjectConstResult.cpp   |   4 +-
 .../Core/ValueObjectConstResultImpl.cpp   |   8 +-
 lldb/source/Core/ValueObjectList.cpp  |  59 +-
 lldb/source/Core/ValueObjectRegister.cpp  |   4 +-
 .../Core/ValueObjectSyntheticFilter.cpp   |  46 +-
 lldb/source/Core/ValueObjectUpdater.cpp   |  18 +-
 lldb/source/DataFormatters/TypeSynthetic.cpp  |   2 +-
 lldb/source/Target/Target.cpp |   2 +-
 21 files changed, 574 insertions(+), 564 deletions(-)

diff --git a/lldb/include/lldb/Core/ValueObject.h 
b/lldb/include/lldb/Core/ValueObject.h
index 20b3086138457f..06184a5bde0715 100644
--- a/lldb/include/lldb/Core/ValueObject.h
+++ b/lldb/include/lldb/Core/ValueObject.h
@@ -409,7 +409,7 @@ class ValueObject {
   Stream &s,
   GetExpressionPathFormat = eGetExpressionPathFormatDereferencePointers);
 
-  lldb::ValueObjectSP GetValueForExpressionPath(
+  std::optional GetValueForExpressionPath(
   llvm::StringRef expression,
   ExpressionPathScanEndReason *reason_to_stop = nullptr,
   ExpressionPathEndResultType *final_value_type = nullptr,
@@ -465,22 +465,24 @@ class ValueObject {
   /// Returns a unique id for this ValueObject.
   lldb::user_id_t GetID() const { return m_id.GetID(); }
 
-  virtual lldb::ValueObjectSP GetChildAtIndex(size_t idx,
-  bool can_create = true);
+  virtual std::optional
+  GetChildAtIndex(size_t idx, bool can_create = true);
 
-  // this will always create the children if necessary
-  lldb::ValueObjectSP GetChildAtIndexPath(llvm::ArrayRef idxs,
-  size_t *index_of_error = nullptr);
+  /// The method always creates missing children in the path, if necessary.
+  std::optional
+  GetChildAtIndexPath(llvm::ArrayRef idxs,
+  size_t *index_of_error = nullptr);
 
-  lldb::ValueObjectSP
+  std::optional
   GetChildAtIndexPath(llvm::ArrayRef> idxs,
   size_t *index_of_error = nullptr);
 
-  // this will always create the children if necessary
-  lldb::ValueObjectSP GetChildAtNamePath(llvm::ArrayRef 
names);
+  /// The method always creates missing children in the path, if necessary.
+  std::optional
+  GetChildAtNamePath(llvm::ArrayRef names);
 
-  virtual lldb::ValueObjectSP GetChildMemberWithName(llvm::StringRef name,
- bool can_create = true);
+  virtual s

[Lldb-commits] [lldb] [lldb] All ValueObjectSP instances are now valid (non-null) but have an error state (PR #74912)


https://github.com/PortalPete edited 
https://github.com/llvm/llvm-project/pull/74912
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][draft] All ValueObjectSP instances are now valid (non-null) but have an error state (PR #74912)


https://github.com/PortalPete edited 
https://github.com/llvm/llvm-project/pull/74912
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][draft] All ValueObjectSP instances are now valid (non-null) but have an error state (PR #74912)


https://github.com/PortalPete edited 
https://github.com/llvm/llvm-project/pull/74912
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb-dap] Implement command directives (PR #74808)


https://github.com/clayborg edited 
https://github.com/llvm/llvm-project/pull/74808
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb-dap] Implement command directives (PR #74808)



@@ -33,14 +39,49 @@ void RunLLDBCommands(llvm::StringRef prefix,
   const char *error = result.GetError();
   strm << error;
 }
+  };
+
+  lldb::SBCommandInterpreter interp = g_dap.debugger.GetCommandInterpreter();
+  for (llvm::StringRef command : commands) {
+lldb::SBCommandReturnObject result;
+bool quiet_on_success = false;
+bool abort_on_error = false;

clayborg wrote:

I am not sure we want to actually abort the `lldb-dap` program if we encounter 
an error, maybe this function can take an extra "bool &fatal_error" parameter 
that can be set to true if and only if we find any `!` prefixed commands that 
fail? Then any callers of this function would then check `fatal_error` and 
return an error for indicating why the current packet failed? 

More comments below

https://github.com/llvm/llvm-project/pull/74808
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb-dap] Implement command directives (PR #74808)



@@ -13,16 +13,22 @@ namespace lldb_dap {
 
 void RunLLDBCommands(llvm::StringRef prefix,
  const llvm::ArrayRef &commands,
- llvm::raw_ostream &strm) {
+ llvm::raw_ostream &strm, bool parse_command_directives) {
   if (commands.empty())

clayborg wrote:

Init the `fatal_error` to `false`
```
fatal_error = false;
```

https://github.com/llvm/llvm-project/pull/74808
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb-dap] Implement command directives (PR #74808)



@@ -33,9 +39,14 @@ namespace lldb_dap {
 /// \param[in] strm
 /// The stream that will receive the prefix, prompt + command and
 /// all command output.
+///
+/// \param[in] parse_command_directives
+/// If \b false, then command prefixes like \b ! or \b ? are not parsed and
+/// each command is executed verbatim.
 void RunLLDBCommands(llvm::StringRef prefix,

clayborg wrote:

return a `std::string` that will be the first error for any `!` prefixed lines 
that fail, and return empty string if no commands started with `!` or some did 
but none failed? Then the callers of this function can decide what to do with 
it.

https://github.com/llvm/llvm-project/pull/74808
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb-dap] Implement command directives (PR #74808)


https://github.com/clayborg requested changes to this pull request.

I would rather not terminate the debug session if we get erorrs on `!`. It 
would be nice if  the `RunLLDBCommands` function could take an extra `bool 
&fatal_error` that clients could check after running when needed to decide what 
to do if this is set to true due to any `!` prefixed commands failing. Then for 
`"launchCommands"` in lldb-dap.cpp's `lldb::SBError LaunchProcess(const 
llvm::json::Object &request)` could use this bool variable to return an error 
when needed:
```
g_dap.target.SetLaunchInfo(launch_info);
bool fatal_error = false;
g_dap.RunLLDBCommands("Running launchCommands:", launchCommands, 
fatal_error);
if (fatal_error)
  error.SetErrorWithString("some required 'launchComands' failed, see debug 
console for details");
```
So each call site that calls RunLLDBCommands(...) would need to watch for this 
variable being set to true and do what is needed to return an error for the 
current command

https://github.com/llvm/llvm-project/pull/74808
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb-dap] Implement command directives (PR #74808)



@@ -13,16 +13,22 @@ namespace lldb_dap {
 
 void RunLLDBCommands(llvm::StringRef prefix,
  const llvm::ArrayRef &commands,
- llvm::raw_ostream &strm) {
+ llvm::raw_ostream &strm, bool parse_command_directives) {

clayborg wrote:

add a `bool &fatal_error` that is an out parameter to indicate we got an error 
on a `!` command, clients will then use this to figure out what to do instead 
of aborting. See inline comments below

https://github.com/llvm/llvm-project/pull/74808
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb-dap] Implement command directives (PR #74808)