https://github.com/JDevlieghere updated
https://github.com/llvm/llvm-project/pull/137751
>From b6618a2ecfe20880e323177f9683f51135b83fd6 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jo...@devlieghere.com>
Date: Mon, 28 Apr 2025 20:51:34 -0700
Subject: [PATCH] [lldb] Support alternatives for scope format entries
This PR implements support for specifying multiple alternatives for
scope format entries. Scopes are used to enclose things that should only
be printed when everything in the scope resolves.
For example, the following scope only resolves if both
`${line.file.basename}` and `${line.number}` resolve.
`
```
{ at ${line.file.basename}:${line.number}}
```
However, the current implementation doesn't let you specify what to
print when they don't resolve. This PR adds support for specifying
multiple alternative scopes, which are evaluated left-to-right.
For example:
```
{ at ${line.file.basename}:${line.number}| in ${function.name}| <unknown
location>}
```
This will resolve to:
- ` at ${line.file.basename}:${line.number}` if the corresponding variables
resolve.
- Otherwise, this resolves to ` in ${function.name}` if `${function.name}`
resolves.
- Otherwise, this resolves to ` <unknown location>` which always resolves.
This PR makes the `|` character a special character within a scope, but
allows it to be escaped.
I ended up with this approach because it fit quite nicely in the
existing architecture of the format entries and by limiting the
functionality to scopes, it sidesteps some complexity, like dealing with
recursion.
---
lldb/include/lldb/Core/FormatEntity.h | 27 +++++---
lldb/source/Core/FormatEntity.cpp | 87 +++++++++++++++++-------
lldb/unittests/Core/FormatEntityTest.cpp | 59 ++++++++++++++--
3 files changed, 135 insertions(+), 38 deletions(-)
diff --git a/lldb/include/lldb/Core/FormatEntity.h
b/lldb/include/lldb/Core/FormatEntity.h
index 97065afe19bfe..6acf6fbe43239 100644
--- a/lldb/include/lldb/Core/FormatEntity.h
+++ b/lldb/include/lldb/Core/FormatEntity.h
@@ -11,10 +11,10 @@
#include "lldb/lldb-enumerations.h"
#include "lldb/lldb-types.h"
+#include "llvm/ADT/SmallVector.h"
#include <algorithm>
#include <cstddef>
#include <cstdint>
-
#include <string>
#include <vector>
@@ -158,9 +158,7 @@ struct Entry {
}
Entry(Type t = Type::Invalid, const char *s = nullptr,
- const char *f = nullptr)
- : string(s ? s : ""), printf_format(f ? f : ""), type(t) {}
-
+ const char *f = nullptr);
Entry(llvm::StringRef s);
Entry(char ch);
@@ -170,15 +168,19 @@ struct Entry {
void AppendText(const char *cstr);
- void AppendEntry(const Entry &&entry) { children.push_back(entry); }
+ void AppendEntry(const Entry &&entry);
+
+ void StartAlternative();
void Clear() {
string.clear();
printf_format.clear();
- children.clear();
+ children_stack.clear();
+ children_stack.emplace_back();
type = Type::Invalid;
fmt = lldb::eFormatDefault;
number = 0;
+ level = 0;
deref = false;
}
@@ -191,7 +193,7 @@ struct Entry {
return false;
if (printf_format != rhs.printf_format)
return false;
- if (children != rhs.children)
+ if (children_stack != rhs.children_stack)
return false;
if (type != rhs.type)
return false;
@@ -202,9 +204,18 @@ struct Entry {
return true;
}
+ std::vector<Entry> &GetChildren();
+
std::string string;
std::string printf_format;
- std::vector<Entry> children;
+
+ /// A stack of children entries, used by Scope entries to provide alterantive
+ /// children. All other entries have a stack of size 1.
+ /// @{
+ llvm::SmallVector<std::vector<Entry>, 1> children_stack;
+ size_t level = 0;
+ /// @}
+
Type type;
lldb::Format fmt = lldb::eFormatDefault;
lldb::addr_t number = 0;
diff --git a/lldb/source/Core/FormatEntity.cpp
b/lldb/source/Core/FormatEntity.cpp
index 40e247a64b78c..031f0e57f132e 100644
--- a/lldb/source/Core/FormatEntity.cpp
+++ b/lldb/source/Core/FormatEntity.cpp
@@ -60,6 +60,7 @@
#include "llvm/Support/Regex.h"
#include "llvm/TargetParser/Triple.h"
+#include <cassert>
#include <cctype>
#include <cinttypes>
#include <cstdio>
@@ -281,31 +282,53 @@ constexpr Definition g_top_level_entries[] = {
constexpr Definition g_root = Entry::DefinitionWithChildren(
"<root>", EntryType::Root, g_top_level_entries);
+FormatEntity::Entry::Entry(Type t, const char *s, const char *f)
+ : string(s ? s : ""), printf_format(f ? f : ""), children_stack({{}}),
+ type(t) {}
+
FormatEntity::Entry::Entry(llvm::StringRef s)
- : string(s.data(), s.size()), printf_format(), children(),
- type(Type::String) {}
+ : string(s.data(), s.size()), children_stack({{}}), type(Type::String) {}
FormatEntity::Entry::Entry(char ch)
- : string(1, ch), printf_format(), children(), type(Type::String) {}
+ : string(1, ch), printf_format(), children_stack({{}}), type(Type::String)
{
+}
+
+std::vector<Entry> &FormatEntity::Entry::GetChildren() {
+ assert(level < children_stack.size());
+ return children_stack[level];
+}
void FormatEntity::Entry::AppendChar(char ch) {
- if (children.empty() || children.back().type != Entry::Type::String)
- children.push_back(Entry(ch));
+ auto &entries = GetChildren();
+ if (entries.empty() || entries.back().type != Entry::Type::String)
+ entries.push_back(Entry(ch));
else
- children.back().string.append(1, ch);
+ entries.back().string.append(1, ch);
}
void FormatEntity::Entry::AppendText(const llvm::StringRef &s) {
- if (children.empty() || children.back().type != Entry::Type::String)
- children.push_back(Entry(s));
+ auto &entries = GetChildren();
+ if (entries.empty() || entries.back().type != Entry::Type::String)
+ entries.push_back(Entry(s));
else
- children.back().string.append(s.data(), s.size());
+ entries.back().string.append(s.data(), s.size());
}
void FormatEntity::Entry::AppendText(const char *cstr) {
return AppendText(llvm::StringRef(cstr));
}
+void FormatEntity::Entry::AppendEntry(const Entry &&entry) {
+ auto &entries = GetChildren();
+ entries.push_back(entry);
+}
+
+void FormatEntity::Entry::StartAlternative() {
+ assert(type == Entry::Type::Scope);
+ children_stack.emplace_back();
+ level++;
+}
+
#define ENUM_TO_CSTR(eee)
\
case FormatEntity::Entry::Type::eee:
\
return #eee
@@ -405,8 +428,9 @@ void FormatEntity::Entry::Dump(Stream &s, int depth) const {
if (deref)
s.Printf("deref = true, ");
s.EOL();
- for (const auto &child : children) {
- child.Dump(s, depth + 1);
+ for (const auto &children : children_stack) {
+ for (const auto &child : children)
+ child.Dump(s, depth + 1);
}
}
@@ -1308,7 +1332,7 @@ bool FormatEntity::Format(const Entry &entry, Stream &s,
return true;
case Entry::Type::Root:
- for (const auto &child : entry.children) {
+ for (const auto &child : entry.children_stack[0]) {
if (!Format(child, s, sc, exe_ctx, addr, valobj, function_changed,
initial_function)) {
return false; // If any item of root fails, then the formatting fails
@@ -1322,19 +1346,26 @@ bool FormatEntity::Format(const Entry &entry, Stream &s,
case Entry::Type::Scope: {
StreamString scope_stream;
- bool success = false;
- for (const auto &child : entry.children) {
- success = Format(child, scope_stream, sc, exe_ctx, addr, valobj,
- function_changed, initial_function);
- if (!success)
- break;
+ auto format_children = [&](const std::vector<Entry> &children) {
+ scope_stream.Clear();
+ for (const auto &child : children) {
+ if (!Format(child, scope_stream, sc, exe_ctx, addr, valobj,
+ function_changed, initial_function))
+ return false;
+ }
+ return true;
+ };
+
+ for (auto &children : entry.children_stack) {
+ if (format_children(children)) {
+ s.Write(scope_stream.GetString().data(),
+ scope_stream.GetString().size());
+ return true;
+ }
}
- // Only if all items in a scope succeed, then do we print the output into
- // the main stream
- if (success)
- s.Write(scope_stream.GetString().data(),
scope_stream.GetString().size());
- }
+
return true; // Scopes always successfully print themselves
+ }
case Entry::Type::Variable:
case Entry::Type::VariableSynthetic:
@@ -2124,7 +2155,7 @@ static Status ParseInternal(llvm::StringRef &format,
Entry &parent_entry,
uint32_t depth) {
Status error;
while (!format.empty() && error.Success()) {
- const size_t non_special_chars = format.find_first_of("${}\\");
+ const size_t non_special_chars = format.find_first_of("${}\\|");
if (non_special_chars == llvm::StringRef::npos) {
// No special characters, just string bytes so add them and we are done
@@ -2161,6 +2192,14 @@ static Status ParseInternal(llvm::StringRef &format,
Entry &parent_entry,
.drop_front(); // Skip the '}' as we are at the end of the
scope
return error;
+ case '|':
+ format = format.drop_front(); // Skip the '|'
+ if (parent_entry.type == Entry::Type::Scope)
+ parent_entry.StartAlternative();
+ else
+ parent_entry.AppendChar('|');
+ break;
+
case '\\': {
format = format.drop_front(); // Skip the '\' character
if (format.empty()) {
diff --git a/lldb/unittests/Core/FormatEntityTest.cpp
b/lldb/unittests/Core/FormatEntityTest.cpp
index 5983c9de99ef7..7e1ff6fb1fa87 100644
--- a/lldb/unittests/Core/FormatEntityTest.cpp
+++ b/lldb/unittests/Core/FormatEntityTest.cpp
@@ -8,16 +8,36 @@
#include "lldb/Core/FormatEntity.h"
#include "lldb/Utility/Status.h"
-
+#include "lldb/Utility/StreamString.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Testing/Support/Error.h"
#include "gtest/gtest.h"
using namespace lldb_private;
+using namespace llvm;
using Definition = FormatEntity::Entry::Definition;
using Entry = FormatEntity::Entry;
-TEST(FormatEntityTest, DefinitionConstructionNameAndType) {
+namespace {
+class FormatEntityTest : public ::testing::Test {
+public:
+ Expected<std::string> Format(StringRef format_str) {
+ StreamString stream;
+ FormatEntity::Entry format;
+ Status status = FormatEntity::Parse(format_str, format);
+ if (status.Fail())
+ return status.ToError();
+
+ FormatEntity::Format(format, stream, nullptr, nullptr, nullptr, nullptr,
+ false, false);
+ return stream.GetString().str();
+ }
+};
+} // namespace
+
+TEST_F(FormatEntityTest, DefinitionConstructionNameAndType) {
Definition d("foo", FormatEntity::Entry::Type::Invalid);
EXPECT_STREQ(d.name, "foo");
@@ -29,7 +49,7 @@ TEST(FormatEntityTest, DefinitionConstructionNameAndType) {
EXPECT_FALSE(d.keep_separator);
}
-TEST(FormatEntityTest, DefinitionConstructionNameAndString) {
+TEST_F(FormatEntityTest, DefinitionConstructionNameAndString) {
Definition d("foo", "string");
EXPECT_STREQ(d.name, "foo");
@@ -41,7 +61,7 @@ TEST(FormatEntityTest, DefinitionConstructionNameAndString) {
EXPECT_FALSE(d.keep_separator);
}
-TEST(FormatEntityTest, DefinitionConstructionNameTypeData) {
+TEST_F(FormatEntityTest, DefinitionConstructionNameTypeData) {
Definition d("foo", FormatEntity::Entry::Type::Invalid, 33);
EXPECT_STREQ(d.name, "foo");
@@ -53,7 +73,7 @@ TEST(FormatEntityTest, DefinitionConstructionNameTypeData) {
EXPECT_FALSE(d.keep_separator);
}
-TEST(FormatEntityTest, DefinitionConstructionNameTypeChildren) {
+TEST_F(FormatEntityTest, DefinitionConstructionNameTypeChildren) {
Definition d("foo", FormatEntity::Entry::Type::Invalid, 33);
Definition parent("parent", FormatEntity::Entry::Type::Invalid, 1, &d);
EXPECT_STREQ(parent.name, "parent");
@@ -153,10 +173,37 @@ constexpr llvm::StringRef lookupStrings[] = {
"${target.file.fullpath}",
"${var.dummy-var-to-test-wildcard}"};
-TEST(FormatEntity, LookupAllEntriesInTree) {
+TEST_F(FormatEntityTest, LookupAllEntriesInTree) {
for (const llvm::StringRef testString : lookupStrings) {
Entry e;
EXPECT_TRUE(FormatEntity::Parse(testString, e).Success())
<< "Formatting " << testString << " did not succeed";
}
}
+
+TEST_F(FormatEntityTest, Scope) {
+ // Scope with one alternative.
+ EXPECT_THAT_EXPECTED(Format("{${frame.pc}|foo}"), HasValue("foo"));
+
+ // Scope with multiple alternatives.
+ EXPECT_THAT_EXPECTED(Format("{${frame.pc}|${function.name}|foo}"),
+ HasValue("foo"));
+
+ // Escaped pipe inside a scope.
+ EXPECT_THAT_EXPECTED(Format("{foo\\|bar}"), HasValue("foo|bar"));
+
+ // Unescaped pipe outside a scope.
+ EXPECT_THAT_EXPECTED(Format("foo|bar"), HasValue("foo|bar"));
+
+ // Nested scopes. Note that scopes always resolve.
+ EXPECT_THAT_EXPECTED(Format("{{${frame.pc}|foo}|{bar}}"), HasValue("foo"));
+ EXPECT_THAT_EXPECTED(Format("{{${frame.pc}}|{bar}}"), HasValue(""));
+
+ // Pipe between scopes.
+ EXPECT_THAT_EXPECTED(Format("{foo}|{bar}"), HasValue("foo|bar"));
+ EXPECT_THAT_EXPECTED(Format("{foo}||{bar}"), HasValue("foo||bar"));
+
+ // Empty space between pipes.
+ EXPECT_THAT_EXPECTED(Format("{{foo}||{bar}}"), HasValue("foo"));
+ EXPECT_THAT_EXPECTED(Format("{${frame.pc}||{bar}}"), HasValue(""));
+}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
