Use C++-style casts (#1576)

CMakeLists.txt

@@ -61,7 +61,7 @@ else()
   if(MORE_WARNINGS)
     add_compile_options(-Werror -Wextra
                         -Walloc-zero -Wcast-align -Wcast-qual -Wduplicated-branches -Wduplicated-cond
-                        -Wfloat-equal -Wlogical-op -Wnull-dereference -Wshift-overflow=2
+                        -Wfloat-equal -Wlogical-op -Wnull-dereference -Wold-style-cast -Wshift-overflow=2
                         -Wstringop-overflow=4 -Wundef -Wuninitialized -Wunused
                         -Wshadow # TODO: -Wshadow=compatible-local?
                         -Wformat=2 -Wformat-overflow=2 -Wformat-truncation=1

Makefile

@@ -201,7 +201,7 @@ checkdiff:
 develop:
 	$Q${MAKE} WARNFLAGS="${WARNFLAGS} -Werror -Wextra \
 		-Walloc-zero -Wcast-align -Wcast-qual -Wduplicated-branches -Wduplicated-cond \
-		-Wfloat-equal -Wlogical-op -Wnull-dereference -Wshift-overflow=2 \
+		-Wfloat-equal -Wlogical-op -Wnull-dereference -Wold-style-cast -Wshift-overflow=2 \
 		-Wstringop-overflow=4 -Wundef -Wuninitialized -Wunused -Wshadow \
 		-Wformat=2 -Wformat-overflow=2 -Wformat-truncation=1 \
 		-Wno-format-nonliteral -Wno-strict-overflow -Wno-unused-but-set-variable \

include/asm/fstack.hpp

@@ -30,8 +30,8 @@ struct FileStackNode {
 	// Meaningless at the root level, but gets written to the object file anyway, so init it
 	uint32_t lineNo = 0;
 
-	// Set only if referenced: ID within the object file, -1 if not output yet
-	uint32_t ID = -1;
+	// Set only if referenced: ID within the object file, `UINT32_MAX` if not output yet
+	uint32_t ID = UINT32_MAX;
 
 	// REPT iteration counts since last named node, in reverse depth order
 	std::vector<uint32_t> &iters() { return data.get<std::vector<uint32_t>>(); }

include/asm/symbol.hpp

@@ -45,7 +45,7 @@ struct Symbol {
 	    >
 	    data;
 
-	uint32_t ID;       // ID of the symbol in the object file (-1 if none)
+	uint32_t ID;       // ID of the symbol in the object file (`UINT32_MAX` if none)
 	uint32_t defIndex; // Ordering of the symbol in the state file
 
 	bool isDefined() const { return type != SYM_REF; }
@@ -55,7 +55,7 @@ struct Symbol {
 	bool isConstant() const {
 		if (type == SYM_LABEL) {
 			Section const *sect = getSection();
-			return sect && sect->org != (uint32_t)-1;
+			return sect && sect->org != UINT32_MAX;
 		}
 		return type == SYM_EQU || type == SYM_VAR;
 	}

include/either.hpp

@@ -43,11 +43,11 @@ union Either {
 	// Generic field accessors; for internal use only.
 	template<typename T>
 	auto &field() {
-		return pick((T *)nullptr);
+		return pick(static_cast<T *>(nullptr));
 	}
 	template<typename T>
 	auto const &field() const {
-		return pick((T *)nullptr);
+		return pick(static_cast<T *>(nullptr));
 	}
 
 public:

include/gfx/rgba.hpp

@@ -28,7 +28,7 @@ struct Rgba {
 		    _5to8(cgbColor),
 		    _5to8(cgbColor >> 5),
 		    _5to8(cgbColor >> 10),
-		    (uint8_t)(cgbColor & 0x8000 ? 0x00 : 0xFF),
+		    static_cast<uint8_t>(cgbColor & 0x8000 ? 0x00 : 0xFF),
 		};
 	}
 

include/itertools.hpp

@@ -18,7 +18,7 @@ class EnumSeq {
 		explicit Iterator(T value) : _value(value) {}
 
 		Iterator &operator++() {
-			_value = (T)(_value + 1);
+			_value = static_cast<T>(_value + 1);
 			return *this;
 		}
 
@@ -29,7 +29,7 @@ class EnumSeq {
 	};
 
 public:
-	explicit EnumSeq(T stop) : _start((T)0), _stop(stop) {}
+	explicit EnumSeq(T stop) : _start(static_cast<T>(0)), _stop(stop) {}
 	explicit EnumSeq(T start, T stop) : _start(start), _stop(stop) {}
 
 	Iterator begin() { return Iterator(_start); }

src/asm/charmap.cpp

@@ -53,7 +53,7 @@ bool charmap_ForEach(
 				mappings[nodeIdx] = mapping;
 			for (unsigned c = 0; c < 256; c++) {
 				if (size_t nextIdx = node.next[c]; nextIdx)
-					prefixes.push({nextIdx, mapping + (char)c});
+					prefixes.push({nextIdx, mapping + static_cast<char>(c)});
 			}
 		}
 		mapFunc(charmap.name);
@@ -64,7 +64,7 @@ bool charmap_ForEach(
 }
 
 void charmap_New(std::string const &name, std::string const *baseName) {
-	size_t baseIdx = (size_t)-1;
+	size_t baseIdx = SIZE_MAX;
 
 	if (baseName != nullptr) {
 		if (auto search = charmapMap.find(*baseName); search == charmapMap.end())
@@ -82,7 +82,7 @@ void charmap_New(std::string const &name, std::string const *baseName) {
 	charmapMap[name] = charmapList.size();
 	Charmap &charmap = charmapList.emplace_back();
 
-	if (baseIdx != (size_t)-1)
+	if (baseIdx != SIZE_MAX)
 		charmap.nodes = charmapList[baseIdx].nodes; // Copies `charmapList[baseIdx].nodes`
 	else
 		charmap.nodes.emplace_back(); // Zero-init the root node
@@ -129,7 +129,7 @@ void charmap_Add(std::string const &mapping, std::vector<int32_t> &&value) {
 	size_t nodeIdx = 0;
 
 	for (char c : mapping) {
-		size_t &nextIdxRef = charmap.nodes[nodeIdx].next[(uint8_t)c];
+		size_t &nextIdxRef = charmap.nodes[nodeIdx].next[static_cast<uint8_t>(c)];
 		size_t nextIdx = nextIdxRef;
 
 		if (!nextIdx) {
@@ -157,7 +157,7 @@ bool charmap_HasChar(std::string const &input) {
 	size_t nodeIdx = 0;
 
 	for (char c : input) {
-		nodeIdx = charmap.nodes[nodeIdx].next[(uint8_t)c];
+		nodeIdx = charmap.nodes[nodeIdx].next[static_cast<uint8_t>(c)];
 
 		if (!nodeIdx)
 			return false;
@@ -184,7 +184,7 @@ size_t charmap_ConvertNext(std::string_view &input, std::vector<int32_t> *output
 	size_t inputIdx = 0;
 
 	for (size_t nodeIdx = 0; inputIdx < input.length();) {
-		nodeIdx = charmap.nodes[nodeIdx].next[(uint8_t)input[inputIdx]];
+		nodeIdx = charmap.nodes[nodeIdx].next[static_cast<uint8_t>(input[inputIdx])];
 
 		if (!nodeIdx)
 			break;

src/asm/fixpoint.cpp

@@ -29,7 +29,7 @@ static int32_t double2fix(double d, int32_t q) {
 		return 0;
 	if (isinf(d))
 		return d < 0 ? INT32_MIN : INT32_MAX;
-	return (int32_t)round(d * pow(2.0, q));
+	return static_cast<int32_t>(round(d * pow(2.0, q)));
 }
 
 static double turn2rad(double t) {

src/asm/format.cpp

@@ -250,15 +250,16 @@ void FormatSpec::appendNumber(std::string &str, uint32_t value) const {
 		}
 
 		double fval = fabs(value / pow(2.0, usePrec));
-		if (useExact)
-			snprintf(valueBuf, sizeof(valueBuf), "%.*fq%zu", (int)useFracWidth, fval, usePrec);
+		if (int fracWidthArg = static_cast<int>(useFracWidth); useExact)
+			snprintf(valueBuf, sizeof(valueBuf), "%.*fq%zu", fracWidthArg, fval, usePrec);
 		else
-			snprintf(valueBuf, sizeof(valueBuf), "%.*f", (int)useFracWidth, fval);
+			snprintf(valueBuf, sizeof(valueBuf), "%.*f", fracWidthArg, fval);
 	} else if (useType == 'd') {
 		// Decimal numbers may be formatted with a '-' sign by `snprintf`, so `abs` prevents that,
 		// with a special case for `INT32_MIN` since `labs(INT32_MIN)` is UB. The sign will be
 		// printed later from `signChar`.
-		uint32_t uval = value != (uint32_t)INT32_MIN ? labs((int32_t)value) : value;
+		uint32_t uval =
+		    value != static_cast<uint32_t>(INT32_MIN) ? labs(static_cast<int32_t>(value)) : value;
 		snprintf(valueBuf, sizeof(valueBuf), "%" PRIu32, uval);
 	} else {
 		char const *spec = useType == 'u'   ? "%" PRIu32

src/asm/fstack.cpp

@@ -170,8 +170,10 @@ bool yywrap() {
 		// If this is a FOR, update the symbol value
 		if (context.isForLoop && fileInfoIters.front() <= context.nbReptIters) {
 			// Avoid arithmetic overflow runtime error
-			uint32_t forValue = (uint32_t)context.forValue + (uint32_t)context.forStep;
-			context.forValue = forValue <= INT32_MAX ? forValue : -(int32_t)~forValue - 1;
+			uint32_t forValue =
+			    static_cast<uint32_t>(context.forValue) + static_cast<uint32_t>(context.forStep);
+			context.forValue =
+			    forValue <= INT32_MAX ? forValue : -static_cast<int32_t>(~forValue) - 1;
 			Symbol *sym = sym_AddVar(context.forName, context.forValue);
 
 			// This error message will refer to the current iteration
@@ -347,9 +349,9 @@ void fstk_RunFor(
 
 	uint32_t count = 0;
 	if (step > 0 && start < stop)
-		count = ((int64_t)stop - start - 1) / step + 1;
+		count = (static_cast<int64_t>(stop) - start - 1) / step + 1;
 	else if (step < 0 && stop < start)
-		count = ((int64_t)start - stop - 1) / -(int64_t)step + 1;
+		count = (static_cast<int64_t>(start) - stop - 1) / -static_cast<int64_t>(step) + 1;
 	else if (step == 0)
 		error("FOR cannot have a step value of 0\n");
 

src/asm/lexer.cpp

@@ -86,7 +86,7 @@ static char *mapFile(int fd, std::string const &path, size_t size) {
 			printf("mmap(%s, MAP_PRIVATE) failed, retrying with MAP_SHARED\n", path.c_str());
 		mappingAddr = mmap(nullptr, size, PROT_READ, MAP_SHARED, fd, 0);
 	}
-	return mappingAddr != MAP_FAILED ? (char *)mappingAddr : nullptr;
+	return mappingAddr != MAP_FAILED ? static_cast<char *>(mappingAddr) : nullptr;
 }
 
 struct FileUnmapDeleter {
@@ -102,7 +102,7 @@ struct FileUnmapDeleter {
 using namespace std::literals;
 
 // Bison 3.6 changed token "types" to "kinds"; cast to int for simple compatibility
-#define T_(name) (int)yy::parser::token::name
+#define T_(name) static_cast<int>(yy::parser::token::name)
 
 struct Token {
 	int type;
@@ -424,7 +424,7 @@ bool LexerState::setFileAsNextState(std::string const &filePath, bool updateStat
 
 		bool isMmapped = false;
 
-		if (size_t size = (size_t)statBuf.st_size; statBuf.st_size > 0) {
+		if (size_t size = static_cast<size_t>(statBuf.st_size); statBuf.st_size > 0) {
 			// Try using `mmap` for better performance
 			if (char *mappingAddr = mapFile(fd, path, size); mappingAddr != nullptr) {
 				close(fd);
@@ -543,7 +543,7 @@ size_t BufferedContent::readMore(size_t startIndex, size_t nbChars) {
 	size += nbReadChars;
 
 	// `nbReadChars` cannot be negative, so it's fine to cast to `size_t`
-	return (size_t)nbReadChars;
+	return static_cast<size_t>(nbReadChars);
 }
 
 void lexer_SetMode(LexerMode mode) {
@@ -709,20 +709,20 @@ int LexerState::peekChar() {
 	// This is `.peekCharAhead()` modified for zero lookahead distance
 	for (Expansion &exp : expansions) {
 		if (exp.offset < exp.size())
-			return (uint8_t)(*exp.contents)[exp.offset];
+			return static_cast<uint8_t>((*exp.contents)[exp.offset]);
 	}
 
 	if (content.holds<ViewedContent>()) {
 		auto &view = content.get<ViewedContent>();
 		if (view.offset < view.span.size)
-			return (uint8_t)view.span.ptr[view.offset];
+			return static_cast<uint8_t>(view.span.ptr[view.offset]);
 	} else {
 		auto &cbuf = content.get<BufferedContent>();
 		if (cbuf.size == 0)
 			cbuf.refill();
 		assume(cbuf.offset < LEXER_BUF_SIZE);
 		if (cbuf.size > 0)
-			return (uint8_t)cbuf.buf[cbuf.offset];
+			return static_cast<uint8_t>(cbuf.buf[cbuf.offset]);
 	}
 
 	// If there aren't enough chars, give up
@@ -738,21 +738,21 @@ int LexerState::peekCharAhead() {
 		// and `.peekCharAhead()` will continue with its parent
 		assume(exp.offset <= exp.size());
 		if (exp.offset + distance < exp.size())
-			return (uint8_t)(*exp.contents)[exp.offset + distance];
+			return static_cast<uint8_t>((*exp.contents)[exp.offset + distance]);
 		distance -= exp.size() - exp.offset;
 	}
 
 	if (content.holds<ViewedContent>()) {
 		auto &view = content.get<ViewedContent>();
 		if (view.offset + distance < view.span.size)
-			return (uint8_t)view.span.ptr[view.offset + distance];
+			return static_cast<uint8_t>(view.span.ptr[view.offset + distance]);
 	} else {
 		auto &cbuf = content.get<BufferedContent>();
 		assume(distance < LEXER_BUF_SIZE);
 		if (cbuf.size <= distance)
 			cbuf.refill();
 		if (cbuf.size > distance)
-			return (uint8_t)cbuf.buf[(cbuf.offset + distance) % LEXER_BUF_SIZE];
+			return static_cast<uint8_t>(cbuf.buf[(cbuf.offset + distance) % LEXER_BUF_SIZE]);
 	}
 
 	// If there aren't enough chars, give up
@@ -1032,11 +1032,12 @@ static uint32_t readFractionalPart(uint32_t integer) {
 		precision = fixPrecision;
 	}
 
-	if (integer >= ((uint64_t)1 << (32 - precision)))
+	if (integer >= (1ULL << (32 - precision)))
 		warning(WARNING_LARGE_CONSTANT, "Magnitude of fixed-point constant is too large\n");
 
 	// Cast to unsigned avoids undefined overflow behavior
-	uint32_t fractional = (uint32_t)round((double)value / divisor * pow(2.0, precision));
+	uint32_t fractional =
+	    static_cast<uint32_t>(round(static_cast<double>(value) / divisor * pow(2.0, precision)));
 
 	return (integer << precision) | fractional;
 }

src/asm/macro.cpp

@@ -55,10 +55,10 @@ void MacroArgs::appendArg(std::shared_ptr<std::string> arg) {
 
 void MacroArgs::shiftArgs(int32_t count) {
 	if (size_t nbArgs = args.size();
-	    count > 0 && ((uint32_t)count > nbArgs || shift > nbArgs - count)) {
+	    count > 0 && (static_cast<uint32_t>(count) > nbArgs || shift > nbArgs - count)) {
 		warning(WARNING_MACRO_SHIFT, "Cannot shift macro arguments past their end\n");
 		shift = nbArgs;
-	} else if (count < 0 && shift < (uint32_t)-count) {
+	} else if (count < 0 && shift < static_cast<uint32_t>(-count)) {
 		warning(WARNING_MACRO_SHIFT, "Cannot shift macro arguments past their beginning\n");
 		shift = 0;
 	} else {

src/asm/main.cpp

@@ -112,7 +112,7 @@ int main(int argc, char *argv[]) {
 	// Support SOURCE_DATE_EPOCH for reproducible builds
 	// https://reproducible-builds.org/docs/source-date-epoch/
 	if (char const *sourceDateEpoch = getenv("SOURCE_DATE_EPOCH"); sourceDateEpoch)
-		now = (time_t)strtoul(sourceDateEpoch, nullptr, 0);
+		now = static_cast<time_t>(strtoul(sourceDateEpoch, nullptr, 0));
 
 	Defer closeDependFile{[&] {
 		if (dependFile)