Replace assert with assume for release build optimization (#1390)

include/file.hpp

@@ -3,7 +3,6 @@
 #ifndef RGBDS_FILE_HPP
 #define RGBDS_FILE_HPP
 
-#include <assert.h>
 #include <fcntl.h>
 #include <fstream>
 #include <ios>
@@ -13,6 +12,7 @@
 #include <string>
 #include <variant>
 
+#include "helpers.hpp" // assume
 #include "platform.hpp"
 
 #include "gfx/main.hpp"
@@ -33,7 +33,7 @@ class File {
 		if (path != "-") {
 			return _file.emplace<std::filebuf>().open(path, mode) ? this : nullptr;
 		} else if (mode & std::ios_base::in) {
-			assert(!(mode & std::ios_base::out));
+			assume(!(mode & std::ios_base::out));
 			_file.emplace<std::streambuf *>(std::cin.rdbuf());
 			if (setmode(STDIN_FILENO, (mode & std::ios_base::binary) ? O_BINARY : O_TEXT) == -1) {
 				fatal(
@@ -43,7 +43,7 @@ class File {
 				);
 			}
 		} else {
-			assert(mode & std::ios_base::out);
+			assume(mode & std::ios_base::out);
 			_file.emplace<std::streambuf *>(std::cout.rdbuf());
 		}
 		return this;

include/helpers.hpp

@@ -3,12 +3,12 @@
 #ifndef HELPERS_H
 #define HELPERS_H
 
-// Ideally, we'd use `__has_attribute` and `__has_builtin`, but these were only introduced in GCC 9
+// Ideally we'd use `std::unreachable`, but it has insufficient compiler support
 #ifdef __GNUC__ // GCC or compatible
-	// In release builds, define "unreachable" as such, but trap in debug builds
 	#ifdef NDEBUG
 		#define unreachable_ __builtin_unreachable
 	#else
+		// In release builds, define "unreachable" as such, but trap in debug builds
 		#define unreachable_ __builtin_trap
 	#endif
 #else
@@ -18,32 +18,52 @@
 }
 #endif
 
-// Use builtins whenever possible, and shim them otherwise
+// Ideally we'd use `[[assume()]]`, but it has insufficient compiler support
+#ifdef NDEBUG
+	#ifdef _MSC_VER
+		#define assume(x) __assume(x)
+	#else
+		//  `[[gnu::assume()]]` for GCC or compatible also has insufficient support (GCC 13+ only)
+		#define assume(x) \
+			do { \
+				if (!(x)) \
+					unreachable_(); \
+			} while (0)
+	#endif
+#else
+	// In release builds, define "assume" as such, but `assert` in debug builds
+	#include <assert.h>
+	#define assume assert
+#endif
+
+// Ideally we'd use `std::bit_width`, but it has insufficient compiler support
 #ifdef __GNUC__ // GCC or compatible
 	#define ctz __builtin_ctz
 	#define clz __builtin_clz
 
 #elif defined(_MSC_VER)
-	#include <assert.h>
 	#include <intrin.h>
 	#pragma intrinsic(_BitScanReverse, _BitScanForward)
+
 static inline int ctz(unsigned int x) {
 	unsigned long cnt;
 
-	assert(x != 0);
+	assume(x != 0);
 	_BitScanForward(&cnt, x);
 	return cnt;
 }
+
 static inline int clz(unsigned int x) {
 	unsigned long cnt;
 
-	assert(x != 0);
+	assume(x != 0);
 	_BitScanReverse(&cnt, x);
 	return 31 - cnt;
 }
 
 #else
 	#include <limits.h>
+
 static inline int ctz(unsigned int x) {
 	int cnt = 0;
 

include/linkdefs.hpp

@@ -3,10 +3,11 @@
 #ifndef RGBDS_LINKDEFS_H
 #define RGBDS_LINKDEFS_H
 
-#include <assert.h>
 #include <stdint.h>
 #include <string>
 
+#include "helpers.hpp" // assume
+
 #define RGBDS_OBJECT_VERSION_STRING "RGBA"
 #define RGBDS_OBJECT_REV            10U
 
@@ -93,7 +94,7 @@ extern struct SectionTypeInfo {
  * @return `true` if the section's definition includes data
  */
 static inline bool sect_HasData(SectionType type) {
-	assert(type != SECTTYPE_INVALID);
+	assume(type != SECTTYPE_INVALID);
 	return type == SECTTYPE_ROM0 || type == SECTTYPE_ROMX;
 }
 

src/asm/fstack.cpp

@@ -3,7 +3,6 @@
 #include "asm/fstack.hpp"
 #include <sys/stat.h>
 
-#include <assert.h>
 #include <errno.h>
 #include <inttypes.h>
 #include <memory>
@@ -50,28 +49,28 @@ static std::vector<std::string> includePaths = {""};
 static std::string preIncludeName;
 
 std::vector<uint32_t> &FileStackNode::iters() {
-	assert(std::holds_alternative<std::vector<uint32_t>>(data));
+	assume(std::holds_alternative<std::vector<uint32_t>>(data));
 	return std::get<std::vector<uint32_t>>(data);
 }
 
 std::vector<uint32_t> const &FileStackNode::iters() const {
-	assert(std::holds_alternative<std::vector<uint32_t>>(data));
+	assume(std::holds_alternative<std::vector<uint32_t>>(data));
 	return std::get<std::vector<uint32_t>>(data);
 }
 
 std::string &FileStackNode::name() {
-	assert(std::holds_alternative<std::string>(data));
+	assume(std::holds_alternative<std::string>(data));
 	return std::get<std::string>(data);
 }
 
 std::string const &FileStackNode::name() const {
-	assert(std::holds_alternative<std::string>(data));
+	assume(std::holds_alternative<std::string>(data));
 	return std::get<std::string>(data);
 }
 
 std::string const &FileStackNode::dump(uint32_t curLineNo) const {
 	if (std::holds_alternative<std::vector<uint32_t>>(data)) {
-		assert(parent); // REPT nodes use their parent's name
+		assume(parent); // REPT nodes use their parent's name
 		std::string const &lastName = parent->dump(lineNo);
 		fputs(" -> ", stderr);
 		fputs(lastName.c_str(), stderr);
@@ -270,7 +269,7 @@ static void newMacroContext(Symbol const &macro, std::shared_ptr<MacroArgs> macr
 	fileInfoName.append(macro.name);
 
 	auto fileInfo = std::make_shared<FileStackNode>(NODE_MACRO, fileInfoName);
-	assert(!contextStack.empty()); // The top level context cannot be a MACRO
+	assume(!contextStack.empty()); // The top level context cannot be a MACRO
 	fileInfo->parent = oldContext.fileInfo;
 	fileInfo->lineNo = lexer_GetLineNo();
 
@@ -295,7 +294,7 @@ static Context &newReptContext(int32_t reptLineNo, ContentSpan const &span, uint
 	}
 
 	auto fileInfo = std::make_shared<FileStackNode>(NODE_REPT, fileInfoIters);
-	assert(!contextStack.empty()); // The top level context cannot be a REPT
+	assume(!contextStack.empty()); // The top level context cannot be a REPT
 	fileInfo->parent = oldContext.fileInfo;
 	fileInfo->lineNo = reptLineNo;
 

src/asm/lexer.cpp

@@ -5,7 +5,6 @@
 #include <sys/types.h>
 
 #include <algorithm>
-#include <assert.h>
 #include <ctype.h>
 #include <errno.h>
 #include <fcntl.h>
@@ -20,7 +19,7 @@
 	#include <unistd.h>
 #endif
 
-#include "helpers.hpp" // QUOTEDSTRLEN
+#include "helpers.hpp" // assume, QUOTEDSTRLEN
 #include "util.hpp"
 
 #include "asm/fixpoint.hpp"
@@ -477,14 +476,14 @@ LexerState::~LexerState() {
 	// scheduled at EOF; `lexerStateEOL` thus becomes a (weak) ref to that lexer state...
 	// It has been possible, due to a bug, that the corresponding fstack context gets popped
 	// before EOL, deleting the associated state... but it would still be switched to at EOL.
-	// This assertion checks that this doesn't happen again.
+	// This assumption checks that this doesn't happen again.
 	// It could be argued that deleting a state that's scheduled for EOF could simply clear
 	// `lexerStateEOL`, but there's currently no situation in which this should happen.
-	assert(this != lexerStateEOL);
+	assume(this != lexerStateEOL);
 }
 
 bool Expansion::advance() {
-	assert(offset <= size());
+	assume(offset <= size());
 	offset++;
 	return offset > size();
 }
@@ -494,11 +493,11 @@ BufferedContent::~BufferedContent() {
 }
 
 void BufferedContent::advance() {
-	assert(offset < LEXER_BUF_SIZE);
+	assume(offset < LEXER_BUF_SIZE);
 	offset++;
 	if (offset == LEXER_BUF_SIZE)
 		offset = 0; // Wrap around if necessary
-	assert(size > 0);
+	assume(size > 0);
 	size--;
 }
 
@@ -528,7 +527,7 @@ void BufferedContent::refill() {
 
 size_t BufferedContent::readMore(size_t startIndex, size_t nbChars) {
 	// This buffer overflow made me lose WEEKS of my life. Never again.
-	assert(startIndex + nbChars <= LEXER_BUF_SIZE);
+	assume(startIndex + nbChars <= LEXER_BUF_SIZE);
 	ssize_t nbReadChars = read(fd, &buf[startIndex], nbChars);
 
 	if (nbReadChars == -1)
@@ -671,7 +670,7 @@ static std::shared_ptr<std::string> readMacroArg(char name) {
 		error("Invalid macro argument '\\0'\n");
 		return nullptr;
 	} else {
-		assert(name > '0' && name <= '9');
+		assume(name > '0' && name <= '9');
 
 		MacroArgs *macroArgs = fstk_GetCurrentMacroArgs();
 		if (!macroArgs) {
@@ -698,11 +697,11 @@ int LexerState::peekChar() {
 		if (view->offset < view->span.size)
 			return (uint8_t)view->span.ptr[view->offset];
 	} else {
-		assert(std::holds_alternative<BufferedContent>(content));
+		assume(std::holds_alternative<BufferedContent>(content));
 		auto &cbuf = std::get<BufferedContent>(content);
 		if (cbuf.size == 0)
 			cbuf.refill();
-		assert(cbuf.offset < LEXER_BUF_SIZE);
+		assume(cbuf.offset < LEXER_BUF_SIZE);
 		if (cbuf.size > 0)
 			return (uint8_t)cbuf.buf[cbuf.offset];
 	}
@@ -718,7 +717,7 @@ int LexerState::peekCharAhead() {
 	for (Expansion &exp : expansions) {
 		// An expansion that has reached its end will have `exp.offset` == `exp.size()`,
 		// and `.peekCharAhead()` will continue with its parent
-		assert(exp.offset <= exp.size());
+		assume(exp.offset <= exp.size());
 		if (exp.offset + distance < exp.size())
 			return (uint8_t)(*exp.contents)[exp.offset + distance];
 		distance -= exp.size() - exp.offset;
@@ -728,9 +727,9 @@ int LexerState::peekCharAhead() {
 		if (view->offset + distance < view->span.size)
 			return (uint8_t)view->span.ptr[view->offset + distance];
 	} else {
-		assert(std::holds_alternative<BufferedContent>(content));
+		assume(std::holds_alternative<BufferedContent>(content));
 		auto &cbuf = std::get<BufferedContent>(content);
-		assert(distance < LEXER_BUF_SIZE);
+		assume(distance < LEXER_BUF_SIZE);
 		if (cbuf.size <= distance)
 			cbuf.refill();
 		if (cbuf.size > distance)
@@ -816,7 +815,7 @@ static void shiftChar() {
 		if (auto *view = std::get_if<ViewedContent>(&lexerState->content); view) {
 			view->offset++;
 		} else {
-			assert(std::holds_alternative<BufferedContent>(lexerState->content));
+			assume(std::holds_alternative<BufferedContent>(lexerState->content));
 			auto &cbuf = std::get<BufferedContent>(lexerState->content);
 			cbuf.advance();
 		}
@@ -1785,7 +1784,7 @@ static Token yylex_NORMAL() {
 					return token;
 
 				// `token` is either an `ID` or a `LOCAL_ID`, and both have a `std::string` value.
-				assert(std::holds_alternative<std::string>(token.value));
+				assume(std::holds_alternative<std::string>(token.value));
 
 				// Local symbols cannot be string expansions
 				if (token.type == T_(ID) && lexerState->expandStrings) {
@@ -1795,7 +1794,7 @@ static Token yylex_NORMAL() {
 					if (sym && sym->type == SYM_EQUS) {
 						std::shared_ptr<std::string> str = sym->getEqus();
 
-						assert(str);
+						assume(str);
 						beginExpansion(str, sym->name);
 						continue; // Restart, reading from the new buffer
 					}
@@ -2172,18 +2171,18 @@ yy::parser::symbol_type yylex() {
 	} 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));
+		assume(std::holds_alternative<std::monostate>(token.value));
 		return yy::parser::symbol_type(token.type);
 	}
 }
 
 static Capture startCapture() {
 	// Due to parser internals, it reads the EOL after the expression before calling this.
 	// Thus, we don't need to keep one in the buffer afterwards.
-	// The following assertion checks that.
-	assert(lexerState->atLineStart);
+	// The following assumption checks that.
+	assume(lexerState->atLineStart);
 
-	assert(!lexerState->capturing && lexerState->captureBuf == nullptr);
+	assume(!lexerState->capturing && lexerState->captureBuf == nullptr);
 	lexerState->capturing = true;
 	lexerState->captureSize = 0;
 
@@ -2194,7 +2193,7 @@ static Capture startCapture() {
 		    .lineNo = lineNo, .span = {.ptr = view->makeSharedContentPtr(), .size = 0}
         };
 	} else {
-		assert(lexerState->captureBuf == nullptr);
+		assume(lexerState->captureBuf == nullptr);
 		lexerState->captureBuf = std::make_shared<std::vector<char>>();
 		// `.span.ptr == nullptr`; indicates to retrieve the capture buffer when done capturing
 		return {

src/asm/output.cpp

@@ -2,7 +2,6 @@
 
 #include "asm/output.hpp"
 
-#include <assert.h>
 #include <deque>
 #include <inttypes.h>
 #include <stdio.h>
@@ -12,7 +11,7 @@
 #include <vector>
 
 #include "error.hpp"
-#include "helpers.hpp" // Defer
+#include "helpers.hpp" // assume, Defer
 
 #include "asm/fstack.hpp"
 #include "asm/lexer.hpp"
@@ -75,7 +74,7 @@ static uint32_t getSectIDIfAny(Section *sect) {
 
 // Write a patch to a file
 static void writepatch(Patch const &patch, FILE *file) {
-	assert(patch.src->ID != (uint32_t)-1);
+	assume(patch.src->ID != (uint32_t)-1);
 	putlong(patch.src->ID, file);
 	putlong(patch.lineNo, file);
 	putlong(patch.offset, file);
@@ -117,7 +116,7 @@ static void writesymbol(Symbol const &sym, FILE *file) {
 	if (!sym.isDefined()) {
 		putc(SYMTYPE_IMPORT, file);
 	} else {
-		assert(sym.src->ID != (uint32_t)-1);
+		assume(sym.src->ID != (uint32_t)-1);
 
 		putc(sym.isExported ? SYMTYPE_EXPORT : SYMTYPE_LOCAL, file);
 		putlong(sym.src->ID, file);

src/asm/parser.y

@@ -2542,7 +2542,7 @@ static std::string strfmt(
 		} else if (auto *n = std::get_if<uint32_t>(&args[argIndex]); n) {
 			fmt.appendNumber(str, *n);
 		} else {
-			assert(std::holds_alternative<std::string>(args[argIndex]));
+			assume(std::holds_alternative<std::string>(args[argIndex]));
 			auto &s = std::get<std::string>(args[argIndex]);
 			fmt.appendString(str, s);
 		}