Util: Add specifier type checking

src/util/format/builder.cpp

@@ -14,6 +14,7 @@
 #include <string_view>
 
 #include "util/format/builder.h"
+#include "util/meta/assert.h"
 
 namespace Util::Format {
 
@@ -53,4 +54,34 @@ void Builder::putF64(double number, uint8_t precision) const
 	m_builder << string;
 }
 
+void Builder::putString(std::string_view string, size_t width, Align align, char fill) const
+{
+	size_t length = string.length();
+	if (width < length) {
+		m_builder.write(string.data(), length);
+		return;
+	}
+
+	switch (align) {
+	case Align::None:
+	case Align::Left:
+		m_builder.write(string.data(), length);
+		m_builder << std::string(width - length, fill);
+		break;
+	case Align::Center: {
+		size_t half = (width - length) / 2;
+		m_builder << std::string(half, fill);
+		m_builder.write(string.data(), length);
+		m_builder << std::string(width - half - length, fill);
+		break;
+	}
+	case Align::Right:
+		m_builder << std::string(width - length, fill);
+		m_builder.write(string.data(), length);
+		break;
+	default:
+		VERIFY_NOT_REACHED();
+	};
+}
+
 } // namespace Util::Format

src/util/format/builder.h

@@ -43,7 +43,7 @@ public:
 	void putF32(float number, uint8_t precision = 6) const;
 	void putF64(double number, uint8_t precision = 6) const;
 	void putCharacter(char character) const { m_builder.write(&character, 1); }
-	void putString(const std::string_view string) const { m_builder.write(string.data(), string.length()); }
+	void putString(std::string_view string, size_t width = 0, Align align = Align::Left, char fill = ' ') const;
 	void putPointer(const void* pointer) const { m_builder << pointer; }
 
 	const std::stringstream& builder() const { return m_builder; }

src/util/format/format.cpp

