diff --git a/diagnostics/diagnostic_emitter.h b/diagnostics/diagnostic_emitter.h index 7f04380cee0b..56f97fb4bb3d 100644 --- a/diagnostics/diagnostic_emitter.h +++ b/diagnostics/diagnostic_emitter.h @@ -7,6 +7,7 @@ #include #include +#include #include "llvm/ADT/Any.h" #include "llvm/ADT/STLExtras.h" @@ -36,16 +37,20 @@ class DiagnosticEmitter { : callback_(std::move(callback)) {} ~DiagnosticEmitter() = default; - // Emits an error unconditionally. `F` is guaranteed to be called. + // Emits an error unconditionally after applying the provided `substitutions`. template - void EmitError( - llvm::function_ref f) { - typename DiagnosticT::Substitutions substitutions; - f(substitutions); + void EmitError(typename DiagnosticT::Substitutions substitutions) { callback_({.short_name = DiagnosticT::ShortName, .message = DiagnosticT::Format(substitutions)}); } + // Emits an error unconditionally when there are no substitutions. + template + std::enable_if_t> + EmitError() { + EmitError({}); + } + // Emits a warning if `F` returns true. `F` may or may not be called if the // warning is disabled. template @@ -74,6 +79,14 @@ inline auto NullDiagnosticEmitter() -> DiagnosticEmitter& { return *emitter; } +// CRTP base class for diagnostics with no substitutions. +template struct SimpleDiagnostic { + struct Substitutions {}; + static auto Format(const Substitutions&) -> std::string { + return Derived::Message.str(); + } +}; + } // namespace Carbon #endif // DIAGNOSTICS_DIAGNOSTICEMITTER_H_ diff --git a/diagnostics/diagnostic_emitter_test.cpp b/diagnostics/diagnostic_emitter_test.cpp index 0c230fca9727..f8319ba2ecb6 100644 --- a/diagnostics/diagnostic_emitter_test.cpp +++ b/diagnostics/diagnostic_emitter_test.cpp @@ -41,14 +41,8 @@ TEST(DiagTest, EmitErrors) { reported.push_back(diagnostic.message); }); - emitter.EmitError( - [](FakeDiagnostic::Substitutions& diagnostic) { - diagnostic.message = "M1"; - }); - emitter.EmitError( - [](FakeDiagnostic::Substitutions& diagnostic) { - diagnostic.message = "M2"; - }); + emitter.EmitError({.message = "M1"}); + emitter.EmitError({.message = "M2"}); EXPECT_THAT(reported, ElementsAre("M1", "M2")); } diff --git a/lexer/tokenized_buffer.cpp b/lexer/tokenized_buffer.cpp index 3b8f0a5fef88..295afef9f7df 100644 --- a/lexer/tokenized_buffer.cpp +++ b/lexer/tokenized_buffer.cpp @@ -21,59 +21,35 @@ namespace Carbon { -struct TrailingComment { +struct TrailingComment : SimpleDiagnostic { static constexpr llvm::StringLiteral ShortName = "syntax-comments"; static constexpr llvm::StringLiteral Message = "Trailing comments are not permitted."; - - struct Substitutions {}; - static auto Format(const Substitutions&) -> std::string { - return Message.str(); - } }; -struct NoWhitespaceAfterCommentIntroducer { +struct NoWhitespaceAfterCommentIntroducer + : SimpleDiagnostic { static constexpr llvm::StringLiteral ShortName = "syntax-comments"; static constexpr llvm::StringLiteral Message = "Whitespace is required after '//'."; - - struct Substitutions {}; - static auto Format(const Substitutions&) -> std::string { - return Message.str(); - } }; -struct UnmatchedClosing { +struct UnmatchedClosing : SimpleDiagnostic { static constexpr llvm::StringLiteral ShortName = "syntax-balanced-delimiters"; static constexpr llvm::StringLiteral Message = "Closing symbol without a corresponding opening symbol."; - - struct Substitutions {}; - static auto Format(const Substitutions&) -> std::string { - return Message.str(); - } }; -struct MismatchedClosing { +struct MismatchedClosing : SimpleDiagnostic { static constexpr llvm::StringLiteral ShortName = "syntax-balanced-delimiters"; static constexpr llvm::StringLiteral Message = "Closing symbol does not match most recent opening symbol."; - - struct Substitutions {}; - static auto Format(const Substitutions&) -> std::string { - return Message.str(); - } }; -struct EmptyDigitSequence { +struct EmptyDigitSequence : SimpleDiagnostic { static constexpr llvm::StringLiteral ShortName = "syntax-invalid-number"; static constexpr llvm::StringLiteral Message = "Empty digit sequence in numeric literal."; - - struct Substitutions {}; - static auto Format(const Substitutions&) -> std::string { - return Message.str(); - } }; struct InvalidDigit { @@ -84,27 +60,19 @@ struct InvalidDigit { int radix; }; static auto Format(const Substitutions& subst) -> std::string { - // TODO: Switch Format to using raw_ostream so we can easily use - // llvm::format here. - llvm::StringRef digit_str(&subst.digit, 1); - return (llvm::Twine("Invalid digit '") + digit_str + "' in " + - (subst.radix == 2 ? "binary" - : subst.radix == 16 ? "hexadecimal" - : "decimal") + - " numeric literal.") + return llvm::formatv("Invalid digit '{0}' in {1} numeric literal.", + subst.digit, + (subst.radix == 2 ? "binary" + : subst.radix == 16 ? "hexadecimal" + : "decimal")) .str(); } }; -struct InvalidDigitSeparator { +struct InvalidDigitSeparator : SimpleDiagnostic { static constexpr llvm::StringLiteral ShortName = "syntax-invalid-number"; static constexpr llvm::StringLiteral Message = "Misplaced digit separator in numeric literal."; - - struct Substitutions {}; - static auto Format(const Substitutions&) -> std::string { - return Message.str(); - } }; struct IrregularDigitSeparators { @@ -116,34 +84,25 @@ struct IrregularDigitSeparators { }; static auto Format(const Substitutions& subst) -> std::string { assert((subst.radix == 10 || subst.radix == 16) && "unexpected radix"); - return (llvm::Twine("Digit separators in ") + - (subst.radix == 10 ? "decimal" : "hexadecimal") + - " should appear every " + (subst.radix == 10 ? "3" : "4") + - " characters from the right.") + return llvm::formatv( + "Digit separators in {0} number should appear every {1} " + "characters from the right.", + (subst.radix == 10 ? "decimal" : "hexadecimal"), + (subst.radix == 10 ? "3" : "4")) .str(); } }; -struct UnknownBaseSpecifier { +struct UnknownBaseSpecifier : SimpleDiagnostic { static constexpr llvm::StringLiteral ShortName = "syntax-invalid-number"; static constexpr llvm::StringLiteral Message = "Unknown base specifier in numeric literal."; - - struct Substitutions {}; - static auto Format(const Substitutions&) -> std::string { - return Message.str(); - } }; -struct BinaryRealLiteral { +struct BinaryRealLiteral : SimpleDiagnostic { static constexpr llvm::StringLiteral ShortName = "syntax-invalid-number"; static constexpr llvm::StringLiteral Message = "Binary real number literals are not supported."; - - struct Substitutions {}; - static auto Format(const Substitutions&) -> std::string { - return Message.str(); - } }; struct WrongRealLiteralExponent { @@ -153,23 +112,17 @@ struct WrongRealLiteralExponent { char expected; }; static auto Format(const Substitutions& subst) -> std::string { - char expected_str[] = {subst.expected, '\0'}; - return (llvm::Twine("Expected '") + expected_str + - "' to introduce exponent.") + return llvm::formatv("Expected '{0}' to introduce exponent.", + subst.expected) .str(); } }; -struct UnrecognizedCharacters { +struct UnrecognizedCharacters : SimpleDiagnostic { static constexpr llvm::StringLiteral ShortName = "syntax-unrecognized-characters"; static constexpr llvm::StringLiteral Message = "Encountered unrecognized characters while parsing."; - - struct Substitutions {}; - static auto Format(const Substitutions&) -> std::string { - return Message.str(); - } }; // TODO(zygoloid): Update this to match whatever we decide qualifies as @@ -387,24 +340,19 @@ class NumericLiteralParser { // next to another digit separator, or at the end. if (!allow_digit_separators || i == 0 || text[i - 1] == '_' || i + 1 == n) { - emitter.EmitError( - [&](InvalidDigitSeparator::Substitutions&) {}); + emitter.EmitError(); recovered_from_error = true; } ++num_digit_separators; continue; } - emitter.EmitError([&](InvalidDigit::Substitutions& subst) { - subst.digit = c; - subst.radix = radix; - }); + emitter.EmitError({.digit = c, .radix = radix}); return {.ok = false}; } if (num_digit_separators == static_cast(text.size())) { - emitter.EmitError( - [&](EmptyDigitSequence::Substitutions&) {}); + emitter.EmitError(); return {.ok = false}; } @@ -425,10 +373,7 @@ class NumericLiteralParser { "given wrong number of digit separators"); auto diagnose_irregular_digit_separators = [&] { - emitter.EmitError( - [&](IrregularDigitSeparators::Substitutions& subst) { - subst.radix = radix; - }); + emitter.EmitError({.radix = radix}); recovered_from_error = true; }; @@ -452,8 +397,7 @@ class NumericLiteralParser { // Check that we don't have a '0' prefix on a non-zero decimal integer. auto CheckLeadingZero() -> bool { if (radix == 10 && int_part.startswith("0") && int_part != "0") { - emitter.EmitError( - [&](UnknownBaseSpecifier::Substitutions& subst) {}); + emitter.EmitError(); return false; } return true; @@ -474,8 +418,7 @@ class NumericLiteralParser { } if (radix == 2) { - emitter.EmitError( - [&](BinaryRealLiteral::Substitutions& subst) {}); + emitter.EmitError(); recovered_from_error = true; // Carry on and parse the binary real literal anyway. } @@ -497,9 +440,7 @@ class NumericLiteralParser { char expected_exponent_kind = (radix == 10 ? 'e' : 'p'); if (literal.text[literal.exponent] != expected_exponent_kind) { emitter.EmitError( - [&](WrongRealLiteralExponent::Substitutions& subst) { - subst.expected = expected_exponent_kind; - }); + {.expected = expected_exponent_kind}); return false; } @@ -617,14 +558,12 @@ class TokenizedBuffer::Lexer { if (source_text.startswith("//")) { // Any comment must be the only non-whitespace on the line. if (set_indent) { - emitter.EmitError( - [](TrailingComment::Substitutions&) {}); + emitter.EmitError(); buffer.has_errors = true; } // The introducer '//' must be followed by whitespace or EOF. if (source_text.size() > 2 && !isSpace(source_text[2])) { - emitter.EmitError( - [](NoWhitespaceAfterCommentIntroducer::Substitutions&) {}); + emitter.EmitError(); buffer.has_errors = true; } while (!source_text.empty() && source_text.front() != '\n') { @@ -779,8 +718,7 @@ class TokenizedBuffer::Lexer { closing_token_info.error_length = kind.GetFixedSpelling().size(); buffer.has_errors = true; - emitter.EmitError( - [](UnmatchedClosing::Substitutions&) {}); + emitter.EmitError(); // Note that this still returns true as we do consume a symbol. return token; } @@ -809,8 +747,7 @@ class TokenizedBuffer::Lexer { open_groups.pop_back(); buffer.has_errors = true; - emitter.EmitError( - [](MismatchedClosing::Substitutions&) {}); + emitter.EmitError(); // TODO: do a smarter backwards scan for where to put the closing // token.