From fda54fd0c3984c2c735788392004cba92dde6108 Mon Sep 17 00:00:00 2001 From: Rangi <35663410+Rangi42@users.noreply.github.com> Date: Tue, 8 Jul 2025 13:59:03 -0400 Subject: [PATCH] Replace `Either` with `std::variant` (#1731) --- include/asm/fstack.hpp | 14 ++-- include/asm/lexer.hpp | 4 +- include/asm/rpn.hpp | 8 +- include/either.hpp | 176 ---------------------------------------- include/file.hpp | 19 ++--- include/link/main.hpp | 13 +-- include/link/symbol.hpp | 8 +- src/asm/fstack.cpp | 2 +- src/asm/lexer.cpp | 51 ++++++------ src/asm/parser.y | 14 ++-- src/asm/rpn.cpp | 2 +- src/link/main.cpp | 2 +- src/link/object.cpp | 14 ++-- src/link/output.cpp | 8 +- src/link/patch.cpp | 10 +-- src/link/sdas_obj.cpp | 10 +-- src/link/symbol.cpp | 10 ++- 17 files changed, 94 insertions(+), 271 deletions(-) delete mode 100644 include/either.hpp diff --git a/include/asm/fstack.hpp b/include/asm/fstack.hpp index a53aab13..3a8c4620 100644 --- a/include/asm/fstack.hpp +++ b/include/asm/fstack.hpp @@ -10,16 +10,16 @@ #include #include #include +#include #include -#include "either.hpp" #include "linkdefs.hpp" #include "asm/lexer.hpp" struct FileStackNode { FileStackNodeType type; - Either< + std::variant< std::vector, // NODE_REPT std::string // NODE_FILE, NODE_MACRO > @@ -34,13 +34,13 @@ struct FileStackNode { uint32_t ID = UINT32_MAX; // REPT iteration counts since last named node, in reverse depth order - std::vector &iters() { return data.get>(); } - std::vector const &iters() const { return data.get>(); } + std::vector &iters() { return std::get>(data); } + std::vector const &iters() const { return std::get>(data); } // File name for files, file::macro name for macros - std::string &name() { return data.get(); } - std::string const &name() const { return data.get(); } + std::string &name() { return std::get(data); } + std::string const &name() const { return std::get(data); } - FileStackNode(FileStackNodeType type_, Either, std::string> data_) + FileStackNode(FileStackNodeType type_, std::variant, std::string> data_) : type(type_), data(data_) {} std::string const &dump(uint32_t curLineNo) const; diff --git a/include/asm/lexer.hpp b/include/asm/lexer.hpp index 173bdfa6..f07af2c6 100644 --- a/include/asm/lexer.hpp +++ b/include/asm/lexer.hpp @@ -8,9 +8,9 @@ #include #include #include +#include #include -#include "either.hpp" #include "platform.hpp" // SSIZE_MAX // This value is a compromise between `LexerState` allocation performance when `mmap` works, and @@ -97,7 +97,7 @@ struct LexerState { bool expandStrings; std::deque expansions; // Front is the innermost current expansion - Either content; + std::variant content; ~LexerState(); diff --git a/include/asm/rpn.hpp b/include/asm/rpn.hpp index 860241b7..db3d96d1 100644 --- a/include/asm/rpn.hpp +++ b/include/asm/rpn.hpp @@ -5,15 +5,15 @@ #include #include +#include #include -#include "either.hpp" #include "linkdefs.hpp" struct Symbol; struct Expression { - Either< + std::variant< int32_t, // If the expression's value is known, it's here std::string // Why the expression is not known, if it isn't > @@ -22,8 +22,8 @@ struct Expression { std::vector rpn{}; // Bytes serializing the RPN expression uint32_t rpnPatchSize = 0; // Size the expression will take in the object file - bool isKnown() const { return data.holds(); } - int32_t value() const { return data.get(); } + bool isKnown() const { return std::holds_alternative(data); } + int32_t value() const { return std::get(data); } int32_t getConstVal() const; Symbol const *symbolOf() const; diff --git a/include/either.hpp b/include/either.hpp deleted file mode 100644 index eb2cac15..00000000 --- a/include/either.hpp +++ /dev/null @@ -1,176 +0,0 @@ -// SPDX-License-Identifier: MIT - -#ifndef RGBDS_EITHER_HPP -#define RGBDS_EITHER_HPP - -#include -#include - -#include "helpers.hpp" // assume - -template -union Either { - typedef T1 type1; - typedef T2 type2; - -private: - template - struct Field { - constexpr static unsigned tag_value = V; - - unsigned tag = tag_value; - T value; - - Field() : value() {} - Field(T &value_) : value(value_) {} - Field(T const &value_) : value(value_) {} - Field(T &&value_) : value(std::move(value_)) {} - }; - - // The `_tag` unifies with the first `tag` member of each `struct`. - constexpr static unsigned nulltag = 0; - unsigned _tag = nulltag; - Field _t1; - Field _t2; - - // Value accessors; the function parameters are dummies for overload resolution. - // Only used to implement `field()` below. - auto &pick(T1 *) { return _t1; } - auto const &pick(T1 *) const { return _t1; } - auto &pick(T2 *) { return _t2; } - auto const &pick(T2 *) const { return _t2; } - - // Generic field accessors; for internal use only. - template - auto &field() { - return pick(static_cast(nullptr)); - } - template - auto const &field() const { - return pick(static_cast(nullptr)); - } - -public: - // Equivalent of `std::monostate` for `std::variant`s. - Either() : _tag() {} - // These constructors cannot be generic over the value type, because that would prevent - // constructible values from being inferred, e.g. a `const char *` string literal for an - // `std::string` field value. - Either(T1 &value) : _t1(value) {} - Either(T2 &value) : _t2(value) {} - Either(T1 const &value) : _t1(value) {} - Either(T2 const &value) : _t2(value) {} - Either(T1 &&value) : _t1(std::move(value)) {} - Either(T2 &&value) : _t2(std::move(value)) {} - - // Destructor manually calls the appropriate value destructor. - ~Either() { - if (_tag == _t1.tag_value) { - _t1.value.~T1(); - } else if (_tag == _t2.tag_value) { - _t2.value.~T2(); - } - } - - // Copy assignment operators for each possible value. - Either &operator=(T1 const &value) { - _t1.tag = _t1.tag_value; - new (&_t1.value) T1(value); - return *this; - } - Either &operator=(T2 const &value) { - _t2.tag = _t2.tag_value; - new (&_t2.value) T2(value); - return *this; - } - - // Move assignment operators for each possible value. - Either &operator=(T1 &&value) { - _t1.tag = _t1.tag_value; - new (&_t1.value) T1(std::move(value)); - return *this; - } - Either &operator=(T2 &&value) { - _t2.tag = _t2.tag_value; - new (&_t2.value) T2(std::move(value)); - return *this; - } - - // Copy assignment operator from another `Either`. - Either &operator=(Either other) { - if (other._tag == other._t1.tag_value) { - *this = other._t1.value; - } else if (other._tag == other._t2.tag_value) { - *this = other._t2.value; - } else { - _tag = nulltag; // LCOV_EXCL_LINE - } - return *this; - } - - // Copy constructor from another `Either`; implemented in terms of value assignment operators. - Either(Either const &other) { - if (other._tag == other._t1.tag_value) { - *this = other._t1.value; - } else if (other._tag == other._t2.tag_value) { - *this = other._t2.value; - } else { - _tag = nulltag; - } - } - - // Move constructor from another `Either`; implemented in terms of value assignment operators. - Either(Either &&other) { - if (other._tag == other._t1.tag_value) { - *this = std::move(other._t1.value); - } else if (other._tag == other._t2.tag_value) { - *this = std::move(other._t2.value); - } else { - _tag = nulltag; - } - } - - // Equivalent of `.emplace()` for `std::variant`s. - template - void emplace(Args &&...args) { - this->~Either(); - if constexpr (std::is_same_v) { - _t1.tag = _t1.tag_value; - new (&_t1.value) T1(std::forward(args)...); - } else if constexpr (std::is_same_v) { - _t2.tag = _t2.tag_value; - new (&_t2.value) T2(std::forward(args)...); - } else { - _tag = nulltag; - } - } - - // Equivalent of `std::holds_alternative()` for `std::variant`s. - bool empty() const { return _tag == nulltag; } - - // Equivalent of `std::holds_alternative()` for `std::variant`s. - template - bool holds() const { - if constexpr (std::is_same_v) { - return _tag == _t1.tag_value; - } else if constexpr (std::is_same_v) { - return _tag == _t2.tag_value; - } else { - return false; - } - } - - // Equivalent of `std::get()` for `std::variant`s. - template - auto &get() { - assume(holds()); - return field().value; - } - template - auto const &get() const { - assume(holds()); - return field().value; - } -}; - -#endif // RGBDS_EITHER_HPP diff --git a/include/file.hpp b/include/file.hpp index aee0e969..bba1fdf0 100644 --- a/include/file.hpp +++ b/include/file.hpp @@ -10,15 +10,15 @@ #include #include #include +#include -#include "either.hpp" #include "helpers.hpp" // assume #include "platform.hpp" -#include "gfx/main.hpp" +#include "gfx/warning.hpp" class File { - Either _file; + std::variant _file; public: File() : _file(nullptr) {} @@ -27,8 +27,7 @@ public: // Returns `nullptr` on error, and a non-null pointer otherwise. File *open(std::string const &path, std::ios_base::openmode mode) { if (path != "-") { - _file.emplace(); - return _file.get().open(path, mode) ? this : nullptr; + return _file.emplace().open(path, mode) ? this : nullptr; } else if (mode & std::ios_base::in) { assume(!(mode & std::ios_base::out)); _file.emplace(std::cin.rdbuf()); @@ -46,8 +45,8 @@ public: return this; } std::streambuf &operator*() { - return _file.holds() ? _file.get() - : *_file.get(); + return std::holds_alternative(_file) ? std::get(_file) + : *std::get(_file); } std::streambuf const &operator*() const { // The non-`const` version does not perform any modifications, so it's okay. @@ -60,9 +59,9 @@ public: } char const *c_str(std::string const &path) const { - return _file.holds() ? path.c_str() - : _file.get() == std::cin.rdbuf() ? "" - : ""; + return std::holds_alternative(_file) ? path.c_str() + : std::get(_file) == std::cin.rdbuf() ? "" + : ""; } }; diff --git a/include/link/main.hpp b/include/link/main.hpp index ab8ccbd7..102e8af4 100644 --- a/include/link/main.hpp +++ b/include/link/main.hpp @@ -6,9 +6,9 @@ #include #include #include +#include #include -#include "either.hpp" #include "linkdefs.hpp" // Variables related to CLI options @@ -39,7 +39,8 @@ extern bool disablePadding; struct FileStackNode { FileStackNodeType type; - Either< + std::variant< + std::monostate, // Default constructed; `.type` and `.data` must be set manually std::vector, // NODE_REPT std::string // NODE_FILE, NODE_MACRO > @@ -50,11 +51,11 @@ struct FileStackNode { uint32_t lineNo; // REPT iteration counts since last named node, in reverse depth order - std::vector &iters() { return data.get>(); } - std::vector const &iters() const { return data.get>(); } + std::vector &iters() { return std::get>(data); } + std::vector const &iters() const { return std::get>(data); } // File name for files, file::macro name for macros - std::string &name() { return data.get(); } - std::string const &name() const { return data.get(); } + std::string &name() { return std::get(data); } + std::string const &name() const { return std::get(data); } std::string const &dump(uint32_t curLineNo) const; }; diff --git a/include/link/symbol.hpp b/include/link/symbol.hpp index cf853fc2..e79012e2 100644 --- a/include/link/symbol.hpp +++ b/include/link/symbol.hpp @@ -7,8 +7,8 @@ #include #include +#include -#include "either.hpp" #include "linkdefs.hpp" struct FileStackNode; @@ -27,14 +27,14 @@ struct Symbol { ExportLevel type; FileStackNode const *src; int32_t lineNo; - Either< + std::variant< int32_t, // Constants just have a numeric value Label // Label values refer to an offset within a specific section > data; - Label &label() { return data.get