@@ -31,7 +31,7 @@ void variadicFormatImpl(Builder& builder, Parser& parser, TypeErasedParameters&
 
 	// Get parameter at index, or next
 	size_t index = indexMaybe.has_value() ? indexMaybe.value() : parameters.tell();
-	VERIFY(index < parameters.size(), "argument not found at index '%zu':'%zu'", index, parameters.size());
+	VERIFY(index < parameters.size(), "argument not found at index '%zu'", index);
 	auto& parameter = parameters.parameter(index);
 
 	// Format the parameter

src/util/format/formatter.cpp

@@ -60,10 +60,10 @@ void Formatter<double>::format(Builder& builder, double value) const
 {
 	if (specifier.precision < 0) {
 		builder.putF64(value);
+		return;
 	}
-	else {
-		builder.putF64(value, specifier.precision);
-	}
+
+	builder.putF64(value, specifier.precision);
 }
 
 // Char
@@ -82,15 +82,26 @@ void Formatter<bool>::format(Builder& builder, bool value) const
 
 // String
 
-void Formatter<const char*>::format(Builder& builder, const char* value) const
-{
-	builder.putString(value != nullptr ? std::string_view { value, strlen(value) } : "(nil)");
-}
-
 template<>
 void Formatter<std::string_view>::format(Builder& builder, std::string_view value) const
 {
-	builder.putString(value);
+	builder.putString(value, specifier.width, specifier.align, specifier.fill);
+}
+
+void Formatter<const char*>::parse(Parser& parser)
+{
+	parser.parseSpecifier(specifier, Parser::ParameterType::CString);
+}
+
+void Formatter<const char*>::format(Builder& builder, const char* value) const
+{
+	if (specifier.type == PresentationType::Pointer) {
+		Formatter<const void*> formatter { specifier };
+		formatter.format(builder, static_cast<const void*>(value));
+		return;
+	}
+
+	builder.putString(value != nullptr ? std::string_view { value, strlen(value) } : "nullptr");
 }
 
 // Pointer

src/util/format/formatter.h

@@ -54,7 +54,7 @@ struct Specifier {
 
 	bool alternativeForm = false;
 	bool zeroPadding = false;
-	int width = 0;
+	size_t width = 0;
 	int8_t precision = -1;
 
 	PresentationType type = PresentationType::None;
@@ -67,16 +67,16 @@ struct Formatter {
 	constexpr void parse(Parser& parser)
 	{
 		if (std::is_integral_v<T>) {
-			parser.parseSpecifier(specifier, Parser::SpecifierType::Integral);
+			parser.parseSpecifier(specifier, Parser::ParameterType::Integral);
 		}
 		else if (std::is_floating_point_v<T>) {
-			parser.parseSpecifier(specifier, Parser::SpecifierType::FloatingPoint);
+			parser.parseSpecifier(specifier, Parser::ParameterType::FloatingPoint);
 		}
 		else if (std::is_same_v<T, char>) {
-			parser.parseSpecifier(specifier, Parser::SpecifierType::Char);
+			parser.parseSpecifier(specifier, Parser::ParameterType::Char);
 		}
 		else if (std::is_same_v<T, std::string_view>) {
-			parser.parseSpecifier(specifier, Parser::SpecifierType::String);
+			parser.parseSpecifier(specifier, Parser::ParameterType::String);
 		}
 	}
 
@@ -124,6 +124,7 @@ struct Formatter<std::string> : Formatter<std::string_view> {
 
 template<>
 struct Formatter<const char*> : Formatter<std::string_view> {
+	void parse(Parser& parser);
 	void format(Builder& builder, const char* value) const;
 };
 
@@ -143,13 +144,13 @@ struct Formatter<T*> {
 
 	constexpr void parse(Parser& parser)
 	{
-		parser.parseSpecifier(specifier, Parser::SpecifierType::Pointer);
+		parser.parseSpecifier(specifier, Parser::ParameterType::Pointer);
 	}
 
 	void format(Builder& builder, T* value) const
 	{
 		value == nullptr
-			? builder.putString("0x0")
+			? builder.putString("nullptr")
 			: builder.putPointer(static_cast<const void*>(value));
 	}
 };

src/util/format/parser.cpp

@@ -158,7 +158,7 @@ std::optional<size_t> Parser::consumeIndex()
 	VERIFY_NOT_REACHED();
 }
 
-void Parser::parseSpecifier(Specifier& specifier, SpecifierType type)
+void Parser::parseSpecifier(Specifier& specifier, ParameterType type)
 {
 	if (consumeSpecific('}') || isEOF()) {
 		return;
@@ -204,7 +204,7 @@ void Parser::parseSpecifier(Specifier& specifier, SpecifierType type)
 		// Sign is only valid for numeric types
 		if (peek0 == '+' || peek0 == '-' || peek0 == ' ') {
 			VERIFY(state < State::AfterSign, "unexpected '%c' at this position", peek0);
-			VERIFY(type == SpecifierType::Integral || type == SpecifierType::FloatingPoint,
+			VERIFY(type == ParameterType::Integral || type == ParameterType::FloatingPoint,
 			       "sign option is only valid for numeric types");
 			state = State::AfterSign;
 			specifier.sign = static_cast<Builder::Sign>(peek0);
@@ -213,7 +213,7 @@ void Parser::parseSpecifier(Specifier& specifier, SpecifierType type)
 		// Alternative form is only valid for numeric types
 		if (peek0 == '#') {
 			VERIFY(state < State::AfterAlternativeForm, "unexpected '#' at this position");
-			VERIFY(type == SpecifierType::Integral || type == SpecifierType::FloatingPoint,
+			VERIFY(type == ParameterType::Integral || type == ParameterType::FloatingPoint,
 			       "'#' option is only valid for numeric types");
 			state = State::AfterAlternativeForm;
 			specifier.alternativeForm = true;
@@ -223,7 +223,7 @@ void Parser::parseSpecifier(Specifier& specifier, SpecifierType type)
 		if (peek0 == '0') {
 			if (state < State::AfterWidth) {
 				VERIFY(state < State::AfterZeroPadding, "unexpected '0' at this position");
-				VERIFY(type == SpecifierType::Integral || type == SpecifierType::FloatingPoint,
+				VERIFY(type == ParameterType::Integral || type == ParameterType::FloatingPoint,
 				       "zero padding option is only valid for numeric types");
 				state = State::AfterZeroPadding;
 				specifier.zeroPadding = true;
@@ -274,7 +274,7 @@ void Parser::parseSpecifier(Specifier& specifier, SpecifierType type)
 			// We parse until after the closing '}', so take this into account
 			widthEnd = tell() - 1;
 		}
-		specifier.width = static_cast<int>(stringToNumber(m_input.substr(widthBegin, widthEnd - widthBegin)));
+		specifier.width = stringToNumber(m_input.substr(widthBegin, widthEnd - widthBegin));
 	}
 
 	if (precisionBegin != std::numeric_limits<size_t>::max()) {
@@ -284,6 +284,73 @@ void Parser::parseSpecifier(Specifier& specifier, SpecifierType type)
 		}
 		specifier.precision = static_cast<int8_t>(stringToNumber(m_input.substr(precisionBegin, precisionEnd - precisionBegin)));
 	}
+
+	checkSpecifierType(specifier, type);
+}
+
+constexpr void Parser::checkSpecifierIntegralType(const Specifier& specifier)
+{
+	switch (specifier.type) {
+	case PresentationType::None:
+	case PresentationType::Binary:
+	case PresentationType::BinaryUppercase:
+	case PresentationType::Decimal:
+	case PresentationType::Octal:
+	case PresentationType::Hex:
+	case PresentationType::HexUppercase:
+	case PresentationType::Character:
+		break;
+	default:
+		VERIFY("invalid type spcifier");
+	};
+}
+
+constexpr void Parser::checkSpecifierType(const Specifier& specifier, ParameterType type)
+{
+	switch (type) {
+	case ParameterType::Integral:
+		checkSpecifierIntegralType(specifier);
+		break;
+	case ParameterType::FloatingPoint:
+		switch (specifier.type) {
+		case PresentationType::Hexfloat:
+		case PresentationType::HexfloatUppercase:
+		case PresentationType::Exponent:
+		case PresentationType::ExponentUppercase:
+		case PresentationType::FixedPoint:
+		case PresentationType::FixedPointUppercase:
+		case PresentationType::General:
+		case PresentationType::GeneralUppercase:
+			break;
+		default:
+			VERIFY("invalid type spcifier");
+		}
+		break;
+	case ParameterType::Char:
+		if (specifier.type != PresentationType::None
+		    && specifier.type != PresentationType::Character) {
+			checkSpecifierIntegralType(specifier);
+		}
+		break;
+	case ParameterType::CString:
+		VERIFY(specifier.type == PresentationType::None
+		           || specifier.type == PresentationType::String
+		           || specifier.type == PresentationType::Pointer,
+		       "invalid type specifier");
+		break;
+	case ParameterType::String:
+		VERIFY(specifier.type == PresentationType::None
+		           || specifier.type == PresentationType::String,
+		       "invalid type specifier");
+		break;
+	case ParameterType::Pointer:
+		VERIFY(specifier.type == PresentationType::None
+		           || specifier.type == PresentationType::Pointer,
+		       "invalid type specifier");
+		break;
+	default:
+		VERIFY_NOT_REACHED();
+	}
 }
 
 } // namespace Util::Format

src/util/format/parser.h

@@ -24,10 +24,11 @@ public:
 		Manual,    // {0},{1}
 	};
 
-	enum class SpecifierType {
+	enum class ParameterType {
 		Integral,
 		FloatingPoint,
 		Char,
+		CString,
 		String,
 		Pointer,
 	};
@@ -41,7 +42,9 @@ public:
 	std::string_view consumeLiteral();
 	std::optional<size_t> consumeIndex();
 
-	void parseSpecifier(Specifier& specifier, SpecifierType type);
+	void parseSpecifier(Specifier& specifier, ParameterType type);
+	constexpr void checkSpecifierIntegralType(const Specifier& specifier);
+	constexpr void checkSpecifierType(const Specifier& specifier, ParameterType type);
 
 private:
 	ArgumentIndexingMode m_mode { ArgumentIndexingMode::Automatic };

test/unit/testutilformat.cpp

@@ -26,7 +26,7 @@ TEST_CASE(FormatBasicTypes)
 	EXPECT_EQ(result, "");
 
 	result = Util::format("{}", nullptr);
-	EXPECT_EQ(result, "(nil)");
+	EXPECT_EQ(result, "nullptr");
 
 	int* number = new int(3);
 	result = Util::format("{}", number);