From 0af1e512c207ad08188354987f73c5fc8607bb01 Mon Sep 17 00:00:00 2001 From: Sylvie <35663410+Rangi42@users.noreply.github.com> Date: Wed, 20 Mar 2024 22:37:54 -0400 Subject: [PATCH] Use `std::get_if` instead of `std::visit` (#1367) `std::visit` is (arguably) cleaner code, but older versions of gcc and clang (not very old; the ones packaged with Ubuntu 22.04 LTS) compile them as tables of function pointers, instead of efficient jump tables. --- include/file.hpp | 44 ++++------ src/asm/fstack.cpp | 43 +++++----- src/asm/lexer.cpp | 183 +++++++++++++++++++----------------------- src/asm/parser.y | 12 ++- src/asm/symbol.cpp | 17 ++-- src/link/main.cpp | 44 +++++----- src/link/object.cpp | 6 +- src/link/patch.cpp | 15 ++-- src/link/sdas_obj.cpp | 16 ++-- src/link/symbol.cpp | 4 +- 10 files changed, 167 insertions(+), 217 deletions(-) diff --git a/include/file.hpp b/include/file.hpp index f8cb917d..35a26492 100644 --- a/include/file.hpp +++ b/include/file.hpp @@ -53,12 +53,8 @@ public: return this; } std::streambuf &operator*() { - return std::visit( - Visitor{ - [](std::filebuf &file) -> std::streambuf & { return file; }, - [](std::streambuf *buf) -> std::streambuf & { return *buf; }}, - _file - ); + auto *file = std::get_if(&_file); + return file ? *file : *std::get(_file); } std::streambuf const &operator*() const { // The non-`const` version does not perform any modifications, so it's okay. @@ -71,32 +67,22 @@ public: } File *close() { - return std::visit( - Visitor{ - [this](std::filebuf &file) { - // This is called by the destructor, and an explicit `close` - // shouldn't close twice. - _file.emplace(nullptr); - return file.close() != nullptr; - }, - [](std::streambuf *buf) { return buf != nullptr; }, - }, - _file - ) - ? this - : nullptr; + if (auto *file = std::get_if(&_file); file) { + // This is called by the destructor, and an explicit `close` shouldn't close twice. + _file.emplace(nullptr); + if (file->close() != nullptr) { + return this; + } + } else if (std::get(_file) != nullptr) { + return this; + } + return nullptr; } char const *c_str(std::string const &path) const { - return std::visit( - Visitor{ - [&path](std::filebuf const &) { return path.c_str(); }, - [](std::streambuf const *buf) { - return buf == std::cin.rdbuf() ? "" : ""; - }, - }, - _file - ); + return std::holds_alternative(_file) ? path.c_str() + : std::get(_file) == std::cin.rdbuf() ? "" + : ""; } }; diff --git a/src/asm/fstack.cpp b/src/asm/fstack.cpp index 079416c8..627f37b0 100644 --- a/src/asm/fstack.cpp +++ b/src/asm/fstack.cpp @@ -62,28 +62,27 @@ std::string const &FileStackNode::name() const { } std::string const &FileStackNode::dump(uint32_t curLineNo) const { - Visitor visitor{ - [this](std::vector const &iters) -> std::string const & { - assert(this->parent); // REPT nodes use their parent's name - std::string const &lastName = this->parent->dump(this->lineNo); - fprintf(stderr, " -> %s", lastName.c_str()); - for (uint32_t i = iters.size(); i--;) - fprintf(stderr, "::REPT~%" PRIu32, iters[i]); - return lastName; - }, - [this](std::string const &name) -> std::string const & { - if (this->parent) { - this->parent->dump(this->lineNo); - fprintf(stderr, " -> %s", name.c_str()); - } else { - fputs(name.c_str(), stderr); - } - return name; - }, - }; - std::string const &topName = std::visit(visitor, data); - fprintf(stderr, "(%" PRIu32 ")", curLineNo); - return topName; + if (std::holds_alternative>(data)) { + assert(parent); // REPT nodes use their parent's name + std::string const &lastName = parent->dump(lineNo); + fputs(" -> ", stderr); + fputs(lastName.c_str(), stderr); + std::vector const &nodeIters = iters(); + for (uint32_t i = nodeIters.size(); i--;) { + fprintf(stderr, "::REPT~%" PRIu32, nodeIters[i]); + } + fprintf(stderr, "(%" PRIu32 ")", curLineNo); + return lastName; + } else { + if (parent) { + parent->dump(lineNo); + fputs(" -> ", stderr); + } + std::string const &nodeName = name(); + fputs(nodeName.c_str(), stderr); + fprintf(stderr, "(%" PRIu32 ")", curLineNo); + return nodeName; + } } void fstk_DumpCurrent() { diff --git a/src/asm/lexer.cpp b/src/asm/lexer.cpp index 2ef301ad..2cd7d68f 100644 --- a/src/asm/lexer.cpp +++ b/src/asm/lexer.cpp @@ -449,14 +449,11 @@ void lexer_OpenFileView( } void lexer_RestartRept(uint32_t lineNo) { - std::visit( - Visitor{ - [](MmappedLexerState &mmap) { mmap.offset = 0; }, - [](ViewedLexerState &view) { view.offset = 0; }, - [](auto &) {}, - }, - lexerState->content - ); + if (auto *mmap = std::get_if(&lexerState->content); mmap) { + mmap->offset = 0; + } else if (auto *view = std::get_if(&lexerState->content); view) { + view->offset = 0; + } initState(*lexerState); lexerState->lineNo = lineNo; } @@ -475,17 +472,12 @@ void lexer_CleanupState(LexerState &state) { // `lexerStateEOL`, but there's currently no situation in which this should happen. assert(&state != lexerStateEOL); - std::visit( - Visitor{ - [](MmappedLexerState &mmap) { - if (!mmap.isReferenced) - munmap(mmap.ptr, mmap.size); - }, - [](BufferedLexerState &cbuf) { close(cbuf.fd); }, - [](auto &) {}, - }, - state.content - ); + if (auto *mmap = std::get_if(&state.content); mmap) { + if (!mmap->isReferenced) + munmap(mmap->ptr, mmap->size); + } else if (auto *cbuf = std::get_if(&state.content); cbuf) { + close(cbuf->fd); + } } void lexer_SetMode(LexerMode mode) { @@ -654,57 +646,52 @@ static int peekInternal(uint8_t distance) { LEXER_BUF_SIZE ); - return std::visit( - Visitor{ - [&distance](MmappedLexerState &mmap) -> int { - if (size_t idx = mmap.offset + distance; idx < mmap.size) - return (uint8_t)mmap.ptr[idx]; - return EOF; - }, - [&distance](ViewedLexerState &view) -> int { - if (size_t idx = view.offset + distance; idx < view.size) - return (uint8_t)view.ptr[idx]; - return EOF; - }, - [&distance](BufferedLexerState &cbuf) -> int { - if (cbuf.nbChars > distance) - return (uint8_t)cbuf.buf[(cbuf.index + distance) % LEXER_BUF_SIZE]; + if (auto *mmap = std::get_if(&lexerState->content); mmap) { + if (size_t idx = mmap->offset + distance; idx < mmap->size) + return (uint8_t)mmap->ptr[idx]; + return EOF; + } else if (auto *view = std::get_if(&lexerState->content); view) { + if (size_t idx = view->offset + distance; idx < view->size) + return (uint8_t)view->ptr[idx]; + return EOF; + } else { + assert(std::holds_alternative(lexerState->content)); + auto &cbuf = std::get(lexerState->content); - // Buffer isn't full enough, read some chars in - size_t target = LEXER_BUF_SIZE - cbuf.nbChars; // Aim: making the buf full + if (cbuf.nbChars > distance) + return (uint8_t)cbuf.buf[(cbuf.index + distance) % LEXER_BUF_SIZE]; - // Compute the index we'll start writing to - size_t writeIndex = (cbuf.index + cbuf.nbChars) % LEXER_BUF_SIZE; + // Buffer isn't full enough, read some chars in + size_t target = LEXER_BUF_SIZE - cbuf.nbChars; // Aim: making the buf full - // If the range to fill passes over the buffer wrapping point, we need two reads - if (writeIndex + target > LEXER_BUF_SIZE) { - size_t nbExpectedChars = LEXER_BUF_SIZE - writeIndex; - size_t nbReadChars = readInternal(cbuf, writeIndex, nbExpectedChars); + // Compute the index we'll start writing to + size_t writeIndex = (cbuf.index + cbuf.nbChars) % LEXER_BUF_SIZE; - cbuf.nbChars += nbReadChars; + // If the range to fill passes over the buffer wrapping point, we need two reads + if (writeIndex + target > LEXER_BUF_SIZE) { + size_t nbExpectedChars = LEXER_BUF_SIZE - writeIndex; + size_t nbReadChars = readInternal(cbuf, writeIndex, nbExpectedChars); - writeIndex += nbReadChars; - if (writeIndex == LEXER_BUF_SIZE) - writeIndex = 0; + cbuf.nbChars += nbReadChars; - // If the read was incomplete, don't perform a second read - target -= nbReadChars; - if (nbReadChars < nbExpectedChars) - target = 0; - } - if (target != 0) - cbuf.nbChars += readInternal(cbuf, writeIndex, target); + writeIndex += nbReadChars; + if (writeIndex == LEXER_BUF_SIZE) + writeIndex = 0; - if (cbuf.nbChars > distance) - return (uint8_t)cbuf.buf[(cbuf.index + distance) % LEXER_BUF_SIZE]; + // If the read was incomplete, don't perform a second read + target -= nbReadChars; + if (nbReadChars < nbExpectedChars) + target = 0; + } + if (target != 0) + cbuf.nbChars += readInternal(cbuf, writeIndex, target); - // If there aren't enough chars even after refilling, give up - return EOF; - }, - [](std::monostate) -> int { return EOF; }, - }, - lexerState->content - ); + if (cbuf.nbChars > distance) + return (uint8_t)cbuf.buf[(cbuf.index + distance) % LEXER_BUF_SIZE]; + + // If there aren't enough chars even after refilling, give up + return EOF; + } } // forward declarations for peek @@ -782,22 +769,20 @@ restart: } else { // Advance within the file contents lexerState->colNo++; - std::visit( - Visitor{ - [](MmappedLexerState &mmap) { mmap.offset++; }, - [](ViewedLexerState &view) { view.offset++; }, - [](BufferedLexerState &cbuf) { - assert(cbuf.index < LEXER_BUF_SIZE); - cbuf.index++; - if (cbuf.index == LEXER_BUF_SIZE) - cbuf.index = 0; // Wrap around if necessary - assert(cbuf.nbChars > 0); - cbuf.nbChars--; - }, - [](std::monostate) {}, - }, - lexerState->content - ); + if (auto *mmap = std::get_if(&lexerState->content); mmap) { + mmap->offset++; + } else if (auto *view = std::get_if(&lexerState->content); view) { + view->offset++; + } else { + assert(std::holds_alternative(lexerState->content)); + auto &cbuf = std::get(lexerState->content); + assert(cbuf.index < LEXER_BUF_SIZE); + cbuf.index++; + if (cbuf.index == LEXER_BUF_SIZE) + cbuf.index = 0; // Wrap around if necessary + assert(cbuf.nbChars > 0); + cbuf.nbChars--; + } } } @@ -2225,13 +2210,16 @@ yy::parser::symbol_type yylex() { lexerState->lastToken = token.type; lexerState->atLineStart = token.type == T_(NEWLINE) || token.type == T_(EOB); - return std::visit( - Visitor{ - [&token](std::monostate) { return yy::parser::symbol_type(token.type); }, - [&token](auto &&value) { return yy::parser::symbol_type(token.type, value); }, - }, - token.value - ); + if (auto *numValue = std::get_if(&token.value); numValue) { + return yy::parser::symbol_type(token.type, *numValue); + } else if (auto *stringValue = std::get_if(&token.value); stringValue) { + return yy::parser::symbol_type(token.type, *stringValue); + } else if (auto *strValue = std::get_if(&token.value); strValue) { + return yy::parser::symbol_type(token.type, *strValue); + } else { + assert(std::holds_alternative(token.value)); + return yy::parser::symbol_type(token.type); + } } static void startCapture(CaptureBody &capture) { @@ -2242,21 +2230,14 @@ static void startCapture(CaptureBody &capture) { lexerState->disableInterpolation = true; capture.lineNo = lexer_GetLineNo(); - capture.body = std::visit( - Visitor{ - [](MmappedLexerState &mmap) -> char const * { - return lexerState->expansions.empty() ? &mmap.ptr[mmap.offset] : nullptr; - }, - [](ViewedLexerState &view) -> char const * { - return lexerState->expansions.empty() ? &view.ptr[view.offset] : nullptr; - }, - [](auto &) -> char const * { return nullptr; }, - }, - lexerState->content - ); - - if (capture.body == nullptr) { - // Indicates to retrieve the capture buffer when done capturing + if (auto *mmap = std::get_if(&lexerState->content); + mmap && lexerState->expansions.empty()) { + capture.body = &mmap->ptr[mmap->offset]; + } else if (auto *view = std::get_if(&lexerState->content); + view && lexerState->expansions.empty()) { + capture.body = &view->ptr[view->offset]; + } else { + capture.body = nullptr; // Indicates to retrieve the capture buffer when done capturing assert(lexerState->captureBuf == nullptr); lexerState->captureBuf = new (std::nothrow) std::vector(); if (!lexerState->captureBuf) @@ -2342,7 +2323,7 @@ bool lexer_CaptureMacroBody(CaptureBody &capture) { startCapture(capture); // If the file is `mmap`ed, we need not to unmap it to keep access to the macro - if (MmappedLexerState *mmap = std::get_if(&lexerState->content); mmap) + if (auto *mmap = std::get_if(&lexerState->content); mmap) mmap->isReferenced = true; int c = EOF; diff --git a/src/asm/parser.y b/src/asm/parser.y index 49eae868..15e0ad9f 100644 --- a/src/asm/parser.y +++ b/src/asm/parser.y @@ -2689,14 +2689,12 @@ static std::string strfmt( } else if (argIndex >= args.size()) { // Will warn after formatting is done. str += '%'; + } else if (auto *n = std::get_if(&args[argIndex]); n) { + str.append(fmt.formatNumber(*n)); } else { - str.append(std::visit( - Visitor{ - [&fmt](uint32_t n) { return fmt.formatNumber(n); }, - [&fmt](std::string const &s) { return fmt.formatString(s); }, - }, - args[argIndex] - )); + assert(std::holds_alternative(args[argIndex])); + auto &s = std::get(args[argIndex]); + str.append(fmt.formatString(s)); } argIndex++; diff --git a/src/asm/symbol.cpp b/src/asm/symbol.cpp index 7b85f07c..ac51bbce 100644 --- a/src/asm/symbol.cpp +++ b/src/asm/symbol.cpp @@ -61,21 +61,20 @@ static int32_t CallbackPC() { int32_t Symbol::getValue() const { assert(std::holds_alternative(data) || std::holds_alternative(data)); - if (int32_t const *value = std::get_if(&data); value) { + if (auto *value = std::get_if(&data); value) { return type == SYM_LABEL ? *value + getSection()->org : *value; } return getOutputValue(); } int32_t Symbol::getOutputValue() const { - return std::visit( - Visitor{ - [](int32_t value) -> int32_t { return value; }, - [](int32_t (*callback)()) -> int32_t { return callback(); }, - [](auto &) -> int32_t { return 0; }, - }, - data - ); + if (auto *value = std::get_if(&data); value) { + return *value; + } else if (auto *callback = std::get_if(&data); callback) { + return (*callback)(); + } else { + return 0; + } } std::string_view *Symbol::getMacro() const { diff --git a/src/link/main.cpp b/src/link/main.cpp index 97092cf3..a6217cea 100644 --- a/src/link/main.cpp +++ b/src/link/main.cpp @@ -66,31 +66,25 @@ std::string const &FileStackNode::name() const { } std::string const &FileStackNode::dump(uint32_t curLineNo) const { - Visitor visitor{ - [this](std::vector const &iters) -> std::string const & { - assert(this->parent); // REPT nodes use their parent's name - std::string const &lastName = this->parent->dump(this->lineNo); - fprintf(stderr, " -> %s", lastName.c_str()); - for (uint32_t iter : iters) - fprintf(stderr, "::REPT~%" PRIu32, iter); - return lastName; - }, - [this](std::string const &name) -> std::string const & { - if (this->parent) { - this->parent->dump(this->lineNo); - fprintf(stderr, " -> %s", name.c_str()); - } else { - fputs(name.c_str(), stderr); - } - return name; - }, - [](std::monostate) -> std::string const & { - unreachable_(); // This should not be possible - }, - }; - std::string const &topName = std::visit(visitor, data); - fprintf(stderr, "(%" PRIu32 ")", curLineNo); - return topName; + if (std::holds_alternative>(data)) { + assert(parent); // REPT nodes use their parent's name + std::string const &lastName = parent->dump(lineNo); + fputs(" -> ", stderr); + fputs(lastName.c_str(), stderr); + for (uint32_t iter : iters()) + fprintf(stderr, "::REPT~%" PRIu32, iter); + fprintf(stderr, "(%" PRIu32 ")", curLineNo); + return lastName; + } else { + if (parent) { + parent->dump(lineNo); + fputs(" -> ", stderr); + } + std::string const &nodeName = name(); + fputs(nodeName.c_str(), stderr); + fprintf(stderr, "(%" PRIu32 ")", curLineNo); + return nodeName; + } } void printDiag( diff --git a/src/link/object.cpp b/src/link/object.cpp index 7b752999..33ac1130 100644 --- a/src/link/object.cpp +++ b/src/link/object.cpp @@ -564,7 +564,7 @@ void obj_ReadFile(char const *fileName, unsigned int fileID) { if (symbol.type == SYMTYPE_EXPORT) sym_AddSymbol(symbol); - if (Label *label = std::get_if