https://github.com/royitaqi updated
https://github.com/llvm/llvm-project/pull/90703
>From 0fd67e2de7e702ce6f7353845454ea7ff9f980d6 Mon Sep 17 00:00:00 2001
From: Roy Shi <roy...@meta.com>
Date: Tue, 30 Apr 2024 21:35:49 -0700
Subject: [PATCH 01/19] Add SBCommandInterpreter::GetTranscript()
---
lldb/include/lldb/API/SBCommandInterpreter.h | 12 +++++++++---
lldb/source/API/SBCommandInterpreter.cpp | 7 ++++++-
2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/lldb/include/lldb/API/SBCommandInterpreter.h
b/lldb/include/lldb/API/SBCommandInterpreter.h
index ba2e049204b8e..d65f06d676f91 100644
--- a/lldb/include/lldb/API/SBCommandInterpreter.h
+++ b/lldb/include/lldb/API/SBCommandInterpreter.h
@@ -247,13 +247,13 @@ class SBCommandInterpreter {
lldb::SBStringList &matches,
lldb::SBStringList &descriptions);
- /// Returns whether an interrupt flag was raised either by the SBDebugger -
+ /// Returns whether an interrupt flag was raised either by the SBDebugger -
/// when the function is not running on the RunCommandInterpreter thread, or
/// by SBCommandInterpreter::InterruptCommand if it is. If your code is
doing
- /// interruptible work, check this API periodically, and interrupt if it
+ /// interruptible work, check this API periodically, and interrupt if it
/// returns true.
bool WasInterrupted() const;
-
+
/// Interrupts the command currently executing in the RunCommandInterpreter
/// thread.
///
@@ -318,6 +318,12 @@ class SBCommandInterpreter {
SBStructuredData GetStatistics();
+ /// Returns a list of handled commands, output and error. Each element in
+ /// the list is a dictionary with three keys: "command" (string), "output"
+ /// (list of strings) and optionally "error" (list of strings). Each string
+ /// in "output" and "error" is a line (without EOL characteres).
+ SBStructuredData GetTranscript();
+
protected:
friend class lldb_private::CommandPluginInterfaceImplementation;
diff --git a/lldb/source/API/SBCommandInterpreter.cpp
b/lldb/source/API/SBCommandInterpreter.cpp
index 83c0951c56db6..242b3f8f09c48 100644
--- a/lldb/source/API/SBCommandInterpreter.cpp
+++ b/lldb/source/API/SBCommandInterpreter.cpp
@@ -150,7 +150,7 @@ bool SBCommandInterpreter::WasInterrupted() const {
bool SBCommandInterpreter::InterruptCommand() {
LLDB_INSTRUMENT_VA(this);
-
+
return (IsValid() ? m_opaque_ptr->InterruptCommand() : false);
}
@@ -571,6 +571,11 @@ SBStructuredData SBCommandInterpreter::GetStatistics() {
return data;
}
+SBStructuredData SBCommandInterpreter::GetTranscript() {
+ LLDB_INSTRUMENT_VA(this);
+ return SBStructuredData();
+}
+
lldb::SBCommand SBCommandInterpreter::AddMultiwordCommand(const char *name,
const char *help) {
LLDB_INSTRUMENT_VA(this, name, help);
>From a1c948ceabaccdc3407e0c4eae0ebc594a9b68b7 Mon Sep 17 00:00:00 2001
From: Roy Shi <roy...@meta.com>
Date: Wed, 1 May 2024 13:45:47 -0700
Subject: [PATCH 02/19] Implement the new API
---
.../lldb/Interpreter/CommandInterpreter.h | 12 +++++--
lldb/include/lldb/Utility/StructuredData.h | 11 +++---
lldb/source/API/SBCommandInterpreter.cpp | 8 ++++-
.../source/Interpreter/CommandInterpreter.cpp | 21 ++++++++++-
lldb/source/Utility/StructuredData.cpp | 35 +++++++++++++++++++
5 files changed, 79 insertions(+), 8 deletions(-)
diff --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h
b/lldb/include/lldb/Interpreter/CommandInterpreter.h
index 70a55a77465bf..9474c41c0dced 100644
--- a/lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -22,6 +22,7 @@
#include "lldb/Utility/Log.h"
#include "lldb/Utility/StreamString.h"
#include "lldb/Utility/StringList.h"
+#include "lldb/Utility/StructuredData.h"
#include "lldb/lldb-forward.h"
#include "lldb/lldb-private.h"
@@ -241,7 +242,7 @@ class CommandInterpreter : public Broadcaster,
eCommandTypesAllThem = 0xFFFF //< all commands
};
- // The CommandAlias and CommandInterpreter both have a hand in
+ // The CommandAlias and CommandInterpreter both have a hand in
// substituting for alias commands. They work by writing special tokens
// in the template form of the Alias command, and then detecting them when
the
// command is executed. These are the special tokens:
@@ -576,7 +577,7 @@ class CommandInterpreter : public Broadcaster,
void SetEchoCommentCommands(bool enable);
bool GetRepeatPreviousCommand() const;
-
+
bool GetRequireCommandOverwrite() const;
const CommandObject::CommandMap &GetUserCommands() const {
@@ -647,6 +648,7 @@ class CommandInterpreter : public Broadcaster,
}
llvm::json::Value GetStatistics();
+ StructuredData::ArraySP GetTranscript() const;
protected:
friend class Debugger;
@@ -766,6 +768,12 @@ class CommandInterpreter : public Broadcaster,
CommandUsageMap m_command_usages;
StreamString m_transcript_stream;
+
+ /// Contains a list of handled commands, output and error. Each element in
+ /// the list is a dictionary with three keys: "command" (string), "output"
+ /// (list of strings) and optionally "error" (list of strings). Each string
+ /// in "output" and "error" is a line (without EOL characteres).
+ StructuredData::ArraySP m_transcript_structured;
};
} // namespace lldb_private
diff --git a/lldb/include/lldb/Utility/StructuredData.h
b/lldb/include/lldb/Utility/StructuredData.h
index 5e63ef92fac3e..72fd035c23e47 100644
--- a/lldb/include/lldb/Utility/StructuredData.h
+++ b/lldb/include/lldb/Utility/StructuredData.h
@@ -290,6 +290,9 @@ class StructuredData {
void GetDescription(lldb_private::Stream &s) const override;
+ static ArraySP SplitString(llvm::StringRef s, char separator, int maxSplit,
+ bool keepEmpty);
+
protected:
typedef std::vector<ObjectSP> collection;
collection m_items;
@@ -366,10 +369,10 @@ class StructuredData {
class String : public Object {
public:
String() : Object(lldb::eStructuredDataTypeString) {}
- explicit String(llvm::StringRef S)
- : Object(lldb::eStructuredDataTypeString), m_value(S) {}
+ explicit String(llvm::StringRef s)
+ : Object(lldb::eStructuredDataTypeString), m_value(s) {}
- void SetValue(llvm::StringRef S) { m_value = std::string(S); }
+ void SetValue(llvm::StringRef s) { m_value = std::string(s); }
llvm::StringRef GetValue() { return m_value; }
@@ -432,7 +435,7 @@ class StructuredData {
}
return success;
}
-
+
template <class IntType>
bool GetValueForKeyAsInteger(llvm::StringRef key, IntType &result) const {
ObjectSP value_sp = GetValueForKey(key);
diff --git a/lldb/source/API/SBCommandInterpreter.cpp
b/lldb/source/API/SBCommandInterpreter.cpp
index 242b3f8f09c48..e96b5a047c64d 100644
--- a/lldb/source/API/SBCommandInterpreter.cpp
+++ b/lldb/source/API/SBCommandInterpreter.cpp
@@ -573,7 +573,13 @@ SBStructuredData SBCommandInterpreter::GetStatistics() {
SBStructuredData SBCommandInterpreter::GetTranscript() {
LLDB_INSTRUMENT_VA(this);
- return SBStructuredData();
+
+ SBStructuredData data;
+ if (!IsValid())
+ return data;
+
+ data.m_impl_up->SetObjectSP(m_opaque_ptr->GetTranscript());
+ return data;
}
lldb::SBCommand SBCommandInterpreter::AddMultiwordCommand(const char *name,
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp
b/lldb/source/Interpreter/CommandInterpreter.cpp
index 4c58ecc3c1848..b5f726d323465 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -51,6 +51,7 @@
#include "lldb/Utility/Log.h"
#include "lldb/Utility/State.h"
#include "lldb/Utility/Stream.h"
+#include "lldb/Utility/StructuredData.h"
#include "lldb/Utility/Timer.h"
#include "lldb/Host/Config.h"
@@ -135,7 +136,8 @@ CommandInterpreter::CommandInterpreter(Debugger &debugger,
m_skip_lldbinit_files(false), m_skip_app_init_files(false),
m_comment_char('#'), m_batch_command_mode(false),
m_truncation_warning(eNoOmission), m_max_depth_warning(eNoOmission),
- m_command_source_depth(0) {
+ m_command_source_depth(0),
+ m_transcript_structured(std::make_shared<StructuredData::Array>()) {
SetEventName(eBroadcastBitThreadShouldExit, "thread-should-exit");
SetEventName(eBroadcastBitResetPrompt, "reset-prompt");
SetEventName(eBroadcastBitQuitCommandReceived, "quit");
@@ -1891,6 +1893,10 @@ bool CommandInterpreter::HandleCommand(const char
*command_line,
m_transcript_stream << "(lldb) " << command_line << '\n';
+ auto transcript_item = std::make_shared<StructuredData::Dictionary>();
+ transcript_item->AddStringItem("command", command_line);
+ m_transcript_structured->AddItem(transcript_item);
+
bool empty_command = false;
bool comment_command = false;
if (command_string.empty())
@@ -2044,6 +2050,15 @@ bool CommandInterpreter::HandleCommand(const char
*command_line,
m_transcript_stream << result.GetOutputData();
m_transcript_stream << result.GetErrorData();
+ // Add output and error to the transcript item after splitting lines. In the
+ // future, other aspects of the command (e.g. perf) can be added, too.
+ transcript_item->AddItem(
+ "output", StructuredData::Array::SplitString(result.GetOutputData(),
'\n',
+ -1, false));
+ transcript_item->AddItem(
+ "error", StructuredData::Array::SplitString(result.GetErrorData(), '\n',
+ -1, false));
+
return result.Succeeded();
}
@@ -3554,3 +3569,7 @@ llvm::json::Value CommandInterpreter::GetStatistics() {
stats.try_emplace(command_usage.getKey(), command_usage.getValue());
return stats;
}
+
+StructuredData::ArraySP CommandInterpreter::GetTranscript() const {
+ return m_transcript_structured;
+}
diff --git a/lldb/source/Utility/StructuredData.cpp
b/lldb/source/Utility/StructuredData.cpp
index 7686d052c599c..278ec93168926 100644
--- a/lldb/source/Utility/StructuredData.cpp
+++ b/lldb/source/Utility/StructuredData.cpp
@@ -10,10 +10,13 @@
#include "lldb/Utility/FileSpec.h"
#include "lldb/Utility/Status.h"
#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringRef.h"
#include "llvm/Support/MemoryBuffer.h"
#include <cerrno>
#include <cinttypes>
#include <cstdlib>
+#include <memory>
+#include <sstream>
using namespace lldb_private;
using namespace llvm;
@@ -289,3 +292,35 @@ void
StructuredData::Null::GetDescription(lldb_private::Stream &s) const {
void StructuredData::Generic::GetDescription(lldb_private::Stream &s) const {
s.Printf("%p", m_object);
}
+
+/// This is the same implementation as `StringRef::split`. Not depending on
+/// `StringRef::split` because it will involve a temporary `SmallVectorImpl`.
+StructuredData::ArraySP StructuredData::Array::SplitString(llvm::StringRef s,
+ char separator,
+ int maxSplit,
+ bool keepEmpty) {
+ auto array_sp = std::make_shared<StructuredData::Array>();
+
+ // Count down from MaxSplit. When MaxSplit is -1, this will just split
+ // "forever". This doesn't support splitting more than 2^31 times
+ // intentionally; if we ever want that we can make MaxSplit a 64-bit integer
+ // but that seems unlikely to be useful.
+ while (maxSplit-- != 0) {
+ size_t idx = s.find(separator);
+ if (idx == llvm::StringLiteral::npos)
+ break;
+
+ // Push this split.
+ if (keepEmpty || idx > 0)
+ array_sp->AddStringItem(s.slice(0, idx));
+
+ // Jump forward.
+ s = s.slice(idx + 1, llvm::StringLiteral::npos);
+ }
+
+ // Push the tail.
+ if (keepEmpty || !s.empty())
+ array_sp->AddStringItem(s);
+
+ return array_sp;
+}
>From efc1c2037da00dacddc3e52812f93377d41d4f82 Mon Sep 17 00:00:00 2001
From: Roy Shi <roy...@meta.com>
Date: Wed, 1 May 2024 14:45:48 -0700
Subject: [PATCH 03/19] Add unittest
---
.../interpreter/TestCommandInterpreterAPI.py | 42 +++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py
b/lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py
index 8f9fbfc255bb0..93d36e3388941 100644
--- a/lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py
+++ b/lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py
@@ -1,5 +1,6 @@
"""Test the SBCommandInterpreter APIs."""
+import json
import lldb
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
@@ -85,3 +86,44 @@ def test_command_output(self):
self.assertEqual(res.GetOutput(), "")
self.assertIsNotNone(res.GetError())
self.assertEqual(res.GetError(), "")
+
+ def test_structured_transcript(self):
+ """Test structured transcript generation and retrieval."""
+ ci = self.dbg.GetCommandInterpreter()
+ self.assertTrue(ci, VALID_COMMAND_INTERPRETER)
+
+ # Send a few commands through the command interpreter
+ res = lldb.SBCommandReturnObject()
+ ci.HandleCommand("version", res)
+ ci.HandleCommand("an-unknown-command", res)
+
+ # Retrieve the transcript and convert it into a Python object
+ transcript = ci.GetTranscript()
+ self.assertTrue(transcript.IsValid())
+
+ stream = lldb.SBStream()
+ self.assertTrue(stream)
+
+ error = transcript.GetAsJSON(stream)
+ self.assertSuccess(error)
+
+ transcript = json.loads(stream.GetData())
+
+ # Validate the transcript.
+ #
+ # Notes:
+ # 1. The following asserts rely on the exact output format of the
+ # commands. Hopefully we are not changing them any time soon.
+ # 2. The transcript will contain a bunch of commands that are run
+ # automatically. We only want to validate for the ones that are
+ # handled in the above, hence the negative indices to find them.
+ self.assertEqual(transcript[-2]["command"], "version")
+ self.assertTrue("lldb version" in transcript[-2]["output"][0])
+ self.assertEqual(transcript[-1],
+ {
+ "command": "an-unknown-command",
+ "output": [],
+ "error": [
+ "error: 'an-unknown-command' is not a valid command.",
+ ],
+ })
>From 6d1190df0ecae0fa49519545526636e84ee9b394 Mon Sep 17 00:00:00 2001
From: Roy Shi <roy...@meta.com>
Date: Wed, 1 May 2024 15:20:38 -0700
Subject: [PATCH 04/19] Add more test asserts and some touch ups
---
.../lldb/Interpreter/CommandInterpreter.h | 2 +-
.../source/Interpreter/CommandInterpreter.cpp | 2 +
lldb/source/Utility/StructuredData.cpp | 2 -
.../interpreter/TestCommandInterpreterAPI.py | 63 ++++++++++++++++---
lldb/test/API/python_api/interpreter/main.c | 5 +-
5 files changed, 60 insertions(+), 14 deletions(-)
diff --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h
b/lldb/include/lldb/Interpreter/CommandInterpreter.h
index 9474c41c0dced..c0846db8f2b8a 100644
--- a/lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -772,7 +772,7 @@ class CommandInterpreter : public Broadcaster,
/// Contains a list of handled commands, output and error. Each element in
/// the list is a dictionary with three keys: "command" (string), "output"
/// (list of strings) and optionally "error" (list of strings). Each string
- /// in "output" and "error" is a line (without EOL characteres).
+ /// in "output" and "error" is a line (without EOL characters).
StructuredData::ArraySP m_transcript_structured;
};
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp
b/lldb/source/Interpreter/CommandInterpreter.cpp
index b5f726d323465..1ec1da437ba3a 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -1893,6 +1893,8 @@ bool CommandInterpreter::HandleCommand(const char
*command_line,
m_transcript_stream << "(lldb) " << command_line << '\n';
+ // The same `transcript_item` will be used below to add output and error of
+ // the command.
auto transcript_item = std::make_shared<StructuredData::Dictionary>();
transcript_item->AddStringItem("command", command_line);
m_transcript_structured->AddItem(transcript_item);
diff --git a/lldb/source/Utility/StructuredData.cpp
b/lldb/source/Utility/StructuredData.cpp
index 278ec93168926..7870334d708fe 100644
--- a/lldb/source/Utility/StructuredData.cpp
+++ b/lldb/source/Utility/StructuredData.cpp
@@ -15,8 +15,6 @@
#include <cerrno>
#include <cinttypes>
#include <cstdlib>
-#include <memory>
-#include <sstream>
using namespace lldb_private;
using namespace llvm;
diff --git a/lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py
b/lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py
index 93d36e3388941..e5cb4a18f7df6 100644
--- a/lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py
+++ b/lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py
@@ -89,6 +89,13 @@ def test_command_output(self):
def test_structured_transcript(self):
"""Test structured transcript generation and retrieval."""
+ # Get command interpreter and create a target
+ self.build()
+ exe = self.getBuildArtifact("a.out")
+
+ target = self.dbg.CreateTarget(exe)
+ self.assertTrue(target, VALID_TARGET)
+
ci = self.dbg.GetCommandInterpreter()
self.assertTrue(ci, VALID_COMMAND_INTERPRETER)
@@ -96,6 +103,10 @@ def test_structured_transcript(self):
res = lldb.SBCommandReturnObject()
ci.HandleCommand("version", res)
ci.HandleCommand("an-unknown-command", res)
+ ci.HandleCommand("breakpoint set -f main.c -l %d" % self.line, res)
+ ci.HandleCommand("r", res)
+ ci.HandleCommand("p a", res)
+ total_number_of_commands = 5
# Retrieve the transcript and convert it into a Python object
transcript = ci.GetTranscript()
@@ -109,17 +120,25 @@ def test_structured_transcript(self):
transcript = json.loads(stream.GetData())
+ # The transcript will contain a bunch of commands that are run
+ # automatically. We only want to validate for the ones that are
+ # listed above, hence trimming to the last parts.
+ transcript = transcript[-total_number_of_commands:]
+
+ print(transcript)
+
# Validate the transcript.
#
- # Notes:
- # 1. The following asserts rely on the exact output format of the
- # commands. Hopefully we are not changing them any time soon.
- # 2. The transcript will contain a bunch of commands that are run
- # automatically. We only want to validate for the ones that are
- # handled in the above, hence the negative indices to find them.
- self.assertEqual(transcript[-2]["command"], "version")
- self.assertTrue("lldb version" in transcript[-2]["output"][0])
- self.assertEqual(transcript[-1],
+ # The following asserts rely on the exact output format of the
+ # commands. Hopefully we are not changing them any time soon.
+
+ # (lldb) version
+ self.assertEqual(transcript[0]["command"], "version")
+ self.assertTrue("lldb version" in transcript[0]["output"][0])
+ self.assertEqual(transcript[0]["error"], [])
+
+ # (lldb) an-unknown-command
+ self.assertEqual(transcript[1],
{
"command": "an-unknown-command",
"output": [],
@@ -127,3 +146,29 @@ def test_structured_transcript(self):
"error: 'an-unknown-command' is not a valid command.",
],
})
+
+ # (lldb) breakpoint set -f main.c -l X
+ self.assertEqual(transcript[2],
+ {
+ "command": "breakpoint set -f main.c -l %d" % self.line,
+ "output": [
+ "Breakpoint 1: where = a.out`main + 29 at main.c:5:5,
address = 0x0000000100000f7d",
+ ],
+ "error": [],
+ })
+
+ # (lldb) r
+ self.assertEqual(transcript[3]["command"], "r")
+ self.assertTrue("Process" in transcript[3]["output"][0])
+ self.assertTrue("launched" in transcript[3]["output"][0])
+ self.assertEqual(transcript[3]["error"], [])
+
+ # (lldb) p a
+ self.assertEqual(transcript[4],
+ {
+ "command": "p a",
+ "output": [
+ "(int) 123",
+ ],
+ "error": [],
+ })
diff --git a/lldb/test/API/python_api/interpreter/main.c
b/lldb/test/API/python_api/interpreter/main.c
index 277aa54a4eea5..366ffde5fdef5 100644
--- a/lldb/test/API/python_api/interpreter/main.c
+++ b/lldb/test/API/python_api/interpreter/main.c
@@ -1,6 +1,7 @@
#include <stdio.h>
int main(int argc, char const *argv[]) {
- printf("Hello world.\n");
- return 0;
+ int a = 123;
+ printf("Hello world.\n");
+ return 0;
}
>From 26a726b2f94713ef8508049115ab93ee91e9a836 Mon Sep 17 00:00:00 2001
From: royitaqi <royit...@users.noreply.github.com>
Date: Wed, 1 May 2024 15:27:33 -0700
Subject: [PATCH 05/19] Apply suggestions from code review
Co-authored-by: Med Ismail Bennani <ism...@bennani.ma>
---
lldb/source/API/SBCommandInterpreter.cpp | 6 ++----
lldb/source/Utility/StructuredData.cpp | 4 ++--
2 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/lldb/source/API/SBCommandInterpreter.cpp
b/lldb/source/API/SBCommandInterpreter.cpp
index e96b5a047c64d..233a2f97fb9f1 100644
--- a/lldb/source/API/SBCommandInterpreter.cpp
+++ b/lldb/source/API/SBCommandInterpreter.cpp
@@ -575,10 +575,8 @@ SBStructuredData SBCommandInterpreter::GetTranscript() {
LLDB_INSTRUMENT_VA(this);
SBStructuredData data;
- if (!IsValid())
- return data;
-
- data.m_impl_up->SetObjectSP(m_opaque_ptr->GetTranscript());
+ if (IsValid())
+ data.m_impl_up->SetObjectSP(m_opaque_ptr->GetTranscript());
return data;
}
diff --git a/lldb/source/Utility/StructuredData.cpp
b/lldb/source/Utility/StructuredData.cpp
index 7870334d708fe..4ca804cb76a74 100644
--- a/lldb/source/Utility/StructuredData.cpp
+++ b/lldb/source/Utility/StructuredData.cpp
@@ -299,9 +299,9 @@ StructuredData::ArraySP
StructuredData::Array::SplitString(llvm::StringRef s,
bool keepEmpty) {
auto array_sp = std::make_shared<StructuredData::Array>();
- // Count down from MaxSplit. When MaxSplit is -1, this will just split
+ // Count down from `maxSplit`. When `maxSplit` is -1, this will just split
// "forever". This doesn't support splitting more than 2^31 times
- // intentionally; if we ever want that we can make MaxSplit a 64-bit integer
+ // intentionally; if we ever want that we can make `maxSplit` a 64-bit
integer
// but that seems unlikely to be useful.
while (maxSplit-- != 0) {
size_t idx = s.find(separator);
>From 52a310b8c236d252233b6e49de48a0c53eab9f45 Mon Sep 17 00:00:00 2001
From: Roy Shi <roy...@meta.com>
Date: Wed, 1 May 2024 16:07:44 -0700
Subject: [PATCH 06/19] Move and add document for Array::SplitString
---
lldb/include/lldb/Utility/StructuredData.h | 25 ++++++++++++++++++++--
lldb/source/Utility/StructuredData.cpp | 2 --
2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/lldb/include/lldb/Utility/StructuredData.h
b/lldb/include/lldb/Utility/StructuredData.h
index 72fd035c23e47..69db0caca2051 100644
--- a/lldb/include/lldb/Utility/StructuredData.h
+++ b/lldb/include/lldb/Utility/StructuredData.h
@@ -290,8 +290,29 @@ class StructuredData {
void GetDescription(lldb_private::Stream &s) const override;
- static ArraySP SplitString(llvm::StringRef s, char separator, int maxSplit,
- bool keepEmpty);
+ /// Creates an Array of substrings by splitting a string around the
occurrences of a separator character.
+ ///
+ /// Note:
+ /// * This is almost the same API and implementation as `StringRef::split`.
+ /// * Not depending on `StringRef::split` because it will involve a
+ /// temporary `SmallVectorImpl`.
+ ///
+ /// \param[in] s
+ /// The input string.
+ ///
+ /// \param[in] separator
+ /// The character to split on.
+ ///
+ /// \param[in] maxSplit
+ /// The maximum number of times the string is split. If \a maxSplit is
>= 0, at most \a maxSplit splits are done and consequently <= \a maxSplit + 1
elements are returned.
+ ///
+ /// \param[in] keepEmpty
+ /// True if empty substrings should be returned. Empty substrings still
count when considering \a maxSplit.
+ ///
+ /// \return
+ /// An array containing the substrings. If \a maxSplit == -1 and \a
keepEmpty == true, then the concatination of the array forms the input string.
+ static ArraySP SplitString(llvm::StringRef s, char separator, int maxSplit
= -1,
+ bool keepEmpty = true);
protected:
typedef std::vector<ObjectSP> collection;
diff --git a/lldb/source/Utility/StructuredData.cpp
b/lldb/source/Utility/StructuredData.cpp
index 4ca804cb76a74..7fa1063e5f01f 100644
--- a/lldb/source/Utility/StructuredData.cpp
+++ b/lldb/source/Utility/StructuredData.cpp
@@ -291,8 +291,6 @@ void
StructuredData::Generic::GetDescription(lldb_private::Stream &s) const {
s.Printf("%p", m_object);
}
-/// This is the same implementation as `StringRef::split`. Not depending on
-/// `StringRef::split` because it will involve a temporary `SmallVectorImpl`.
StructuredData::ArraySP StructuredData::Array::SplitString(llvm::StringRef s,
char separator,
int maxSplit,
>From 9beff0b2fdbac700f2aec6047ea90356ffecbce7 Mon Sep 17 00:00:00 2001
From: Roy Shi <roy...@meta.com>
Date: Wed, 1 May 2024 16:53:25 -0700
Subject: [PATCH 07/19] Add unit test for Array::SplitString
---
lldb/unittests/Utility/StructuredDataTest.cpp | 63 +++++++++++++++++++
1 file changed, 63 insertions(+)
diff --git a/lldb/unittests/Utility/StructuredDataTest.cpp
b/lldb/unittests/Utility/StructuredDataTest.cpp
index e536039f365a4..f107490946334 100644
--- a/lldb/unittests/Utility/StructuredDataTest.cpp
+++ b/lldb/unittests/Utility/StructuredDataTest.cpp
@@ -12,6 +12,7 @@
#include "lldb/Utility/Status.h"
#include "lldb/Utility/StreamString.h"
#include "lldb/Utility/StructuredData.h"
+#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Path.h"
using namespace lldb;
@@ -112,3 +113,65 @@ TEST(StructuredDataTest, ParseJSONFromFile) {
object_sp->Dump(S, false);
EXPECT_EQ("[1,2,3]", S.GetString());
}
+
+struct ArraySplitStringTestCase {
+ llvm::StringRef s;
+ char separator;
+ int maxSplit;
+ bool keepEmpty;
+ std::vector<std::string> expected;
+};
+
+TEST(StructuredDataTest, ArraySplitString) {
+ ArraySplitStringTestCase test_cases[] = {
+ // Happy path
+ {
+ "1,2,,3",
+ ',',
+ -1,
+ true,
+ {"1", "2", "", "3"},
+ },
+ // No splits
+ {
+ "1,2,,3",
+ ',',
+ 0,
+ true,
+ {"1,2,,3"},
+ },
+ // 1 split
+ {
+ "1,2,,3",
+ ',',
+ 1,
+ true,
+ {"1", "2,,3"},
+ },
+ // No empty substrings
+ {
+ "1,2,,3",
+ ',',
+ -1,
+ false,
+ {"1", "2", "3"},
+ },
+ // Empty substrings count towards splits
+ {
+ ",1,2,3",
+ ',',
+ 1,
+ false,
+ {"1,2,3"},
+ },
+ };
+ for (const auto &test_case : test_cases) {
+ auto array = StructuredData::Array::SplitString(
+ test_case.s, test_case.separator, test_case.maxSplit,
+ test_case.keepEmpty);
+ EXPECT_EQ(test_case.expected.size(), array->GetSize());
+ for (unsigned int i = 0; i < test_case.expected.size(); ++i) {
+ EXPECT_EQ(test_case.expected[i],
array->GetItemAtIndexAsString(i)->str());
+ }
+ }
+}
>From 6c6c5c272511f8c62c7ef14eefe67904875b4d2c Mon Sep 17 00:00:00 2001
From: Roy Shi <roy...@meta.com>
Date: Wed, 1 May 2024 17:07:12 -0700
Subject: [PATCH 08/19] Fix format
---
lldb/include/lldb/Utility/StructuredData.h | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/lldb/include/lldb/Utility/StructuredData.h
b/lldb/include/lldb/Utility/StructuredData.h
index 69db0caca2051..8217d2bf33b80 100644
--- a/lldb/include/lldb/Utility/StructuredData.h
+++ b/lldb/include/lldb/Utility/StructuredData.h
@@ -290,7 +290,8 @@ class StructuredData {
void GetDescription(lldb_private::Stream &s) const override;
- /// Creates an Array of substrings by splitting a string around the
occurrences of a separator character.
+ /// Creates an Array of substrings by splitting a string around the
+ /// occurrences of a separator character.
///
/// Note:
/// * This is almost the same API and implementation as `StringRef::split`.
@@ -304,15 +305,20 @@ class StructuredData {
/// The character to split on.
///
/// \param[in] maxSplit
- /// The maximum number of times the string is split. If \a maxSplit is
>= 0, at most \a maxSplit splits are done and consequently <= \a maxSplit + 1
elements are returned.
+ /// The maximum number of times the string is split. If \a maxSplit is >=
+ /// 0, at most \a maxSplit splits are done and consequently <= \a
maxSplit
+ /// + 1 elements are returned.
///
/// \param[in] keepEmpty
- /// True if empty substrings should be returned. Empty substrings still
count when considering \a maxSplit.
+ /// True if empty substrings should be returned. Empty substrings still
+ /// count when considering \a maxSplit.
///
/// \return
- /// An array containing the substrings. If \a maxSplit == -1 and \a
keepEmpty == true, then the concatination of the array forms the input string.
- static ArraySP SplitString(llvm::StringRef s, char separator, int maxSplit
= -1,
- bool keepEmpty = true);
+ /// An array containing the substrings. If \a maxSplit == -1 and \a
+ /// keepEmpty == true, then the concatination of the array forms the
input
+ /// string.
+ static ArraySP SplitString(llvm::StringRef s, char separator,
+ int maxSplit = -1, bool keepEmpty = true);
protected:
typedef std::vector<ObjectSP> collection;
>From fbf8e9ab4cfe232218edb1eb4fe0c22d9a68f4a9 Mon Sep 17 00:00:00 2001
From: Roy Shi <roy...@meta.com>
Date: Wed, 1 May 2024 17:23:14 -0700
Subject: [PATCH 09/19] Improve Python API test reliability
---
.../interpreter/TestCommandInterpreterAPI.py | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py
b/lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py
index e5cb4a18f7df6..5bb9f579ad13f 100644
--- a/lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py
+++ b/lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py
@@ -125,8 +125,6 @@ def test_structured_transcript(self):
# listed above, hence trimming to the last parts.
transcript = transcript[-total_number_of_commands:]
- print(transcript)
-
# Validate the transcript.
#
# The following asserts rely on the exact output format of the
@@ -147,18 +145,15 @@ def test_structured_transcript(self):
],
})
- # (lldb) breakpoint set -f main.c -l X
- self.assertEqual(transcript[2],
- {
- "command": "breakpoint set -f main.c -l %d" % self.line,
- "output": [
- "Breakpoint 1: where = a.out`main + 29 at main.c:5:5,
address = 0x0000000100000f7d",
- ],
- "error": [],
- })
+ # (lldb) breakpoint set -f main.c -l <line>
+ self.assertEqual(transcript[2]["command"], "breakpoint set -f main.c
-l %d" % self.line)
+ # Breakpoint 1: where = a.out`main + 29 at main.c:5:3, address =
0x0000000100000f7d
+ self.assertTrue("Breakpoint 1: where = a.out`main + 29 at main.c:5:3,
address =" in transcript[2]["output"][0])
+ self.assertEqual(transcript[2]["error"], [])
# (lldb) r
self.assertEqual(transcript[3]["command"], "r")
+ # Process 25494 launched:
'<path>/TestCommandInterpreterAPI.test_structured_transcript/a.out' (x86_64)
self.assertTrue("Process" in transcript[3]["output"][0])
self.assertTrue("launched" in transcript[3]["output"][0])
self.assertEqual(transcript[3]["error"], [])
>From 836a719f0f2124627ebb0ea625486bcc8cdf3cd5 Mon Sep 17 00:00:00 2001
From: Roy Shi <roy...@meta.com>
Date: Thu, 2 May 2024 21:08:46 -0700
Subject: [PATCH 10/19] Update implementation of Array::SplitString, and other
smaller changes
---
lldb/include/lldb/API/SBCommandInterpreter.h | 2 +-
.../lldb/Interpreter/CommandInterpreter.h | 2 +-
lldb/include/lldb/Utility/StructuredData.h | 11 ++------
.../source/Interpreter/CommandInterpreter.cpp | 6 ++--
lldb/source/Utility/StructuredData.cpp | 28 +++++--------------
5 files changed, 15 insertions(+), 34 deletions(-)
diff --git a/lldb/include/lldb/API/SBCommandInterpreter.h
b/lldb/include/lldb/API/SBCommandInterpreter.h
index d65f06d676f91..f56f7d844b0d1 100644
--- a/lldb/include/lldb/API/SBCommandInterpreter.h
+++ b/lldb/include/lldb/API/SBCommandInterpreter.h
@@ -321,7 +321,7 @@ class SBCommandInterpreter {
/// Returns a list of handled commands, output and error. Each element in
/// the list is a dictionary with three keys: "command" (string), "output"
/// (list of strings) and optionally "error" (list of strings). Each string
- /// in "output" and "error" is a line (without EOL characteres).
+ /// in "output" and "error" is a line (without EOL characters).
SBStructuredData GetTranscript();
protected:
diff --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h
b/lldb/include/lldb/Interpreter/CommandInterpreter.h
index c0846db8f2b8a..0938ad6ae78ab 100644
--- a/lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -773,7 +773,7 @@ class CommandInterpreter : public Broadcaster,
/// the list is a dictionary with three keys: "command" (string), "output"
/// (list of strings) and optionally "error" (list of strings). Each string
/// in "output" and "error" is a line (without EOL characters).
- StructuredData::ArraySP m_transcript_structured;
+ StructuredData::ArraySP m_transcript;
};
} // namespace lldb_private
diff --git a/lldb/include/lldb/Utility/StructuredData.h
b/lldb/include/lldb/Utility/StructuredData.h
index 8217d2bf33b80..563eb1b1a284a 100644
--- a/lldb/include/lldb/Utility/StructuredData.h
+++ b/lldb/include/lldb/Utility/StructuredData.h
@@ -293,11 +293,6 @@ class StructuredData {
/// Creates an Array of substrings by splitting a string around the
/// occurrences of a separator character.
///
- /// Note:
- /// * This is almost the same API and implementation as `StringRef::split`.
- /// * Not depending on `StringRef::split` because it will involve a
- /// temporary `SmallVectorImpl`.
- ///
/// \param[in] s
/// The input string.
///
@@ -396,10 +391,10 @@ class StructuredData {
class String : public Object {
public:
String() : Object(lldb::eStructuredDataTypeString) {}
- explicit String(llvm::StringRef s)
- : Object(lldb::eStructuredDataTypeString), m_value(s) {}
+ explicit String(llvm::StringRef S)
+ : Object(lldb::eStructuredDataTypeString), m_value(S) {}
- void SetValue(llvm::StringRef s) { m_value = std::string(s); }
+ void SetValue(llvm::StringRef S) { m_value = std::string(S); }
llvm::StringRef GetValue() { return m_value; }
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp
b/lldb/source/Interpreter/CommandInterpreter.cpp
index 1ec1da437ba3a..d5e37dee186a6 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -137,7 +137,7 @@ CommandInterpreter::CommandInterpreter(Debugger &debugger,
m_comment_char('#'), m_batch_command_mode(false),
m_truncation_warning(eNoOmission), m_max_depth_warning(eNoOmission),
m_command_source_depth(0),
- m_transcript_structured(std::make_shared<StructuredData::Array>()) {
+ m_transcript(std::make_shared<StructuredData::Array>()) {
SetEventName(eBroadcastBitThreadShouldExit, "thread-should-exit");
SetEventName(eBroadcastBitResetPrompt, "reset-prompt");
SetEventName(eBroadcastBitQuitCommandReceived, "quit");
@@ -1897,7 +1897,7 @@ bool CommandInterpreter::HandleCommand(const char
*command_line,
// the command.
auto transcript_item = std::make_shared<StructuredData::Dictionary>();
transcript_item->AddStringItem("command", command_line);
- m_transcript_structured->AddItem(transcript_item);
+ m_transcript->AddItem(transcript_item);
bool empty_command = false;
bool comment_command = false;
@@ -3573,5 +3573,5 @@ llvm::json::Value CommandInterpreter::GetStatistics() {
}
StructuredData::ArraySP CommandInterpreter::GetTranscript() const {
- return m_transcript_structured;
+ return m_transcript;
}
diff --git a/lldb/source/Utility/StructuredData.cpp
b/lldb/source/Utility/StructuredData.cpp
index 7fa1063e5f01f..e5f9a69fab2ab 100644
--- a/lldb/source/Utility/StructuredData.cpp
+++ b/lldb/source/Utility/StructuredData.cpp
@@ -295,28 +295,14 @@ StructuredData::ArraySP
StructuredData::Array::SplitString(llvm::StringRef s,
char separator,
int maxSplit,
bool keepEmpty) {
- auto array_sp = std::make_shared<StructuredData::Array>();
+ // Split the string into a small vector.
+ llvm::SmallVector<StringRef> small_vec;
+ s.split(small_vec, separator, maxSplit, keepEmpty);
- // Count down from `maxSplit`. When `maxSplit` is -1, this will just split
- // "forever". This doesn't support splitting more than 2^31 times
- // intentionally; if we ever want that we can make `maxSplit` a 64-bit
integer
- // but that seems unlikely to be useful.
- while (maxSplit-- != 0) {
- size_t idx = s.find(separator);
- if (idx == llvm::StringLiteral::npos)
- break;
-
- // Push this split.
- if (keepEmpty || idx > 0)
- array_sp->AddStringItem(s.slice(0, idx));
-
- // Jump forward.
- s = s.slice(idx + 1, llvm::StringLiteral::npos);
+ // Copy the substrings from the small vector into the output array.
+ auto array_sp = std::make_shared<StructuredData::Array>();
+ for (auto substring : small_vec) {
+ array_sp->AddStringItem(std::move(substring));
}
-
- // Push the tail.
- if (keepEmpty || !s.empty())
- array_sp->AddStringItem(s);
-
return array_sp;
}
>From 6f82462d7dff7c5d97c58a84b4eac9aca8d7fc99 Mon Sep 17 00:00:00 2001
From: Roy Shi <roy...@meta.com>
Date: Mon, 6 May 2024 10:47:47 -0700
Subject: [PATCH 11/19] Update comment, more reliable test, minor fix
---
lldb/source/Utility/StructuredData.cpp | 2 +-
.../interpreter/TestCommandInterpreterAPI.py | 15 +++++++++++----
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/lldb/source/Utility/StructuredData.cpp
b/lldb/source/Utility/StructuredData.cpp
index e5f9a69fab2ab..f4e87b7167a1f 100644
--- a/lldb/source/Utility/StructuredData.cpp
+++ b/lldb/source/Utility/StructuredData.cpp
@@ -302,7 +302,7 @@ StructuredData::ArraySP
StructuredData::Array::SplitString(llvm::StringRef s,
// Copy the substrings from the small vector into the output array.
auto array_sp = std::make_shared<StructuredData::Array>();
for (auto substring : small_vec) {
- array_sp->AddStringItem(std::move(substring));
+ array_sp->AddStringItem(substring);
}
return array_sp;
}
diff --git a/lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py
b/lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py
index 5bb9f579ad13f..54179892eabda 100644
--- a/lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py
+++ b/lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py
@@ -120,9 +120,16 @@ def test_structured_transcript(self):
transcript = json.loads(stream.GetData())
- # The transcript will contain a bunch of commands that are run
- # automatically. We only want to validate for the ones that are
- # listed above, hence trimming to the last parts.
+ print('TRANSCRIPT')
+ print(transcript)
+
+ # The transcript will contain a bunch of commands that are from
+ # a general setup code. See `def setUpCommands(cls)` in
+ # `lldb/packages/Python/lldbsuite/test/lldbtest.py`.
+ # https://shorturl.at/bJKVW
+ #
+ # We only want to validate for the ones that are listed above, hence
+ # trimming to the last parts.
transcript = transcript[-total_number_of_commands:]
# Validate the transcript.
@@ -148,7 +155,7 @@ def test_structured_transcript(self):
# (lldb) breakpoint set -f main.c -l <line>
self.assertEqual(transcript[2]["command"], "breakpoint set -f main.c
-l %d" % self.line)
# Breakpoint 1: where = a.out`main + 29 at main.c:5:3, address =
0x0000000100000f7d
- self.assertTrue("Breakpoint 1: where = a.out`main + 29 at main.c:5:3,
address =" in transcript[2]["output"][0])
+ self.assertTrue("Breakpoint 1: where = a.out`main " in
transcript[2]["output"][0])
self.assertEqual(transcript[2]["error"], [])
# (lldb) r
>From b00905d4b2d370fe05c277ad09c6325265d89f44 Mon Sep 17 00:00:00 2001
From: Roy Shi <roy...@meta.com>
Date: Fri, 10 May 2024 13:19:16 -0700
Subject: [PATCH 12/19] Remove Array::SplitString + add test for JSON output
---
lldb/include/lldb/Utility/StructuredData.h | 25 ---------
.../source/Interpreter/CommandInterpreter.cpp | 12 ++---
lldb/source/Utility/StructuredData.cpp | 16 ------
.../interpreter/TestCommandInterpreterAPI.py | 46 ++++++++--------
lldb/unittests/Utility/StructuredDataTest.cpp | 54 -------------------
5 files changed, 29 insertions(+), 124 deletions(-)
diff --git a/lldb/include/lldb/Utility/StructuredData.h
b/lldb/include/lldb/Utility/StructuredData.h
index 563eb1b1a284a..78e3234e34f2a 100644
--- a/lldb/include/lldb/Utility/StructuredData.h
+++ b/lldb/include/lldb/Utility/StructuredData.h
@@ -290,31 +290,6 @@ class StructuredData {
void GetDescription(lldb_private::Stream &s) const override;
- /// Creates an Array of substrings by splitting a string around the
- /// occurrences of a separator character.
- ///
- /// \param[in] s
- /// The input string.
- ///
- /// \param[in] separator
- /// The character to split on.
- ///
- /// \param[in] maxSplit
- /// The maximum number of times the string is split. If \a maxSplit is >=
- /// 0, at most \a maxSplit splits are done and consequently <= \a
maxSplit
- /// + 1 elements are returned.
- ///
- /// \param[in] keepEmpty
- /// True if empty substrings should be returned. Empty substrings still
- /// count when considering \a maxSplit.
- ///
- /// \return
- /// An array containing the substrings. If \a maxSplit == -1 and \a
- /// keepEmpty == true, then the concatination of the array forms the
input
- /// string.
- static ArraySP SplitString(llvm::StringRef s, char separator,
- int maxSplit = -1, bool keepEmpty = true);
-
protected:
typedef std::vector<ObjectSP> collection;
collection m_items;
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp
b/lldb/source/Interpreter/CommandInterpreter.cpp
index d5e37dee186a6..0f5900a135a52 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2052,14 +2052,10 @@ bool CommandInterpreter::HandleCommand(const char
*command_line,
m_transcript_stream << result.GetOutputData();
m_transcript_stream << result.GetErrorData();
- // Add output and error to the transcript item after splitting lines. In the
- // future, other aspects of the command (e.g. perf) can be added, too.
- transcript_item->AddItem(
- "output", StructuredData::Array::SplitString(result.GetOutputData(),
'\n',
- -1, false));
- transcript_item->AddItem(
- "error", StructuredData::Array::SplitString(result.GetErrorData(), '\n',
- -1, false));
+ // Add output and error to the transcript item. In the future, other aspects
+ // of the command (e.g. perf) can be added, too.
+ transcript_item->AddStringItem("output", result.GetOutputData());
+ transcript_item->AddStringItem("error", result.GetErrorData());
return result.Succeeded();
}
diff --git a/lldb/source/Utility/StructuredData.cpp
b/lldb/source/Utility/StructuredData.cpp
index f4e87b7167a1f..aab2ed32f3133 100644
--- a/lldb/source/Utility/StructuredData.cpp
+++ b/lldb/source/Utility/StructuredData.cpp
@@ -290,19 +290,3 @@ void
StructuredData::Null::GetDescription(lldb_private::Stream &s) const {
void StructuredData::Generic::GetDescription(lldb_private::Stream &s) const {
s.Printf("%p", m_object);
}
-
-StructuredData::ArraySP StructuredData::Array::SplitString(llvm::StringRef s,
- char separator,
- int maxSplit,
- bool keepEmpty) {
- // Split the string into a small vector.
- llvm::SmallVector<StringRef> small_vec;
- s.split(small_vec, separator, maxSplit, keepEmpty);
-
- // Copy the substrings from the small vector into the output array.
- auto array_sp = std::make_shared<StructuredData::Array>();
- for (auto substring : small_vec) {
- array_sp->AddStringItem(substring);
- }
- return array_sp;
-}
diff --git a/lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py
b/lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py
index 54179892eabda..d29cee8d0141c 100644
--- a/lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py
+++ b/lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py
@@ -106,7 +106,8 @@ def test_structured_transcript(self):
ci.HandleCommand("breakpoint set -f main.c -l %d" % self.line, res)
ci.HandleCommand("r", res)
ci.HandleCommand("p a", res)
- total_number_of_commands = 5
+ ci.HandleCommand("statistics dump", res)
+ total_number_of_commands = 6
# Retrieve the transcript and convert it into a Python object
transcript = ci.GetTranscript()
@@ -120,9 +121,6 @@ def test_structured_transcript(self):
transcript = json.loads(stream.GetData())
- print('TRANSCRIPT')
- print(transcript)
-
# The transcript will contain a bunch of commands that are from
# a general setup code. See `def setUpCommands(cls)` in
# `lldb/packages/Python/lldbsuite/test/lldbtest.py`.
@@ -132,45 +130,51 @@ def test_structured_transcript(self):
# trimming to the last parts.
transcript = transcript[-total_number_of_commands:]
- # Validate the transcript.
+ # The following validates individual commands in the transcript.
#
- # The following asserts rely on the exact output format of the
+ # Note: Some of the asserts rely on the exact output format of the
# commands. Hopefully we are not changing them any time soon.
# (lldb) version
self.assertEqual(transcript[0]["command"], "version")
- self.assertTrue("lldb version" in transcript[0]["output"][0])
- self.assertEqual(transcript[0]["error"], [])
+ self.assertTrue("lldb version" in transcript[0]["output"])
+ self.assertEqual(transcript[0]["error"], "")
# (lldb) an-unknown-command
self.assertEqual(transcript[1],
{
"command": "an-unknown-command",
- "output": [],
- "error": [
- "error: 'an-unknown-command' is not a valid command.",
- ],
+ "output": "",
+ "error": "error: 'an-unknown-command' is not a valid
command.\n",
})
# (lldb) breakpoint set -f main.c -l <line>
self.assertEqual(transcript[2]["command"], "breakpoint set -f main.c
-l %d" % self.line)
# Breakpoint 1: where = a.out`main + 29 at main.c:5:3, address =
0x0000000100000f7d
- self.assertTrue("Breakpoint 1: where = a.out`main " in
transcript[2]["output"][0])
- self.assertEqual(transcript[2]["error"], [])
+ self.assertTrue("Breakpoint 1: where = a.out`main " in
transcript[2]["output"])
+ self.assertEqual(transcript[2]["error"], "")
# (lldb) r
self.assertEqual(transcript[3]["command"], "r")
# Process 25494 launched:
'<path>/TestCommandInterpreterAPI.test_structured_transcript/a.out' (x86_64)
- self.assertTrue("Process" in transcript[3]["output"][0])
- self.assertTrue("launched" in transcript[3]["output"][0])
- self.assertEqual(transcript[3]["error"], [])
+ self.assertTrue("Process" in transcript[3]["output"])
+ self.assertTrue("launched" in transcript[3]["output"])
+ self.assertEqual(transcript[3]["error"], "")
# (lldb) p a
self.assertEqual(transcript[4],
{
"command": "p a",
- "output": [
- "(int) 123",
- ],
- "error": [],
+ "output": "(int) 123\n",
+ "error": "",
})
+
+ # (lldb) statistics dump
+ statistics_dump = json.loads(transcript[5]["output"])
+ # Dump result should be valid JSON
+ self.assertTrue(statistics_dump is not json.JSONDecodeError)
+ # Dump result should contain expected fields
+ self.assertTrue("commands" in statistics_dump)
+ self.assertTrue("memory" in statistics_dump)
+ self.assertTrue("modules" in statistics_dump)
+ self.assertTrue("targets" in statistics_dump)
diff --git a/lldb/unittests/Utility/StructuredDataTest.cpp
b/lldb/unittests/Utility/StructuredDataTest.cpp
index f107490946334..411ebebb67585 100644
--- a/lldb/unittests/Utility/StructuredDataTest.cpp
+++ b/lldb/unittests/Utility/StructuredDataTest.cpp
@@ -121,57 +121,3 @@ struct ArraySplitStringTestCase {
bool keepEmpty;
std::vector<std::string> expected;
};
-
-TEST(StructuredDataTest, ArraySplitString) {
- ArraySplitStringTestCase test_cases[] = {
- // Happy path
- {
- "1,2,,3",
- ',',
- -1,
- true,
- {"1", "2", "", "3"},
- },
- // No splits
- {
- "1,2,,3",
- ',',
- 0,
- true,
- {"1,2,,3"},
- },
- // 1 split
- {
- "1,2,,3",
- ',',
- 1,
- true,
- {"1", "2,,3"},
- },
- // No empty substrings
- {
- "1,2,,3",
- ',',
- -1,
- false,
- {"1", "2", "3"},
- },
- // Empty substrings count towards splits
- {
- ",1,2,3",
- ',',
- 1,
- false,
- {"1,2,3"},
- },
- };
- for (const auto &test_case : test_cases) {
- auto array = StructuredData::Array::SplitString(
- test_case.s, test_case.separator, test_case.maxSplit,
- test_case.keepEmpty);
- EXPECT_EQ(test_case.expected.size(), array->GetSize());
- for (unsigned int i = 0; i < test_case.expected.size(); ++i) {
- EXPECT_EQ(test_case.expected[i],
array->GetItemAtIndexAsString(i)->str());
- }
- }
-}
>From e2e26d83f270e5989f05f4bbe65760ffa8f1a036 Mon Sep 17 00:00:00 2001
From: Roy Shi <roy...@meta.com>
Date: Fri, 10 May 2024 13:26:13 -0700
Subject: [PATCH 13/19] Clean-ups
---
lldb/include/lldb/Utility/StructuredData.h | 2 +-
lldb/source/Utility/StructuredData.cpp | 1 -
lldb/unittests/Utility/StructuredDataTest.cpp | 9 ---------
3 files changed, 1 insertion(+), 11 deletions(-)
diff --git a/lldb/include/lldb/Utility/StructuredData.h
b/lldb/include/lldb/Utility/StructuredData.h
index 78e3234e34f2a..5e63ef92fac3e 100644
--- a/lldb/include/lldb/Utility/StructuredData.h
+++ b/lldb/include/lldb/Utility/StructuredData.h
@@ -432,7 +432,7 @@ class StructuredData {
}
return success;
}
-
+
template <class IntType>
bool GetValueForKeyAsInteger(llvm::StringRef key, IntType &result) const {
ObjectSP value_sp = GetValueForKey(key);
diff --git a/lldb/source/Utility/StructuredData.cpp
b/lldb/source/Utility/StructuredData.cpp
index aab2ed32f3133..7686d052c599c 100644
--- a/lldb/source/Utility/StructuredData.cpp
+++ b/lldb/source/Utility/StructuredData.cpp
@@ -10,7 +10,6 @@
#include "lldb/Utility/FileSpec.h"
#include "lldb/Utility/Status.h"
#include "llvm/ADT/StringExtras.h"
-#include "llvm/ADT/StringRef.h"
#include "llvm/Support/MemoryBuffer.h"
#include <cerrno>
#include <cinttypes>
diff --git a/lldb/unittests/Utility/StructuredDataTest.cpp
b/lldb/unittests/Utility/StructuredDataTest.cpp
index 411ebebb67585..e536039f365a4 100644
--- a/lldb/unittests/Utility/StructuredDataTest.cpp
+++ b/lldb/unittests/Utility/StructuredDataTest.cpp
@@ -12,7 +12,6 @@
#include "lldb/Utility/Status.h"
#include "lldb/Utility/StreamString.h"
#include "lldb/Utility/StructuredData.h"
-#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Path.h"
using namespace lldb;
@@ -113,11 +112,3 @@ TEST(StructuredDataTest, ParseJSONFromFile) {
object_sp->Dump(S, false);
EXPECT_EQ("[1,2,3]", S.GetString());
}
-
-struct ArraySplitStringTestCase {
- llvm::StringRef s;
- char separator;
- int maxSplit;
- bool keepEmpty;
- std::vector<std::string> expected;
-};
>From e21855f1a4cebc0f9f1d92b959f17c1b1cc298a6 Mon Sep 17 00:00:00 2001
From: Roy Shi <roy...@meta.com>
Date: Mon, 13 May 2024 15:07:15 -0700
Subject: [PATCH 14/19] Use assertIn in python test
---
.../interpreter/TestCommandInterpreterAPI.py | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py
b/lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py
index d29cee8d0141c..52506e7dc7bc7 100644
--- a/lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py
+++ b/lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py
@@ -137,7 +137,7 @@ def test_structured_transcript(self):
# (lldb) version
self.assertEqual(transcript[0]["command"], "version")
- self.assertTrue("lldb version" in transcript[0]["output"])
+ self.assertIn("lldb version", transcript[0]["output"])
self.assertEqual(transcript[0]["error"], "")
# (lldb) an-unknown-command
@@ -151,14 +151,14 @@ def test_structured_transcript(self):
# (lldb) breakpoint set -f main.c -l <line>
self.assertEqual(transcript[2]["command"], "breakpoint set -f main.c
-l %d" % self.line)
# Breakpoint 1: where = a.out`main + 29 at main.c:5:3, address =
0x0000000100000f7d
- self.assertTrue("Breakpoint 1: where = a.out`main " in
transcript[2]["output"])
+ self.assertIn("Breakpoint 1: where = a.out`main ",
transcript[2]["output"])
self.assertEqual(transcript[2]["error"], "")
# (lldb) r
self.assertEqual(transcript[3]["command"], "r")
# Process 25494 launched:
'<path>/TestCommandInterpreterAPI.test_structured_transcript/a.out' (x86_64)
- self.assertTrue("Process" in transcript[3]["output"])
- self.assertTrue("launched" in transcript[3]["output"])
+ self.assertIn("Process", transcript[3]["output"])
+ self.assertIn("launched", transcript[3]["output"])
self.assertEqual(transcript[3]["error"], "")
# (lldb) p a
@@ -174,7 +174,7 @@ def test_structured_transcript(self):
# Dump result should be valid JSON
self.assertTrue(statistics_dump is not json.JSONDecodeError)
# Dump result should contain expected fields
- self.assertTrue("commands" in statistics_dump)
- self.assertTrue("memory" in statistics_dump)
- self.assertTrue("modules" in statistics_dump)
- self.assertTrue("targets" in statistics_dump)
+ self.assertIn("commands", statistics_dump)
+ self.assertIn("memory", statistics_dump)
+ self.assertIn("modules", statistics_dump)
+ self.assertIn("targets", statistics_dump)
>From 7a240d92a2d12f27499a43e8330d10d8030e2157 Mon Sep 17 00:00:00 2001
From: Roy Shi <roy...@meta.com>
Date: Thu, 16 May 2024 21:56:11 -0700
Subject: [PATCH 15/19] Add a setting; add elapsed seconds; make a copy of the
returned StructuredData::Array; add tests
---
lldb/include/lldb/API/SBCommandInterpreter.h | 8 +-
.../lldb/Interpreter/CommandInterpreter.h | 7 +-
lldb/source/API/SBCommandInterpreter.cpp | 4 +-
.../source/Interpreter/CommandInterpreter.cpp | 48 ++++--
.../Interpreter/InterpreterProperties.td | 4 +
.../interpreter/TestCommandInterpreterAPI.py | 139 ++++++++++++++----
6 files changed, 157 insertions(+), 53 deletions(-)
diff --git a/lldb/include/lldb/API/SBCommandInterpreter.h
b/lldb/include/lldb/API/SBCommandInterpreter.h
index f56f7d844b0d1..a82f735c012ae 100644
--- a/lldb/include/lldb/API/SBCommandInterpreter.h
+++ b/lldb/include/lldb/API/SBCommandInterpreter.h
@@ -319,9 +319,11 @@ class SBCommandInterpreter {
SBStructuredData GetStatistics();
/// Returns a list of handled commands, output and error. Each element in
- /// the list is a dictionary with three keys: "command" (string), "output"
- /// (list of strings) and optionally "error" (list of strings). Each string
- /// in "output" and "error" is a line (without EOL characters).
+ /// the list is a dictionary with the following keys/values:
+ /// - "command" (string): The command that was executed.
+ /// - "output" (string): The output of the command. Empty ("") if no output.
+ /// - "error" (string): The error of the command. Empty ("") if no error.
+ /// - "seconds" (float): The time it took to execute the command.
SBStructuredData GetTranscript();
protected:
diff --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h
b/lldb/include/lldb/Interpreter/CommandInterpreter.h
index 0938ad6ae78ab..6d49ae1af1b8f 100644
--- a/lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -561,6 +561,9 @@ class CommandInterpreter : public Broadcaster,
bool GetPromptOnQuit() const;
void SetPromptOnQuit(bool enable);
+ bool GetSaveTranscript() const;
+ void SetSaveTranscript(bool enable);
+
bool GetSaveSessionOnQuit() const;
void SetSaveSessionOnQuit(bool enable);
@@ -648,7 +651,7 @@ class CommandInterpreter : public Broadcaster,
}
llvm::json::Value GetStatistics();
- StructuredData::ArraySP GetTranscript() const;
+ const StructuredData::Array& GetTranscript() const;
protected:
friend class Debugger;
@@ -773,7 +776,7 @@ class CommandInterpreter : public Broadcaster,
/// the list is a dictionary with three keys: "command" (string), "output"
/// (list of strings) and optionally "error" (list of strings). Each string
/// in "output" and "error" is a line (without EOL characters).
- StructuredData::ArraySP m_transcript;
+ StructuredData::Array m_transcript;
};
} // namespace lldb_private
diff --git a/lldb/source/API/SBCommandInterpreter.cpp
b/lldb/source/API/SBCommandInterpreter.cpp
index 233a2f97fb9f1..6a912f0d108b8 100644
--- a/lldb/source/API/SBCommandInterpreter.cpp
+++ b/lldb/source/API/SBCommandInterpreter.cpp
@@ -6,6 +6,7 @@
//
//===----------------------------------------------------------------------===//
+#include "lldb/Utility/StructuredData.h"
#include "lldb/lldb-types.h"
#include "lldb/Interpreter/CommandInterpreter.h"
@@ -576,7 +577,8 @@ SBStructuredData SBCommandInterpreter::GetTranscript() {
SBStructuredData data;
if (IsValid())
- data.m_impl_up->SetObjectSP(m_opaque_ptr->GetTranscript());
+ // A deep copy is performed by `std::make_shared` on the
`StructuredData::Array`, via its implicitly-declared copy constructor. This
ensures thread-safety between the user changing the returned `SBStructuredData`
and the `CommandInterpreter` changing its internal `m_transcript`.
+
data.m_impl_up->SetObjectSP(std::make_shared<StructuredData::Array>(m_opaque_ptr->GetTranscript()));
return data;
}
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp
b/lldb/source/Interpreter/CommandInterpreter.cpp
index 0f5900a135a52..ef2b6d39f3d35 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -136,8 +136,7 @@ CommandInterpreter::CommandInterpreter(Debugger &debugger,
m_skip_lldbinit_files(false), m_skip_app_init_files(false),
m_comment_char('#'), m_batch_command_mode(false),
m_truncation_warning(eNoOmission), m_max_depth_warning(eNoOmission),
- m_command_source_depth(0),
- m_transcript(std::make_shared<StructuredData::Array>()) {
+ m_command_source_depth(0) {
SetEventName(eBroadcastBitThreadShouldExit, "thread-should-exit");
SetEventName(eBroadcastBitResetPrompt, "reset-prompt");
SetEventName(eBroadcastBitQuitCommandReceived, "quit");
@@ -163,6 +162,17 @@ void CommandInterpreter::SetPromptOnQuit(bool enable) {
SetPropertyAtIndex(idx, enable);
}
+bool CommandInterpreter::GetSaveTranscript() const {
+ const uint32_t idx = ePropertySaveTranscript;
+ return GetPropertyAtIndexAs<bool>(
+ idx, g_interpreter_properties[idx].default_uint_value != 0);
+}
+
+void CommandInterpreter::SetSaveTranscript(bool enable) {
+ const uint32_t idx = ePropertySaveTranscript;
+ SetPropertyAtIndex(idx, enable);
+}
+
bool CommandInterpreter::GetSaveSessionOnQuit() const {
const uint32_t idx = ePropertySaveSessionOnQuit;
return GetPropertyAtIndexAs<bool>(
@@ -1891,13 +1901,16 @@ bool CommandInterpreter::HandleCommand(const char
*command_line,
else
add_to_history = (lazy_add_to_history == eLazyBoolYes);
- m_transcript_stream << "(lldb) " << command_line << '\n';
-
// The same `transcript_item` will be used below to add output and error of
// the command.
- auto transcript_item = std::make_shared<StructuredData::Dictionary>();
- transcript_item->AddStringItem("command", command_line);
- m_transcript->AddItem(transcript_item);
+ StructuredData::DictionarySP transcript_item;
+ if (GetSaveTranscript()) {
+ m_transcript_stream << "(lldb) " << command_line << '\n';
+
+ transcript_item = std::make_shared<StructuredData::Dictionary>();
+ transcript_item->AddStringItem("command", command_line);
+ m_transcript.AddItem(transcript_item);
+ }
bool empty_command = false;
bool comment_command = false;
@@ -2002,7 +2015,7 @@ bool CommandInterpreter::HandleCommand(const char
*command_line,
// Take care of things like setting up the history command & calling the
// appropriate Execute method on the CommandObject, with the appropriate
// arguments.
-
+ StatsDuration execute_time;
if (cmd_obj != nullptr) {
bool generate_repeat_command = add_to_history;
// If we got here when empty_command was true, then this command is a
@@ -2043,19 +2056,24 @@ bool CommandInterpreter::HandleCommand(const char
*command_line,
log, "HandleCommand, command line after removing command name(s):
'%s'",
remainder.c_str());
+ ElapsedTime elapsed(execute_time);
cmd_obj->Execute(remainder.c_str(), result);
}
LLDB_LOGF(log, "HandleCommand, command %s",
(result.Succeeded() ? "succeeded" : "did not succeed"));
- m_transcript_stream << result.GetOutputData();
- m_transcript_stream << result.GetErrorData();
+ // To test whether or not transcript should be saved, `transcript_item` is
+ // used instead of `GetSaveTrasncript()`. This is because the latter will
+ // fail when the command is "settings set interpreter.save-transcript true".
+ if (transcript_item) {
+ m_transcript_stream << result.GetOutputData();
+ m_transcript_stream << result.GetErrorData();
- // Add output and error to the transcript item. In the future, other aspects
- // of the command (e.g. perf) can be added, too.
- transcript_item->AddStringItem("output", result.GetOutputData());
- transcript_item->AddStringItem("error", result.GetErrorData());
+ transcript_item->AddStringItem("output", result.GetOutputData());
+ transcript_item->AddStringItem("error", result.GetErrorData());
+ transcript_item->AddFloatItem("seconds", execute_time.get().count());
+ }
return result.Succeeded();
}
@@ -3568,6 +3586,6 @@ llvm::json::Value CommandInterpreter::GetStatistics() {
return stats;
}
-StructuredData::ArraySP CommandInterpreter::GetTranscript() const {
+const StructuredData::Array& CommandInterpreter::GetTranscript() const {
return m_transcript;
}
diff --git a/lldb/source/Interpreter/InterpreterProperties.td
b/lldb/source/Interpreter/InterpreterProperties.td
index 2155ee61ccffb..a5fccbbca091c 100644
--- a/lldb/source/Interpreter/InterpreterProperties.td
+++ b/lldb/source/Interpreter/InterpreterProperties.td
@@ -9,6 +9,10 @@ let Definition = "interpreter" in {
Global,
DefaultTrue,
Desc<"If true, LLDB will prompt you before quitting if there are any live
processes being debugged. If false, LLDB will quit without asking in any
case.">;
+ def SaveTranscript: Property<"save-transcript", "Boolean">,
+ Global,
+ DefaultFalse,
+ Desc<"If true, commands will be saved into a transcript buffer for user
access.">;
def SaveSessionOnQuit: Property<"save-session-on-quit", "Boolean">,
Global,
DefaultFalse,
diff --git a/lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py
b/lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py
index 52506e7dc7bc7..95643eef0d344 100644
--- a/lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py
+++ b/lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py
@@ -16,8 +16,7 @@ def setUp(self):
# Find the line number to break on inside main.cpp.
self.line = line_number("main.c", "Hello world.")
- def test_with_process_launch_api(self):
- """Test the SBCommandInterpreter APIs."""
+ def buildAndCreateTarget(self):
self.build()
exe = self.getBuildArtifact("a.out")
@@ -28,6 +27,11 @@ def test_with_process_launch_api(self):
# Retrieve the associated command interpreter from our debugger.
ci = self.dbg.GetCommandInterpreter()
self.assertTrue(ci, VALID_COMMAND_INTERPRETER)
+ return ci
+
+ def test_with_process_launch_api(self):
+ """Test the SBCommandInterpreter APIs."""
+ ci = self.buildAndCreateTarget()
# Exercise some APIs....
@@ -87,19 +91,30 @@ def test_command_output(self):
self.assertIsNotNone(res.GetError())
self.assertEqual(res.GetError(), "")
+ def getTranscriptAsPythonObject(self, ci):
+ """Retrieve the transcript and convert it into a Python object"""
+ structured_data = ci.GetTranscript()
+ self.assertTrue(structured_data.IsValid())
+
+ stream = lldb.SBStream()
+ self.assertTrue(stream)
+
+ error = structured_data.GetAsJSON(stream)
+ self.assertSuccess(error)
+
+ return json.loads(stream.GetData())
+
def test_structured_transcript(self):
"""Test structured transcript generation and retrieval."""
- # Get command interpreter and create a target
- self.build()
- exe = self.getBuildArtifact("a.out")
-
- target = self.dbg.CreateTarget(exe)
- self.assertTrue(target, VALID_TARGET)
+ ci = self.buildAndCreateTarget()
- ci = self.dbg.GetCommandInterpreter()
- self.assertTrue(ci, VALID_COMMAND_INTERPRETER)
+ # Make sure the "save-transcript" setting is on
+ self.runCmd("settings set interpreter.save-transcript true")
- # Send a few commands through the command interpreter
+ # Send a few commands through the command interpreter.
+ #
+ # Using `ci.HandleCommand` because some commands will fail so that we
+ # can test the "error" field in the saved transcript.
res = lldb.SBCommandReturnObject()
ci.HandleCommand("version", res)
ci.HandleCommand("an-unknown-command", res)
@@ -109,31 +124,25 @@ def test_structured_transcript(self):
ci.HandleCommand("statistics dump", res)
total_number_of_commands = 6
- # Retrieve the transcript and convert it into a Python object
- transcript = ci.GetTranscript()
- self.assertTrue(transcript.IsValid())
+ # Get transcript as python object
+ transcript = self.getTranscriptAsPythonObject(ci)
- stream = lldb.SBStream()
- self.assertTrue(stream)
-
- error = transcript.GetAsJSON(stream)
- self.assertSuccess(error)
-
- transcript = json.loads(stream.GetData())
-
- # The transcript will contain a bunch of commands that are from
- # a general setup code. See `def setUpCommands(cls)` in
- # `lldb/packages/Python/lldbsuite/test/lldbtest.py`.
- # https://shorturl.at/bJKVW
- #
- # We only want to validate for the ones that are listed above, hence
- # trimming to the last parts.
- transcript = transcript[-total_number_of_commands:]
+ # All commands should have expected fields.
+ for command in transcript:
+ self.assertIn("command", command)
+ self.assertIn("output", command)
+ self.assertIn("error", command)
+ self.assertIn("seconds", command)
# The following validates individual commands in the transcript.
#
- # Note: Some of the asserts rely on the exact output format of the
- # commands. Hopefully we are not changing them any time soon.
+ # Notes:
+ # 1. Some of the asserts rely on the exact output format of the
+ # commands. Hopefully we are not changing them any time soon.
+ # 2. We are removing the "seconds" field from each command, so that
+ # some of the validations below can be easier / more readable.
+ for command in transcript:
+ del(command["seconds"])
# (lldb) version
self.assertEqual(transcript[0]["command"], "version")
@@ -178,3 +187,69 @@ def test_structured_transcript(self):
self.assertIn("memory", statistics_dump)
self.assertIn("modules", statistics_dump)
self.assertIn("targets", statistics_dump)
+
+ def test_save_transcript_setting_default(self):
+ ci = self.buildAndCreateTarget()
+ res = lldb.SBCommandReturnObject()
+
+ # The setting's default value should be "false"
+ self.runCmd("settings show interpreter.save-transcript",
"interpreter.save-transcript (boolean) = false\n")
+ # self.assertEqual(res.GetOutput(), )
+
+ def test_save_transcript_setting_off(self):
+ ci = self.buildAndCreateTarget()
+
+ # Make sure the setting is off
+ self.runCmd("settings set interpreter.save-transcript false")
+
+ # The transcript should be empty after running a command
+ self.runCmd("version")
+ transcript = self.getTranscriptAsPythonObject(ci)
+ self.assertEqual(transcript, [])
+
+ def test_save_transcript_setting_on(self):
+ ci = self.buildAndCreateTarget()
+ res = lldb.SBCommandReturnObject()
+
+ # Make sure the setting is on
+ self.runCmd("settings set interpreter.save-transcript true")
+
+ # The transcript should contain one item after running a command
+ self.runCmd("version")
+ transcript = self.getTranscriptAsPythonObject(ci)
+ self.assertEqual(len(transcript), 1)
+ self.assertEqual(transcript[0]["command"], "version")
+
+ def test_save_transcript_returns_copy(self):
+ """
+ Test that the returned structured data is *at least* a shallow copy.
+
+ We believe that a deep copy *is* performed in
`SBCommandInterpreter::GetTranscript`.
+ However, the deep copy cannot be tested and doesn't need to be tested,
+ because there is no logic in the command interpreter to modify a
+ transcript item (representing a command) after it has been returned.
+ """
+ ci = self.buildAndCreateTarget()
+
+ # Make sure the setting is on
+ self.runCmd("settings set interpreter.save-transcript true")
+
+ # Run commands and get the transcript as structured data
+ self.runCmd("version")
+ structured_data_1 = ci.GetTranscript()
+ self.assertTrue(structured_data_1.IsValid())
+ self.assertEqual(structured_data_1.GetSize(), 1)
+
self.assertEqual(structured_data_1.GetItemAtIndex(0).GetValueForKey("command").GetStringValue(100),
"version")
+
+ # Run some more commands and get the transcript as structured data
again
+ self.runCmd("help")
+ structured_data_2 = ci.GetTranscript()
+ self.assertTrue(structured_data_2.IsValid())
+ self.assertEqual(structured_data_2.GetSize(), 2)
+
self.assertEqual(structured_data_2.GetItemAtIndex(0).GetValueForKey("command").GetStringValue(100),
"version")
+
self.assertEqual(structured_data_2.GetItemAtIndex(1).GetValueForKey("command").GetStringValue(100),
"help")
+
+ # Now, the first structured data should remain unchanged
+ self.assertTrue(structured_data_1.IsValid())
+ self.assertEqual(structured_data_1.GetSize(), 1)
+
self.assertEqual(structured_data_1.GetItemAtIndex(0).GetValueForKey("command").GetStringValue(100),
"version")
>From f4791a61738ae872ef84223cc9f1c152e5382607 Mon Sep 17 00:00:00 2001
From: Roy Shi <roy...@meta.com>
Date: Thu, 16 May 2024 22:35:14 -0700
Subject: [PATCH 16/19] Fix format
---
lldb/include/lldb/Interpreter/CommandInterpreter.h | 2 +-
lldb/source/API/SBCommandInterpreter.cpp | 9 +++++++--
lldb/source/Interpreter/CommandInterpreter.cpp | 2 +-
3 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h
b/lldb/include/lldb/Interpreter/CommandInterpreter.h
index 6d49ae1af1b8f..2c6abbefb44c9 100644
--- a/lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -651,7 +651,7 @@ class CommandInterpreter : public Broadcaster,
}
llvm::json::Value GetStatistics();
- const StructuredData::Array& GetTranscript() const;
+ const StructuredData::Array &GetTranscript() const;
protected:
friend class Debugger;
diff --git a/lldb/source/API/SBCommandInterpreter.cpp
b/lldb/source/API/SBCommandInterpreter.cpp
index 6a912f0d108b8..5e071874f0111 100644
--- a/lldb/source/API/SBCommandInterpreter.cpp
+++ b/lldb/source/API/SBCommandInterpreter.cpp
@@ -577,8 +577,13 @@ SBStructuredData SBCommandInterpreter::GetTranscript() {
SBStructuredData data;
if (IsValid())
- // A deep copy is performed by `std::make_shared` on the
`StructuredData::Array`, via its implicitly-declared copy constructor. This
ensures thread-safety between the user changing the returned `SBStructuredData`
and the `CommandInterpreter` changing its internal `m_transcript`.
-
data.m_impl_up->SetObjectSP(std::make_shared<StructuredData::Array>(m_opaque_ptr->GetTranscript()));
+ // A deep copy is performed by `std::make_shared` on the
+ // `StructuredData::Array`, via its implicitly-declared copy constructor.
+ // This ensures thread-safety between the user changing the returned
+ // `SBStructuredData` and the `CommandInterpreter` changing its internal
+ // `m_transcript`.
+ data.m_impl_up->SetObjectSP(
+
std::make_shared<StructuredData::Array>(m_opaque_ptr->GetTranscript()));
return data;
}
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp
b/lldb/source/Interpreter/CommandInterpreter.cpp
index ef2b6d39f3d35..811726e30af4d 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -3586,6 +3586,6 @@ llvm::json::Value CommandInterpreter::GetStatistics() {
return stats;
}
-const StructuredData::Array& CommandInterpreter::GetTranscript() const {
+const StructuredData::Array &CommandInterpreter::GetTranscript() const {
return m_transcript;
}
>From 7ffc57b18cbeaeb904db853b36c8e4422c2c6be4 Mon Sep 17 00:00:00 2001
From: Roy Shi <roy...@meta.com>
Date: Thu, 16 May 2024 22:38:54 -0700
Subject: [PATCH 17/19] Attempt to revert unnecessary format changes
---
lldb/include/lldb/API/SBCommandInterpreter.h | 6 +++---
lldb/include/lldb/Interpreter/CommandInterpreter.h | 4 ++--
lldb/source/API/SBCommandInterpreter.cpp | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/lldb/include/lldb/API/SBCommandInterpreter.h
b/lldb/include/lldb/API/SBCommandInterpreter.h
index a82f735c012ae..8ac36344b3a79 100644
--- a/lldb/include/lldb/API/SBCommandInterpreter.h
+++ b/lldb/include/lldb/API/SBCommandInterpreter.h
@@ -247,13 +247,13 @@ class SBCommandInterpreter {
lldb::SBStringList &matches,
lldb::SBStringList &descriptions);
- /// Returns whether an interrupt flag was raised either by the SBDebugger -
+ /// Returns whether an interrupt flag was raised either by the SBDebugger -
/// when the function is not running on the RunCommandInterpreter thread, or
/// by SBCommandInterpreter::InterruptCommand if it is. If your code is
doing
- /// interruptible work, check this API periodically, and interrupt if it
+ /// interruptible work, check this API periodically, and interrupt if it
/// returns true.
bool WasInterrupted() const;
-
+
/// Interrupts the command currently executing in the RunCommandInterpreter
/// thread.
///
diff --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h
b/lldb/include/lldb/Interpreter/CommandInterpreter.h
index 2c6abbefb44c9..cbc5942f1c782 100644
--- a/lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -242,7 +242,7 @@ class CommandInterpreter : public Broadcaster,
eCommandTypesAllThem = 0xFFFF //< all commands
};
- // The CommandAlias and CommandInterpreter both have a hand in
+ // The CommandAlias and CommandInterpreter both have a hand in
// substituting for alias commands. They work by writing special tokens
// in the template form of the Alias command, and then detecting them when
the
// command is executed. These are the special tokens:
@@ -580,7 +580,7 @@ class CommandInterpreter : public Broadcaster,
void SetEchoCommentCommands(bool enable);
bool GetRepeatPreviousCommand() const;
-
+
bool GetRequireCommandOverwrite() const;
const CommandObject::CommandMap &GetUserCommands() const {
diff --git a/lldb/source/API/SBCommandInterpreter.cpp
b/lldb/source/API/SBCommandInterpreter.cpp
index 5e071874f0111..7a35473283684 100644
--- a/lldb/source/API/SBCommandInterpreter.cpp
+++ b/lldb/source/API/SBCommandInterpreter.cpp
@@ -151,7 +151,7 @@ bool SBCommandInterpreter::WasInterrupted() const {
bool SBCommandInterpreter::InterruptCommand() {
LLDB_INSTRUMENT_VA(this);
-
+
return (IsValid() ? m_opaque_ptr->InterruptCommand() : false);
}
>From 95aaa8bdf4c1f625602996fc770789cd6de52459 Mon Sep 17 00:00:00 2001
From: Roy Shi <roy...@meta.com>
Date: Fri, 17 May 2024 09:17:26 -0700
Subject: [PATCH 18/19] Fix TestSessionSave.py; update a comment
---
lldb/include/lldb/Interpreter/CommandInterpreter.h | 10 ++++++----
.../API/commands/session/save/TestSessionSave.py | 12 ++++++++++++
2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h
b/lldb/include/lldb/Interpreter/CommandInterpreter.h
index cbc5942f1c782..a336d614d02aa 100644
--- a/lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -772,10 +772,12 @@ class CommandInterpreter : public Broadcaster,
StreamString m_transcript_stream;
- /// Contains a list of handled commands, output and error. Each element in
- /// the list is a dictionary with three keys: "command" (string), "output"
- /// (list of strings) and optionally "error" (list of strings). Each string
- /// in "output" and "error" is a line (without EOL characters).
+ /// Contains a list of handled commands and their details. Each element in
+ /// the list is a dictionary with the following keys/values:
+ /// - "command" (string): The command that was executed.
+ /// - "output" (string): The output of the command. Empty ("") if no output.
+ /// - "error" (string): The error of the command. Empty ("") if no error.
+ /// - "seconds" (float): The time it took to execute the command.
StructuredData::Array m_transcript;
};
diff --git a/lldb/test/API/commands/session/save/TestSessionSave.py
b/lldb/test/API/commands/session/save/TestSessionSave.py
index 172a764523046..98985c66010bb 100644
--- a/lldb/test/API/commands/session/save/TestSessionSave.py
+++ b/lldb/test/API/commands/session/save/TestSessionSave.py
@@ -25,6 +25,12 @@ def test_session_save(self):
raw = ""
interpreter = self.dbg.GetCommandInterpreter()
+ # Make sure "save-transcript" is on, so that all the following setings
+ # and commands are saved into the trasncript. Note that this cannot be
+ # a part of the `settings`, because this command itself won't be saved
+ # into the transcript.
+ self.runCmd("settings set interpreter.save-transcript true")
+
settings = [
"settings set interpreter.echo-commands true",
"settings set interpreter.echo-comment-commands true",
@@ -95,6 +101,12 @@ def test_session_save_on_quit(self):
raw = ""
interpreter = self.dbg.GetCommandInterpreter()
+ # Make sure "save-transcript" is on, so that all the following setings
+ # and commands are saved into the trasncript. Note that this cannot be
+ # a part of the `settings`, because this command itself won't be saved
+ # into the transcript.
+ self.runCmd("settings set interpreter.save-transcript true")
+
td = tempfile.TemporaryDirectory()
settings = [
>From d55005ad8bda067b7fe42b3bd48fd15534364846 Mon Sep 17 00:00:00 2001
From: Roy Shi <roy...@meta.com>
Date: Mon, 20 May 2024 16:19:05 -0400
Subject: [PATCH 19/19] Add comments the new setting
---
lldb/include/lldb/Interpreter/CommandInterpreter.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h
b/lldb/include/lldb/Interpreter/CommandInterpreter.h
index a336d614d02aa..ccc30cf4f1a82 100644
--- a/lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -770,6 +770,8 @@ class CommandInterpreter : public Broadcaster,
typedef llvm::StringMap<uint64_t> CommandUsageMap;
CommandUsageMap m_command_usages;
+ /// Turn on settings `interpreter.save-transcript` for LLDB to populate
+ /// this stream. Otherwise this stream is empty.
StreamString m_transcript_stream;
/// Contains a list of handled commands and their details. Each element in
@@ -778,6 +780,9 @@ class CommandInterpreter : public Broadcaster,
/// - "output" (string): The output of the command. Empty ("") if no output.
/// - "error" (string): The error of the command. Empty ("") if no error.
/// - "seconds" (float): The time it took to execute the command.
+ ///
+ /// Turn on settings `interpreter.save-transcript` for LLDB to populate
+ /// this list. Otherwise this list is empty.
StructuredData::Array m_transcript;
};
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
