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

@@ -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<std::filebuf>(&_file);
+		return file ? *file : *std::get<std::streambuf *>(_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<std::streambuf *>(nullptr);
-			               return file.close() != nullptr;
-		               },
-		               [](std::streambuf *buf) { return buf != nullptr; },
-		           },
-		           _file
-		       )
-		           ? this
-		           : nullptr;
+		if (auto *file = std::get_if<std::filebuf>(&_file); file) {
+			// This is called by the destructor, and an explicit `close` shouldn't close twice.
+			_file.emplace<std::streambuf *>(nullptr);
+			if (file->close() != nullptr) {
+				return this;
+			}
+		} else if (std::get<std::streambuf *>(_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() ? "<stdin>" : "<stdout>";
-		        },
-		    },
-		    _file
-		);
+		return std::holds_alternative<std::filebuf>(_file)             ? path.c_str()
+		       : std::get<std::streambuf *>(_file) == std::cin.rdbuf() ? "<stdin>"
+		                                                               : "<stdout>";
 	}
 };
 

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<uint32_t> 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<std::vector<uint32_t>>(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<uint32_t> 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() {

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<MmappedLexerState>(&lexerState->content); mmap) {
+		mmap->offset = 0;
+	} else if (auto *view = std::get_if<ViewedLexerState>(&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<MmappedLexerState>(&state.content); mmap) {
+		if (!mmap->isReferenced)
+			munmap(mmap->ptr, mmap->size);
+	} else if (auto *cbuf = std::get_if<BufferedLexerState>(&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<MmappedLexerState>(&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<ViewedLexerState>(&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<BufferedLexerState>(lexerState->content));
+		auto &cbuf = std::get<BufferedLexerState>(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<MmappedLexerState>(&lexerState->content); mmap) {
+			mmap->offset++;
+		} else if (auto *view = std::get_if<ViewedLexerState>(&lexerState->content); view) {
+			view->offset++;
+		} else {
+			assert(std::holds_alternative<BufferedLexerState>(lexerState->content));
+			auto &cbuf = std::get<BufferedLexerState>(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<uint32_t>(&token.value); numValue) {
+		return yy::parser::symbol_type(token.type, *numValue);
+	} else if (auto *stringValue = std::get_if<String>(&token.value); stringValue) {
+		return yy::parser::symbol_type(token.type, *stringValue);
+	} else if (auto *strValue = std::get_if<std::string>(&token.value); strValue) {
+		return yy::parser::symbol_type(token.type, *strValue);
+	} else {
+		assert(std::holds_alternative<std::monostate>(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<MmappedLexerState>(&lexerState->content);
+	    mmap && lexerState->expansions.empty()) {
+		capture.body = &mmap->ptr[mmap->offset];
+	} else if (auto *view = std::get_if<ViewedLexerState>(&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<char>();
 		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<MmappedLexerState>(&lexerState->content); mmap)
+	if (auto *mmap = std::get_if<MmappedLexerState>(&lexerState->content); mmap)
 		mmap->isReferenced = true;
 
 	int c = EOF;

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<uint32_t>(&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<std::string>(args[argIndex]));
+			auto &s = std::get<std::string>(args[argIndex]);
+			str.append(fmt.formatString(s));
 		}
 
 		argIndex++;

src/asm/symbol.cpp

@@ -61,21 +61,20 @@ static int32_t CallbackPC() {
 
 int32_t Symbol::getValue() const {
 	assert(std::holds_alternative<int32_t>(data) || std::holds_alternative<int32_t (*)()>(data));
-	if (int32_t const *value = std::get_if<int32_t>(&data); value) {
+	if (auto *value = std::get_if<int32_t>(&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<int32_t>(&data); value) {
+		return *value;
+	} else if (auto *callback = std::get_if<int32_t (*)()>(&data); callback) {
+		return (*callback)();
+	} else {
+		return 0;
+	}
 }
 
 std::string_view *Symbol::getMacro() const {

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<uint32_t> 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<std::vector<uint32_t>>(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(

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<Label>(&symbol.data); label)
+		if (auto *label = std::get_if<Label>(&symbol.data); label)
 			nbSymPerSect[label->sectionID]++;
 	}
 
@@ -603,7 +603,7 @@ void obj_ReadFile(char const *fileName, unsigned int fileID) {
 
 	// Give symbols' section pointers to their sections
 	for (uint32_t i = 0; i < nbSymbols; i++) {
-		if (Label *label = std::get_if<Label>(&fileSymbols[i].data); label) {
+		if (auto *label = std::get_if<Label>(&fileSymbols[i].data); label) {
 			Section *section = fileSections[label->sectionID].get();
 
 			label->section = section;
@@ -620,7 +620,7 @@ void obj_ReadFile(char const *fileName, unsigned int fileID) {
 	// This has to run **after** all the `sect_AddSection()` calls,
 	// so that `sect_GetSection()` will work
 	for (uint32_t i = 0; i < nbSymbols; i++) {
-		if (Label *label = std::get_if<Label>(&fileSymbols[i].data); label) {
+		if (auto *label = std::get_if<Label>(&fileSymbols[i].data); label) {
 			if (Section *section = label->section; section->modifier != SECTION_NORMAL) {
 				if (section->modifier == SECTION_FRAGMENT)
 					// Add the fragment's offset to the symbol's

src/link/patch.cpp

@@ -218,7 +218,7 @@ static int32_t computeRPNExpr(Patch const &patch, std::vector<Symbol> const &fil
 				);
 				isError = true;
 				value = 1;
-			} else if (Label const *label = std::get_if<Label>(&symbol->data); label) {
+			} else if (auto *label = std::get_if<Label>(&symbol->data); label) {
 				value = label->section->bank;
 			} else {
 				error(
@@ -383,16 +383,11 @@ static int32_t computeRPNExpr(Patch const &patch, std::vector<Symbol> const &fil
 					    fileSymbols[value].name.c_str()
 					);
 					isError = true;
+				} else if (auto *label = std::get_if<Label>(&symbol->data); label) {
+					value = label->section->org + label->offset;
 				} else {
-					value = std::visit(
-					    Visitor{
-					        [](int32_t val) -> int32_t { return val; },
-					        [](Label label) -> int32_t {
-						        return label.section->org + label.offset;
-					        },
-					    },
-					    symbol->data
-					);
+					assert(std::holds_alternative<int32_t>(symbol->data));
+					value = std::get<int32_t>(symbol->data);
 				}
 			}
 			break;

src/link/sdas_obj.cpp

@@ -392,16 +392,14 @@ void sdobj_ReadFile(FileStackNode const &where, FILE *file, std::vector<Symbol>
 				if (other) {
 					// The same symbol can only be defined twice if neither
 					// definition is in a floating section
-					auto visitor = Visitor{
-					    [](int32_t value) -> std::tuple<Section *, int32_t> {
-						    return {nullptr, value};
-					    },
-					    [](Label label) -> std::tuple<Section *, int32_t> {
-						    return {label.section, label.offset};
-					    },
+					auto checkSymbol = [](Symbol const &sym) -> std::tuple<Section *, int32_t> {
+						if (auto *label = std::get_if<Label>(&sym.data); label)
+							return {label->section, label->offset};
+						assert(std::holds_alternative<int32_t>(sym.data));
+						return {nullptr, std::get<int32_t>(sym.data)};
 					};
-					auto [symbolSection, symbolValue] = std::visit(visitor, symbol.data);
-					auto [otherSection, otherValue] = std::visit(visitor, other->data);
+					auto [symbolSection, symbolValue] = checkSymbol(symbol);
+					auto [otherSection, otherValue] = checkSymbol(*other);
 
 					if ((otherSection && !otherSection->isAddressFixed)
 					    || (symbolSection && !symbolSection->isAddressFixed)) {

src/link/symbol.cpp

@@ -27,8 +27,8 @@ Label const &Symbol::label() const {
 
 void sym_AddSymbol(Symbol &symbol) {
 	Symbol *other = sym_GetSymbol(symbol.name);
-	int32_t const *symValue = std::get_if<int32_t>(&symbol.data);
-	int32_t const *otherValue = other ? std::get_if<int32_t>(&other->data) : nullptr;
+	auto *symValue = std::get_if<int32_t>(&symbol.data);
+	auto *otherValue = other ? std::get_if<int32_t>(&other->data) : nullptr;
 
 	// Check if the symbol already exists with a different value
 	if (other && !(symValue && otherValue && *symValue == *otherValue)) {