Use C++20 concepts to require constraints on template parameters (#1977)

include/backtrace.hpp

@@ -9,6 +9,7 @@
 #include <stdio.h>
 #include <vector>
 
+#include "helpers.hpp" // InvocableR
 #include "style.hpp"
 
 #define TRACE_SEPARATOR  "<-"
@@ -25,8 +26,12 @@ extern Tracing tracing;
 
 bool trace_ParseTraceDepth(char const *arg);
 
-template<typename NodeT, typename NameFnT, typename LineNoFnT>
+template<typename NodeT>
-void trace_PrintBacktrace(std::vector<NodeT> const &stack, NameFnT getName, LineNoFnT getLineNo) {
+void trace_PrintBacktrace(
+    std::vector<NodeT> const &stack,
+    InvocableR<char const *, NodeT const &> auto getName,
+    InvocableR<uint32_t, NodeT const &> auto getLineNo
+) {
 	size_t n = stack.size();
 	if (n == 0) {
 		return; // LCOV_EXCL_LINE

include/diagnostics.hpp

@@ -30,7 +30,7 @@ struct WarningState {
 
 std::pair<WarningState, std::optional<uint32_t>> getInitialWarningState(std::string &flag);
 
-template<typename LevelEnumT>
+template<Enum LevelEnumT>
 struct WarningFlag {
 	char const *name;
 	LevelEnumT level;
@@ -38,14 +38,14 @@ struct WarningFlag {
 
 enum WarningBehavior { DISABLED, ENABLED, ERROR };
 
-template<typename WarningEnumT>
+template<Enum WarningEnumT>
 struct ParamWarning {
 	WarningEnumT firstID;
 	WarningEnumT lastID;
 	uint8_t defaultLevel;
 };
 
-template<typename WarningEnumT>
+template<Enum WarningEnumT>
 struct DiagnosticsState {
 	WarningState flagStates[WarningEnumT::NB_WARNINGS];
 	WarningState metaStates[WarningEnumT::NB_WARNINGS];
@@ -53,7 +53,7 @@ struct DiagnosticsState {
 	bool warningsAreErrors = false;
 };
 
-template<typename LevelEnumT, typename WarningEnumT>
+template<Enum LevelEnumT, Enum WarningEnumT>
 struct Diagnostics {
 	std::vector<WarningFlag<LevelEnumT>> metaWarnings;
 	std::vector<WarningFlag<LevelEnumT>> warningFlags;
@@ -71,7 +71,7 @@ struct Diagnostics {
 	void processWarningFlag(char const *flag);
 };
 
-template<typename LevelEnumT, typename WarningEnumT>
+template<Enum LevelEnumT, Enum WarningEnumT>
 WarningBehavior Diagnostics<LevelEnumT, WarningEnumT>::getWarningBehavior(WarningEnumT id) const {
 	// Check if warnings are globally disabled
 	if (!state.warningsEnabled) {
@@ -120,7 +120,7 @@ WarningBehavior Diagnostics<LevelEnumT, WarningEnumT>::getWarningBehavior(Warnin
 	return WarningBehavior::DISABLED;
 }
 
-template<typename LevelEnumT, typename WarningEnumT>
+template<Enum LevelEnumT, Enum WarningEnumT>
 void Diagnostics<LevelEnumT, WarningEnumT>::processWarningFlag(char const *flag) {
 	std::string rootFlag = flag;
 

include/helpers.hpp

@@ -3,6 +3,8 @@
 #ifndef RGBDS_HELPERS_HPP
 #define RGBDS_HELPERS_HPP
 
+#include <type_traits>
+
 // Ideally we'd use `std::unreachable`, but it has insufficient compiler support
 #ifdef __GNUC__ // GCC or compatible
 	#ifdef NDEBUG
@@ -101,12 +103,29 @@ static inline int clz(unsigned int x) {
 
 // MSVC does not inline `strlen()` or `.length()` of a constant string
 template<int SizeOfString>
+    requires(SizeOfString > 0)
 static constexpr int literal_strlen(char const (&)[SizeOfString]) {
 	return SizeOfString - 1; // Don't count the ending '\0'
 }
 
+// Concept for template parameters that must be an `enum` type
+template<typename EnumT>
+concept Enum = std::is_enum_v<EnumT>;
+
+// Concept for template parameters that must be a function returning `void`
+template<typename ProcedureFnT, typename... Args>
+concept Procedure = std::is_void_v<std::invoke_result_t<ProcedureFnT, Args...>>;
+
+// Concept for template parameters that must be a function returning a particular type
+template<typename FnT, typename ReturnT, typename... Args>
+concept InvocableR = std::is_invocable_r_v<ReturnT, FnT, Args...>;
+
+// Concept for template parameters that must match a particular type modulo cv-qualifiers
+template<typename T, typename UnqualifiedT>
+concept QualifiedEquivalent = std::is_same_v<std::remove_cvref_t<T>, UnqualifiedT>;
+
 // For ad-hoc RAII in place of a `defer` statement or cross-platform `__attribute__((cleanup))`
-template<typename DeferredFnT>
+template<Procedure DeferredFnT>
 struct Defer {
 	DeferredFnT deferred;
 	Defer(DeferredFnT func) : deferred(func) {}

include/itertools.hpp

@@ -12,6 +12,8 @@
 #include <unordered_map>
 #include <utility>
 
+#include "helpers.hpp" // Enum
+
 // A wrapper around iterables to reverse their iteration order; used in `for`-each loops.
 template<typename IterableT>
 struct ReversedIterable {
@@ -78,7 +80,7 @@ public:
 };
 
 // An iterable of `enum` values in the half-open range [start, stop).
-template<typename EnumT>
+template<Enum EnumT>
 class EnumSeq {
 	EnumT _start;
 	EnumT _stop;
@@ -169,7 +171,7 @@ template<typename IterableT>
 using ZipHolder = std::conditional_t<
     std::is_lvalue_reference_v<IterableT>,
     IterableT,
-    std::remove_cv_t<std::remove_reference_t<IterableT>>>;
+    std::remove_cvref_t<IterableT>>;
 
 // Iterates over N containers at once, yielding tuples of N items at a time.
 // Does the same number of iterations as the first container's iterator!

include/link/section.hpp

@@ -8,6 +8,7 @@
 #include <string>
 #include <vector>
 
+#include "helpers.hpp" // QualifiedEquivalent
 #include "linkdefs.hpp"
 
 struct FileStackNode;
@@ -52,7 +53,7 @@ struct Section {
 
 private:
 	// Template class for both const and non-const iterators over the "pieces" of this section
-	template<typename SectionT>
+	template<QualifiedEquivalent<Section> SectionT>
 	class PiecesIterable {
 		SectionT *_firstPiece;
 

include/util.hpp

@@ -39,8 +39,11 @@ bool startsIdentifier(int c);
 bool continuesIdentifier(int c);
 
 template<uint32_t Base>
+inline constexpr bool BaseV = Base > 0 && Base <= 36;
+
+template<uint32_t Base>
+    requires BaseV<Base>
 bool isDigit(int c) {
-	static_assert(Base <= 36, "Base must be 36 or less to allow digits 0-9A-Z");
 	if constexpr (Base <= 10) {
 		return c >= '0' && c < static_cast<int>('0' + Base);
 	} else {
@@ -50,8 +53,8 @@ bool isDigit(int c) {
 }
 
 template<uint32_t Base>
+    requires BaseV<Base>
 uint8_t parseDigit(int c) {
-	static_assert(Base <= 36, "Base must be 36 or less to allow digits 0-9A-Z");
 	assume(isDigit<Base>(c));
 	if constexpr (Base <= 10) {
 		return c - '0';

src/asm/charmap.cpp

@@ -3,6 +3,7 @@
 #include "asm/charmap.hpp"
 
 #include <algorithm>
+#include <concepts> // predicate
 #include <map>
 #include <optional>
 #include <stack>
@@ -65,8 +66,9 @@ struct Charmap {
 };
 
 // Traverse the trie depth-first to derive the character mappings in definition order
-template<typename CallbackFnT>
+bool forEachChar(
-bool forEachChar(Charmap const &charmap, CallbackFnT callback) {
+    Charmap const &charmap, std::predicate<size_t, std::string const &> auto callback
+) {
 	// clang-format off: nested initializers
 	for (std::stack<std::pair<size_t, std::string>> prefixes({{0, ""}}); !prefixes.empty();) {
 		// clang-format on

src/asm/lexer.cpp

@@ -4,6 +4,7 @@
 #include <sys/stat.h>
 
 #include <algorithm>
+#include <concepts> // predicate
 #include <errno.h>
 #include <fcntl.h>
 #include <fstream>
@@ -527,6 +528,7 @@ static void shiftChar();
 static int bumpChar();
 static int nextChar();
 template<uint32_t Base>
+    requires BaseV<Base>
 static uint32_t readNumber(int initial, char const *prefix);
 
 static uint32_t readBracketedMacroArgNum() {
@@ -818,8 +820,7 @@ static int nextChar() {
 	return peek();
 }
 
-template<typename PredicateFnT>
+static int skipChars(std::predicate<int> auto predicate) {
-static int skipChars(PredicateFnT predicate) {
 	int c = peek();
 	while (predicate(c)) {
 		c = nextChar();
@@ -1099,6 +1100,7 @@ void lexer_SetGfxDigits(char const digits[4]) {
 }
 
 template<uint32_t Base>
+    requires BaseV<Base>
 static uint32_t readNumber(int initial, char const *prefix) {
 	auto isSomeDigit = [](int c) {
 		if constexpr (Base == 2) {
@@ -2283,8 +2285,7 @@ yy::parser::symbol_type yylex() {
 	}
 }
 
-template<typename CallbackFnT>
+static Capture makeCapture(char const *name, InvocableR<int, int> auto callback) {
-static Capture makeCapture(char const *name, CallbackFnT callback) {
 	// 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 assumption checks that.

src/asm/parser.y

@@ -32,6 +32,7 @@
 
 %code {
 	#include <algorithm>
+	#include <concepts> // invocable
 	#include <inttypes.h>
 	#include <optional>
 	#include <stdio.h>
@@ -57,9 +58,10 @@
 
 	yy::parser::symbol_type yylex(); // Provided by lexer.cpp
 
-	template<typename NumCallbackFnT, typename StrCallbackFnT>
 	static auto handleSymbolByType(
-	    std::string const &symName, NumCallbackFnT numCallback, StrCallbackFnT strCallback
+	    std::string const &symName,
+	    std::invocable<Expression const &> auto numCallback,
+	    std::invocable<std::string const &> auto strCallback
 	) {
 		if (Symbol *sym = sym_FindScopedSymbol(symName); sym && sym->type == SYM_EQUS) {
 			return strCallback(*sym->getEqus());

src/gfx/pal_spec.cpp

@@ -5,6 +5,7 @@
 #include <algorithm>
 #include <array>
 #include <charconv>
+#include <concepts> // unsigned_integral
 #include <errno.h>
 #include <fstream>
 #include <inttypes.h>
@@ -202,7 +203,7 @@ static void warnExtraColors(
 }
 
 // Parses the initial part of a string_view, advancing the "read index" as it does
-template<typename UintT> // Should be uint*_t
+template<std::unsigned_integral UintT>
 static std::optional<UintT> parseDec(std::string const &str, size_t &n) {
 	UintT value = 0;
 	auto result = std::from_chars(str.data() + n, str.data() + str.length(), value);

src/link/lexer.cpp

@@ -95,6 +95,7 @@ static std::string readKeyword(int initial) {
 }
 
 template<uint32_t Base>
+    requires BaseV<Base>
 static yy::parser::symbol_type readNumber(int initial, char const *prefix, char const *name) {
 	LexerStackEntry &context = lexerStack.back();
 	uint32_t number;

src/link/output.cpp

@@ -310,8 +310,9 @@ static bool compareSymbols(SortedSymbol const &sym1, SortedSymbol const &sym2) {
 	       < std::tie(sym2.addr, sym2_local, sym2.parentAddr, sym2_name);
 }
 
-template<typename CallbackFnT>
+static void forEachSortedSection(
-static void forEachSortedSection(SortedSections const &bankSections, CallbackFnT callback) {
+    SortedSections const &bankSections, Procedure<Section const &> auto callback
+) {
 	for (Section const *sect : bankSections.zeroLenSections) {
 		for (Section const &piece : sect->pieces()) {
 			callback(piece);
@@ -411,8 +412,7 @@ static void writeSectionName(std::string const &name, FILE *file) {
 	}
 }
 
-template<typename CallbackFnT>
+uint16_t forEachSection(SortedSections const &sectList, Procedure<Section const &> auto callback) {
-uint16_t forEachSection(SortedSections const &sectList, CallbackFnT callback) {
 	uint16_t used = 0;
 	auto section = sectList.sections.begin();
 	auto zeroLenSection = sectList.zeroLenSections.begin();