Skip to content
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.

Sign up for GitHub

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

Jump to bottom

PPCSymbolDB: GetDescription by std::string_view #12704

Merged
merged 1 commit into from
Apr 15, 2024
Merged

PPCSymbolDB: GetDescription by std::string_view #12704

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion Source/Core/Core/Debugger/DebugInterface.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <cstddef>
#include <optional>
#include <string>
#include <string_view>
#include <vector>

#include "Common/CommonTypes.h"
Expand Down Expand Up @@ -103,7 +104,7 @@ class DebugInterface
{
return 0xFFFFFFFF;
}
virtual std::string GetDescription(u32 /*address*/) const = 0;
virtual std::string_view GetDescription(u32 /*address*/) const = 0;
virtual void Clear(const CPUThreadGuard& guard) = 0;
};
} // namespace Core
10 changes: 5 additions & 5 deletions Source/Core/Core/Debugger/Debugger_SymbolMap.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ bool GetCallstack(const Core::CPUThreadGuard& guard, std::vector<CallstackEntry>
});

WalkTheStack(guard, [&output, &ppc_symbol_db](u32 func_addr) {
std::string func_desc = ppc_symbol_db.GetDescription(func_addr);
std::string_view func_desc = ppc_symbol_db.GetDescription(func_addr);
if (func_desc.empty() || func_desc == "Invalid")
func_desc = "(unknown)";

Expand All @@ -106,14 +106,14 @@ void PrintCallstack(const Core::CPUThreadGuard& guard, Common::Log::LogType type
GENERIC_LOG_FMT(type, level, " LR = 0 - this is bad");
}

if (ppc_symbol_db.GetDescription(ppc_state.pc) != ppc_symbol_db.GetDescription(LR(ppc_state)))
if (const std::string_view lr_desc = ppc_symbol_db.GetDescription(LR(ppc_state));
lr_desc != ppc_symbol_db.GetDescription(ppc_state.pc))
{
GENERIC_LOG_FMT(type, level, " * {} [ LR = {:08x} ]",
ppc_symbol_db.GetDescription(LR(ppc_state)), LR(ppc_state));
GENERIC_LOG_FMT(type, level, " * {} [ LR = {:08x} ]", lr_desc, LR(ppc_state));
}

WalkTheStack(guard, [type, level, &ppc_symbol_db](u32 func_addr) {
std::string func_desc = ppc_symbol_db.GetDescription(func_addr);
std::string_view func_desc = ppc_symbol_db.GetDescription(func_addr);
if (func_desc.empty() || func_desc == "Invalid")
func_desc = "(unknown)";
GENERIC_LOG_FMT(type, level, " * {} [ addr = {:08x} ]", func_desc, func_addr);
Expand Down
2 changes: 1 addition & 1 deletion Source/Core/Core/Debugger/PPCDebugInterface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ u32 PPCDebugInterface::GetColor(const Core::CPUThreadGuard* guard, u32 address)
}
// =============

std::string PPCDebugInterface::GetDescription(u32 address) const
std::string_view PPCDebugInterface::GetDescription(u32 address) const
{
return m_ppc_symbol_db.GetDescription(address);
}
Expand Down
2 changes: 1 addition & 1 deletion Source/Core/Core/Debugger/PPCDebugInterface.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ class PPCDebugInterface final : public Core::DebugInterface
void Step() override {}
void RunToBreakpoint() override;
u32 GetColor(const Core::CPUThreadGuard* guard, u32 address) const override;
std::string GetDescription(u32 address) const override;
std::string_view GetDescription(u32 address) const override;

std::shared_ptr<Core::NetworkCaptureLogger> NetworkLogger();

Expand Down
8 changes: 3 additions & 5 deletions Source/Core/Core/PowerPC/PPCSymbolDB.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,11 @@ Common::Symbol* PPCSymbolDB::GetSymbolFromAddr(u32 addr)
return nullptr;
}

std::string PPCSymbolDB::GetDescription(u32 addr)
std::string_view PPCSymbolDB::GetDescription(u32 addr)
Copy link
Contributor

@iwubcode iwubcode Apr 13, 2024 •
edited

Choose a reason for hiding this comment

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

Would be nice if this was const too (and GetSymbolFromAddr)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, PPCSymbolDB and its base class, Common::SymbolDB, lack a lot of const-correctness as technical debt from them being global variables for so many years. I can turn this PR into a general "PPCSymbolDB updates" PR if that's desired, I just saw this as a quick and simple fix to get approved fast.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nah, can be saved for another day. Code as is looks good. Thanks @mitaclaw

{
Common::Symbol* symbol = GetSymbolFromAddr(addr);
if (symbol)
if (const Common::Symbol* const symbol = GetSymbolFromAddr(addr))
return symbol->name;
else
return " --- ";
return " --- ";
}

void PPCSymbolDB::FillInCallers()
Expand Down
3 changes: 2 additions & 1 deletion Source/Core/Core/PowerPC/PPCSymbolDB.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#pragma once

#include <string>
#include <string_view>

#include "Common/CommonTypes.h"
#include "Common/SymbolDB.h"
Expand All @@ -27,7 +28,7 @@ class PPCSymbolDB : public Common::SymbolDB

Common::Symbol* GetSymbolFromAddr(u32 addr) override;

std::string GetDescription(u32 addr);
std::string_view GetDescription(u32 addr);

void FillInCallers();

Expand Down
1 change: 1 addition & 0 deletions Source/Core/DolphinQt/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,7 @@ add_executable(dolphin-emu
QtUtils/ElidedButton.h
QtUtils/FileOpenEventFilter.cpp
QtUtils/FileOpenEventFilter.h
QtUtils/FromStdString.h
QtUtils/ImageConverter.cpp
QtUtils/ImageConverter.h
QtUtils/ModalMessageBox.cpp
Expand Down
15 changes: 4 additions & 11 deletions Source/Core/DolphinQt/Debugger/BreakpointWidget.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,11 +193,8 @@ void BreakpointWidget::Update()
m_table->setItem(i, 0, active);
m_table->setItem(i, 1, create_item(QStringLiteral("BP")));

if (ppc_symbol_db.GetSymbolFromAddr(bp.address))
{
m_table->setItem(
i, 2, create_item(QString::fromStdString(ppc_symbol_db.GetDescription(bp.address))));
}
if (const Common::Symbol* const symbol = ppc_symbol_db.GetSymbolFromAddr(bp.address))
m_table->setItem(i, 2, create_item(QString::fromStdString(symbol->name)));

m_table->setItem(i, 3,
create_item(QStringLiteral("%1").arg(bp.address, 8, 16, QLatin1Char('0'))));
Expand Down Expand Up @@ -234,12 +231,8 @@ void BreakpointWidget::Update()
m_table->setItem(i, 0, active);
m_table->setItem(i, 1, create_item(QStringLiteral("MBP")));

if (ppc_symbol_db.GetSymbolFromAddr(mbp.start_address))
{
m_table->setItem(
i, 2,
create_item(QString::fromStdString(ppc_symbol_db.GetDescription(mbp.start_address))));
}
if (const Common::Symbol* const symbol = ppc_symbol_db.GetSymbolFromAddr(mbp.start_address))
m_table->setItem(i, 2, create_item(QString::fromStdString(symbol->name)));

if (mbp.is_ranged)
{
Expand Down
7 changes: 4 additions & 3 deletions Source/Core/DolphinQt/Debugger/CodeViewWidget.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
#include "DolphinQt/Debugger/AssembleInstructionDialog.h"
#include "DolphinQt/Debugger/PatchInstructionDialog.h"
#include "DolphinQt/Host.h"
#include "DolphinQt/QtUtils/FromStdString.h"
#include "DolphinQt/QtUtils/SetWindowDecorations.h"
#include "DolphinQt/Resources.h"
#include "DolphinQt/Settings.h"
Expand Down Expand Up @@ -326,15 +327,15 @@ void CodeViewWidget::Update(const Core::CPUThreadGuard* guard)

std::string ins = (split == std::string::npos ? disas : disas.substr(0, split));
std::string param = (split == std::string::npos ? "" : disas.substr(split + 1));
std::string desc = debug_interface.GetDescription(addr);
const std::string_view desc = debug_interface.GetDescription(addr);

// Adds whitespace and a minimum size to ins and param. Helps to prevent frequent resizing while
// scrolling.
const QString ins_formatted =
QStringLiteral("%1").arg(QString::fromStdString(ins), -7, QLatin1Char(' '));
const QString param_formatted =
QStringLiteral("%1").arg(QString::fromStdString(param), -19, QLatin1Char(' '));
const QString desc_formatted = QStringLiteral("%1 ").arg(QString::fromStdString(desc));
const QString desc_formatted = QStringLiteral("%1 ").arg(QtUtils::FromStdString(desc));

auto* ins_item = new QTableWidgetItem(ins_formatted);
auto* param_item = new QTableWidgetItem(param_formatted);
Expand Down Expand Up @@ -374,7 +375,7 @@ void CodeViewWidget::Update(const Core::CPUThreadGuard* guard)
branch.is_link = IsBranchInstructionWithLink(ins);

description_item->setText(
tr("--> %1").arg(QString::fromStdString(debug_interface.GetDescription(branch_addr))));
tr("--> %1").arg(QtUtils::FromStdString(debug_interface.GetDescription(branch_addr))));
param_item->setForeground(dark_theme ? QColor(255, 135, 255) : Qt::magenta);
}

Expand Down
3 changes: 2 additions & 1 deletion Source/Core/DolphinQt/Debugger/ThreadWidget.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "Core/PowerPC/PowerPC.h"
#include "Core/System.h"
#include "DolphinQt/Host.h"
#include "DolphinQt/QtUtils/FromStdString.h"
#include "DolphinQt/Settings.h"

ThreadWidget::ThreadWidget(QWidget* parent) : QDockWidget(parent)
Expand Down Expand Up @@ -461,7 +462,7 @@ void ThreadWidget::UpdateThreadCallstack(const Core::CPUThreadGuard& guard,
m_callstack_table->setItem(i, 2, new QTableWidgetItem(format_hex(lr_save)));
m_callstack_table->setItem(
i, 3,
new QTableWidgetItem(QString::fromStdString(
new QTableWidgetItem(QtUtils::FromStdString(
guard.GetSystem().GetPowerPC().GetDebugInterface().GetDescription(lr_save))));
}
else
Expand Down
1 change: 1 addition & 0 deletions Source/Core/DolphinQt/DolphinQt.vcxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,7 @@
<QtMoc Include="QtUtils\DoubleClickEventFilter.h" />
<QtMoc Include="QtUtils\ElidedButton.h" />
<QtMoc Include="QtUtils\FileOpenEventFilter.h" />
<ClInclude Include="QtUtils\FromStdString.h" />
<QtMoc Include="QtUtils\ParallelProgressDialog.h" />
<QtMoc Include="QtUtils\PartiallyClosableTabWidget.h" />
<ClInclude Include="QtUtils\SetWindowDecorations.h" />
Expand Down
23 changes: 23 additions & 0 deletions Source/Core/DolphinQt/QtUtils/FromStdString.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright 2024 Dolphin Emulator Project
// SPDX-License-Identifier: GPL-2.0-or-later

#pragma once

#include <QString>
#include <string_view>

namespace QtUtils
{
inline QString FromStdString(std::string_view s)
{
return QString::fromUtf8(s.data(), s.size());
}
inline QString FromStdString(std::u8string_view s)
{
return QString::fromUtf8(s.data(), s.size());
}
inline QString FromStdString(std::u16string_view s)
{
return QString::fromUtf16(s.data(), s.size());
}
} // namespace QtUtils