From 0a1249ea40e4d8d3c1665c415a1516ca10c1f812 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 7 Jun 2019 17:05:23 -0400 Subject: [PATCH 1/6] DSP/LabelMap: Amend class formatting Places the most important stuff, the public interface first. --- Source/Core/Core/DSP/LabelMap.h | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/Source/Core/Core/DSP/LabelMap.h b/Source/Core/Core/DSP/LabelMap.h index d3252a6feb..ac1cb42041 100644 --- a/Source/Core/Core/DSP/LabelMap.h +++ b/Source/Core/Core/DSP/LabelMap.h @@ -21,6 +21,16 @@ enum LabelType class LabelMap { +public: + LabelMap(); + ~LabelMap() {} + void RegisterDefaults(); + void RegisterLabel(const std::string& label, u16 lval, LabelType type = LABEL_VALUE); + void DeleteLabel(const std::string& label); + bool GetLabelValue(const std::string& label, u16* value, LabelType type = LABEL_ANY) const; + void Clear(); + +private: struct label_t { label_t(const std::string& lbl, s32 address, LabelType ltype) @@ -32,14 +42,5 @@ class LabelMap LabelType type; }; std::vector labels; - -public: - LabelMap(); - ~LabelMap() {} - void RegisterDefaults(); - void RegisterLabel(const std::string& label, u16 lval, LabelType type = LABEL_VALUE); - void DeleteLabel(const std::string& label); - bool GetLabelValue(const std::string& label, u16* value, LabelType type = LABEL_ANY) const; - void Clear(); }; } // namespace DSP From 98ec2ab2ac0d3f751efc566ba0a3c728dabe27ea Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 7 Jun 2019 17:07:04 -0400 Subject: [PATCH 2/6] DSP/LabelMap: Default constructor and destructor We also move the destructor definition into the cpp file, as that will allow us to make the entire label_t type hidden from external view in a following change. --- Source/Core/Core/DSP/LabelMap.cpp | 6 +++--- Source/Core/Core/DSP/LabelMap.h | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Source/Core/Core/DSP/LabelMap.cpp b/Source/Core/Core/DSP/LabelMap.cpp index e2e157ba79..ef3c6479f8 100644 --- a/Source/Core/Core/DSP/LabelMap.cpp +++ b/Source/Core/Core/DSP/LabelMap.cpp @@ -11,9 +11,9 @@ namespace DSP { -LabelMap::LabelMap() -{ -} +LabelMap::LabelMap() = default; + +LabelMap::~LabelMap() = default; void LabelMap::RegisterDefaults() { diff --git a/Source/Core/Core/DSP/LabelMap.h b/Source/Core/Core/DSP/LabelMap.h index ac1cb42041..4f541959a9 100644 --- a/Source/Core/Core/DSP/LabelMap.h +++ b/Source/Core/Core/DSP/LabelMap.h @@ -23,7 +23,8 @@ class LabelMap { public: LabelMap(); - ~LabelMap() {} + ~LabelMap(); + void RegisterDefaults(); void RegisterLabel(const std::string& label, u16 lval, LabelType type = LABEL_VALUE); void DeleteLabel(const std::string& label); From 747128b093b2b54cdd69645adc20d74f97759335 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 7 Jun 2019 17:13:08 -0400 Subject: [PATCH 3/6] DSP/LabelMap: Collapse DeleteLabel loop into std::find_if Same thing, no explicit loops. --- Source/Core/Core/DSP/LabelMap.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Source/Core/Core/DSP/LabelMap.cpp b/Source/Core/Core/DSP/LabelMap.cpp index ef3c6479f8..79aa503f45 100644 --- a/Source/Core/Core/DSP/LabelMap.cpp +++ b/Source/Core/Core/DSP/LabelMap.cpp @@ -4,6 +4,7 @@ #include "Core/DSP/LabelMap.h" +#include #include #include @@ -44,14 +45,13 @@ void LabelMap::RegisterLabel(const std::string& label, u16 lval, LabelType type) void LabelMap::DeleteLabel(const std::string& label) { - for (std::vector::iterator iter = labels.begin(); iter != labels.end(); ++iter) - { - if (!label.compare(iter->name)) - { - labels.erase(iter); - return; - } - } + const auto iter = std::find_if(labels.cbegin(), labels.cend(), + [&label](const auto& entry) { return entry.name == label; }); + + if (iter == labels.cend()) + return; + + labels.erase(iter); } bool LabelMap::GetLabelValue(const std::string& name, u16* value, LabelType type) const From a3ed4ceec5dda61f74a80fc1e7f4d9978a20cad2 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 7 Jun 2019 17:26:22 -0400 Subject: [PATCH 4/6] DSP/LabelMap: Use std::optional with GetLabelValue() Rather than use a bool and out parameter, we can collapse them into one by using a std::optional. --- Source/Core/Core/DSP/DSPAssembler.cpp | 6 +++--- Source/Core/Core/DSP/LabelMap.cpp | 20 ++++++++++---------- Source/Core/Core/DSP/LabelMap.h | 3 ++- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/Source/Core/Core/DSP/DSPAssembler.cpp b/Source/Core/Core/DSP/DSPAssembler.cpp index 5926bf9895..549e7df8f2 100644 --- a/Source/Core/Core/DSP/DSPAssembler.cpp +++ b/Source/Core/Core/DSP/DSPAssembler.cpp @@ -200,9 +200,9 @@ s32 DSPAssembler::ParseValue(const char* str) else // Everything else is a label. { // Lookup label - u16 value; - if (m_labels.GetLabelValue(ptr, &value)) - return value; + if (const std::optional value = m_labels.GetLabelValue(ptr)) + return *value; + if (m_cur_pass == 2) ShowError(AssemblerError::UnknownLabel, str); } diff --git a/Source/Core/Core/DSP/LabelMap.cpp b/Source/Core/Core/DSP/LabelMap.cpp index 79aa503f45..d2ac003f23 100644 --- a/Source/Core/Core/DSP/LabelMap.cpp +++ b/Source/Core/Core/DSP/LabelMap.cpp @@ -33,11 +33,11 @@ void LabelMap::RegisterDefaults() void LabelMap::RegisterLabel(const std::string& label, u16 lval, LabelType type) { - u16 old_value; - if (GetLabelValue(label, &old_value) && old_value != lval) + const std::optional old_value = GetLabelValue(label); + if (old_value && old_value != lval) { printf("WARNING: Redefined label %s to %04x - old value %04x\n", label.c_str(), lval, - old_value); + *old_value); DeleteLabel(label); } labels.emplace_back(label, lval, type); @@ -54,16 +54,15 @@ void LabelMap::DeleteLabel(const std::string& label) labels.erase(iter); } -bool LabelMap::GetLabelValue(const std::string& name, u16* value, LabelType type) const +std::optional LabelMap::GetLabelValue(const std::string& name, LabelType type) const { - for (auto& label : labels) + for (const auto& label : labels) { - if (!name.compare(label.name)) + if (name == label.name) { - if (type & label.type) + if ((type & label.type) != 0) { - *value = label.addr; - return true; + return label.addr; } else { @@ -71,7 +70,8 @@ bool LabelMap::GetLabelValue(const std::string& name, u16* value, LabelType type } } } - return false; + + return std::nullopt; } void LabelMap::Clear() diff --git a/Source/Core/Core/DSP/LabelMap.h b/Source/Core/Core/DSP/LabelMap.h index 4f541959a9..5458785610 100644 --- a/Source/Core/Core/DSP/LabelMap.h +++ b/Source/Core/Core/DSP/LabelMap.h @@ -4,6 +4,7 @@ #pragma once +#include #include #include @@ -28,7 +29,7 @@ public: void RegisterDefaults(); void RegisterLabel(const std::string& label, u16 lval, LabelType type = LABEL_VALUE); void DeleteLabel(const std::string& label); - bool GetLabelValue(const std::string& label, u16* value, LabelType type = LABEL_ANY) const; + std::optional GetLabelValue(const std::string& label, LabelType type = LABEL_ANY) const; void Clear(); private: From 32427af79e0773489281b02f90af4bd2c5f161f6 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 7 Jun 2019 17:37:45 -0400 Subject: [PATCH 5/6] DSP/LabelMap: Make label_t's definition hidden Given this is a private struct and it's used in a container that supports incomplete types, we can forward-declare it and move it into the cpp file. While we're at it, we can change the name to Label to follow our formatting guidelines. --- Source/Core/Core/DSP/LabelMap.cpp | 11 +++++++++++ Source/Core/Core/DSP/LabelMap.h | 13 ++----------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/Source/Core/Core/DSP/LabelMap.cpp b/Source/Core/Core/DSP/LabelMap.cpp index d2ac003f23..dd045c5a44 100644 --- a/Source/Core/Core/DSP/LabelMap.cpp +++ b/Source/Core/Core/DSP/LabelMap.cpp @@ -12,6 +12,17 @@ namespace DSP { +struct LabelMap::Label +{ + Label(const std::string& lbl, s32 address, LabelType ltype) + : name(lbl), addr(address), type(ltype) + { + } + std::string name; + s32 addr; + LabelType type; +}; + LabelMap::LabelMap() = default; LabelMap::~LabelMap() = default; diff --git a/Source/Core/Core/DSP/LabelMap.h b/Source/Core/Core/DSP/LabelMap.h index 5458785610..e9bcd3950b 100644 --- a/Source/Core/Core/DSP/LabelMap.h +++ b/Source/Core/Core/DSP/LabelMap.h @@ -33,16 +33,7 @@ public: void Clear(); private: - struct label_t - { - label_t(const std::string& lbl, s32 address, LabelType ltype) - : name(lbl), addr(address), type(ltype) - { - } - std::string name; - s32 addr; - LabelType type; - }; - std::vector labels; + struct Label; + std::vector