[Lldb-commits] [PATCH] D112708: [lldb] Make TypeSystemClang::GetFullyUnqualifiedType work for constant arrays

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik accepted this revision.
shafik added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112708

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

[Lldb-commits] [PATCH] D112706: [lldb/test] Allow indentation in inline tests

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

I agree, we should be trying to get ride of inline tests. The last time I had 
to debug one it was not a fun experience. Maybe some of the recent improvements 
have helped there though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112706

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

[Lldb-commits] [PATCH] D112709: [lldb] Fix matchers for char array formatters

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

Nice catch!




Comment at: 
lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp:21
+MAKE_VARS();
+MAKE_VARS(const);
+

It feels like writing out the decls by hand would have been quicker but perhaps 
less satisfying...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112709

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

[Lldb-commits] [PATCH] D112212: [lldb/test] Print build commands in trace mode

[Jonas Devlieghere via Phabricator via lldb-commits]

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

Thanks everyone. LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112212

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

[Lldb-commits] [PATCH] D112632: [lldb] [Host/Terminal] Fix warnings with termios disabled

[Nico Weber via Phabricator via lldb-commits]

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

Thanks!




Comment at: lldb/source/Host/common/Terminal.cpp:39
+
 llvm::Expected Terminal::GetData() {
   if (!FileDescriptorIsValid())

nit: should GetData() even exist if `!LLDB_ENABLE_TERMIOS`? Looks like it's 
only called `#if LLDB_ENABLE_TERMIOS` now. Maybe the whole method definition 
should be inside an ifdef.

If it should keep existing, either move the !FileDescriptorIsValid() check into 
the #if inside it (imho preferred), or move all the callers of it outside that 
#if. The current mix is a bit self-inconsistent.


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

https://reviews.llvm.org/D112632

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

[Lldb-commits] [lldb] 2aa3b56 - [lldb] Fix TestMacCatalyst.py

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2021-10-28T17:57:55-07:00
New Revision: 2aa3b56339423969f0c89fe10d81f32ff77db2d2

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

LOG: [lldb] Fix TestMacCatalyst.py

Added: 


Modified: 
lldb/test/API/macosx/macCatalyst/Makefile

Removed: 




diff  --git a/lldb/test/API/macosx/macCatalyst/Makefile 
b/lldb/test/API/macosx/macCatalyst/Makefile
index f358ff85ea9f0..d162c33d774f9 100644
--- a/lldb/test/API/macosx/macCatalyst/Makefile
+++ b/lldb/test/API/macosx/macCatalyst/Makefile
@@ -1,6 +1,6 @@
 C_SOURCES := main.c
 
-override TRIPLE := $(ARCH)-apple-ios13.0-macabi
+override TRIPLE := $(ARCH)-apple-ios13.1-macabi
 CFLAGS_EXTRAS := -target $(TRIPLE)
 
 # FIXME: rdar://problem/54986190



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

[Lldb-commits] [lldb] d1e9514 - To avoid the obvious problem, use a different port...

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2021-10-28T17:45:31-07:00
New Revision: d1e9514ac89b1ceed51114159b7dd4888ce58974

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

LOG: To avoid the obvious problem, use a different port...

There's another test that opens an hard-coded port to talk to debugserver
(TestPlatformSDK.py).  Make sure this port and the one in that other
test are different to avoid that potential conflict.

Added: 


Modified: 
lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py 
b/lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py
index 157d16fe025b..4fbe7199267e 100644
--- a/lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py
+++ b/lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py
@@ -18,7 +18,7 @@ class TestStopAtEntry(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
 
 # The port used by debugserver.
-PORT = 54637
+PORT = 54638
 
 # The number of attempts.
 ATTEMPTS = 10



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

[Lldb-commits] [PATCH] D112747: Restore process events after a launch that stopped at the entry point

[Jim Ingham via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe655769c4a7b: Fix a bug in Launch when using an async 
debugger  remote platform. (authored by jingham).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112747

Files:
  lldb/source/Target/Process.cpp
  lldb/test/API/functionalities/launch_stop_at_entry/Makefile
  lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py
  lldb/test/API/functionalities/launch_stop_at_entry/main.c

Index: lldb/test/API/functionalities/launch_stop_at_entry/main.c
===
--- /dev/null
+++ lldb/test/API/functionalities/launch_stop_at_entry/main.c
@@ -0,0 +1,5 @@
+int main(int argc, char **argv) {
+  /* We just want to make sure this ran, so
+ it doesn't actually need to do anything. */
+  return 0;
+}
Index: lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py
===
--- /dev/null
+++ lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py
@@ -0,0 +1,163 @@
+import lldb
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+from lldbgdbserverutils import get_debugserver_exe
+
+import os
+import platform
+import shutil
+import time
+import socket
+
+
+class TestStopAtEntry(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+# The port used by debugserver.
+PORT = 54637
+
+# The number of attempts.
+ATTEMPTS = 10
+
+# Time given to the binary to launch and to debugserver to attach to it for
+# every attempt. We'll wait a maximum of 10 times 2 seconds while the
+# inferior will wait 10 times 10 seconds.
+TIMEOUT = 2
+
+def no_debugserver(self):
+if get_debugserver_exe() is None:
+return 'no debugserver'
+return None
+
+def port_not_available(self):
+s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+if s.connect_ex(('127.0.0.1', self.PORT)) == 0:
+return '{} not available'.format(self.PORT)
+return None
+
+@skipUnlessDarwin
+def test_stop_default_platform_sync(self):
+self.do_test_stop_at_entry(True, False)
+
+@skipUnlessDarwin
+def test_stop_default_platform_async(self):
+self.do_test_stop_at_entry(False, False)
+
+@skipUnlessDarwin
+@expectedFailureIfFn(no_debugserver)
+@expectedFailureIfFn(port_not_available)
+def test_stop_remote_platform_sync(self):
+self.do_test_stop_at_entry(True, True)
+
+@skipUnlessDarwin
+@expectedFailureIfFn(no_debugserver)
+@expectedFailureIfFn(port_not_available)
+def test_stop_remote_platform_async(self):
+self.do_test_stop_at_entry(False, True)
+
+def do_test_stop_at_entry(self, synchronous, remote):
+"""Test the normal launch path in either sync or async mode"""
+self.build()
+
+target = lldbutil.run_to_breakpoint_make_target(self)
+launch_info = target.GetLaunchInfo()
+launch_info.SetLaunchFlags(lldb.eLaunchFlagStopAtEntry)
+old_async = self.dbg.GetAsync()
+def cleanup ():
+self.dbg.SetAsync(old_async)
+self.addTearDownHook(cleanup)
+
+if not synchronous:
+self.dbg.SetAsync(True)
+listener = lldb.SBListener("test-process-listener")
+mask = listener.StartListeningForEventClass(self.dbg, lldb.SBProcess.GetBroadcasterClassName(), lldb.SBProcess.eBroadcastBitStateChanged)
+self.assertEqual(mask, lldb.SBProcess.eBroadcastBitStateChanged, "Got right mask for listener")
+launch_info.SetListener(listener)
+else:
+self.dbg.SetAsync(False)
+
+if remote:
+self.setup_remote_platform()
+
+error = lldb.SBError()
+
+process = target.Launch(launch_info, error)
+self.assertTrue(error.Success(), "Launch failed: {0}".format(error.description))
+# If we are asynchronous, we have to wait for the events:
+if not synchronous:
+listener = launch_info.GetListener()
+event = lldb.SBEvent()
+result = listener.WaitForEvent(30, event)
+self.assertTrue(result, "Timed out waiting for event from process")
+state = lldb.SBProcess.GetStateFromEvent(event)
+self.assertEqual(state, lldb.eStateStopped, "Didn't get a stopped state after launch")
+
+# Okay, we should be stopped.  Make sure we are indeed at the
+# entry point.  I only know how to do this on darwin:
+self.assertEqual(len(process.threads), 1, "Should only have one thread at entry")
+thread = process.threads[0]
+frame = thread.GetFrameAtIndex(0)
+stop_func = frame.name
+

[Lldb-commits] [lldb] e655769 - Fix a bug in Launch when using an async debugger & remote platform.

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2021-10-28T17:02:43-07:00
New Revision: e655769c4a7b5a17b17eace3bd160b3b015b75ed

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

LOG: Fix a bug in Launch when using an async debugger & remote platform.

We weren't setting the listener back to the unhijacked one in this
case, so that a continue after the stop fails.  It thinks the process
is still running.  Also add tests for this behavior.

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

Added: 
lldb/test/API/functionalities/launch_stop_at_entry/Makefile
lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py
lldb/test/API/functionalities/launch_stop_at_entry/main.c

Modified: 
lldb/source/Target/Process.cpp

Removed: 




diff  --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 79ede760fb819..d901d523a1036 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -2580,6 +2580,9 @@ Status Process::Launch(ProcessLaunchInfo _info) {
   // stopped or crashed. Directly set the state.  This is done to
   // prevent a stop message with a bunch of spurious output on thread
   // status, as well as not pop a ProcessIOHandler.
+  // We are done with the launch hijack listener, and this stop should
+  // go to the public state listener:
+  RestoreProcessEvents();
   SetPublicState(state, false);
 
   if (PrivateStateThreadIsValid())

diff  --git a/lldb/test/API/functionalities/launch_stop_at_entry/Makefile 
b/lldb/test/API/functionalities/launch_stop_at_entry/Makefile
new file mode 100644
index 0..10495940055b6
--- /dev/null
+++ b/lldb/test/API/functionalities/launch_stop_at_entry/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules

diff  --git 
a/lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py 
b/lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py
new file mode 100644
index 0..157d16fe025b7
--- /dev/null
+++ b/lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py
@@ -0,0 +1,163 @@
+import lldb
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+from lldbgdbserverutils import get_debugserver_exe
+
+import os
+import platform
+import shutil
+import time
+import socket
+
+
+class TestStopAtEntry(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+# The port used by debugserver.
+PORT = 54637
+
+# The number of attempts.
+ATTEMPTS = 10
+
+# Time given to the binary to launch and to debugserver to attach to it for
+# every attempt. We'll wait a maximum of 10 times 2 seconds while the
+# inferior will wait 10 times 10 seconds.
+TIMEOUT = 2
+
+def no_debugserver(self):
+if get_debugserver_exe() is None:
+return 'no debugserver'
+return None
+
+def port_not_available(self):
+s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+if s.connect_ex(('127.0.0.1', self.PORT)) == 0:
+return '{} not available'.format(self.PORT)
+return None
+
+@skipUnlessDarwin
+def test_stop_default_platform_sync(self):
+self.do_test_stop_at_entry(True, False)
+
+@skipUnlessDarwin
+def test_stop_default_platform_async(self):
+self.do_test_stop_at_entry(False, False)
+
+@skipUnlessDarwin
+@expectedFailureIfFn(no_debugserver)
+@expectedFailureIfFn(port_not_available)
+def test_stop_remote_platform_sync(self):
+self.do_test_stop_at_entry(True, True)
+
+@skipUnlessDarwin
+@expectedFailureIfFn(no_debugserver)
+@expectedFailureIfFn(port_not_available)
+def test_stop_remote_platform_async(self):
+self.do_test_stop_at_entry(False, True)
+
+def do_test_stop_at_entry(self, synchronous, remote):
+"""Test the normal launch path in either sync or async mode"""
+self.build()
+
+target = lldbutil.run_to_breakpoint_make_target(self)
+launch_info = target.GetLaunchInfo()
+launch_info.SetLaunchFlags(lldb.eLaunchFlagStopAtEntry)
+old_async = self.dbg.GetAsync()
+def cleanup ():
+self.dbg.SetAsync(old_async)
+self.addTearDownHook(cleanup)
+
+if not synchronous:
+self.dbg.SetAsync(True)
+listener = lldb.SBListener("test-process-listener")
+mask = listener.StartListeningForEventClass(self.dbg, 
lldb.SBProcess.GetBroadcasterClassName(), 
lldb.SBProcess.eBroadcastBitStateChanged)
+self.assertEqual(mask, lldb.SBProcess.eBroadcastBitStateChanged, 
"Got right mask for listener")
+   

[Lldb-commits] [PATCH] D112747: Restore process events after a launch that stopped at the entry point

[Greg Clayton via Phabricator via lldb-commits]

clayborg accepted this revision.
clayborg added a comment.

ok, lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112747

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

[Lldb-commits] [PATCH] D112747: Restore process events after a launch that stopped at the entry point

[Jim Ingham via Phabricator via lldb-commits]

jingham added inline comments.



Comment at: 
lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py:21
+# The port used by debugserver.
+PORT = 54637
+

jingham wrote:
> clayborg wrote:
> > This hard coded port worries me for buildbot flakiness. Do other tests do 
> > this?
> The test I copied it from does...
That is "TestPlatformSDK.py".  I didn't centralize that logic because the two 
uses were different, and the implementation is not very many lines.  If we see 
flakiness in either of these tests we can centralize this logic and handle the 
port fetching better.  But I'd rather not do that as part of this patch. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112747

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

[Lldb-commits] [PATCH] D112747: Restore process events after a launch that stopped at the entry point

[Jim Ingham via Phabricator via lldb-commits]

jingham added inline comments.



Comment at: 
lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py:21
+# The port used by debugserver.
+PORT = 54637
+

clayborg wrote:
> This hard coded port worries me for buildbot flakiness. Do other tests do 
> this?
The test I copied it from does...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112747

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

Re: [Lldb-commits] [PATCH] D112691: Include target settings in "statistics dump" output.

[Jim Ingham via lldb-commits]

> On Oct 28, 2021, at 4:10 PM, Greg Clayton via Phabricator 
>  wrote:
> 
> clayborg added a comment.
> 
>> In D112691#3095010 , @jingham 
>> wrote:
>> I can see wanting to dump statistics at various points in the running of a 
>> process, maybe triggered by breakpoints, for instance.  In that case I 
>> wouldn't want to dump the settings data - if it is indeed redundant (see 
>> above) every time.  Having the settings as a separate emission would make 
>> that possible.  And just like we add gdb-remote as a convenience, it would 
>> be fine to have some low level commands that you can reassemble and then a 
>> portmanteau command that generates a "good for most purposes" report.
> 
> Sounds good. We can do this with a separate command and keep the "statistics 
> dump" cleaner. And yes, we do want to write this data out at various points 
> in the process' lifetime to see where and when delays are introduced, so 
> keeping this to just stats is a smart.
> 
>> Also, we already have "settings read" and "settings write" so adding another 
>> way to dump them seems redundant.  You are dumping a subset, but the 
>> "settings write" command can do that as well.  If the format's not one you 
>> like, I think we should be able to change that as the successful round trip 
>> is the main thing.
> 
> I just tried "settings write -f /tmp/a" after loading up lldb with "lldb 
> a.out 1 2 3", but the output doesn't seem to contain any of the right 
> settings? target.arg0 and target.run-args are not saved at all and have no 
> value? Is this command tested? I will opt for "settings show" and saving the 
> entire output out to a file instead.

There is a teeny test that seems to mostly test error conditions in 
TestSettingsWrite.test.

Writing out the settings in the form of a list of "setting set" commands seems 
neither an efficient nor a generally useful way to save settings.  It would be 
much better to write & read them in JSON now that we have the facility to 
handle JSON easily.  We should probably fix this one day.  But given the way 
it's currently implemented, I retract my suggestion that you use it.

Jim


> 
> 
> Repository:
>  rG LLVM Github Monorepo
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D112691/new/
> 
> https://reviews.llvm.org/D112691
> 

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

[Lldb-commits] [PATCH] D112747: Restore process events after a launch that stopped at the entry point

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: 
lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py:21
+# The port used by debugserver.
+PORT = 54637
+

This hard coded port worries me for buildbot flakiness. Do other tests do this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112747

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

[Lldb-commits] [PATCH] D112691: Include target settings in "statistics dump" output.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

> In D112691#3095010 , @jingham wrote:
> I can see wanting to dump statistics at various points in the running of a 
> process, maybe triggered by breakpoints, for instance.  In that case I 
> wouldn't want to dump the settings data - if it is indeed redundant (see 
> above) every time.  Having the settings as a separate emission would make 
> that possible.  And just like we add gdb-remote as a convenience, it would be 
> fine to have some low level commands that you can reassemble and then a 
> portmanteau command that generates a "good for most purposes" report.

Sounds good. We can do this with a separate command and keep the "statistics 
dump" cleaner. And yes, we do want to write this data out at various points in 
the process' lifetime to see where and when delays are introduced, so keeping 
this to just stats is a smart.

> Also, we already have "settings read" and "settings write" so adding another 
> way to dump them seems redundant.  You are dumping a subset, but the 
> "settings write" command can do that as well.  If the format's not one you 
> like, I think we should be able to change that as the successful round trip 
> is the main thing.

I just tried "settings write -f /tmp/a" after loading up lldb with "lldb a.out 
1 2 3", but the output doesn't seem to contain any of the right settings? 
target.arg0 and target.run-args are not saved at all and have no value? Is this 
command tested? I will opt for "settings show" and saving the entire output out 
to a file instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112691

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

[Lldb-commits] [PATCH] D112677: Remove unused ValueObjectDynamicValue::SetOwningSP & backing ivar

[Jim Ingham via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1227fa7e9040: Remove unused 
ValueObjectDynamicValue::SetOwningSP  backing ivar (authored by jingham).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112677

Files:
  lldb/include/lldb/Core/ValueObjectDynamicValue.h
  lldb/source/Core/ValueObjectDynamicValue.cpp


Index: lldb/source/Core/ValueObjectDynamicValue.cpp
===
--- lldb/source/Core/ValueObjectDynamicValue.cpp
+++ lldb/source/Core/ValueObjectDynamicValue.cpp
@@ -36,10 +36,6 @@
   SetName(parent.GetName());
 }
 
-ValueObjectDynamicValue::~ValueObjectDynamicValue() {
-  m_owning_valobj_sp.reset();
-}
-
 CompilerType ValueObjectDynamicValue::GetCompilerTypeImpl() {
   const bool success = UpdateValueIfNeeded(false);
   if (success) {
Index: lldb/include/lldb/Core/ValueObjectDynamicValue.h
===
--- lldb/include/lldb/Core/ValueObjectDynamicValue.h
+++ lldb/include/lldb/Core/ValueObjectDynamicValue.h
@@ -32,7 +32,7 @@
 /// set lldb type.
 class ValueObjectDynamicValue : public ValueObject {
 public:
-  ~ValueObjectDynamicValue() override;
+  ~ValueObjectDynamicValue() = default;
 
   llvm::Optional GetByteSize() override;
 
@@ -68,14 +68,6 @@
 
   lldb::ValueObjectSP GetStaticValue() override { return m_parent->GetSP(); }
 
-  void SetOwningSP(lldb::ValueObjectSP _sp) {
-if (m_owning_valobj_sp == owning_sp)
-  return;
-
-assert(m_owning_valobj_sp.get() == nullptr);
-m_owning_valobj_sp = owning_sp;
-  }
-
   bool SetValueFromCString(const char *value_str, Status ) override;
 
   bool SetData(DataExtractor , Status ) override;
@@ -117,7 +109,6 @@
 
   Address m_address; ///< The variable that this value object is based upon
   TypeAndOrName m_dynamic_type_info; // We can have a type_sp or just a name
-  lldb::ValueObjectSP m_owning_valobj_sp;
   lldb::DynamicValueType m_use_dynamic;
   TypeImpl m_type_impl;
 


Index: lldb/source/Core/ValueObjectDynamicValue.cpp
===
--- lldb/source/Core/ValueObjectDynamicValue.cpp
+++ lldb/source/Core/ValueObjectDynamicValue.cpp
@@ -36,10 +36,6 @@
   SetName(parent.GetName());
 }
 
-ValueObjectDynamicValue::~ValueObjectDynamicValue() {
-  m_owning_valobj_sp.reset();
-}
-
 CompilerType ValueObjectDynamicValue::GetCompilerTypeImpl() {
   const bool success = UpdateValueIfNeeded(false);
   if (success) {
Index: lldb/include/lldb/Core/ValueObjectDynamicValue.h
===
--- lldb/include/lldb/Core/ValueObjectDynamicValue.h
+++ lldb/include/lldb/Core/ValueObjectDynamicValue.h
@@ -32,7 +32,7 @@
 /// set lldb type.
 class ValueObjectDynamicValue : public ValueObject {
 public:
-  ~ValueObjectDynamicValue() override;
+  ~ValueObjectDynamicValue() = default;
 
   llvm::Optional GetByteSize() override;
 
@@ -68,14 +68,6 @@
 
   lldb::ValueObjectSP GetStaticValue() override { return m_parent->GetSP(); }
 
-  void SetOwningSP(lldb::ValueObjectSP _sp) {
-if (m_owning_valobj_sp == owning_sp)
-  return;
-
-assert(m_owning_valobj_sp.get() == nullptr);
-m_owning_valobj_sp = owning_sp;
-  }
-
   bool SetValueFromCString(const char *value_str, Status ) override;
 
   bool SetData(DataExtractor , Status ) override;
@@ -117,7 +109,6 @@
 
   Address m_address; ///< The variable that this value object is based upon
   TypeAndOrName m_dynamic_type_info; // We can have a type_sp or just a name
-  lldb::ValueObjectSP m_owning_valobj_sp;
   lldb::DynamicValueType m_use_dynamic;
   TypeImpl m_type_impl;
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 1227fa7 - Remove unused ValueObjectDynamicValue::SetOwningSP & backing ivar

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2021-10-28T16:06:01-07:00
New Revision: 1227fa7e9040469d648cc7709a298901002f4c27

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

LOG: Remove unused ValueObjectDynamicValue::SetOwningSP & backing ivar

This has no uses and the ValueObjectDynamicValue already tracks
its ownership through the parent it is passed when made.  I can't
find any vestiges of the use of this API, maybe it was from some
earlier design?

Resetting the backing ivar was the only job the destructor did, so I
set that to default as well.

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

Added: 


Modified: 
lldb/include/lldb/Core/ValueObjectDynamicValue.h
lldb/source/Core/ValueObjectDynamicValue.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/ValueObjectDynamicValue.h 
b/lldb/include/lldb/Core/ValueObjectDynamicValue.h
index 8822a1d392499..09dcd0f968be4 100644
--- a/lldb/include/lldb/Core/ValueObjectDynamicValue.h
+++ b/lldb/include/lldb/Core/ValueObjectDynamicValue.h
@@ -32,7 +32,7 @@ class Status;
 /// set lldb type.
 class ValueObjectDynamicValue : public ValueObject {
 public:
-  ~ValueObjectDynamicValue() override;
+  ~ValueObjectDynamicValue() = default;
 
   llvm::Optional GetByteSize() override;
 
@@ -68,14 +68,6 @@ class ValueObjectDynamicValue : public ValueObject {
 
   lldb::ValueObjectSP GetStaticValue() override { return m_parent->GetSP(); }
 
-  void SetOwningSP(lldb::ValueObjectSP _sp) {
-if (m_owning_valobj_sp == owning_sp)
-  return;
-
-assert(m_owning_valobj_sp.get() == nullptr);
-m_owning_valobj_sp = owning_sp;
-  }
-
   bool SetValueFromCString(const char *value_str, Status ) override;
 
   bool SetData(DataExtractor , Status ) override;
@@ -117,7 +109,6 @@ class ValueObjectDynamicValue : public ValueObject {
 
   Address m_address; ///< The variable that this value object is based upon
   TypeAndOrName m_dynamic_type_info; // We can have a type_sp or just a name
-  lldb::ValueObjectSP m_owning_valobj_sp;
   lldb::DynamicValueType m_use_dynamic;
   TypeImpl m_type_impl;
 

diff  --git a/lldb/source/Core/ValueObjectDynamicValue.cpp 
b/lldb/source/Core/ValueObjectDynamicValue.cpp
index d775094965094..bf087f33c0e90 100644
--- a/lldb/source/Core/ValueObjectDynamicValue.cpp
+++ b/lldb/source/Core/ValueObjectDynamicValue.cpp
@@ -36,10 +36,6 @@ ValueObjectDynamicValue::ValueObjectDynamicValue(
   SetName(parent.GetName());
 }
 
-ValueObjectDynamicValue::~ValueObjectDynamicValue() {
-  m_owning_valobj_sp.reset();
-}
-
 CompilerType ValueObjectDynamicValue::GetCompilerTypeImpl() {
   const bool success = UpdateValueIfNeeded(false);
   if (success) {



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

[Lldb-commits] [PATCH] D112747: Restore process events after a launch that stopped at the entry point

[Jim Ingham via Phabricator via lldb-commits]

jingham marked 3 inline comments as done.
jingham added a comment.

Thanks for pointing these out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112747

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

[Lldb-commits] [PATCH] D112747: Restore process events after a launch that stopped at the entry point

[Jim Ingham via Phabricator via lldb-commits]

jingham updated this revision to Diff 383184.
jingham added a comment.

Address Ismail's comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112747

Files:
  lldb/source/Target/Process.cpp
  lldb/test/API/functionalities/launch_stop_at_entry/Makefile
  lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py
  lldb/test/API/functionalities/launch_stop_at_entry/main.c

Index: lldb/test/API/functionalities/launch_stop_at_entry/main.c
===
--- /dev/null
+++ lldb/test/API/functionalities/launch_stop_at_entry/main.c
@@ -0,0 +1,5 @@
+int main(int argc, char **argv) {
+  /* We just want to make sure this ran, so
+ it doesn't actually need to do anything. */
+  return 0;
+}
Index: lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py
===
--- /dev/null
+++ lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py
@@ -0,0 +1,163 @@
+import lldb
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+from lldbgdbserverutils import get_debugserver_exe
+
+import os
+import platform
+import shutil
+import time
+import socket
+
+
+class TestStopAtEntry(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+# The port used by debugserver.
+PORT = 54637
+
+# The number of attempts.
+ATTEMPTS = 10
+
+# Time given to the binary to launch and to debugserver to attach to it for
+# every attempt. We'll wait a maximum of 10 times 2 seconds while the
+# inferior will wait 10 times 10 seconds.
+TIMEOUT = 2
+
+def no_debugserver(self):
+if get_debugserver_exe() is None:
+return 'no debugserver'
+return None
+
+def port_not_available(self):
+s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+if s.connect_ex(('127.0.0.1', self.PORT)) == 0:
+return '{} not available'.format(self.PORT)
+return None
+
+@skipUnlessDarwin
+def test_stop_default_platform_sync(self):
+self.do_test_stop_at_entry(True, False)
+
+@skipUnlessDarwin
+def test_stop_default_platform_async(self):
+self.do_test_stop_at_entry(False, False)
+
+@skipUnlessDarwin
+@expectedFailureIfFn(no_debugserver)
+@expectedFailureIfFn(port_not_available)
+def test_stop_remote_platform_sync(self):
+self.do_test_stop_at_entry(True, True)
+
+@skipUnlessDarwin
+@expectedFailureIfFn(no_debugserver)
+@expectedFailureIfFn(port_not_available)
+def test_stop_remote_platform_async(self):
+self.do_test_stop_at_entry(False, True)
+
+def do_test_stop_at_entry(self, synchronous, remote):
+"""Test the normal launch path in either sync or async mode"""
+self.build()
+
+target = lldbutil.run_to_breakpoint_make_target(self)
+launch_info = target.GetLaunchInfo()
+launch_info.SetLaunchFlags(lldb.eLaunchFlagStopAtEntry)
+old_async = self.dbg.GetAsync()
+def cleanup ():
+self.dbg.SetAsync(old_async)
+self.addTearDownHook(cleanup)
+
+if not synchronous:
+self.dbg.SetAsync(True)
+listener = lldb.SBListener("test-process-listener")
+mask = listener.StartListeningForEventClass(self.dbg, lldb.SBProcess.GetBroadcasterClassName(), lldb.SBProcess.eBroadcastBitStateChanged)
+self.assertEqual(mask, lldb.SBProcess.eBroadcastBitStateChanged, "Got right mask for listener")
+launch_info.SetListener(listener)
+else:
+self.dbg.SetAsync(False)
+
+if remote:
+self.setup_remote_platform()
+
+error = lldb.SBError()
+
+process = target.Launch(launch_info, error)
+self.assertTrue(error.Success(), "Launch failed: {0}".format(error.description))
+# If we are asynchronous, we have to wait for the events:
+if not synchronous:
+listener = launch_info.GetListener()
+event = lldb.SBEvent()
+result = listener.WaitForEvent(30, event)
+self.assertTrue(result, "Timed out waiting for event from process")
+state = lldb.SBProcess.GetStateFromEvent(event)
+self.assertEqual(state, lldb.eStateStopped, "Didn't get a stopped state after launch")
+
+# Okay, we should be stopped.  Make sure we are indeed at the
+# entry point.  I only know how to do this on darwin:
+self.assertEqual(len(process.threads), 1, "Should only have one thread at entry")
+thread = process.threads[0]
+frame = thread.GetFrameAtIndex(0)
+stop_func = frame.name
+self.assertEqual(stop_func, "_dyld_start")
+
+# Now make sure that we can resume the process and have 

[Lldb-commits] [PATCH] D112747: Restore process events after a launch that stopped at the entry point

[Med Ismail Bennani via Phabricator via lldb-commits]

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

LGTM besides some minor things. Thanks for fixing this @jingham !




Comment at: 
lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py:85
+#self.dbg.HandleCommand("log enable lldb process")
+#self.dbg.HandleCommand("log enable gdb-remote packets process")
+

Did you forget to remove these comments or did you leave them on purpose  ?



Comment at: 
lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py:89
+self.assertTrue(error.Success(), "Launch failed: 
{0}".format(error.description))
+# If we are synchronous, we have to wait for the events:
+if not synchronous:

This comment doesn't match the following condition



Comment at: 
lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py:134
+#FIXME: This should be a cleanup
+self.dbg.SetAsync(old_async)
+

As you mentioned in the FIXME above, this should be a cleanup (similarly to 
what `TestPlatformSDK.py` does)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112747

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

Re: [Lldb-commits] [PATCH] D112691: Include target settings in "statistics dump" output.

[Jim Ingham via lldb-commits]

> On Oct 28, 2021, at 3:18 PM, Greg Clayton via Phabricator 
>  wrote:
> 
> clayborg added a comment.
> 
> In D112691#3095010 , @jingham wrote:
> 
>> Do you care about the history of these settings?  After all, the problem 
>> might arise because someone set a setting then unset it.  Your statistics 
>> approach wouldn't catch that.  If you are really trying to build an 
>> architecture where we can track this sort of problem down, then you might 
>> need more of a history approach, where the settings and certain other 
>> changes in the state of the debugger mark epochs, and you aggregate data 
>> into those epochs?
> 
> Personally I find most people set the important settings once and them leave 
> them alone for the debug session. History of settings and timings could be 
> nice, but we have no infrastructure to associate timestamps with events in 
> the debug session right now, that being said it could be added.

Yeah, I don't know that it's necessary to get too far out before we see a 
variety of actual uses of this feature.  I was mostly speculating on potential 
issues... 

> 
> If we don't want this in the statistics dump I can fully understand, though I 
> do like a one stop command people can run when they want to report issues 
> that may involve performance or other things going wrong with the debug 
> session.

I can see wanting to dump statistics at various points in the running of a 
process, maybe triggered by breakpoints, for instance.  In that case I wouldn't 
want to dump the settings data - if it is indeed redundant (see above) every 
time.  Having the settings as a separate emission would make that possible.  
And just like we add gdb-remote as a convenience, it would be fine to have some 
low level commands that you can reassemble and then a portmanteau command that 
generates a "good for most purposes" report.

Also, we already have "settings read" and "settings write" so adding another 
way to dump them seems redundant.  You are dumping a subset, but the "settings 
write" command can do that as well.  If the format's not one you like, I think 
we should be able to change that as the successful round trip is the main thing.

Jim


> 
> 
> Repository:
>  rG LLVM Github Monorepo
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D112691/new/
> 
> https://reviews.llvm.org/D112691
> 

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

[Lldb-commits] [PATCH] D112691: Include target settings in "statistics dump" output.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D112691#3095010 , @jingham wrote:

> Do you care about the history of these settings?  After all, the problem 
> might arise because someone set a setting then unset it.  Your statistics 
> approach wouldn't catch that.  If you are really trying to build an 
> architecture where we can track this sort of problem down, then you might 
> need more of a history approach, where the settings and certain other changes 
> in the state of the debugger mark epochs, and you aggregate data into those 
> epochs?

Personally I find most people set the important settings once and them leave 
them alone for the debug session. History of settings and timings could be 
nice, but we have no infrastructure to associate timestamps with events in the 
debug session right now, that being said it could be added.

If we don't want this in the statistics dump I can fully understand, though I 
do like a one stop command people can run when they want to report issues that 
may involve performance or other things going wrong with the debug session.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112691

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

[Lldb-commits] [PATCH] D112691: Include target settings in "statistics dump" output.

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

Do you care about the history of these settings?  After all, the problem might 
arise because someone set a setting then unset it.  If you are really trying to 
build an architecture where we can track this sort of problem down, then you 
might need more of a history approach, where the settings and certain other 
changes in the state of the debugger mark epochs, and you aggregate data into 
those epochs?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112691

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

[Lldb-commits] [PATCH] D112691: Include target settings in "statistics dump" output.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D112691#3093889 , @JDevlieghere 
wrote:

> I understand the need for something like this to make some of the statistics 
> more meaningful, but this is stretching the notion of statistics. 
> Conceptually, this is approaching something like a dump of the debugger for 
> issue/performance analysis. I think that idea is really exciting, and from 
> that perspective there's a lot more useful information we could add to it. 
> Long term, I can see this output be something that we ask users to include in 
> every bug report.

That is kind of what I am going for with this command. One main issue is when 
breakpoints fail to resolve we often don't know why, and knowing if we a) have 
debug info enabled and b) have the right settings to remap things really will 
help. I was thinking the same thing though: this info is not really statistics.

Would adding a "--target-state" option that would only include target stuff if 
requested make more sense? See my comment below as well

> TL;DR: Can we make this more generic and keep the separation between 
> statistics/metrics and a more general concept that includes these target 
> settings?

I am open to ideas on how to do this kind of thing. We can always gather this 
information on our own or with a separate command.

We could do a "target read" and "target write" just like we do for breakpoints? 
The idea would be to serialize all settings needed for a target into JSON and 
be able to load a target from JSON. This could be really handy for bug reports. 
If we have a command that does this, we could easily add an option to the 
"statistics dump" command to include this state with --target-state so we can 
still get it all in one place, but the user must ask for it? The description 
for --target-state could be something like:

  -t (--target-state)
  Include the serialization of the target in this information to help with 
being able to reproduce the conditions that led to the statistics

Lemme know what you think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112691

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

[Lldb-commits] [PATCH] D112752: [formatters] Add a libstdcpp formatter for multimap and unify tests across stdlibs

[Danil Stefaniuc via Phabricator via lldb-commits]

danilashtefan created this revision.
danilashtefan requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112752

Files:
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/multimap/Makefile
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/multimap/TestDataFormatterGenericMultiMap.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/multimap/main.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/multimap/Makefile
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/multimap/TestDataFormatterLibccMultiMap.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/multimap/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/multimap/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/multimap/main.cpp
@@ -1,77 +0,0 @@
-#include 
-#include 
-
-#define intint_map std::multimap 
-#define strint_map std::multimap 
-#define intstr_map std::multimap 
-#define strstr_map std::multimap 
-
-int g_the_foo = 0;
-
-int thefoo_rw(int arg = 1)
-{
-	if (arg < 0)
-		arg = 0;
-	if (!arg)
-		arg = 1;
-	g_the_foo += arg;
-	return g_the_foo;
-}
-
-int main()
-{
-intint_map ii;
-
-ii.emplace(0,0); // Set break point at this line.
-ii.emplace(1,1);
-	thefoo_rw(1);  // Set break point at this line.
-ii.emplace(2,0);
-	ii.emplace(3,1);
-	thefoo_rw(1);  // Set break point at this line.
-	ii.emplace(4,0);
-	ii.emplace(5,1);
-	ii.emplace(6,0);
-	ii.emplace(7,1);
-thefoo_rw(1);  // Set break point at this line.
-ii.emplace(85,1234567);
-
-ii.clear();
-
-strint_map si;
-thefoo_rw(1);  // Set break point at this line.
-	
-si.emplace("zero",0);
-	thefoo_rw(1);  // Set break point at this line.
-	si.emplace("one",1);
-	si.emplace("two",2);
-	si.emplace("three",3);
-	thefoo_rw(1);  // Set break point at this line.
-	si.emplace("four",4);
-
-si.clear();
-thefoo_rw(1);  // Set break point at this line.
-	
-intstr_map is;
-thefoo_rw(1);  // Set break point at this line.
-is.emplace(85,"goofy");
-is.emplace(1,"is");
-is.emplace(2,"smart");
-is.emplace(3,"!!!");
-thefoo_rw(1);  // Set break point at this line.
-	
-is.clear();
-thefoo_rw(1);  // Set break point at this line.
-	
-strstr_map ss;
-thefoo_rw(1);  // Set break point at this line.
-	
-ss.emplace("ciao","hello");
-ss.emplace("casa","house");
-ss.emplace("gatto","cat");
-thefoo_rw(1);  // Set break point at this line.
-ss.emplace("a Mac..","..is always a Mac!");
-
-ss.clear();
-thefoo_rw(1);  // Set break point at this line.
-return 0;
-}
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/multimap/TestDataFormatterGenericMultiMap.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/multimap/TestDataFormatterGenericMultiMap.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/multimap/TestDataFormatterGenericMultiMap.py
@@ -10,19 +10,38 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
+USE_LIBSTDCPP = "USE_LIBSTDCPP"
+USE_LIBCPP = "USE_LIBCPP"
 
-class LibcxxMultiMapDataFormatterTestCase(TestBase):
+class GenericMultiMapDataFormatterTestCase(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
 def setUp(self):
 TestBase.setUp(self)
 self.namespace = 'std'
-
-@add_test_categories(["libc++"])
-def test_with_run_command(self):
+
+def findVariable(self, name):
+var = self.frame().FindVariable(name)
+self.assertTrue(var.IsValid())
+return var
+
+def getVariableType(self, name):
+var = self.findVariable(name)
+return var.GetType().GetDisplayTypeName()
+
+def check(self, var_name, size):
+var = self.findVariable(var_name)
+self.assertEqual(var.GetNumChildren(), size)
+children = []
+for i in range(size):
+child = var.GetChildAtIndex(i)
+children.append(ValueCheck(value=child.GetValue()))
+self.expect_var_path(var_name, type=self.getVariableType(var_name), children=children)
+
+def do_test_with_run_command(self, stdlib_type):
 """Test that that file and class static variables display correctly."""
-self.build()
+self.build(dictionary={stdlib_type: "1"})
 self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
 
 bkpt = self.target().FindBreakpointByID(
@@ -65,6 +84,8 @@
 '[0] = (first 

[Lldb-commits] [PATCH] D112586: [lldb] Remove forgotten FIXME on CPlusPlus formatters

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

In D112586#3094519 , @ljmf00 wrote:

> In D112586#3093490 , @teemperor 
> wrote:
>
>> LGTM, thanks.
>
> Note: I can't land it

I can land it for you tomorrow (unless someone beats me to it :) )


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112586

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

[Lldb-commits] [PATCH] D112658: [lldb] Refactor C/C++ string and char summary providers

[Luís Ferreira via Phabricator via lldb-commits]

ljmf00 added a comment.

In D112658#3092586 , @labath wrote:

> Thanks for doing this. Just a couple of small remarks.

Can you re-review?




Comment at: lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp:35
 
-bool lldb_private::formatters::Char8StringSummaryProvider(
-ValueObject , Stream , const TypeSummaryOptions &) {
-  ProcessSP process_sp = valobj.GetProcessSP();
-  if (!process_sp)
-return false;
+namespace {
+

labath wrote:
> We don't use [[ 
> https://llvm.org/docs/CodingStandards.html#anonymous-namespaces | anonymous 
> namespaces]] like this. `static` is sufficient.
Done



Comment at: lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp:76-85
+if (ElemType == StringPrinter::StringElementType::UTF8) {
+  options.SetPrefixToken("u8");
+  valobj.GetValueAsCString(lldb::eFormatUnicode8, value);
+} else if (ElemType == StringPrinter::StringElementType::UTF16) {
+  options.SetPrefixToken("u");
+  valobj.GetValueAsCString(lldb::eFormatUnicode16, value);
+} else if (ElemType == StringPrinter::StringElementType::UTF32) {

labath wrote:
> Maybe a helper function like `pair 
> getElementTraits(StringElementType)` would reduce the repetition further? Or 
> possibly it could be a static array indexed by `ElemType`.
Done. I used `constexpr auto`, I'm not sure if `auto` is a practice here in the 
LLVM codebase or I should explicitly specify the pair type.


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

https://reviews.llvm.org/D112658

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

[Lldb-commits] [PATCH] D112658: [lldb] Refactor C/C++ string and char summary providers

[Luís Ferreira via Phabricator via lldb-commits]

ljmf00 updated this revision to Diff 383121.

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

https://reviews.llvm.org/D112658

Files:
  lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp

Index: lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp
@@ -32,8 +32,25 @@
 using namespace lldb_private;
 using namespace lldb_private::formatters;
 
-bool lldb_private::formatters::Char8StringSummaryProvider(
-ValueObject , Stream , const TypeSummaryOptions &) {
+using StringElementType = StringPrinter::StringElementType;
+
+static constexpr std::pair getElementTraits(StringElementType ElemType)
+{
+  switch(ElemType)
+  {
+case StringElementType::UTF8:
+  return std::make_pair("u8", lldb::eFormatUnicode8);
+case StringElementType::UTF16:
+  return std::make_pair("u", lldb::eFormatUnicode16);
+case StringElementType::UTF32:
+  return std::make_pair("U", lldb::eFormatUnicode32);
+default:
+  return std::make_pair(nullptr, lldb::eFormatInvalid);
+  }
+}
+
+template
+static bool CharStringSummaryProvider(ValueObject , Stream ) {
   ProcessSP process_sp = valobj.GetProcessSP();
   if (!process_sp)
 return false;
@@ -46,65 +63,55 @@
   options.SetLocation(valobj_addr);
   options.SetProcessSP(process_sp);
   options.SetStream();
-  options.SetPrefixToken("u8");
+  options.SetPrefixToken(getElementTraits(ElemType).first);
 
-  if (!StringPrinter::ReadStringAndDumpToStream<
-  StringPrinter::StringElementType::UTF8>(options)) {
+  if (!StringPrinter::ReadStringAndDumpToStream(options))
 stream.Printf("Summary Unavailable");
-return true;
-  }
 
   return true;
 }
 
-bool lldb_private::formatters::Char16StringSummaryProvider(
-ValueObject , Stream , const TypeSummaryOptions &) {
-  ProcessSP process_sp = valobj.GetProcessSP();
-  if (!process_sp)
-return false;
+template
+static bool CharSummaryProvider(ValueObject , Stream ) {
+  DataExtractor data;
+  Status error;
+  valobj.GetData(data, error);
 
-  lldb::addr_t valobj_addr = GetArrayAddressOrPointerValue(valobj);
-  if (valobj_addr == 0 || valobj_addr == LLDB_INVALID_ADDRESS)
+  if (error.Fail())
 return false;
 
-  StringPrinter::ReadStringAndDumpToStreamOptions options(valobj);
-  options.SetLocation(valobj_addr);
-  options.SetProcessSP(process_sp);
-  options.SetStream();
-  options.SetPrefixToken("u");
+  std::string value;
+  StringPrinter::ReadBufferAndDumpToStreamOptions options(valobj);
 
-  if (!StringPrinter::ReadStringAndDumpToStream<
-  StringPrinter::StringElementType::UTF16>(options)) {
-stream.Printf("Summary Unavailable");
-return true;
-  }
+  constexpr auto ElemTraits = getElementTraits(ElemType);
+  valobj.GetValueAsCString(ElemTraits.second, value);
 
-  return true;
-}
+  if (!value.empty())
+stream.Printf("%s ", value.c_str());
 
-bool lldb_private::formatters::Char32StringSummaryProvider(
-ValueObject , Stream , const TypeSummaryOptions &) {
-  ProcessSP process_sp = valobj.GetProcessSP();
-  if (!process_sp)
-return false;
+  options.SetData(std::move(data));
+  options.SetStream();
+  options.SetPrefixToken(ElemTraits.first);
+  options.SetQuote('\'');
+  options.SetSourceSize(1);
+  options.SetBinaryZeroIsTerminator(false);
 
-  lldb::addr_t valobj_addr = GetArrayAddressOrPointerValue(valobj);
-  if (valobj_addr == 0 || valobj_addr == LLDB_INVALID_ADDRESS)
-return false;
+  return StringPrinter::ReadBufferAndDumpToStream(options);
+}
 
-  StringPrinter::ReadStringAndDumpToStreamOptions options(valobj);
-  options.SetLocation(valobj_addr);
-  options.SetProcessSP(process_sp);
-  options.SetStream();
-  options.SetPrefixToken("U");
+bool lldb_private::formatters::Char8StringSummaryProvider(
+ValueObject , Stream , const TypeSummaryOptions &) {
+  return CharStringSummaryProvider(valobj, stream);
+}
 
-  if (!StringPrinter::ReadStringAndDumpToStream<
-  StringPrinter::StringElementType::UTF32>(options)) {
-stream.Printf("Summary Unavailable");
-return true;
-  }
+bool lldb_private::formatters::Char16StringSummaryProvider(
+ValueObject , Stream , const TypeSummaryOptions &) {
+  return CharStringSummaryProvider(valobj, stream);
+}
 
-  return true;
+bool lldb_private::formatters::Char32StringSummaryProvider(
+ValueObject , Stream , const TypeSummaryOptions &) {
+  return CharStringSummaryProvider(valobj, stream);
 }
 
 bool lldb_private::formatters::WCharStringSummaryProvider(
@@ -139,13 +146,13 @@
   switch (wchar_size) {
   case 8:
 return StringPrinter::ReadStringAndDumpToStream<
-StringPrinter::StringElementType::UTF8>(options);
+StringElementType::UTF8>(options);
   case 16:
 return StringPrinter::ReadStringAndDumpToStream<
-

[Lldb-commits] [PATCH] D112586: [lldb] Remove forgotten FIXME on CPlusPlus formatters

[Luís Ferreira via Phabricator via lldb-commits]

ljmf00 added a comment.

In D112586#3093490 , @teemperor wrote:

> LGTM, thanks.

Note: I can't land it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112586

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

[Lldb-commits] [PATCH] D112747: Restore process events after a launch that stopped at the entry point

[Jim Ingham via Phabricator via lldb-commits]

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

We found a bug introduced by changes for the ScriptedProcess.  It only 
manifests itself is you run a process with stop-at-entry set to true and when 
you are using an async debugger and when you are using a remote process.  The 
solution is trivial.  The majority of the patch is a test for stop-at-entry in 
the four cases of "sync & async" debugger crossed with "remote or local" 
process.  The test both tests that stop-at-entry really does stop at entry, and 
then tests that we can resume afterwards.

I wrote the test for darwin because I don't know how to assert that we 
successfully stopped at entry on Linux or Windows.  If someone more familiar 
with those platforms wants to extend the test to either of those hosts, it 
should be straightforward.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112747

Files:
  lldb/source/Target/Process.cpp
  lldb/test/API/functionalities/launch_stop_at_entry/Makefile
  lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py
  lldb/test/API/functionalities/launch_stop_at_entry/main.c

Index: lldb/test/API/functionalities/launch_stop_at_entry/main.c
===
--- /dev/null
+++ lldb/test/API/functionalities/launch_stop_at_entry/main.c
@@ -0,0 +1,5 @@
+int main(int argc, char **argv) {
+  /* We just want to make sure this ran, so
+ it doesn't actually need to do anything. */
+  return 0;
+}
Index: lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py
===
--- /dev/null
+++ lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py
@@ -0,0 +1,165 @@
+import lldb
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+from lldbgdbserverutils import get_debugserver_exe
+
+import os
+import platform
+import shutil
+import time
+import socket
+
+
+class TestStopAtEntry(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+# The port used by debugserver.
+PORT = 54637
+
+# The number of attempts.
+ATTEMPTS = 10
+
+# Time given to the binary to launch and to debugserver to attach to it for
+# every attempt. We'll wait a maximum of 10 times 2 seconds while the
+# inferior will wait 10 times 10 seconds.
+TIMEOUT = 2
+
+def no_debugserver(self):
+if get_debugserver_exe() is None:
+return 'no debugserver'
+return None
+
+def port_not_available(self):
+s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+if s.connect_ex(('127.0.0.1', self.PORT)) == 0:
+return '{} not available'.format(self.PORT)
+return None
+
+@skipUnlessDarwin
+def test_stop_default_platform_sync(self):
+self.do_test_stop_at_entry(True, False)
+
+@skipUnlessDarwin
+def test_stop_default_platform_async(self):
+self.do_test_stop_at_entry(False, False)
+
+@skipUnlessDarwin
+@expectedFailureIfFn(no_debugserver)
+@expectedFailureIfFn(port_not_available)
+def test_stop_remote_platform_sync(self):
+self.do_test_stop_at_entry(True, True)
+
+@skipUnlessDarwin
+@expectedFailureIfFn(no_debugserver)
+@expectedFailureIfFn(port_not_available)
+def test_stop_remote_platform_async(self):
+self.do_test_stop_at_entry(False, True)
+
+def do_test_stop_at_entry(self, synchronous, remote):
+"""Test the normal launch path in either sync or async mode"""
+self.build()
+
+target = lldbutil.run_to_breakpoint_make_target(self)
+launch_info = target.GetLaunchInfo()
+launch_info.SetLaunchFlags(lldb.eLaunchFlagStopAtEntry)
+old_async = self.dbg.GetAsync()
+if not synchronous:
+self.dbg.SetAsync(True)
+listener = lldb.SBListener("test-process-listener")
+mask = listener.StartListeningForEventClass(self.dbg, lldb.SBProcess.GetBroadcasterClassName(), lldb.SBProcess.eBroadcastBitStateChanged)
+self.assertEqual(mask, lldb.SBProcess.eBroadcastBitStateChanged, "Got right mask for listener")
+launch_info.SetListener(listener)
+else:
+self.dbg.SetAsync(False)
+
+if remote:
+self.setup_remote_platform()
+
+error = lldb.SBError()
+
+#self.dbg.HandleCommand("log enable lldb process")
+#self.dbg.HandleCommand("log enable gdb-remote packets process")
+
+process = target.Launch(launch_info, error)
+self.assertTrue(error.Success(), "Launch failed: {0}".format(error.description))
+# If we are synchronous, we have to wait for the events:
+if not synchronous:
+

[Lldb-commits] [PATCH] D111890: [lldb] [Host] Make Terminal methods return llvm::Error

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

Thanks, that sounds like a good idea. Changed D112632 
 to do that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111890

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

[Lldb-commits] [PATCH] D112632: [lldb] [Host/Terminal] Fix warnings with termios disabled

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 383112.
mgorny retitled this revision from "[lldb] [Host/Terminal] Add 
llvm_unreachable() to appease Windows" to "[lldb] [Host/Terminal] Fix warnings 
with termios disabled".
mgorny edited the summary of this revision.
mgorny added a comment.

Create the error for `!LLDB_ENABLE_TERMIOS` via a helper function, as suggested 
by Nico Weber.


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

https://reviews.llvm.org/D112632

Files:
  lldb/source/Host/common/Terminal.cpp

Index: lldb/source/Host/common/Terminal.cpp
===
--- lldb/source/Host/common/Terminal.cpp
+++ lldb/source/Host/common/Terminal.cpp
@@ -29,6 +29,13 @@
 
 bool Terminal::IsATerminal() const { return m_fd >= 0 && ::isatty(m_fd); }
 
+#if !LLDB_ENABLE_TERMIOS
+static llvm::Error termiosMissingError() {
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "termios support missing in LLDB");
+}
+#endif
+
 llvm::Expected Terminal::GetData() {
   if (!FileDescriptorIsValid())
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
@@ -46,8 +53,7 @@
 "unable to get teletype attributes");
   return data;
 #else // !LLDB_ENABLE_TERMIOS
-  return llvm::createStringError(llvm::inconvertibleErrorCode(),
- "termios support missing in LLDB");
+  return termiosMissingError();
 #endif // LLDB_ENABLE_TERMIOS
 }
 
@@ -62,44 +68,48 @@
 "unable to set teletype attributes");
   return llvm::Error::success();
 #else // !LLDB_ENABLE_TERMIOS
-  llvm_unreachable("SetData() should not be called if !LLDB_ENABLE_TERMIOS");
+  return termiosMissingError();
 #endif // LLDB_ENABLE_TERMIOS
 }
 
 llvm::Error Terminal::SetEcho(bool enabled) {
+#if LLDB_ENABLE_TERMIOS
   llvm::Expected data = GetData();
   if (!data)
 return data.takeError();
 
-#if LLDB_ENABLE_TERMIOS
   struct termios _termios = data->m_termios;
   fd_termios.c_lflag &= ~ECHO;
   if (enabled)
 fd_termios.c_lflag |= ECHO;
   return SetData(data.get());
+#else // !LLDB_ENABLE_TERMIOS
+  return termiosMissingError();
 #endif // LLDB_ENABLE_TERMIOS
 }
 
 llvm::Error Terminal::SetCanonical(bool enabled) {
+#if LLDB_ENABLE_TERMIOS
   llvm::Expected data = GetData();
   if (!data)
 return data.takeError();
 
-#if LLDB_ENABLE_TERMIOS
   struct termios _termios = data->m_termios;
   fd_termios.c_lflag &= ~ICANON;
   if (enabled)
 fd_termios.c_lflag |= ICANON;
   return SetData(data.get());
+#else // !LLDB_ENABLE_TERMIOS
+  return termiosMissingError();
 #endif // LLDB_ENABLE_TERMIOS
 }
 
 llvm::Error Terminal::SetRaw() {
+#if LLDB_ENABLE_TERMIOS
   llvm::Expected data = GetData();
   if (!data)
 return data.takeError();
 
-#if LLDB_ENABLE_TERMIOS
   struct termios _termios = data->m_termios;
   ::cfmakeraw(_termios);
 
@@ -109,7 +119,9 @@
   fd_termios.c_cc[VTIME] = 0;
 
   return SetData(data.get());
-#endif // #if LLDB_ENABLE_TERMIOS
+#else // !LLDB_ENABLE_TERMIOS
+  return termiosMissingError();
+#endif // LLDB_ENABLE_TERMIOS
 }
 
 #if LLDB_ENABLE_TERMIOS
@@ -258,11 +270,11 @@
 #endif
 
 llvm::Error Terminal::SetBaudRate(unsigned int baud_rate) {
+#if LLDB_ENABLE_TERMIOS
   llvm::Expected data = GetData();
   if (!data)
 return data.takeError();
 
-#if LLDB_ENABLE_TERMIOS
   struct termios _termios = data->m_termios;
   llvm::Optional val = baudRateToConst(baud_rate);
   if (!val) // invalid value
@@ -278,15 +290,17 @@
 std::error_code(errno, std::generic_category()),
 "setting output baud rate failed");
   return SetData(data.get());
-#endif // #if LLDB_ENABLE_TERMIOS
+#else // !LLDB_ENABLE_TERMIOS
+  return termiosMissingError();
+#endif // LLDB_ENABLE_TERMIOS
 }
 
 llvm::Error Terminal::SetStopBits(unsigned int stop_bits) {
+#if LLDB_ENABLE_TERMIOS
   llvm::Expected data = GetData();
   if (!data)
 return data.takeError();
 
-#if LLDB_ENABLE_TERMIOS
   struct termios _termios = data->m_termios;
   switch (stop_bits) {
   case 1:
@@ -301,15 +315,17 @@
 "invalid stop bit count: %d (must be 1 or 2)", stop_bits);
   }
   return SetData(data.get());
-#endif // #if LLDB_ENABLE_TERMIOS
+#else // !LLDB_ENABLE_TERMIOS
+  return termiosMissingError();
+#endif // LLDB_ENABLE_TERMIOS
 }
 
 llvm::Error Terminal::SetParity(Terminal::Parity parity) {
+#if LLDB_ENABLE_TERMIOS
   llvm::Expected data = GetData();
   if (!data)
 return data.takeError();
 
-#if LLDB_ENABLE_TERMIOS
   struct termios _termios = data->m_termios;
   fd_termios.c_cflag &= ~(
 #if defined(CMSPAR)
@@ -332,15 +348,17 @@
 }
   }
   return SetData(data.get());
-#endif // #if LLDB_ENABLE_TERMIOS
+#else // !LLDB_ENABLE_TERMIOS
+  return termiosMissingError();
+#endif // LLDB_ENABLE_TERMIOS
 }
 
 llvm::Error Terminal::SetParityCheck(Terminal::ParityCheck parity_check) {
+#if LLDB_ENABLE_TERMIOS
   llvm::Expected data = GetData();
   if (!data)
 return data.takeError();
 
-#if 

[Lldb-commits] [PATCH] D112439: normalize file path when searching the source map

[Xu Jun via Phabricator via lldb-commits]

xujuntwt95329 updated this revision to Diff 383083.
xujuntwt95329 added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112439

Files:
  lldb/source/Target/PathMappingList.cpp
  lldb/unittests/Target/CMakeLists.txt
  lldb/unittests/Target/FindFileTest.cpp

Index: lldb/unittests/Target/FindFileTest.cpp
===
--- /dev/null
+++ lldb/unittests/Target/FindFileTest.cpp
@@ -0,0 +1,97 @@
+//===-- FindFileTest.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "TestingSupport/TestUtilities.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Target/PathMappingList.h"
+#include "lldb/Utility/FileSpec.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
+#include "gtest/gtest.h"
+#include 
+
+using namespace llvm;
+using namespace llvm::sys::fs;
+using namespace lldb_private;
+
+namespace {
+struct Matches {
+  FileSpec original;
+  llvm::StringRef remapped;
+  Matches(const char *o, const char *r) : original(o), remapped(r) {}
+  Matches(const char *o, llvm::sys::path::Style style, const char *r)
+  : original(o, style), remapped(r) {}
+};
+
+class FindFileTest : public testing::Test {
+public:
+  void SetUp() override {
+FileSystem::Initialize();
+HostInfo::Initialize();
+  }
+  void TearDown() override {
+HostInfo::Terminate();
+FileSystem::Terminate();
+  }
+};
+} // namespace
+
+static void TestFileFindings(const PathMappingList ,
+ llvm::ArrayRef matches,
+ llvm::ArrayRef fails) {
+  for (const auto  : fails) {
+SCOPED_TRACE(fail.GetCString());
+EXPECT_FALSE(map.FindFile(fail));
+  }
+
+  for (const auto  : matches) {
+SCOPED_TRACE(match.original.GetPath() + " -> " + match.remapped);
+llvm::Optional remapped;
+
+EXPECT_TRUE(bool(remapped = map.FindFile(match.original)));
+EXPECT_TRUE(FileSpec(remapped.getValue()).GetPath() ==
+ConstString(match.remapped).GetStringRef());
+  }
+}
+
+TEST_F(FindFileTest, FindFileTests) {
+  const auto *Info = testing::UnitTest::GetInstance()->current_test_info();
+  llvm::SmallString<128> DirName, FileName;
+  int fd;
+
+  ASSERT_NO_ERROR(createUniqueDirectory(Info->name(), DirName));
+
+  sys::path::append(FileName, Twine(DirName), Twine("test"));
+  ASSERT_NO_ERROR(openFile(FileName, fd, CD_CreateAlways, FA_Read, OF_None));
+
+  llvm::FileRemover dir_remover(DirName);
+  llvm::FileRemover file_remover(FileName);
+  PathMappingList map;
+
+  map.Append(ConstString("/old"), ConstString(DirName.str()), false);
+  map.Append(ConstString(R"(C:\foo)"), ConstString(DirName.str()), false);
+
+  Matches matches[] = {
+  {"/old", llvm::sys::path::Style::posix, DirName.c_str()},
+  {"/old/test", llvm::sys::path::Style::posix, FileName.c_str()},
+  {R"(C:\foo)", llvm::sys::path::Style::windows, DirName.c_str()},
+  {R"(C:\foo\test)", llvm::sys::path::Style::windows, FileName.c_str()}};
+
+  ArrayRef fails{
+  // path not mapped
+  FileSpec("/foo", llvm::sys::path::Style::posix),
+  FileSpec("/new", llvm::sys::path::Style::posix),
+  FileSpec(R"(C:\new)", llvm::sys::path::Style::windows),
+  // path mapped, but file not exist
+  FileSpec("/old/test1", llvm::sys::path::Style::posix),
+  FileSpec(R"(C:\foo\test2)", llvm::sys::path::Style::windows)};
+
+  TestFileFindings(map, matches, fails);
+}
Index: lldb/unittests/Target/CMakeLists.txt
===
--- lldb/unittests/Target/CMakeLists.txt
+++ lldb/unittests/Target/CMakeLists.txt
@@ -7,6 +7,7 @@
   PathMappingListTest.cpp
   RemoteAwarePlatformTest.cpp
   StackFrameRecognizerTest.cpp
+  FindFileTest.cpp
 
   LINK_LIBS
   lldbCore
Index: lldb/source/Target/PathMappingList.cpp
===
--- lldb/source/Target/PathMappingList.cpp
+++ lldb/source/Target/PathMappingList.cpp
@@ -218,7 +218,12 @@
 }
 
 llvm::Optional PathMappingList::FindFile(const FileSpec _spec) const {
-  if (auto remapped = RemapPath(orig_spec.GetPath(), /*only_if_exists=*/true))
+  // We must normalize the orig_spec again using the host's path style,
+  // otherwise there will be mismatch between the host and remote platform
+  // if they use different path styles.
+  if (auto remapped = RemapPath(
+  NormalizePath(ConstString(orig_spec.GetCString())).GetStringRef(),
+  /*only_if_exists=*/true))
 return remapped;
 
   

[Lldb-commits] [lldb] e50f02b - [lldb] [Host/ConnectionFileDescriptor] Refactor to improve code reuse

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-10-28T19:24:10+02:00
New Revision: e50f02ba7ed846f944a369e6738174a5eabfbdcc

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

LOG: [lldb] [Host/ConnectionFileDescriptor] Refactor to improve code reuse

Refactor ConnectionFileDescriptor to improve code reuse for different
types of sockets.  Unify method naming.

While at it, remove some (now-)dead code from Socket.

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

Added: 


Modified: 
lldb/include/lldb/Host/Socket.h
lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
lldb/source/Host/common/Socket.cpp
lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp

Removed: 




diff  --git a/lldb/include/lldb/Host/Socket.h b/lldb/include/lldb/Host/Socket.h
index 9176433df7ce..01f790ee11fb 100644
--- a/lldb/include/lldb/Host/Socket.h
+++ b/lldb/include/lldb/Host/Socket.h
@@ -84,18 +84,6 @@ class Socket : public IOObject {
   static llvm::Expected>
   UdpConnect(llvm::StringRef host_and_port, bool child_processes_inherit);
 
-  static Status UnixDomainConnect(llvm::StringRef host_and_port,
-  bool child_processes_inherit,
-  Socket *);
-  static Status UnixDomainAccept(llvm::StringRef host_and_port,
- bool child_processes_inherit, Socket 
*);
-  static Status UnixAbstractConnect(llvm::StringRef host_and_port,
-bool child_processes_inherit,
-Socket *);
-  static Status UnixAbstractAccept(llvm::StringRef host_and_port,
-   bool child_processes_inherit,
-   Socket *);
-
   int GetOption(int level, int option_name, int _value);
   int SetOption(int level, int option_name, int option_value);
 

diff  --git a/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h 
b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
index 12eab5fc8ae8..35773d5907e9 100644
--- a/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
+++ b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
@@ -16,6 +16,7 @@
 #include "lldb/lldb-forward.h"
 
 #include "lldb/Host/Pipe.h"
+#include "lldb/Host/Socket.h"
 #include "lldb/Utility/Connection.h"
 #include "lldb/Utility/IOObject.h"
 
@@ -73,9 +74,18 @@ class ConnectionFileDescriptor : public Connection {
   void CloseCommandPipe();
 
   lldb::ConnectionStatus
-  SocketListenAndAccept(llvm::StringRef host_and_port,
-socket_id_callback_type socket_id_callback,
-Status *error_ptr);
+  AcceptSocket(Socket::SocketProtocol socket_protocol,
+   llvm::StringRef socket_name,
+   llvm::function_ref post_listen_callback,
+   Status *error_ptr);
+
+  lldb::ConnectionStatus ConnectSocket(Socket::SocketProtocol socket_protocol,
+   llvm::StringRef socket_name,
+   Status *error_ptr);
+
+  lldb::ConnectionStatus AcceptTCP(llvm::StringRef host_and_port,
+   socket_id_callback_type socket_id_callback,
+   Status *error_ptr);
 
   lldb::ConnectionStatus ConnectTCP(llvm::StringRef host_and_port,
 socket_id_callback_type socket_id_callback,
@@ -86,24 +96,24 @@ class ConnectionFileDescriptor : public Connection {
 Status *error_ptr);
 
   lldb::ConnectionStatus
-  NamedSocketConnect(llvm::StringRef socket_name,
+  ConnectNamedSocket(llvm::StringRef socket_name,
  socket_id_callback_type socket_id_callback,
  Status *error_ptr);
 
   lldb::ConnectionStatus
-  NamedSocketAccept(llvm::StringRef socket_name,
+  AcceptNamedSocket(llvm::StringRef socket_name,
 socket_id_callback_type socket_id_callback,
 Status *error_ptr);
 
   lldb::ConnectionStatus
-  UnixAbstractSocketAccept(llvm::StringRef socket_name,
-   socket_id_callback_type socket_id_callback,
-   Status *error_ptr);
+  AcceptAbstractSocket(llvm::StringRef socket_name,
+   socket_id_callback_type socket_id_callback,
+   Status *error_ptr);
 
   lldb::ConnectionStatus
-  UnixAbstractSocketConnect(llvm::StringRef socket_name,
-socket_id_callback_type socket_id_callback,
-Status *error_ptr);
+  ConnectAbstractSocket(llvm::StringRef socket_name,
+socket_id_callback_type socket_id_callback,
+Status *error_ptr);
 
   

[Lldb-commits] [PATCH] D111890: [lldb] [Host] Make Terminal methods return llvm::Error

[Nico Weber via Phabricator via lldb-commits]

thakis added a comment.

In D111890#3090653 , @mgorny wrote:

> In D111890#3090632 , @thakis wrote:
>
>> This causes lots of warnings on Windows, where TERMIOS is false: 
>> http://45.33.8.238/win/47744/step_4.txt (look for "Terminal.cpp"). Could you 
>> take a look? It looks like return statements are genuinely missing there.
>
> When `LLDB_ENABLE_TERMIOS` is false, `GetData()` always returns an error, so 
> the code below should not be reachable. I suppose I could add 
> `llvm_unreachable()` for that. I'll try that and make a diff if it works.

Maybe it'd be easier to read if you move

  return llvm::createStringError(llvm::inconvertibleErrorCode(),
 "termios support missing in LLDB");

into some function and then do

  llvm::Error Terminal::SetCanonical(bool enabled) {
  #if LLDB_ENABLE_TERMIOS
...actual body...
  #else
return ThatErrorFunction();
  #endif
  ``
  
  (etc) in all the functions instead of hiding it in GetData()? Then it's 
obvious in every function what's going on.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111890

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

[Lldb-commits] [PATCH] D112691: Include target settings in "statistics dump" output.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

I understand the need for something like this to make some of the statistics 
more meaningful, but this is stretching the notion of statistics. Conceptually, 
this is approaching something like a dump of the debugger for issue/performance 
analysis. I think that idea is really exciting, and from that perspective 
there's a lot more useful information we could add to it. Long term, I can see 
this output be something that we ask users to include in every bug report.

TL;DR: Can we make this more generic and keep the separation between 
statistics/metrics and a more general concept that includes these target 
settings?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112691

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

[Lldb-commits] [PATCH] D110827: [LLDB] Provide target specific directories to libclang

[Raphael Isemann via Phabricator via lldb-commits]

teemperor requested changes to this revision.
teemperor added a comment.
This revision now requires changes to proceed.

Apologies for the delay, this slipped out of my review queue by accident.

The patch looks in general pretty much ready, I just have a few nits here and 
there.

(Also just to clarify what this code does. It scans the support files of some 
(LLDB) modules and then returns a list of include paths that are needed to 
compile the correct libc++/libc of the target. I fixed up the class docs a bit 
to make this clearer).




Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp:34
 
-bool CppModuleConfiguration::analyzeFile(const FileSpec ) {
+static void getTargetIncludePaths(const llvm::Triple ,
+  std::vector ) {

Could this just return a `llvm::SmallVector` ? Then you can 
just use it directly in the for-range below without any variable.

Some docs would be nice:
```
lang=c++
/// Returns the target-specific default include paths for libc.
```



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp:46
+
+static bool guessIncludePath(llvm::StringRef pathToFile,
+ llvm::StringRef pattern, llvm::StringRef ) 
{

Could this just return `llvm::Optional`? Also the parameter should 
technically be `path_to_file` (this file uses LLDB's `variable_naming_style` 
instead of the `usualNamingStyle`, even though the existing code already has 
some code style issues where it deviated towards LLVM's style but oh well)



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp:46
+
+static bool guessIncludePath(llvm::StringRef pathToFile,
+ llvm::StringRef pattern, llvm::StringRef ) 
{

teemperor wrote:
> Could this just return `llvm::Optional`? Also the parameter 
> should technically be `path_to_file` (this file uses LLDB's 
> `variable_naming_style` instead of the `usualNamingStyle`, even though the 
> existing code already has some code style issues where it deviated towards 
> LLVM's style but oh well)
```
lang=c++
/// Returns the include path matching the given pattern for the given file path 
(or None if the path doesn't match the pattern).
```



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp:78
+
+posix_dir.consume_back("c++/v1");
+llvm::SmallVector tmp;

```
lang=c++
// Check if this is a target-specific libc++ include directory.
```



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp:81
+return m_std_target_inc.TrySet(
+(posix_dir + triple.str() + "/c++/v1").toStringRef(tmp));
   }

`.str()` instead of `.toStringRef(tmp)` should also work.



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.h:46
+  /// If valid, the per-target include path used for the std module.
+  SetOncePath m_std_target_inc;
   /// If valid, the include path to the C library (e.g. /usr/include).

Please add a comment that this is optional unlike the other members:
```/// This is an optional path only required on some systems.```



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.h:51
+  /// (e.g. /usr/include/x86_64-linux-gnu).
+  SetOncePath m_c_target_inc;
   /// The Clang resource include path for this configuration.

Same as above.



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.h:65
   /// Creates a configuration by analyzing the given list of used source files.
-  explicit CppModuleConfiguration(const FileSpecList _files);
+  explicit CppModuleConfiguration(const FileSpecList _files,
+  const llvm::Triple );

```
lang=c++
/// The triple (if valid) is used to search for target-specific include paths.
```



Comment at: lldb/unittests/Expression/CppModuleConfigurationTest.cpp:80
+   libcpp_target));
 }
 

Could you please duplicate this test and make the target-specific directory its 
own new test? E.g. `TEST_F(CppModuleConfigurationTest, 
LinuxTargetSpecificInclude)`. Just to make sure we also still support the old 
behaviour.



Comment at: lldb/unittests/Expression/CppModuleConfigurationTest.cpp:132
   EXPECT_THAT(config.GetIncludeDirs(),
-  testing::ElementsAre(libcpp, ResourceInc(), usr));
+  testing::ElementsAre(libcpp, ResourceInc(), usr, libcpp_target));
 }

Same as above: both the new and old behaviour should work so just copy this 
into a new test.


Repository:
  rG LLVM Github Monorepo

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

[Lldb-commits] [lldb] f5c65be - [lldb][NFC] Improve CppModuleConfiguration documentation a bit

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2021-10-28T16:39:06+02:00
New Revision: f5c65be510430605829f8ccf46b855e39b424d1e

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

LOG: [lldb][NFC] Improve CppModuleConfiguration documentation a bit

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.h

Removed: 




diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.h 
b/lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.h
index 425106bba0a38..907db5d625dcf 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.h
@@ -16,9 +16,9 @@ namespace lldb_private {
 
 /// A Clang configuration when importing C++ modules.
 ///
-/// Includes a list of include paths that should be used when importing
-/// and a list of modules that can be imported. Currently only used when
-/// importing the 'std' module and its dependencies.
+/// This class computes a list of include paths and module names that can be
+/// imported given a list of source files. Currently only used when importing
+/// the 'std' module and its dependencies.
 class CppModuleConfiguration {
   /// Utility class for a path that can only be set once.
   class SetOncePath {



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

[Lldb-commits] [PATCH] D112708: [lldb] Make TypeSystemClang::GetFullyUnqualifiedType work for constant arrays

[Raphael Isemann via Phabricator via lldb-commits]

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

In D112708#3093318 , @labath wrote:

> In D112708#3093252 , @teemperor 
> wrote:
>
>> LGTM, could you also extend a non-unit test to test the change within the 
>> whole FormatManager/etc. setup? I think `TestDataFormatterAdv.py` already 
>> has a very similar section about ignoring cv on types.
>
> I've added a simple check to that test. I didn't want to go overboard, as I 
> already have an made an exhaustive test in the follow-up patch.

Fair, LGTM then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112708

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

[Lldb-commits] [PATCH] D112586: [lldb] Remove forgotten FIXME on CPlusPlus formatters

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added subscribers: labath, teemperor.
teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.

LGTM, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112586

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

Re: [Lldb-commits] [lldb] aee4925 - Recommit: Compress formatting of array type names (int [4] -> int[4])

[Pavel Labath via lldb-commits]

On 26/10/2021 23:14, Jim Ingham via lldb-commits wrote:




On Oct 26, 2021, at 12:45 PM, David Blaikie  wrote:

On Tue, Oct 26, 2021 at 12:15 PM Raphael Isemann  wrote:
Not sure how many LLDB users would (or can) write AST matchers though. And
 (lldb) type summary add "foo[4]" ...
is arguably easier on the eyes than
 type summary add
constantArrayType(allOf(hasSize(4),hasDeclaration(namedDecl(hasName("foo")`

Though presumably (& the example I think Jim was giving) more often the desire 
would be to write a matcher for arrays of any dimension, where it's:

"foo[\d+]" or similar (maybe having to escape the "[]"
compared to
arrayType(hasElementType(hasDeclaration(cxxRecordDecl(hasName("foo")
  
Which is, admittedly, still pretty verbose. But more type checked (for better and worse - annoying to get right (took me ages to figure out I needed "hasDeclaration" in there) but more robust once it is right)


(that's my best shot at a good AST matcher, not sure if we could make
this shorter, but that's probably what a user ends up with).

In this case it seems just comparing tokens instead of direct strings
is enough to make most users happy. And it doesn't require building
clang ASTs (which is always expensive).

Yeah, as much as I like the abstract purity of structural comparison, token 
comparison is probably close enough. (guess that has to involve LLDB walking 
through typedefs incrementally, etc - if typedefs can be used in pretty printer 
matching/searching, otherwise got to go strip out all the typedefs (from the 
users regex (if they are allowed there) and from the type of the thing being 
printed) and other sugar before comparison - guess that's done today somewhere)




The formatter matching does use a type hierarchy if it has one.  We walk the 
chain of typedefs looking for a match, first one wins.  We also match down 
class hierarchies, since if you only format the base class fields, it would be 
annoying to have to write a new formatter for each derived class just to get 
the base class fields printed.

We could also invent some mini-language for the type specification, like "char 
[*]" to mean an array of any size.  But I hesitate to introduce non-language 
features in here because then some other language we need to support will use those 
characters making them ambiguous.  And it's better to rely on facts people already know 
to specify this sort of thing, if possible.  But maybe having a slightly smart tokenizer 
and a few conventions for the common cases might make specifying matches less awkward.

But it doesn't require that we know about then names being matched, and will 
fall back now on either plain string matching or regex-es.



I've recently approached this issue from a slightly different angle 
(trying to format sizeless char arrays (char[]), and I've learned a 
couple of things which might be interesting to this discussion:
- besides typedefs, we also strip cv qualifiers (so a formatter for char 
* applies to const char * as well). However, we didn't to this for array 
types (so one had to include char [.*] and const char [.*] separately. 
(D112708 extends this behavior to arrays too)
- our formatters were not handling cv arrays separately, and the reason 
this worked is because they used unachored regexes, so "char [.*]" 
matched the last part of "const volatile char [47]"
- the unachored matching meant that these formatters were getting 
applied to completely unrelated types as well (e.g. MyStruct). 
(D112709 fixes the second two issues)


While I don't think it's realistic to expect that our users will learn 
how clang ast matchers (or any similar language) work, I think all of 
these examples show that the regex approach has a lot of pitfalls as 
well. It might be good to consider switching to a full-match regexes at 
least, as I think the template trick will affect most formatters, and I 
doubt users will remember to surround their expressions with ^$.


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

[Lldb-commits] [PATCH] D112708: [lldb] Make TypeSystemClang::GetFullyUnqualifiedType work for constant arrays

[Pavel Labath via Phabricator via lldb-commits]

labath marked 2 inline comments as done.
labath added a comment.

In D112708#3093252 , @teemperor wrote:

> LGTM, could you also extend a non-unit test to test the change within the 
> whole FormatManager/etc. setup? I think `TestDataFormatterAdv.py` already has 
> a very similar section about ignoring cv on types.

I've added a simple check to that test. I didn't want to go overboard, as I 
already have an made an exhaustive test in the follow-up patch.




Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:4243
+   ast->getAsConstantArrayType(qual_type)) {
+arr->getElementType();
+qual_type = ast->getConstantArrayType(

teemperor wrote:
> ?
oops


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112708

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

[Lldb-commits] [PATCH] D112708: [lldb] Make TypeSystemClang::GetFullyUnqualifiedType work for constant arrays

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 383009.
labath added a comment.

new version


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112708

Files:
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-advanced/TestDataFormatterAdv.py
  lldb/test/API/functionalities/data-formatter/data-formatter-advanced/main.cpp
  lldb/unittests/Symbol/TestTypeSystemClang.cpp


Index: lldb/unittests/Symbol/TestTypeSystemClang.cpp
===
--- lldb/unittests/Symbol/TestTypeSystemClang.cpp
+++ lldb/unittests/Symbol/TestTypeSystemClang.cpp
@@ -912,6 +912,32 @@
   EXPECT_EQ(method->getDeclName().getObjCSelector().getAsString(), "foo");
 }
 
+TEST_F(TestTypeSystemClang, GetFullyUnqualifiedType) {
+  CompilerType bool_ = m_ast->GetBasicType(eBasicTypeBool);
+  CompilerType cv_bool = bool_.AddConstModifier().AddVolatileModifier();
+
+  // const volatile bool -> bool
+  EXPECT_EQ(bool_, cv_bool.GetFullyUnqualifiedType());
+
+  // const volatile bool[47] -> bool[47]
+  EXPECT_EQ(bool_.GetArrayType(47),
+cv_bool.GetArrayType(47).GetFullyUnqualifiedType());
+
+  // const volatile bool[47][42] -> bool[47][42]
+  EXPECT_EQ(
+  bool_.GetArrayType(42).GetArrayType(47),
+  cv_bool.GetArrayType(42).GetArrayType(47).GetFullyUnqualifiedType());
+
+  // const volatile bool * -> bool *
+  EXPECT_EQ(bool_.GetPointerType(),
+cv_bool.GetPointerType().GetFullyUnqualifiedType());
+
+  // const volatile bool *[47] -> bool *[47]
+  EXPECT_EQ(
+  bool_.GetPointerType().GetArrayType(47),
+  cv_bool.GetPointerType().GetArrayType(47).GetFullyUnqualifiedType());
+}
+
 TEST(TestScratchTypeSystemClang, InferSubASTFromLangOpts) {
   LangOptions lang_opts;
   EXPECT_EQ(
Index: 
lldb/test/API/functionalities/data-formatter/data-formatter-advanced/main.cpp
===
--- 
lldb/test/API/functionalities/data-formatter/data-formatter-advanced/main.cpp
+++ 
lldb/test/API/functionalities/data-formatter/data-formatter-advanced/main.cpp
@@ -144,7 +144,8 @@
 cool_array[2].character = 'Q';
 
 int int_array[] = {1,2,3,4,5};
-
+const int const_int_array[] = {11, 12, 13, 14, 15};
+
 IWrapPointers wrapper;
 
 *int_array = -1;
Index: 
lldb/test/API/functionalities/data-formatter/data-formatter-advanced/TestDataFormatterAdv.py
===
--- 
lldb/test/API/functionalities/data-formatter/data-formatter-advanced/TestDataFormatterAdv.py
+++ 
lldb/test/API/functionalities/data-formatter/data-formatter-advanced/TestDataFormatterAdv.py
@@ -65,10 +65,12 @@
 self.runCmd("type summary clear")
 
 self.runCmd(
-"type summary add --summary-string \"${var[]}\" -x 
\"int\\[[0-9]\\]")
+"type summary add --summary-string \"${var[]}\" -x 
\"^int\\[[0-9]\\]")
 
 self.expect("frame variable int_array",
 substrs=['1,2,3,4,5'])
+self.expect("frame variable const_int_array",
+substrs=['11,12,13,14,15'])
 
 # this will fail if we don't do [] as regex correctly
 self.runCmd(
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -4238,7 +4238,13 @@
   if (qual_type->isPointerType())
 qual_type = ast->getPointerType(
 GetFullyUnqualifiedType_Impl(ast, qual_type->getPointeeType()));
-  else
+  else if (const ConstantArrayType *arr =
+   ast->getAsConstantArrayType(qual_type)) {
+qual_type = ast->getConstantArrayType(
+GetFullyUnqualifiedType_Impl(ast, arr->getElementType()),
+arr->getSize(), arr->getSizeExpr(), arr->getSizeModifier(),
+arr->getIndexTypeQualifiers().getAsOpaqueValue());
+  } else
 qual_type = qual_type.getUnqualifiedType();
   qual_type.removeLocalConst();
   qual_type.removeLocalRestrict();


Index: lldb/unittests/Symbol/TestTypeSystemClang.cpp
===
--- lldb/unittests/Symbol/TestTypeSystemClang.cpp
+++ lldb/unittests/Symbol/TestTypeSystemClang.cpp
@@ -912,6 +912,32 @@
   EXPECT_EQ(method->getDeclName().getObjCSelector().getAsString(), "foo");
 }
 
+TEST_F(TestTypeSystemClang, GetFullyUnqualifiedType) {
+  CompilerType bool_ = m_ast->GetBasicType(eBasicTypeBool);
+  CompilerType cv_bool = bool_.AddConstModifier().AddVolatileModifier();
+
+  // const volatile bool -> bool
+  EXPECT_EQ(bool_, cv_bool.GetFullyUnqualifiedType());
+
+  // const volatile bool[47] -> bool[47]
+  EXPECT_EQ(bool_.GetArrayType(47),
+

[Lldb-commits] [PATCH] D112706: [lldb/test] Allow indentation in inline tests

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG349295fcf37e: [lldb/test] Allow indentation in inline tests 
(authored by labath).

Changed prior to commit:
  https://reviews.llvm.org/D112706?vs=382991=383000#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112706

Files:
  lldb/packages/Python/lldbsuite/test/lldbinline.py
  lldb/test/API/test_utils/TestInlineTest.py


Index: lldb/test/API/test_utils/TestInlineTest.py
===
--- /dev/null
+++ lldb/test/API/test_utils/TestInlineTest.py
@@ -0,0 +1,33 @@
+from lldbsuite.test.lldbinline import CommandParser
+from lldbsuite.test.lldbtest import Base
+import textwrap
+
+
+class TestCommandParser(Base):
+
+mydir = Base.compute_mydir(__file__)
+
+def test_indentation(self):
+"""Test indentation handling"""
+filename = self.getBuildArtifact("test_file.cpp")
+with open(filename, "w") as f:
+f.write(textwrap.dedent("""\
+int q;
+int w; //% first break
+int e;
+int r; //% second break
+//% continue second
+//%   continuing indented
+  //% not indented
+int t; //% third break
+"""))
+p = CommandParser()
+p.parse_source_files([filename])
+
+def bkpt(line, cmd):
+return {'file_name': filename, 'line_number': line, 'command': cmd}
+self.assertEqual(
+p.breakpoints, [
+bkpt(2, 'first break'),
+bkpt(4, 'second break\ncontinue second\n  continuing 
indented\nnot indented'),
+bkpt(8, "third break")])
Index: lldb/packages/Python/lldbsuite/test/lldbinline.py
===
--- lldb/packages/Python/lldbsuite/test/lldbinline.py
+++ lldb/packages/Python/lldbsuite/test/lldbinline.py
@@ -3,6 +3,7 @@
 
 # System modules
 import os
+import textwrap
 
 # Third-party modules
 import io
@@ -38,7 +39,7 @@
 new_breakpoint = True
 
 if len(parts) == 2:
-command = parts[1].strip()  # take off whitespace
+command = parts[1].rstrip()
 new_breakpoint = parts[0].strip() != ""
 
 return (command, new_breakpoint)
@@ -68,6 +69,8 @@
 else:
 current_breakpoint['command'] = current_breakpoint[
 'command'] + "\n" + command
+for bkpt in self.breakpoints:
+bkpt['command'] = textwrap.dedent(bkpt['command'])
 
 def set_breakpoints(self, target):
 for breakpoint in self.breakpoints:


Index: lldb/test/API/test_utils/TestInlineTest.py
===
--- /dev/null
+++ lldb/test/API/test_utils/TestInlineTest.py
@@ -0,0 +1,33 @@
+from lldbsuite.test.lldbinline import CommandParser
+from lldbsuite.test.lldbtest import Base
+import textwrap
+
+
+class TestCommandParser(Base):
+
+mydir = Base.compute_mydir(__file__)
+
+def test_indentation(self):
+"""Test indentation handling"""
+filename = self.getBuildArtifact("test_file.cpp")
+with open(filename, "w") as f:
+f.write(textwrap.dedent("""\
+int q;
+int w; //% first break
+int e;
+int r; //% second break
+//% continue second
+//%   continuing indented
+  //% not indented
+int t; //% third break
+"""))
+p = CommandParser()
+p.parse_source_files([filename])
+
+def bkpt(line, cmd):
+return {'file_name': filename, 'line_number': line, 'command': cmd}
+self.assertEqual(
+p.breakpoints, [
+bkpt(2, 'first break'),
+bkpt(4, 'second break\ncontinue second\n  continuing indented\nnot indented'),
+bkpt(8, "third break")])
Index: lldb/packages/Python/lldbsuite/test/lldbinline.py
===
--- lldb/packages/Python/lldbsuite/test/lldbinline.py
+++ lldb/packages/Python/lldbsuite/test/lldbinline.py
@@ -3,6 +3,7 @@
 
 # System modules
 import os
+import textwrap
 
 # Third-party modules
 import io
@@ -38,7 +39,7 @@
 new_breakpoint = True
 
 if len(parts) == 2:
-command = parts[1].strip()  # take off whitespace
+command = parts[1].rstrip()
 new_breakpoint = parts[0].strip() != ""
 
 return (command, new_breakpoint)
@@ -68,6 +69,8 @@
 else:
 current_breakpoint['command'] = current_breakpoint[
 

[Lldb-commits] [lldb] 349295f - [lldb/test] Allow indentation in inline tests

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2021-10-28T14:39:02+02:00
New Revision: 349295fcf37ed1ff1ea98c18ea1b391741823916

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

LOG: [lldb/test] Allow indentation in inline tests

This makes it possible to use for loops (and other language constructs)
in inline tests.

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

Added: 
lldb/test/API/test_utils/TestInlineTest.py

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

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/lldbinline.py 
b/lldb/packages/Python/lldbsuite/test/lldbinline.py
index 0d1cb24a54dfd..e8d4d4d62c1f6 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbinline.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbinline.py
@@ -3,6 +3,7 @@
 
 # System modules
 import os
+import textwrap
 
 # Third-party modules
 import io
@@ -38,7 +39,7 @@ def parse_one_command(self, line):
 new_breakpoint = True
 
 if len(parts) == 2:
-command = parts[1].strip()  # take off whitespace
+command = parts[1].rstrip()
 new_breakpoint = parts[0].strip() != ""
 
 return (command, new_breakpoint)
@@ -68,6 +69,8 @@ def parse_source_files(self, source_files):
 else:
 current_breakpoint['command'] = current_breakpoint[
 'command'] + "\n" + command
+for bkpt in self.breakpoints:
+bkpt['command'] = textwrap.dedent(bkpt['command'])
 
 def set_breakpoints(self, target):
 for breakpoint in self.breakpoints:

diff  --git a/lldb/test/API/test_utils/TestInlineTest.py 
b/lldb/test/API/test_utils/TestInlineTest.py
new file mode 100644
index 0..21a3e2fd4bf2a
--- /dev/null
+++ b/lldb/test/API/test_utils/TestInlineTest.py
@@ -0,0 +1,33 @@
+from lldbsuite.test.lldbinline import CommandParser
+from lldbsuite.test.lldbtest import Base
+import textwrap
+
+
+class TestCommandParser(Base):
+
+mydir = Base.compute_mydir(__file__)
+
+def test_indentation(self):
+"""Test indentation handling"""
+filename = self.getBuildArtifact("test_file.cpp")
+with open(filename, "w") as f:
+f.write(textwrap.dedent("""\
+int q;
+int w; //% first break
+int e;
+int r; //% second break
+//% continue second
+//%   continuing indented
+  //% not indented
+int t; //% third break
+"""))
+p = CommandParser()
+p.parse_source_files([filename])
+
+def bkpt(line, cmd):
+return {'file_name': filename, 'line_number': line, 'command': cmd}
+self.assertEqual(
+p.breakpoints, [
+bkpt(2, 'first break'),
+bkpt(4, 'second break\ncontinue second\n  continuing 
indented\nnot indented'),
+bkpt(8, "third break")])



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

[Lldb-commits] [PATCH] D112708: [lldb] Make TypeSystemClang::GetFullyUnqualifiedType work for constant arrays

[Raphael Isemann via Phabricator via lldb-commits]

teemperor requested changes to this revision.
teemperor added a comment.
This revision now requires changes to proceed.

LGTM, could you also extend a non-unit test to test the change within the whole 
FormatManager/etc. setup? I think `TestDataFormatterAdv.py` already has a very 
similar section about ignoring cv on types.




Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:4243
+   ast->getAsConstantArrayType(qual_type)) {
+arr->getElementType();
+qual_type = ast->getConstantArrayType(

?



Comment at: lldb/unittests/Symbol/TestTypeSystemClang.cpp:919
+  EXPECT_EQ(bool_, cv_bool.GetFullyUnqualifiedType());
+  EXPECT_EQ(bool_.GetArrayType(47),
+cv_bool.GetArrayType(47).GetFullyUnqualifiedType());

My head has a hard time parsing what those asserts do or test. Maybe add some 
summary above or something similar?
```
lang=c++
// unqualified(const volatile bool[47]) -> bool[47]
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112708

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

[Lldb-commits] [PATCH] D112706: [lldb/test] Allow indentation in inline tests

[Pavel Labath via Phabricator via lldb-commits]

labath marked an inline comment as done.
labath added a comment.

In D112706#3093179 , @teemperor wrote:

> On a more general note: I wish we would just kill off inline tests instead of 
> trying to make them usable. But I think that's a discussion for the mailing 
> lists...

I actually kinda like the inline tests, as they allow you to put the source 
code and the test expectations into the same file (which makes them very 
similar to llvm tests). But they do leave a lot to be desired...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112706

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

[Lldb-commits] [PATCH] D112706: [lldb/test] Allow indentation in inline tests

[Pavel Labath via Phabricator via lldb-commits]

labath marked 2 inline comments as done.
labath added inline comments.



Comment at: lldb/test/API/test_utils/TestInlineTest.py:10
+
+def test(self):
+filename = self.getBuildArtifact("test_file.cpp")

teemperor wrote:
> teemperor wrote:
> > nit: maybe give this a more unique name + a doc string.
> Also I think this should be a NO_DEBUG_INFO_TESTCASE ?
Fun fact: tests which inherit from `Base` (instead of `TestBase`) don't 
automatically get debug info multiplication. (They also don't get a lot of 
other things, which is why this isn't suitable for regular tests, but this 
isn't a regular test.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112706

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

[Lldb-commits] [PATCH] D112709: [lldb] Fix matchers for char array formatters

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: teemperor, jingham.
labath requested review of this revision.
Herald added a project: LLDB.

They were being applied too narrowly (they didn't cover signed char *,
for instance), and too broadly (they covered SomeTemplate) at
the same time.

Depends on D112708 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112709

Files:
  lldb/source/DataFormatters/FormatManager.cpp
  lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp


Index: lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
+++ lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
@@ -6,6 +6,27 @@
   char overflow[4];
 };
 
+#define MAKE_VARS3(c, v, s)
\
+  c v s char c##v##s##chararray[] = #c #v #s "char";   
\
+  c v s char *c##v##s##charstar = c##v##s##chararray
+#define MAKE_VARS2(c, v)   
\
+  MAKE_VARS3(c, v, );  
\
+  MAKE_VARS3(c, v, signed);
\
+  MAKE_VARS3(c, v, unsigned)
+#define MAKE_VARS(c)   
\
+  MAKE_VARS2(c, ); 
\
+  MAKE_VARS2(c, volatile)
+
+MAKE_VARS();
+MAKE_VARS(const);
+
+template
+struct S {
+  int x = 0;
+};
+S Schar5;
+S Scharstar;
+
 int main (int argc, char const *argv[])
 {
 A a, b, c;
@@ -15,7 +36,7 @@
 memcpy(b.data, "FO\0BAR", 7);
 memcpy(c.data, "F\0O\0AR", 7);
 std::string stdstring("Hello\t\tWorld\nI am here\t\tto say hello\n"); 
//%self.addTearDownHook(lambda x: x.runCmd("setting set escape-non-printables 
true"))
-const char* constcharstar = stdstring.c_str();
+const char *charwithtabs = stdstring.c_str();
 std::string longstring(
 "I am a very long string; in fact I am longer than any reasonable length that 
a string should be; quite long indeed; oh my, so many words; so many letters; 
this is kind of like writing a poem; except in real life all that is happening"
 " is just me producing a very very long set of words; there is text here, text 
there, text everywhere; it fills me with glee to see so much text; all around 
me it's just letters, and symbols, and other pleasant drawings that cause me"
@@ -32,15 +53,30 @@
   );
 const char* longconstcharstar = longstring.c_str();
 return 0; //% if self.TraceOn(): self.runCmd('frame variable')
+//%
 //% self.expect_var_path('stdstring', summary='"Hello\\t\\tWorld\\nI am 
here\\t\\tto say hello\\n"')
-//% self.expect_var_path('constcharstar', summary='"Hello\\t\\tWorld\\nI 
am here\\t\\tto say hello\\n"')
+//% self.expect_var_path('charwithtabs', summary='"Hello\\t\\tWorld\\nI am 
here\\t\\tto say hello\\n"')
 //% self.expect_var_path("a.data", summary='"FOOB"')
 //% self.expect_var_path("b.data", summary=r'"FO\0B"')
 //% self.expect_var_path("c.data", summary=r'"F\0O"')
 //%
+//% for c in ["", "const"]:
+//%   for v in ["", "volatile"]:
+//% for s in ["", "signed", "unsigned"]:
+//%   summary = '"'+c+v+s+'char"'
+//%   self.expect_var_path(c+v+s+"chararray", summary=summary)
+//% # These should be printed normally
+//%   self.expect_var_path(c+v+s+"charstar", summary=summary)
+//% Schar5 = self.expect_var_path("Schar5",
+//% children=[ValueCheck(name="x", value="0")])
+//% self.assertIsNone(Schar5.GetSummary())
+//% Scharstar = self.expect_var_path("Scharstar",
+//% children=[ValueCheck(name="x", value="0")])
+//% self.assertIsNone(Scharstar.GetSummary())
+//%
 //% self.runCmd("setting set escape-non-printables false")
 //% self.expect_var_path('stdstring', summary='"Hello\t\tWorld\nI am 
here\t\tto say hello\n"')
-//% self.expect_var_path('constcharstar', summary='"Hello\t\tWorld\nI am 
here\t\tto say hello\n"')
+//% self.expect_var_path('charwithtabs', summary='"Hello\t\tWorld\nI am 
here\t\tto say hello\n"')
 //% 
self.assertTrue(self.frame().FindVariable('longstring').GetSummary().endswith('"...'))
 //% 
self.assertTrue(self.frame().FindVariable('longconstcharstar').GetSummary().endswith('"...'))
 // FIXME: make "b.data" and "c.data" work sanely
Index: lldb/source/DataFormatters/FormatManager.cpp
===
--- lldb/source/DataFormatters/FormatManager.cpp
+++ lldb/source/DataFormatters/FormatManager.cpp
@@ -724,7 +724,7 @@
   lldb::TypeSummaryImplSP string_array_format(
   new StringSummaryFormat(string_array_flags, "${var%char[]}"));
 
-  RegularExpression 

[Lldb-commits] [PATCH] D112706: [lldb/test] Allow indentation in inline tests

[Raphael Isemann via Phabricator via lldb-commits]

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

LGTM, thanks.

On a more general note: I wish we would just kill off inline tests instead of 
trying to make them usable. But I think that's a discussion for the mailing 
lists...




Comment at: lldb/test/API/test_utils/TestInlineTest.py:10
+
+def test(self):
+filename = self.getBuildArtifact("test_file.cpp")

nit: maybe give this a more unique name + a doc string.



Comment at: lldb/test/API/test_utils/TestInlineTest.py:10
+
+def test(self):
+filename = self.getBuildArtifact("test_file.cpp")

teemperor wrote:
> nit: maybe give this a more unique name + a doc string.
Also I think this should be a NO_DEBUG_INFO_TESTCASE ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112706

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

[Lldb-commits] [PATCH] D112708: [lldb] Make TypeSystemClang::GetFullyUnqualifiedType work for constant arrays

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: teemperor, shafik.
labath requested review of this revision.
Herald added a project: LLDB.

Unqualify (constant) arrays recursively, just like we do for pointers.
This allows for better pretty printer matching.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112708

Files:
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/unittests/Symbol/TestTypeSystemClang.cpp


Index: lldb/unittests/Symbol/TestTypeSystemClang.cpp
===
--- lldb/unittests/Symbol/TestTypeSystemClang.cpp
+++ lldb/unittests/Symbol/TestTypeSystemClang.cpp
@@ -912,6 +912,22 @@
   EXPECT_EQ(method->getDeclName().getObjCSelector().getAsString(), "foo");
 }
 
+TEST_F(TestTypeSystemClang, GetFullyUnqualifiedType) {
+  CompilerType bool_ = m_ast->GetBasicType(eBasicTypeBool);
+  CompilerType cv_bool = bool_.AddConstModifier().AddVolatileModifier();
+  EXPECT_EQ(bool_, cv_bool.GetFullyUnqualifiedType());
+  EXPECT_EQ(bool_.GetArrayType(47),
+cv_bool.GetArrayType(47).GetFullyUnqualifiedType());
+  EXPECT_EQ(
+  bool_.GetArrayType(42).GetArrayType(47),
+  cv_bool.GetArrayType(42).GetArrayType(47).GetFullyUnqualifiedType());
+  EXPECT_EQ(bool_.GetPointerType(),
+cv_bool.GetPointerType().GetFullyUnqualifiedType());
+  EXPECT_EQ(
+  bool_.GetPointerType().GetArrayType(47),
+  cv_bool.GetPointerType().GetArrayType(47).GetFullyUnqualifiedType());
+}
+
 TEST(TestScratchTypeSystemClang, InferSubASTFromLangOpts) {
   LangOptions lang_opts;
   EXPECT_EQ(
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -4238,7 +4238,14 @@
   if (qual_type->isPointerType())
 qual_type = ast->getPointerType(
 GetFullyUnqualifiedType_Impl(ast, qual_type->getPointeeType()));
-  else
+  else if (const ConstantArrayType *arr =
+   ast->getAsConstantArrayType(qual_type)) {
+arr->getElementType();
+qual_type = ast->getConstantArrayType(
+GetFullyUnqualifiedType_Impl(ast, arr->getElementType()),
+arr->getSize(), arr->getSizeExpr(), arr->getSizeModifier(),
+arr->getIndexTypeQualifiers().getAsOpaqueValue());
+  } else
 qual_type = qual_type.getUnqualifiedType();
   qual_type.removeLocalConst();
   qual_type.removeLocalRestrict();


Index: lldb/unittests/Symbol/TestTypeSystemClang.cpp
===
--- lldb/unittests/Symbol/TestTypeSystemClang.cpp
+++ lldb/unittests/Symbol/TestTypeSystemClang.cpp
@@ -912,6 +912,22 @@
   EXPECT_EQ(method->getDeclName().getObjCSelector().getAsString(), "foo");
 }
 
+TEST_F(TestTypeSystemClang, GetFullyUnqualifiedType) {
+  CompilerType bool_ = m_ast->GetBasicType(eBasicTypeBool);
+  CompilerType cv_bool = bool_.AddConstModifier().AddVolatileModifier();
+  EXPECT_EQ(bool_, cv_bool.GetFullyUnqualifiedType());
+  EXPECT_EQ(bool_.GetArrayType(47),
+cv_bool.GetArrayType(47).GetFullyUnqualifiedType());
+  EXPECT_EQ(
+  bool_.GetArrayType(42).GetArrayType(47),
+  cv_bool.GetArrayType(42).GetArrayType(47).GetFullyUnqualifiedType());
+  EXPECT_EQ(bool_.GetPointerType(),
+cv_bool.GetPointerType().GetFullyUnqualifiedType());
+  EXPECT_EQ(
+  bool_.GetPointerType().GetArrayType(47),
+  cv_bool.GetPointerType().GetArrayType(47).GetFullyUnqualifiedType());
+}
+
 TEST(TestScratchTypeSystemClang, InferSubASTFromLangOpts) {
   LangOptions lang_opts;
   EXPECT_EQ(
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -4238,7 +4238,14 @@
   if (qual_type->isPointerType())
 qual_type = ast->getPointerType(
 GetFullyUnqualifiedType_Impl(ast, qual_type->getPointeeType()));
-  else
+  else if (const ConstantArrayType *arr =
+   ast->getAsConstantArrayType(qual_type)) {
+arr->getElementType();
+qual_type = ast->getConstantArrayType(
+GetFullyUnqualifiedType_Impl(ast, arr->getElementType()),
+arr->getSize(), arr->getSizeExpr(), arr->getSizeModifier(),
+arr->getIndexTypeQualifiers().getAsOpaqueValue());
+  } else
 qual_type = qual_type.getUnqualifiedType();
   qual_type.removeLocalConst();
   qual_type.removeLocalRestrict();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D112706: [lldb/test] Allow indentation in inline tests

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: teemperor, mgorny.
labath requested review of this revision.
Herald added a project: LLDB.

This makes it possible to use for loops (and other language constructs)
in inline tests.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112706

Files:
  lldb/packages/Python/lldbsuite/test/lldbinline.py
  lldb/test/API/test_utils/TestInlineTest.py


Index: lldb/test/API/test_utils/TestInlineTest.py
===
--- /dev/null
+++ lldb/test/API/test_utils/TestInlineTest.py
@@ -0,0 +1,32 @@
+from lldbsuite.test.lldbinline import CommandParser
+from lldbsuite.test.lldbtest import Base
+import textwrap
+
+
+class TestCommandParser(Base):
+
+mydir = Base.compute_mydir(__file__)
+
+def test(self):
+filename = self.getBuildArtifact("test_file.cpp")
+with open(filename, "w") as f:
+f.write(textwrap.dedent("""\
+int q;
+int w; //% first break
+int e;
+int r; //% second break
+//% continue second
+//%   continuing indented
+  //% not indented
+int t; //% third break
+"""))
+p = CommandParser()
+p.parse_source_files([filename])
+
+def bkpt(line, cmd):
+return {'file_name': filename, 'line_number': line, 'command': cmd}
+self.assertEqual(
+p.breakpoints, [
+bkpt(2, 'first break'),
+bkpt(4, 'second break\ncontinue second\n  continuing 
indented\nnot indented'),
+bkpt(8, "third break")])
Index: lldb/packages/Python/lldbsuite/test/lldbinline.py
===
--- lldb/packages/Python/lldbsuite/test/lldbinline.py
+++ lldb/packages/Python/lldbsuite/test/lldbinline.py
@@ -3,6 +3,7 @@
 
 # System modules
 import os
+import textwrap
 
 # Third-party modules
 import io
@@ -38,7 +39,7 @@
 new_breakpoint = True
 
 if len(parts) == 2:
-command = parts[1].strip()  # take off whitespace
+command = parts[1].rstrip()
 new_breakpoint = parts[0].strip() != ""
 
 return (command, new_breakpoint)
@@ -68,6 +69,8 @@
 else:
 current_breakpoint['command'] = current_breakpoint[
 'command'] + "\n" + command
+for bkpt in self.breakpoints:
+bkpt['command'] = textwrap.dedent(bkpt['command'])
 
 def set_breakpoints(self, target):
 for breakpoint in self.breakpoints:


Index: lldb/test/API/test_utils/TestInlineTest.py
===
--- /dev/null
+++ lldb/test/API/test_utils/TestInlineTest.py
@@ -0,0 +1,32 @@
+from lldbsuite.test.lldbinline import CommandParser
+from lldbsuite.test.lldbtest import Base
+import textwrap
+
+
+class TestCommandParser(Base):
+
+mydir = Base.compute_mydir(__file__)
+
+def test(self):
+filename = self.getBuildArtifact("test_file.cpp")
+with open(filename, "w") as f:
+f.write(textwrap.dedent("""\
+int q;
+int w; //% first break
+int e;
+int r; //% second break
+//% continue second
+//%   continuing indented
+  //% not indented
+int t; //% third break
+"""))
+p = CommandParser()
+p.parse_source_files([filename])
+
+def bkpt(line, cmd):
+return {'file_name': filename, 'line_number': line, 'command': cmd}
+self.assertEqual(
+p.breakpoints, [
+bkpt(2, 'first break'),
+bkpt(4, 'second break\ncontinue second\n  continuing indented\nnot indented'),
+bkpt(8, "third break")])
Index: lldb/packages/Python/lldbsuite/test/lldbinline.py
===
--- lldb/packages/Python/lldbsuite/test/lldbinline.py
+++ lldb/packages/Python/lldbsuite/test/lldbinline.py
@@ -3,6 +3,7 @@
 
 # System modules
 import os
+import textwrap
 
 # Third-party modules
 import io
@@ -38,7 +39,7 @@
 new_breakpoint = True
 
 if len(parts) == 2:
-command = parts[1].strip()  # take off whitespace
+command = parts[1].rstrip()
 new_breakpoint = parts[0].strip() != ""
 
 return (command, new_breakpoint)
@@ -68,6 +69,8 @@
 else:
 current_breakpoint['command'] = current_breakpoint[
 'command'] + "\n" + command
+for bkpt in self.breakpoints:
+bkpt['command'] = textwrap.dedent(bkpt['command'])
 
 def set_breakpoints(self, target):
 for breakpoint in 

[Lldb-commits] [lldb] 5f4980f - [lldb] Remove ConstString from Process, ScriptInterpreter and StructuredData plugin names

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2021-10-28T10:15:03+02:00
New Revision: 5f4980f004f052367b947ff3aa6cc142cea1c23f

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

LOG: [lldb] Remove ConstString from Process, ScriptInterpreter and 
StructuredData plugin names

Added: 


Modified: 
lldb/include/lldb/Core/PluginManager.h
lldb/include/lldb/Target/ProcessTrace.h
lldb/source/Core/IOHandlerCursesGUI.cpp
lldb/source/Core/PluginManager.cpp
lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h
lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h
lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
lldb/source/Plugins/Process/elf-core/ProcessElfCore.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
lldb/source/Plugins/Process/mach-core/ProcessMachCore.h
lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
lldb/source/Plugins/Process/minidump/ProcessMinidump.h
lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
lldb/source/Plugins/Process/scripted/ScriptedProcess.h
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
lldb/source/Plugins/ScriptInterpreter/None/ScriptInterpreterNone.cpp
lldb/source/Plugins/ScriptInterpreter/None/ScriptInterpreterNone.h
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.h
lldb/source/Target/Process.cpp
lldb/source/Target/ProcessTrace.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/PluginManager.h 
b/lldb/include/lldb/Core/PluginManager.h
index e027968c1b4d4..dffceb93ebc63 100644
--- a/lldb/include/lldb/Core/PluginManager.h
+++ b/lldb/include/lldb/Core/PluginManager.h
@@ -230,7 +230,7 @@ class PluginManager {
CompletionRequest );
   // Process
   static bool
-  RegisterPlugin(ConstString name, const char *description,
+  RegisterPlugin(llvm::StringRef name, llvm::StringRef description,
  ProcessCreateInstance create_callback,
  DebuggerInitializeCallback debugger_init_callback = nullptr);
 
@@ -239,17 +239,17 @@ class PluginManager {
   static ProcessCreateInstance GetProcessCreateCallbackAtIndex(uint32_t idx);
 
   static ProcessCreateInstance
-  GetProcessCreateCallbackForPluginName(ConstString name);
+  GetProcessCreateCallbackForPluginName(llvm::StringRef name);
 
-  static const char *GetProcessPluginNameAtIndex(uint32_t idx);
+  static llvm::StringRef GetProcessPluginNameAtIndex(uint32_t idx);
 
-  static const char *GetProcessPluginDescriptionAtIndex(uint32_t idx);
+  static llvm::StringRef GetProcessPluginDescriptionAtIndex(uint32_t idx);
 
   static void AutoCompleteProcessName(llvm::StringRef partial_name,
   CompletionRequest );
 
   // ScriptInterpreter
-  static bool RegisterPlugin(ConstString name, const char *description,
+  static bool RegisterPlugin(llvm::StringRef name, llvm::StringRef description,
  lldb::ScriptLanguage script_lang,
  ScriptInterpreterCreateInstance create_callback);
 
@@ -297,7 +297,7 @@ class PluginManager {
   /// \return
   ///Returns true upon success; otherwise, false.
   static bool
-  RegisterPlugin(ConstString name, const char *description,
+  RegisterPlugin(llvm::StringRef name, llvm::StringRef description,
  StructuredDataPluginCreateInstance create_callback,
  DebuggerInitializeCallback debugger_init_callback = nullptr,
  StructuredDataFilterLaunchInfo filter_callback = nullptr);

diff  --git a/lldb/include/lldb/Target/ProcessTrace.h 
b/lldb/include/lldb/Target/ProcessTrace.h
index 8fa7928910d7d..037dea232cc02 100644
--- a/lldb/include/lldb/Target/ProcessTrace.h
+++ b/lldb/include/lldb/Target/ProcessTrace.h
@@ -23,9 +23,9 @@ class ProcessTrace : public PostMortemProcess {
 
   static void Terminate();
 
-  static ConstString GetPluginNameStatic();
+  static llvm::StringRef GetPluginNameStatic() { return "trace"; }
 
-  static const char *GetPluginDescriptionStatic();
+  static llvm::StringRef GetPluginDescriptionStatic();
 
   

[Lldb-commits] [PATCH] D112629: [lldb] [Host/Socket] Make DecodeHostAndPort() return a dedicated struct

[Michał Górny via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG073c5d0e4706: [lldb] [Host/Socket] Make DecodeHostAndPort() 
return a dedicated struct (authored by mgorny).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D112629?vs=382698=382943#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112629

Files:
  lldb/include/lldb/Host/Socket.h
  lldb/source/Host/common/Socket.cpp
  lldb/source/Host/common/TCPSocket.cpp
  lldb/source/Host/common/UDPSocket.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/tools/lldb-server/Acceptor.cpp
  lldb/unittests/Host/SocketTest.cpp

Index: lldb/unittests/Host/SocketTest.cpp
===
--- lldb/unittests/Host/SocketTest.cpp
+++ lldb/unittests/Host/SocketTest.cpp
@@ -33,65 +33,40 @@
 };
 
 TEST_P(SocketTest, DecodeHostAndPort) {
-  std::string host_str;
-  std::string port_str;
-  uint16_t port;
-
-  EXPECT_THAT_ERROR(
-  Socket::DecodeHostAndPort("localhost:1138", host_str, port_str, port),
-  llvm::Succeeded());
-  EXPECT_STREQ("localhost", host_str.c_str());
-  EXPECT_STREQ("1138", port_str.c_str());
-  EXPECT_EQ(1138, port);
-
-  EXPECT_THAT_ERROR(
-  Socket::DecodeHostAndPort("google.com:65536", host_str, port_str, port),
+  EXPECT_THAT_EXPECTED(Socket::DecodeHostAndPort("localhost:1138"),
+   llvm::HasValue(Socket::HostAndPort{"localhost", 1138}));
+
+  EXPECT_THAT_EXPECTED(
+  Socket::DecodeHostAndPort("google.com:65536"),
   llvm::FailedWithMessage(
   "invalid host:port specification: 'google.com:65536'"));
 
-  EXPECT_THAT_ERROR(
-  Socket::DecodeHostAndPort("google.com:-1138", host_str, port_str, port),
+  EXPECT_THAT_EXPECTED(
+  Socket::DecodeHostAndPort("google.com:-1138"),
   llvm::FailedWithMessage(
   "invalid host:port specification: 'google.com:-1138'"));
 
-  EXPECT_THAT_ERROR(
-  Socket::DecodeHostAndPort("google.com:65536", host_str, port_str, port),
+  EXPECT_THAT_EXPECTED(
+  Socket::DecodeHostAndPort("google.com:65536"),
   llvm::FailedWithMessage(
   "invalid host:port specification: 'google.com:65536'"));
 
-  EXPECT_THAT_ERROR(
-  Socket::DecodeHostAndPort("12345", host_str, port_str, port),
-  llvm::Succeeded());
-  EXPECT_STREQ("", host_str.c_str());
-  EXPECT_STREQ("12345", port_str.c_str());
-  EXPECT_EQ(12345, port);
-
-  EXPECT_THAT_ERROR(Socket::DecodeHostAndPort("*:0", host_str, port_str, port),
-llvm::Succeeded());
-  EXPECT_STREQ("*", host_str.c_str());
-  EXPECT_STREQ("0", port_str.c_str());
-  EXPECT_EQ(0, port);
-
-  EXPECT_THAT_ERROR(
-  Socket::DecodeHostAndPort("*:65535", host_str, port_str, port),
-  llvm::Succeeded());
-  EXPECT_STREQ("*", host_str.c_str());
-  EXPECT_STREQ("65535", port_str.c_str());
-  EXPECT_EQ(65535, port);
-
-  EXPECT_THAT_ERROR(
-  Socket::DecodeHostAndPort("[::1]:12345", host_str, port_str, port),
-  llvm::Succeeded());
-  EXPECT_STREQ("::1", host_str.c_str());
-  EXPECT_STREQ("12345", port_str.c_str());
-  EXPECT_EQ(12345, port);
-
-  EXPECT_THAT_ERROR(Socket::DecodeHostAndPort("[abcd:12fg:AF58::1]:12345",
-  host_str, port_str, port),
-llvm::Succeeded());
-  EXPECT_STREQ("abcd:12fg:AF58::1", host_str.c_str());
-  EXPECT_STREQ("12345", port_str.c_str());
-  EXPECT_EQ(12345, port);
+  EXPECT_THAT_EXPECTED(Socket::DecodeHostAndPort("12345"),
+   llvm::HasValue(Socket::HostAndPort{"", 12345}));
+
+  EXPECT_THAT_EXPECTED(Socket::DecodeHostAndPort("*:0"),
+   llvm::HasValue(Socket::HostAndPort{"*", 0}));
+
+  EXPECT_THAT_EXPECTED(Socket::DecodeHostAndPort("*:65535"),
+   llvm::HasValue(Socket::HostAndPort{"*", 65535}));
+
+  EXPECT_THAT_EXPECTED(
+  Socket::DecodeHostAndPort("[::1]:12345"),
+  llvm::HasValue(Socket::HostAndPort{"::1", 12345}));
+
+  EXPECT_THAT_EXPECTED(
+  Socket::DecodeHostAndPort("[abcd:12fg:AF58::1]:12345"),
+  llvm::HasValue(Socket::HostAndPort{"abcd:12fg:AF58::1", 12345}));
 }
 
 #if LLDB_ENABLE_POSIX
Index: lldb/tools/lldb-server/Acceptor.cpp
===
--- lldb/tools/lldb-server/Acceptor.cpp
+++ lldb/tools/lldb-server/Acceptor.cpp
@@ -92,12 +92,8 @@
 else
   name = name.drop_front(res->scheme.size() + strlen("://"));
   } else {
-std::string host_str;
-std::string port_str;
-uint16_t port;
 // Try to match socket name as $host:port - e.g., localhost:
-if (!llvm::errorToBool(
-Socket::DecodeHostAndPort(name, host_str, port_str, port)))
+if (!llvm::errorToBool(Socket::DecodeHostAndPort(name).takeError()))
   socket_protocol = Socket::ProtocolTcp;
   }
 
Index: 

[Lldb-commits] [lldb] 073c5d0 - [lldb] [Host/Socket] Make DecodeHostAndPort() return a dedicated struct

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-10-28T09:57:50+02:00
New Revision: 073c5d0e4706f14b599f62c7bb115b843d0e4962

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

LOG: [lldb] [Host/Socket] Make DecodeHostAndPort() return a dedicated struct

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

Added: 


Modified: 
lldb/include/lldb/Host/Socket.h
lldb/source/Host/common/Socket.cpp
lldb/source/Host/common/TCPSocket.cpp
lldb/source/Host/common/UDPSocket.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/tools/lldb-server/Acceptor.cpp
lldb/unittests/Host/SocketTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Host/Socket.h b/lldb/include/lldb/Host/Socket.h
index 780c6e6a1..9176433df7cea 100644
--- a/lldb/include/lldb/Host/Socket.h
+++ b/lldb/include/lldb/Host/Socket.h
@@ -47,6 +47,15 @@ class Socket : public IOObject {
 ProtocolUnixAbstract
   };
 
+  struct HostAndPort {
+std::string hostname;
+uint16_t port;
+
+bool operator==(const HostAndPort ) const {
+  return port == R.port && hostname == R.hostname;
+}
+  };
+
   static const NativeSocket kInvalidSocketValue;
 
   ~Socket() override;
@@ -102,9 +111,8 @@ class Socket : public IOObject {
   bool IsValid() const override { return m_socket != kInvalidSocketValue; }
   WaitableHandle GetWaitableHandle() override;
 
-  static llvm::Error DecodeHostAndPort(llvm::StringRef host_and_port,
-   std::string _str,
-   std::string _str, uint16_t );
+  static llvm::Expected
+  DecodeHostAndPort(llvm::StringRef host_and_port);
 
   // If this Socket is connected then return the URI used to connect.
   virtual std::string GetRemoteConnectionURI() const { return ""; };
@@ -129,6 +137,9 @@ class Socket : public IOObject {
   bool m_should_close_fd;
 };
 
+llvm::raw_ostream <<(llvm::raw_ostream ,
+  const Socket::HostAndPort );
+
 } // namespace lldb_private
 
 #endif // LLDB_HOST_SOCKET_H

diff  --git a/lldb/source/Host/common/Socket.cpp 
b/lldb/source/Host/common/Socket.cpp
index 1eaa69532592a..b3fdb933bf739 100644
--- a/lldb/source/Host/common/Socket.cpp
+++ b/lldb/source/Host/common/Socket.cpp
@@ -167,13 +167,6 @@ Socket::TcpListen(llvm::StringRef host_and_port, bool 
child_processes_inherit,
   Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION));
   LLDB_LOG(log, "host_and_port = {0}", host_and_port);
 
-  std::string host_str;
-  std::string port_str;
-  uint16_t port;
-  if (llvm::Error decode_error =
-  DecodeHostAndPort(host_and_port, host_str, port_str, port))
-return std::move(decode_error);
-
   std::unique_ptr listen_socket(
   new TCPSocket(true, child_processes_inherit));
 
@@ -181,12 +174,6 @@ Socket::TcpListen(llvm::StringRef host_and_port, bool 
child_processes_inherit,
   if (error.Fail())
 return error.ToError();
 
-  // We were asked to listen on port zero which means we must now read the
-  // actual port that was given to us as port zero is a special code for
-  // "find an open port for me".
-  if (port == 0)
-port = listen_socket->GetLocalPortNumber();
-
   return std::move(listen_socket);
 }
 
@@ -260,28 +247,22 @@ Status Socket::UnixAbstractAccept(llvm::StringRef name,
   return error;
 }
 
-llvm::Error Socket::DecodeHostAndPort(llvm::StringRef host_and_port,
-  std::string _str,
-  std::string _str, uint16_t ) {
+llvm::Expected Socket::DecodeHostAndPort(llvm::StringRef 
host_and_port) {
   static llvm::Regex g_regex("([^:]+|\\[[0-9a-fA-F:]+.*\\]):([0-9]+)");
+  HostAndPort ret;
   llvm::SmallVector matches;
   if (g_regex.match(host_and_port, )) {
-host_str = matches[1].str();
-port_str = matches[2].str();
+ret.hostname = matches[1].str();
 // IPv6 addresses are wrapped in [] when specified with ports
-if (host_str.front() == '[' && host_str.back() == ']')
-  host_str = host_str.substr(1, host_str.size() - 2);
-if (to_integer(matches[2], port, 10))
-  return llvm::Error::success();
+if (ret.hostname.front() == '[' && ret.hostname.back() == ']')
+  ret.hostname = ret.hostname.substr(1, ret.hostname.size() - 2);
+if (to_integer(matches[2], ret.port, 10))
+  return ret;
   } else {
-// If this was unsuccessful, then check if it's simply a signed 32-bit
+// If this was unsuccessful, then check if it's simply an unsigned 16-bit
 // integer, representing a port with an empty host.
-host_str.clear();
-port_str.clear();
-if (to_integer(host_and_port, port, 10)) {
-  port_str = host_and_port.str();
-  return llvm::Error::success();
-}
+

[Lldb-commits] [PATCH] D112658: [lldb] Refactor C/C++ string and char summary providers

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thanks for doing this. Just a couple of small remarks.




Comment at: lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp:35
 
-bool lldb_private::formatters::Char8StringSummaryProvider(
-ValueObject , Stream , const TypeSummaryOptions &) {
-  ProcessSP process_sp = valobj.GetProcessSP();
-  if (!process_sp)
-return false;
+namespace {
+

We don't use [[ https://llvm.org/docs/CodingStandards.html#anonymous-namespaces 
| anonymous namespaces]] like this. `static` is sufficient.



Comment at: lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp:76-85
+if (ElemType == StringPrinter::StringElementType::UTF8) {
+  options.SetPrefixToken("u8");
+  valobj.GetValueAsCString(lldb::eFormatUnicode8, value);
+} else if (ElemType == StringPrinter::StringElementType::UTF16) {
+  options.SetPrefixToken("u");
+  valobj.GetValueAsCString(lldb::eFormatUnicode16, value);
+} else if (ElemType == StringPrinter::StringElementType::UTF32) {

Maybe a helper function like `pair 
getElementTraits(StringElementType)` would reduce the repetition further? Or 
possibly it could be a static array indexed by `ElemType`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112658

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

[Lldb-commits] [PATCH] D112629: [lldb] [Host/Socket] Make DecodeHostAndPort() return a dedicated struct

[Pavel Labath via Phabricator via lldb-commits]

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



Comment at: lldb/source/Host/common/Socket.cpp:198-199
   } else {
 // If this was unsuccessful, then check if it's simply a signed 32-bit
 // integer, representing a port with an empty host.
+if (to_integer(host_and_port, ret.port, 10))

I guess this isn't true anymore.


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

https://reviews.llvm.org/D112629

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

[Lldb-commits] [PATCH] D112691: Include target settings in "statistics dump" output.

[Greg Clayton via Phabricator via lldb-commits]

clayborg created this revision.
clayborg added reviewers: wallace, aadsm, jingham, JDevlieghere.
clayborg requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Many target settings can be set that can affect how breakpoints are found 
during a debug session. Having these settings along with the statistics are 
important for being able to reproduce issues clients are having.

A new "settings" key/value pair is added to each target dictionary that 
contains settings:

  "settings": {
"target.arg0": "/tmp/a.out",
"target.clang-module-search-paths": [
  "/tmp",
  "/var"
],
"target.debug-file-search-paths": [
  "/tmp",
  "/var"
],
"target.exec-search-paths": [
  "/tmp",
  "/var"
],
"target.inline-breakpoint-strategy": "headers",
"target.preload-symbols": true,
"target.run-args": [
  "a",
  "b",
  "c"
],
"target.skip-prologue": true,
"target.source-map": [
  [
"/tmp",
"/var"
  ]
]
  },


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112691

Files:
  lldb/include/lldb/Core/FileSpecList.h
  lldb/include/lldb/Target/PathMappingList.h
  lldb/source/Core/FileSpecList.cpp
  lldb/source/Target/PathMappingList.cpp
  lldb/source/Target/Statistics.cpp
  lldb/test/API/commands/statistics/basic/TestStats.py

Index: lldb/test/API/commands/statistics/basic/TestStats.py
===
--- lldb/test/API/commands/statistics/basic/TestStats.py
+++ lldb/test/API/commands/statistics/basic/TestStats.py
@@ -1,5 +1,6 @@
 import lldb
 import json
+import os
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
@@ -154,7 +155,7 @@
 target = self.createTestTarget()
 debug_stats = self.get_stats()
 debug_stat_keys = [
-'modules', 
+'modules',
 'targets',
 'totalSymbolTableParseTime',
 'totalSymbolTableIndexTime',
@@ -217,7 +218,7 @@
   lldb.SBFileSpec("main.c"))
 debug_stats = self.get_stats()
 debug_stat_keys = [
-'modules', 
+'modules',
 'targets',
 'totalSymbolTableParseTime',
 'totalSymbolTableIndexTime',
@@ -255,7 +256,7 @@
 target = self.createTestTarget(file_path=exe)
 debug_stats = self.get_stats()
 debug_stat_keys = [
-'modules', 
+'modules',
 'targets',
 'totalSymbolTableParseTime',
 'totalSymbolTableIndexTime',
@@ -315,7 +316,7 @@
 "details": {...},
 "id": 2,
 "resolveTime": 4.363258166997
-}
+}
 ]
 }
 ],
@@ -333,7 +334,7 @@
 self.runCmd("b a_function")
 debug_stats = self.get_stats()
 debug_stat_keys = [
-'modules', 
+'modules',
 'targets',
 'totalSymbolTableParseTime',
 'totalSymbolTableIndexTime',
@@ -363,5 +364,134 @@
 'resolveTime'
 ]
 for breakpoint in breakpoints:
-self.verify_keys(breakpoint, 'target_stats["breakpoints"]', 
+self.verify_keys(breakpoint, 'target_stats["breakpoints"]',
  bp_keys_exist, None)
+
+def test_settings(self):
+"""Test "statistics dump --settings"
+
+Output expected to be something like:
+
+(lldb) statistics dump --settings
+{
+  "targetCreateTime": 0.265668995,
+  "totalBreakpointResolveTime": 0.00314094199,
+  "totalDebugInfoIndexTime": 0,
+  "totalDebugInfoParseTime": 0.000372883001,
+  "totalDebugInfoSize": 2193,
+  "totalSymbolTableIndexTime": 0.0568344889,
+  "totalSymbolTableParseTime": 0.09397942199979
+  "settings": {
+"target.arg0": "/.../commands/target/metrics/TestStats.test_settings/a.out",
+"target.clang-module-search-paths": [],
+"target.debug-file-search-paths": [],
+"target.exec-search-paths": [
+  "/.../commands/target/metrics/TestStats.test_settings"
+],
+"target.inline-breakpoint-strategy": "always",
+"target.preload-symbols": true,
+"target.run-args": [],
+"target.skip-prologue": true,
+"target.source-map": []
+},
+  }
+
+"""
+exe = self.getBuildArtifact("a.out")
+exe_dir = os.path.dirname(exe)
+path1 = self.getSourceDir()
+path2 = exe_dir
+target = self.createTestTarget(file_path=exe)
+