Clean up SimpleDiagnostic inheritance (#1004)

toolchain/diagnostics/diagnostic_emitter.h

@@ -68,6 +68,33 @@ class DiagnosticLocationTranslator {
       -> Diagnostic::Location = 0;
 };
 
+// CRTP base class for diagnostics. `DiagnosticEmitter` requires `ShortName` and
+// `Format`; `Message` is used by the default `Format` implementation. A simple
+// child will look like:
+//
+//   struct MySimpleError : DiagnosticBase<MyError> {
+//     static constexpr llvm::StringLiteral ShortName = "short-name";
+//     static constexpr llvm::StringLiteral Message = "A message.";
+//   };
+//
+//   emitter.EmitError<MySimpleError>(location);
+//
+// A complex child may provide an alternate `Format` implementation:
+//
+//   struct MyComplexError : DiagnosticBase<MyComplexError> {
+//     static constexpr llvm::StringLiteral ShortName = "short-name";
+//
+//     auto Format() -> std::string { return llvm::formatv("See {0}.", ref); }
+//
+//     std::string ref;
+//   };
+//
+//   emitter.EmitError<MyComplexError>(location, {.ref = "ref"; });
+template <typename Derived>
+struct DiagnosticBase {
+  static auto Format() -> std::string { return Derived::Message.str(); }
+};
+
 // Manages the creation of reports, the testing if diagnostics are enabled, and
 // the collection of reports.
 //
@@ -88,7 +115,9 @@ class DiagnosticEmitter {
   ~DiagnosticEmitter() = default;
 
   // Emits an error unconditionally.
-  template <typename DiagnosticT>
+  template <typename DiagnosticT,
+            typename = std::enable_if_t<
+                std::is_base_of_v<DiagnosticBase<DiagnosticT>, DiagnosticT>>>
   auto EmitError(LocationT location, DiagnosticT diag) -> void {
     // TODO: Encode the diagnostic kind in the Diagnostic object rather than
     // hardcoding an "error: " prefix.
@@ -143,12 +172,6 @@ inline auto ConsoleDiagnosticConsumer() -> DiagnosticConsumer& {
   return *consumer;
 }
 
-// CRTP base class for diagnostics with no substitutions.
-template <typename Derived>
-struct SimpleDiagnostic {
-  static auto Format() -> std::string { return Derived::Message.str(); }
-};
-
 // Diagnostic consumer adaptor that tracks whether any errors have been
 // produced.
 class ErrorTrackingDiagnosticConsumer : public DiagnosticConsumer {

toolchain/diagnostics/diagnostic_emitter_test.cpp

@@ -14,7 +14,7 @@
 namespace Carbon::Testing {
 namespace {
 
-struct FakeDiagnostic {
+struct FakeDiagnostic : DiagnosticBase<FakeDiagnostic> {
   static constexpr llvm::StringLiteral ShortName = "fake-diagnostic";
   // TODO: consider ways to put the Message into `format` to allow dynamic
   // selection of the message.

toolchain/lexer/numeric_literal.cpp

@@ -14,13 +14,13 @@
 namespace Carbon {
 
 namespace {
-struct EmptyDigitSequence : SimpleDiagnostic<EmptyDigitSequence> {
+struct EmptyDigitSequence : DiagnosticBase<EmptyDigitSequence> {
   static constexpr llvm::StringLiteral ShortName = "syntax-invalid-number";
   static constexpr llvm::StringLiteral Message =
       "Empty digit sequence in numeric literal.";
 };
 
-struct InvalidDigit {
+struct InvalidDigit : DiagnosticBase<InvalidDigit> {
   static constexpr llvm::StringLiteral ShortName = "syntax-invalid-number";
 
   auto Format() -> std::string {
@@ -35,13 +35,13 @@ struct InvalidDigit {
   int radix;
 };
 
-struct InvalidDigitSeparator : SimpleDiagnostic<InvalidDigitSeparator> {
+struct InvalidDigitSeparator : DiagnosticBase<InvalidDigitSeparator> {
   static constexpr llvm::StringLiteral ShortName = "syntax-invalid-number";
   static constexpr llvm::StringLiteral Message =
       "Misplaced digit separator in numeric literal.";
 };
 
-struct IrregularDigitSeparators {
+struct IrregularDigitSeparators : DiagnosticBase<IrregularDigitSeparators> {
   static constexpr llvm::StringLiteral ShortName =
       "syntax-irregular-digit-separators";
 
@@ -58,19 +58,19 @@ struct IrregularDigitSeparators {
   int radix;
 };
 
-struct UnknownBaseSpecifier : SimpleDiagnostic<UnknownBaseSpecifier> {
+struct UnknownBaseSpecifier : DiagnosticBase<UnknownBaseSpecifier> {
   static constexpr llvm::StringLiteral ShortName = "syntax-invalid-number";
   static constexpr llvm::StringLiteral Message =
       "Unknown base specifier in numeric literal.";
 };
 
-struct BinaryRealLiteral : SimpleDiagnostic<BinaryRealLiteral> {
+struct BinaryRealLiteral : DiagnosticBase<BinaryRealLiteral> {
   static constexpr llvm::StringLiteral ShortName = "syntax-invalid-number";
   static constexpr llvm::StringLiteral Message =
       "Binary real number literals are not supported.";
 };
 
-struct WrongRealLiteralExponent {
+struct WrongRealLiteralExponent : DiagnosticBase<WrongRealLiteralExponent> {
   static constexpr llvm::StringLiteral ShortName = "syntax-invalid-number";
 
   auto Format() -> std::string {
@@ -81,7 +81,7 @@ struct WrongRealLiteralExponent {
   char expected;
 };
 
-struct TooManyDigits {
+struct TooManyDigits : DiagnosticBase<TooManyDigits> {
   static constexpr llvm::StringLiteral ShortName = "syntax-invalid-number";
 
   auto Format() -> std::string {

toolchain/lexer/string_literal.cpp

@@ -17,27 +17,27 @@ namespace Carbon {
 using LexerDiagnosticEmitter = DiagnosticEmitter<const char*>;
 
 struct ContentBeforeStringTerminator
-    : SimpleDiagnostic<ContentBeforeStringTerminator> {
+    : DiagnosticBase<ContentBeforeStringTerminator> {
   static constexpr llvm::StringLiteral ShortName = "syntax-invalid-string";
   static constexpr llvm::StringLiteral Message =
       "Only whitespace is permitted before the closing `\"\"\"` of a "
       "multi-line string.";
 };
 
-struct UnicodeEscapeTooLarge : SimpleDiagnostic<UnicodeEscapeTooLarge> {
+struct UnicodeEscapeTooLarge : DiagnosticBase<UnicodeEscapeTooLarge> {
   static constexpr llvm::StringLiteral ShortName = "syntax-invalid-string";
   static constexpr llvm::StringLiteral Message =
       "Code point specified by `\\u{...}` escape is greater than 0x10FFFF.";
 };
 
-struct UnicodeEscapeSurrogate : SimpleDiagnostic<UnicodeEscapeSurrogate> {
+struct UnicodeEscapeSurrogate : DiagnosticBase<UnicodeEscapeSurrogate> {
   static constexpr llvm::StringLiteral ShortName = "syntax-invalid-string";
   static constexpr llvm::StringLiteral Message =
       "Code point specified by `\\u{...}` escape is a surrogate character.";
 };
 
 struct UnicodeEscapeMissingBracedDigits
-    : SimpleDiagnostic<UnicodeEscapeMissingBracedDigits> {
+    : DiagnosticBase<UnicodeEscapeMissingBracedDigits> {
   static constexpr llvm::StringLiteral ShortName = "syntax-invalid-string";
   static constexpr llvm::StringLiteral Message =
       "Escape sequence `\\u` must be followed by a braced sequence of "
@@ -45,30 +45,30 @@ struct UnicodeEscapeMissingBracedDigits
 };
 
 struct HexadecimalEscapeMissingDigits
-    : SimpleDiagnostic<HexadecimalEscapeMissingDigits> {
+    : DiagnosticBase<HexadecimalEscapeMissingDigits> {
   static constexpr llvm::StringLiteral ShortName = "syntax-invalid-string";
   static constexpr llvm::StringLiteral Message =
       "Escape sequence `\\x` must be followed by two "
       "uppercase hexadecimal digits, for example `\\x0F`.";
 };
 
-struct DecimalEscapeSequence : SimpleDiagnostic<DecimalEscapeSequence> {
+struct DecimalEscapeSequence : DiagnosticBase<DecimalEscapeSequence> {
   static constexpr llvm::StringLiteral ShortName = "syntax-invalid-string";
   static constexpr llvm::StringLiteral Message =
       "Decimal digit follows `\\0` escape sequence. Use `\\x00` instead of "
       "`\\0` if the next character is a digit.";
 };
 
-struct UnknownEscapeSequence {
+struct UnknownEscapeSequence : DiagnosticBase<UnknownEscapeSequence> {
   static constexpr llvm::StringLiteral ShortName = "syntax-invalid-string";
   static constexpr const char* Message = "Unrecognized escape sequence `{0}`.";
 
-  char first;
-
   auto Format() -> std::string { return llvm::formatv(Message, first).str(); }
+
+  char first;
 };
 
-struct MismatchedIndentInString : SimpleDiagnostic<MismatchedIndentInString> {
+struct MismatchedIndentInString : DiagnosticBase<MismatchedIndentInString> {
   static constexpr llvm::StringLiteral ShortName = "syntax-invalid-string";
   static constexpr llvm::StringLiteral Message =
       "Indentation does not match that of the closing \"\"\" in multi-line "
@@ -76,7 +76,7 @@ struct MismatchedIndentInString : SimpleDiagnostic<MismatchedIndentInString> {
 };
 
 struct InvalidHorizontalWhitespaceInString
-    : SimpleDiagnostic<InvalidHorizontalWhitespaceInString> {
+    : DiagnosticBase<InvalidHorizontalWhitespaceInString> {
   static constexpr llvm::StringLiteral ShortName = "syntax-invalid-string";
   static constexpr llvm::StringLiteral Message =
       "Whitespace other than plain space must be expressed with an escape "

toolchain/lexer/tokenized_buffer.cpp

@@ -25,32 +25,32 @@
 
 namespace Carbon {
 
-struct TrailingComment : SimpleDiagnostic<TrailingComment> {
+struct TrailingComment : DiagnosticBase<TrailingComment> {
   static constexpr llvm::StringLiteral ShortName = "syntax-comments";
   static constexpr llvm::StringLiteral Message =
       "Trailing comments are not permitted.";
 };
 
 struct NoWhitespaceAfterCommentIntroducer
-    : SimpleDiagnostic<NoWhitespaceAfterCommentIntroducer> {
+    : DiagnosticBase<NoWhitespaceAfterCommentIntroducer> {
   static constexpr llvm::StringLiteral ShortName = "syntax-comments";
   static constexpr llvm::StringLiteral Message =
       "Whitespace is required after '//'.";
 };
 
-struct UnmatchedClosing : SimpleDiagnostic<UnmatchedClosing> {
+struct UnmatchedClosing : DiagnosticBase<UnmatchedClosing> {
   static constexpr llvm::StringLiteral ShortName = "syntax-balanced-delimiters";
   static constexpr llvm::StringLiteral Message =
       "Closing symbol without a corresponding opening symbol.";
 };
 
-struct MismatchedClosing : SimpleDiagnostic<MismatchedClosing> {
+struct MismatchedClosing : DiagnosticBase<MismatchedClosing> {
   static constexpr llvm::StringLiteral ShortName = "syntax-balanced-delimiters";
   static constexpr llvm::StringLiteral Message =
       "Closing symbol does not match most recent opening symbol.";
 };
 
-struct UnrecognizedCharacters : SimpleDiagnostic<UnrecognizedCharacters> {
+struct UnrecognizedCharacters : DiagnosticBase<UnrecognizedCharacters> {
   static constexpr llvm::StringLiteral ShortName =
       "syntax-unrecognized-characters";
   static constexpr llvm::StringLiteral Message =

toolchain/parser/parser_impl.cpp

@@ -17,10 +17,10 @@
 
 namespace Carbon {
 
-struct StackLimitExceeded {
+struct StackLimitExceeded : DiagnosticBase<StackLimitExceeded> {
   static constexpr llvm::StringLiteral ShortName = "syntax-error";
 
-  auto Format() -> std::string {
+  static auto Format() -> std::string {
     return llvm::formatv("Exceeded recursion limit ({0})",
                          ParseTree::StackDepthLimit);
   }
@@ -55,46 +55,43 @@ class ParseTree::Parser::ScopedStackStep {
     return (error_return_expr);                    \
   }
 
-struct UnexpectedTokenInCodeBlock
-    : SimpleDiagnostic<UnexpectedTokenInCodeBlock> {
+struct UnexpectedTokenInCodeBlock : DiagnosticBase<UnexpectedTokenInCodeBlock> {
   static constexpr llvm::StringLiteral ShortName = "syntax-error";
   static constexpr llvm::StringLiteral Message =
       "Unexpected token in code block.";
 };
 
-struct ExpectedFunctionName : SimpleDiagnostic<ExpectedFunctionName> {
+struct ExpectedFunctionName : DiagnosticBase<ExpectedFunctionName> {
   static constexpr llvm::StringLiteral ShortName = "syntax-error";
   static constexpr llvm::StringLiteral Message =
       "Expected function name after `fn` keyword.";
 };
 
-struct ExpectedFunctionParams : SimpleDiagnostic<ExpectedFunctionParams> {
+struct ExpectedFunctionParams : DiagnosticBase<ExpectedFunctionParams> {
   static constexpr llvm::StringLiteral ShortName = "syntax-error";
   static constexpr llvm::StringLiteral Message =
       "Expected `(` after function name.";
 };
 
-struct ExpectedFunctionBodyOrSemi
-    : SimpleDiagnostic<ExpectedFunctionBodyOrSemi> {
+struct ExpectedFunctionBodyOrSemi : DiagnosticBase<ExpectedFunctionBodyOrSemi> {
   static constexpr llvm::StringLiteral ShortName = "syntax-error";
   static constexpr llvm::StringLiteral Message =
       "Expected function definition or `;` after function declaration.";
 };
 
-struct ExpectedVariableName : SimpleDiagnostic<ExpectedVariableName> {
+struct ExpectedVariableName : DiagnosticBase<ExpectedVariableName> {
   static constexpr llvm::StringLiteral ShortName = "syntax-error";
   static constexpr llvm::StringLiteral Message =
       "Expected pattern in `var` declaration.";
 };
 
-struct ExpectedParameterName : SimpleDiagnostic<ExpectedParameterName> {
+struct ExpectedParameterName : DiagnosticBase<ExpectedParameterName> {
   static constexpr llvm::StringLiteral ShortName = "syntax-error";
   static constexpr llvm::StringLiteral Message =
       "Expected parameter declaration.";
 };
 
-struct ExpectedStructLiteralField
-    : SimpleDiagnostic<ExpectedStructLiteralField> {
+struct ExpectedStructLiteralField : DiagnosticBase<ExpectedStructLiteralField> {
   static constexpr llvm::StringLiteral ShortName = "syntax-error";
 
   auto Format() -> std::string {
@@ -116,23 +113,23 @@ struct ExpectedStructLiteralField
   bool can_be_value;
 };
 
-struct UnrecognizedDeclaration : SimpleDiagnostic<UnrecognizedDeclaration> {
+struct UnrecognizedDeclaration : DiagnosticBase<UnrecognizedDeclaration> {
   static constexpr llvm::StringLiteral ShortName = "syntax-error";
   static constexpr llvm::StringLiteral Message =
       "Unrecognized declaration introducer.";
 };
 
-struct ExpectedCodeBlock : SimpleDiagnostic<ExpectedCodeBlock> {
+struct ExpectedCodeBlock : DiagnosticBase<ExpectedCodeBlock> {
   static constexpr llvm::StringLiteral ShortName = "syntax-error";
   static constexpr llvm::StringLiteral Message = "Expected braced code block.";
 };
 
-struct ExpectedExpression : SimpleDiagnostic<ExpectedExpression> {
+struct ExpectedExpression : DiagnosticBase<ExpectedExpression> {
   static constexpr llvm::StringLiteral ShortName = "syntax-error";
   static constexpr llvm::StringLiteral Message = "Expected expression.";
 };
 
-struct ExpectedParenAfter : SimpleDiagnostic<ExpectedParenAfter> {
+struct ExpectedParenAfter : DiagnosticBase<ExpectedParenAfter> {
   static constexpr llvm::StringLiteral ShortName = "syntax-error";
   static constexpr const char* Message = "Expected `(` after `{0}`.";
 
@@ -143,7 +140,7 @@ struct ExpectedParenAfter : SimpleDiagnostic<ExpectedParenAfter> {
   TokenKind introducer;
 };
 
-struct ExpectedCloseParen : SimpleDiagnostic<ExpectedCloseParen> {
+struct ExpectedCloseParen : DiagnosticBase<ExpectedCloseParen> {
   static constexpr llvm::StringLiteral ShortName = "syntax-error";
   static constexpr llvm::StringLiteral Message =
       "Unexpected tokens before `)`.";
@@ -153,13 +150,13 @@ struct ExpectedCloseParen : SimpleDiagnostic<ExpectedCloseParen> {
 };
 
 struct ExpectedSemiAfterExpression
-    : SimpleDiagnostic<ExpectedSemiAfterExpression> {
+    : DiagnosticBase<ExpectedSemiAfterExpression> {
   static constexpr llvm::StringLiteral ShortName = "syntax-error";
   static constexpr llvm::StringLiteral Message =
       "Expected `;` after expression.";
 };
 
-struct ExpectedSemiAfter : SimpleDiagnostic<ExpectedSemiAfter> {
+struct ExpectedSemiAfter : DiagnosticBase<ExpectedSemiAfter> {
   static constexpr llvm::StringLiteral ShortName = "syntax-error";
   static constexpr const char* Message = "Expected `;` after `{0}`.";
 
@@ -170,15 +167,14 @@ struct ExpectedSemiAfter : SimpleDiagnostic<ExpectedSemiAfter> {
   TokenKind preceding;
 };
 
-struct ExpectedIdentifierAfterDot
-    : SimpleDiagnostic<ExpectedIdentifierAfterDot> {
+struct ExpectedIdentifierAfterDot : DiagnosticBase<ExpectedIdentifierAfterDot> {
   static constexpr llvm::StringLiteral ShortName = "syntax-error";
   static constexpr llvm::StringLiteral Message =
       "Expected identifier after `.`.";
 };
 
 struct UnexpectedTokenAfterListElement
-    : SimpleDiagnostic<UnexpectedTokenAfterListElement> {
+    : DiagnosticBase<UnexpectedTokenAfterListElement> {
   static constexpr llvm::StringLiteral ShortName = "syntax-error";
   static constexpr const char* Message = "Expected `,` or `{0}`.";
 
@@ -190,7 +186,7 @@ struct UnexpectedTokenAfterListElement
 };
 
 struct BinaryOperatorRequiresWhitespace
-    : SimpleDiagnostic<BinaryOperatorRequiresWhitespace> {
+    : DiagnosticBase<BinaryOperatorRequiresWhitespace> {
   static constexpr llvm::StringLiteral ShortName = "syntax-error";
   static constexpr const char* Message =
       "Whitespace missing {0} binary operator.";
@@ -209,8 +205,7 @@ struct BinaryOperatorRequiresWhitespace
   bool has_trailing_space;
 };
 
-struct UnaryOperatorHasWhitespace
-    : SimpleDiagnostic<UnaryOperatorHasWhitespace> {
+struct UnaryOperatorHasWhitespace : DiagnosticBase<UnaryOperatorHasWhitespace> {
   static constexpr llvm::StringLiteral ShortName = "syntax-error";
   static constexpr const char* Message =
       "Whitespace is not allowed {0} this unary operator.";
@@ -223,7 +218,7 @@ struct UnaryOperatorHasWhitespace
 };
 
 struct UnaryOperatorRequiresWhitespace
-    : SimpleDiagnostic<UnaryOperatorRequiresWhitespace> {
+    : DiagnosticBase<UnaryOperatorRequiresWhitespace> {
   static constexpr llvm::StringLiteral ShortName = "syntax-error";
   static constexpr const char* Message =
       "Whitespace is required {0} this unary operator.";
@@ -236,7 +231,7 @@ struct UnaryOperatorRequiresWhitespace
 };
 
 struct OperatorRequiresParentheses
-    : SimpleDiagnostic<OperatorRequiresParentheses> {
+    : DiagnosticBase<OperatorRequiresParentheses> {
   static constexpr llvm::StringLiteral ShortName = "syntax-error";
   static constexpr llvm::StringLiteral Message =
       "Parentheses are required to disambiguate operator precedence.";