Skip to content

[Demangling] Refactor Demangler range tracking #140762

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions lldb/include/lldb/Core/DemangledNameInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ struct DemangledNameInfo {
/// \endcode
std::pair<size_t, size_t> ScopeRange;

/// Indicates the [start, end) of the function argument lits.
/// Indicates the [start, end) of the function argument list.
/// E.g.,
/// \code{.cpp}
/// int (*getFunc<float>(float, double))(int, int)
Expand All @@ -59,10 +59,24 @@ struct DemangledNameInfo {
/// \endcode
std::pair<size_t, size_t> QualifiersRange;

/// Indicates the [start, end) of the function's prefix. This is a
/// catch-all range for anything that is not tracked by the rest of
/// the pairs.
std::pair<size_t, size_t> PrefixRange;

/// Indicates the [start, end) of the function's suffix. This is a
/// catch-all range for anything that is not tracked by the rest of
/// the pairs.
std::pair<size_t, size_t> SuffixRange;

/// Returns \c true if this object holds a valid basename range.
bool hasBasename() const {
return BasenameRange.second > BasenameRange.first &&
BasenameRange.second > 0;
return BasenameRange.second > BasenameRange.first;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate why this was needed?

}

/// Returns \c true if this object holds a valid arguments range.
bool hasArguments() const {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems unused?

return ArgumentsRange.second > ArgumentsRange.first;
}
};

Expand Down
1 change: 1 addition & 0 deletions lldb/include/lldb/Core/FormatEntity.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ struct Entry {
FunctionNameWithArgs,
FunctionNameNoArgs,
FunctionMangledName,
FunctionPrefix,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we add a new variable here we will need to update the documentation under lldb/docs/use/formatting.rst

FunctionScope,
FunctionBasename,
FunctionTemplateArguments,
Expand Down
3 changes: 3 additions & 0 deletions lldb/source/Core/FormatEntity.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ constexpr Definition g_function_child_entries[] = {
Definition("initial-function", EntryType::FunctionInitial),
Definition("changed", EntryType::FunctionChanged),
Definition("is-optimized", EntryType::FunctionIsOptimized),
Definition("prefix", EntryType::FunctionPrefix),
Definition("scope", EntryType::FunctionScope),
Definition("basename", EntryType::FunctionBasename),
Definition("template-arguments", EntryType::FunctionTemplateArguments),
Expand Down Expand Up @@ -385,6 +386,7 @@ const char *FormatEntity::Entry::TypeToCString(Type t) {
ENUM_TO_CSTR(FunctionNameWithArgs);
ENUM_TO_CSTR(FunctionNameNoArgs);
ENUM_TO_CSTR(FunctionMangledName);
ENUM_TO_CSTR(FunctionPrefix);
ENUM_TO_CSTR(FunctionScope);
ENUM_TO_CSTR(FunctionBasename);
ENUM_TO_CSTR(FunctionTemplateArguments);
Expand Down Expand Up @@ -1835,6 +1837,7 @@ bool FormatEntity::Format(const Entry &entry, Stream &s,
return true;
}

case Entry::Type::FunctionPrefix:
case Entry::Type::FunctionScope:
case Entry::Type::FunctionBasename:
case Entry::Type::FunctionTemplateArguments:
Expand Down
2 changes: 2 additions & 0 deletions lldb/source/Core/Mangled.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,8 @@ GetItaniumDemangledStr(const char *M) {

TrackingOutputBuffer OB(demangled_cstr, demangled_size);
demangled_cstr = ipd.finishDemangle(&OB);
OB.NameInfo.SuffixRange.first = OB.NameInfo.QualifiersRange.second;
OB.NameInfo.SuffixRange.second = std::string(demangled_cstr).length();
info = std::move(OB.NameInfo);

assert(demangled_cstr &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -401,8 +401,8 @@ GetDemangledFunctionSuffix(const SymbolContext &sc) {
if (!info->hasBasename())
return std::nullopt;

return demangled_name.slice(info->QualifiersRange.second,
llvm::StringRef::npos);
return demangled_name.slice(info->SuffixRange.first,
info->SuffixRange.second);
}

static bool PrintDemangledArgumentList(Stream &s, const SymbolContext &sc) {
Expand Down
2 changes: 1 addition & 1 deletion lldb/unittests/Core/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ add_lldb_unittest(LLDBCoreTests
DumpDataExtractorTest.cpp
DumpRegisterInfoTest.cpp
FormatEntityTest.cpp
MangledTest.cpp
ItaniumMangledTest.cpp
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't mind renaming the file but there are already swift mangling tests in this file, so ItaniumMangled isn't quite correct. What's the motivation for the rename?

ModuleSpecTest.cpp
PluginManagerTest.cpp
ProgressReportTest.cpp
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//===-- MangledTest.cpp ---------------------------------------------------===//
//===-- ItaniumMangledTest.cpp --------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
Expand Down Expand Up @@ -401,7 +401,7 @@ TEST(MangledTest, DemangledNameInfo_SetValue) {
EXPECT_FALSE(mangled.GetDemangledInfo()->hasBasename());
}

struct DemanglingPartsTestCase {
struct ItaniumDemanglingPartsTestCase {
const char *mangled;
DemangledNameInfo expected_info;
std::string_view basename;
Expand All @@ -410,7 +410,7 @@ struct DemanglingPartsTestCase {
bool valid_basename = true;
};

DemanglingPartsTestCase g_demangling_parts_test_cases[] = {
ItaniumDemanglingPartsTestCase g_demangling_itanium_parts_test_cases[] = {
// clang-format off
{ "_ZNVKO3BarIN2ns3QuxIiEEE1CIPFi3FooIS_IiES6_EEE6methodIS6_EENS5_IT_SC_E5InnerIiEESD_SD_",
{ /*.BasenameRange=*/{92, 98}, /*.ScopeRange=*/{36, 92}, /*.ArgumentsRange=*/{ 108, 158 },
Expand Down Expand Up @@ -555,7 +555,7 @@ DemanglingPartsTestCase g_demangling_parts_test_cases[] = {
};

struct DemanglingPartsTestFixture
: public ::testing::TestWithParam<DemanglingPartsTestCase> {};
: public ::testing::TestWithParam<ItaniumDemanglingPartsTestCase> {};

namespace {
class TestAllocator {
Expand Down Expand Up @@ -608,5 +608,6 @@ TEST_P(DemanglingPartsTestFixture, DemanglingParts) {
std::free(OB.getBuffer());
}

INSTANTIATE_TEST_SUITE_P(DemanglingPartsTests, DemanglingPartsTestFixture,
::testing::ValuesIn(g_demangling_parts_test_cases));
INSTANTIATE_TEST_SUITE_P(
DemanglingPartsTests, DemanglingPartsTestFixture,
::testing::ValuesIn(g_demangling_itanium_parts_test_cases));
Loading