Explorar el Código

Restructure Diagnostic objects to allow late formatting (#1131)

Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Jon Meow hace 4 años
padre
commit
aaca540a05

+ 40 - 13
toolchain/diagnostics/BUILD

@@ -7,7 +7,46 @@ package(default_visibility = ["//visibility:public"])
 cc_library(
     name = "diagnostic_emitter",
     hdrs = ["diagnostic_emitter.h"],
-    deps = ["@llvm-project//llvm:Support"],
+    deps = [
+        ":diagnostic_kind",
+        "@llvm-project//llvm:Support",
+    ],
+)
+
+cc_test(
+    name = "diagnostic_emitter_test",
+    size = "small",
+    srcs = ["diagnostic_emitter_test.cpp"],
+    deps = [
+        ":diagnostic_emitter",
+        ":mocks",
+        "@com_google_googletest//:gtest_main",
+        "@llvm-project//llvm:Support",
+    ],
+)
+
+cc_library(
+    name = "diagnostic_kind",
+    srcs = ["diagnostic_kind.cpp"],
+    hdrs = ["diagnostic_kind.h"],
+    textual_hdrs = [
+        "diagnostic_registry.def",
+    ],
+    deps = [
+        "//common:ostream",
+        "@llvm-project//llvm:Support",
+    ],
+)
+
+cc_test(
+    name = "diagnostic_kind_test",
+    size = "small",
+    srcs = ["diagnostic_kind_test.cpp"],
+    deps = [
+        ":diagnostic_kind",
+        "@com_google_googletest//:gtest_main",
+        "@llvm-project//llvm:Support",
+    ],
 )
 
 cc_library(
@@ -53,15 +92,3 @@ cc_library(
         "@llvm-project//llvm:Support",
     ],
 )
-
-cc_test(
-    name = "diagnostic_emitter_test",
-    size = "small",
-    srcs = ["diagnostic_emitter_test.cpp"],
-    deps = [
-        ":diagnostic_emitter",
-        ":mocks",
-        "@com_google_googletest//:gtest_main",
-        "@llvm-project//llvm:Support",
-    ],
-)

+ 120 - 93
toolchain/diagnostics/diagnostic_emitter.h

@@ -2,8 +2,8 @@
 // Exceptions. See /LICENSE for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
-#ifndef TOOLCHAIN_DIAGNOSTICS_DIAGNOSTICEMITTER_H_
-#define TOOLCHAIN_DIAGNOSTICS_DIAGNOSTICEMITTER_H_
+#ifndef TOOLCHAIN_DIAGNOSTICS_DIAGNOSTIC_EMITTER_H_
+#define TOOLCHAIN_DIAGNOSTICS_DIAGNOSTIC_EMITTER_H_
 
 #include <functional>
 #include <string>
@@ -13,36 +13,72 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/raw_ostream.h"
+#include "toolchain/diagnostics/diagnostic_kind.h"
 
 namespace Carbon {
 
+enum class DiagnosticLevel : int8_t {
+  // A warning diagnostic, indicating a likely problem with the program.
+  Warning,
+  // An error diagnostic, indicating that the program is not valid.
+  Error,
+};
+
+// Provides a definition of a diagnostic. For example:
+//   CARBON_DIAGNOSTIC(MyDiagnostic, Error, "Invalid code!");
+//   CARBON_DIAGNOSTIC(MyDiagnostic, Warning, "Found {0}, expected {1}.",
+//              llvm::StringRef, llvm::StringRef);
+//
+// Arguments are passed to llvm::formatv; see:
+// https://llvm.org/doxygen/FormatVariadic_8h_source.html
+//
+// See `DiagnosticEmitter::Emit` for comments about argument lifetimes.
+#define CARBON_DIAGNOSTIC(DiagnosticName, Level, Format, ...) \
+  static constexpr auto DiagnosticName =                      \
+      Internal::DiagnosticBase<__VA_ARGS__>(                  \
+          ::Carbon::DiagnosticKind::DiagnosticName,           \
+          ::Carbon::DiagnosticLevel::Level, Format)
+
+struct DiagnosticLocation {
+  // Name of the file or buffer that this diagnostic refers to.
+  // TODO: Move this out of DiagnosticLocation, as part of an expectation that
+  // files will be compiled separately, so storing the file's path
+  // per-diagnostic is wasteful.
+  std::string file_name;
+  // 1-based line number.
+  int32_t line_number;
+  // 1-based column number.
+  int32_t column_number;
+};
+
 // An instance of a single error or warning.  Information about the diagnostic
 // can be recorded into it for more complex consumers.
-//
-// TODO: turn this into a much more reasonable API when we add some actual
-// uses of it.
 struct Diagnostic {
-  enum Level {
-    // A warning diagnostic, indicating a likely problem with the program.
-    Warning,
-    // An error diagnostic, indicating that the program is not valid.
-    Error,
-  };
+  // The diagnostic's kind.
+  DiagnosticKind kind;
 
-  struct Location {
-    // Name of the file or buffer that this diagnostic refers to.
-    std::string file_name;
-    // 1-based line number.
-    int32_t line_number;
-    // 1-based column number.
-    int32_t column_number;
-  };
+  // The diagnostic's level.
+  DiagnosticLevel level;
+
+  // The calculated location of the diagnostic.
+  DiagnosticLocation location;
+
+  // The diagnostic's format string. This, along with format_args, will be
+  // passed to format_fn.
+  llvm::StringLiteral format;
+
+  // A list of format arguments.
+  //
+  // These may be used by non-standard consumers to inspect diagnostic details
+  // without needing to parse the formatted string; however, it should be
+  // understood that diagnostic formats are subject to change and the llvm::Any
+  // offers limited compile-time type safety. Integration tests are required.
+  llvm::SmallVector<llvm::Any, 0> format_args;
 
-  Level level;
-  Location location;
-  llvm::StringRef short_name;
-  std::string message;
+  // Returns the formatted string. By default, this uses llvm::formatv.
+  std::function<std::string(const Diagnostic&)> format_fn;
 };
 
 // Receives diagnostics as they are emitted.
@@ -68,36 +104,46 @@ class DiagnosticLocationTranslator {
   virtual ~DiagnosticLocationTranslator() = default;
 
   [[nodiscard]] virtual auto GetLocation(LocationT loc)
-      -> Diagnostic::Location = 0;
+      -> DiagnosticLocation = 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>
+namespace Internal {
+
+// Use the DIAGNOSTIC macro to instantiate this.
+// This stores static information about a diagnostic category.
+template <typename... Args>
 struct DiagnosticBase {
-  static auto Format() -> std::string { return Derived::Message.str(); }
+  constexpr DiagnosticBase(DiagnosticKind kind, DiagnosticLevel level,
+                           llvm::StringLiteral format)
+      : Kind(kind), Level(level), Format(format) {}
+
+  // Calls formatv with the diagnostic's arguments.
+  auto FormatFn(const Diagnostic& diagnostic) const -> std::string {
+    return FormatFnImpl(diagnostic,
+                        std::make_index_sequence<sizeof...(Args)>());
+  };
+
+  // The diagnostic's kind.
+  DiagnosticKind Kind;
+  // The diagnostic's level.
+  DiagnosticLevel Level;
+  // The diagnostic's format for llvm::formatv.
+  llvm::StringLiteral Format;
+
+ private:
+  // Handles the cast of llvm::Any to Args types for formatv.
+  template <std::size_t... N>
+  inline auto FormatFnImpl(const Diagnostic& diagnostic,
+                           std::index_sequence<N...> /*indices*/) const
+      -> std::string {
+    assert(diagnostic.format_args.size() == sizeof...(Args));
+    return llvm::formatv(diagnostic.format.data(),
+                         llvm::any_cast<Args>(diagnostic.format_args[N])...);
+  }
 };
 
+}  // namespace Internal
+
 // Manages the creation of reports, the testing if diagnostics are enabled, and
 // the collection of reports.
 //
@@ -117,42 +163,25 @@ class DiagnosticEmitter {
       : translator_(&translator), consumer_(&consumer) {}
   ~DiagnosticEmitter() = default;
 
-  // Emits an error unconditionally.
-  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.
-    consumer_->HandleDiagnostic({.level = Diagnostic::Error,
-                                 .location = translator_->GetLocation(location),
-                                 .short_name = DiagnosticT::ShortName,
-                                 .message = diag.Format()});
-  }
-
-  // Emits a stateless error unconditionally.
-  template <typename DiagnosticT>
-  auto EmitError(LocationT location)
-      -> std::enable_if_t<std::is_empty_v<DiagnosticT>> {
-    EmitError<DiagnosticT>(location, {});
-  }
-
-  // Emits a warning if `F` returns true.  `F` may or may not be called if the
-  // warning is disabled.
-  template <typename DiagnosticT>
-  auto EmitWarningIf(LocationT location,
-                     llvm::function_ref<bool(DiagnosticT&)> f) -> void {
-    // TODO(kfm): check if this warning is enabled at `location`.
-    DiagnosticT diag;
-    if (f(diag)) {
-      // TODO: Encode the diagnostic kind in the Diagnostic object rather than
-      // hardcoding a "warning: " prefix.
-      consumer_->HandleDiagnostic(
-          {.level = Diagnostic::Warning,
-           .location = translator_->GetLocation(location),
-           .short_name = DiagnosticT::ShortName,
-           .message = diag.Format()});
-    }
+  // Emits an error.
+  //
+  // When passing arguments, they may be buffered. As a consequence, lifetimes
+  // may outlive the `Emit` call.
+  template <typename... Args>
+  void Emit(LocationT location,
+            const Internal::DiagnosticBase<Args...>& diagnostic_base,
+            // Disable type deduction based on `args`; the type of
+            // `diagnostic_base` determines the diagnostic's parameter types.
+            typename std::common_type_t<Args>... args) {
+    consumer_->HandleDiagnostic({
+        .kind = diagnostic_base.Kind,
+        .level = diagnostic_base.Level,
+        .location = translator_->GetLocation(location),
+        .format = diagnostic_base.Format,
+        .format_args = {std::move(args)...},
+        .format_fn = [&diagnostic_base](const Diagnostic& diagnostic)
+            -> std::string { return diagnostic_base.FormatFn(diagnostic); },
+    });
   }
 
  private:
@@ -162,13 +191,11 @@ class DiagnosticEmitter {
 
 inline auto ConsoleDiagnosticConsumer() -> DiagnosticConsumer& {
   struct Consumer : DiagnosticConsumer {
-    auto HandleDiagnostic(const Diagnostic& d) -> void override {
-      if (!d.location.file_name.empty()) {
-        llvm::errs() << d.location.file_name << ":" << d.location.line_number
-                     << ":" << d.location.column_number << ": ";
-      }
-
-      llvm::errs() << d.message << "\n";
+    auto HandleDiagnostic(const Diagnostic& diagnostic) -> void override {
+      llvm::errs() << diagnostic.location.file_name << ":"
+                   << diagnostic.location.line_number << ":"
+                   << diagnostic.location.column_number << ": "
+                   << diagnostic.format_fn(diagnostic) << "\n";
     }
   };
   static auto* consumer = new Consumer;
@@ -183,7 +210,7 @@ class ErrorTrackingDiagnosticConsumer : public DiagnosticConsumer {
       : next_consumer_(&next_consumer) {}
 
   auto HandleDiagnostic(const Diagnostic& diagnostic) -> void override {
-    seen_error_ |= diagnostic.level == Diagnostic::Error;
+    seen_error_ |= diagnostic.level == DiagnosticLevel::Error;
     next_consumer_->HandleDiagnostic(diagnostic);
   }
 
@@ -200,4 +227,4 @@ class ErrorTrackingDiagnosticConsumer : public DiagnosticConsumer {
 
 }  // namespace Carbon
 
-#endif  // TOOLCHAIN_DIAGNOSTICS_DIAGNOSTICEMITTER_H_
+#endif  // TOOLCHAIN_DIAGNOSTICS_DIAGNOSTIC_EMITTER_H_

+ 33 - 60
toolchain/diagnostics/diagnostic_emitter_test.cpp

@@ -14,75 +14,48 @@
 namespace Carbon::Testing {
 namespace {
 
-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.
-  static constexpr llvm::StringLiteral Message = "{0}";
-
-  auto Format() -> std::string {
-    // Work around a bug in Clang's unused const variable warning by marking it
-    // used here with a no-op.
-    static_cast<void>(ShortName);
-
-    return llvm::formatv(Message.data(), message).str();
-  }
-
-  std::string message;
-};
-
 struct FakeDiagnosticLocationTranslator : DiagnosticLocationTranslator<int> {
-  auto GetLocation(int n) -> Diagnostic::Location override {
-    return {.file_name = "test", .line_number = 1, .column_number = n};
+  auto GetLocation(int n) -> DiagnosticLocation override {
+    return {.line_number = 1, .column_number = n};
   }
 };
 
-TEST(DiagTest, EmitErrors) {
-  FakeDiagnosticLocationTranslator translator;
-  Testing::MockDiagnosticConsumer consumer;
-  DiagnosticEmitter<int> emitter(translator, consumer);
+class DiagnosticEmitterTest : public ::testing::Test {
+ protected:
+  DiagnosticEmitterTest() : emitter_(translator_, consumer_) {}
 
-  EXPECT_CALL(consumer, HandleDiagnostic(
-                            AllOf(DiagnosticLevel(Diagnostic::Error),
-                                  DiagnosticAt(1, 1), DiagnosticMessage("M1"),
-                                  DiagnosticShortName("fake-diagnostic"))));
-  EXPECT_CALL(consumer, HandleDiagnostic(
-                            AllOf(DiagnosticLevel(Diagnostic::Error),
-                                  DiagnosticAt(1, 2), DiagnosticMessage("M2"),
-                                  DiagnosticShortName("fake-diagnostic"))));
+  FakeDiagnosticLocationTranslator translator_;
+  Testing::MockDiagnosticConsumer consumer_;
+  DiagnosticEmitter<int> emitter_;
+};
 
-  emitter.EmitError<FakeDiagnostic>(1, {.message = "M1"});
-  emitter.EmitError<FakeDiagnostic>(2, {.message = "M2"});
+TEST_F(DiagnosticEmitterTest, EmitSimpleError) {
+  CARBON_DIAGNOSTIC(TestDiagnostic, Error, "simple error");
+  EXPECT_CALL(consumer_, HandleDiagnostic(IsDiagnostic(
+                             DiagnosticKind::TestDiagnostic,
+                             DiagnosticLevel::Error, 1, 1, "simple error")));
+  EXPECT_CALL(consumer_, HandleDiagnostic(IsDiagnostic(
+                             DiagnosticKind::TestDiagnostic,
+                             DiagnosticLevel::Error, 1, 2, "simple error")));
+  emitter_.Emit(1, TestDiagnostic);
+  emitter_.Emit(2, TestDiagnostic);
 }
 
-TEST(DiagTest, EmitWarnings) {
-  std::vector<std::string> reported;
-
-  FakeDiagnosticLocationTranslator translator;
-  Testing::MockDiagnosticConsumer consumer;
-  DiagnosticEmitter<int> emitter(translator, consumer);
-
-  EXPECT_CALL(consumer, HandleDiagnostic(
-                            AllOf(DiagnosticLevel(Diagnostic::Warning),
-                                  DiagnosticAt(1, 3), DiagnosticMessage("M1"),
-                                  DiagnosticShortName("fake-diagnostic"))));
-  EXPECT_CALL(consumer, HandleDiagnostic(
-                            AllOf(DiagnosticLevel(Diagnostic::Warning),
-                                  DiagnosticAt(1, 5), DiagnosticMessage("M3"),
-                                  DiagnosticShortName("fake-diagnostic"))));
+TEST_F(DiagnosticEmitterTest, EmitSimpleWarning) {
+  CARBON_DIAGNOSTIC(TestDiagnostic, Warning, "simple warning");
+  EXPECT_CALL(consumer_,
+              HandleDiagnostic(IsDiagnostic(DiagnosticKind::TestDiagnostic,
+                                            DiagnosticLevel::Warning, 1, 1,
+                                            "simple warning")));
+  emitter_.Emit(1, TestDiagnostic);
+}
 
-  emitter.EmitWarningIf<FakeDiagnostic>(3, [](FakeDiagnostic& diagnostic) {
-    diagnostic.message = "M1";
-    return true;
-  });
-  emitter.EmitWarningIf<FakeDiagnostic>(4, [](FakeDiagnostic& diagnostic) {
-    diagnostic.message = "M2";
-    return false;
-  });
-  emitter.EmitWarningIf<FakeDiagnostic>(5, [](FakeDiagnostic& diagnostic) {
-    diagnostic.message = "M3";
-    return true;
-  });
+TEST_F(DiagnosticEmitterTest, EmitOneArgDiagnostic) {
+  CARBON_DIAGNOSTIC(TestDiagnostic, Error, "arg: `{0}`", llvm::StringRef);
+  EXPECT_CALL(consumer_, HandleDiagnostic(IsDiagnostic(
+                             DiagnosticKind::TestDiagnostic,
+                             DiagnosticLevel::Error, 1, 1, "arg: `str`")));
+  emitter_.Emit(1, TestDiagnostic, "str");
 }
 
 }  // namespace

+ 19 - 0
toolchain/diagnostics/diagnostic_kind.cpp

@@ -0,0 +1,19 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#include "toolchain/diagnostics/diagnostic_kind.h"
+
+namespace Carbon {
+
+auto operator<<(llvm::raw_ostream& out, DiagnosticKind kind)
+    -> llvm::raw_ostream& {
+  static constexpr llvm::StringLiteral Names[] = {
+#define DIAGNOSTIC_KIND(DiagnosticName) #DiagnosticName,
+#include "toolchain/diagnostics/diagnostic_registry.def"
+  };
+  out << Names[static_cast<int>(kind)];
+  return out;
+}
+
+}  // namespace Carbon

+ 37 - 0
toolchain/diagnostics/diagnostic_kind.h

@@ -0,0 +1,37 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#ifndef TOOLCHAIN_DIAGNOSTICS_DIAGNOSTIC_KIND_H_
+#define TOOLCHAIN_DIAGNOSTICS_DIAGNOSTIC_KIND_H_
+
+#include "common/ostream.h"
+#include "llvm/ADT/StringRef.h"
+
+namespace Carbon {
+
+// An enumeration of all diagnostics provided by the toolchain. Diagnostics must
+// be added to diagnostic_registry.def, and defined locally to where they're
+// used using the `DIAGNOSTIC` macro in diagnostic_emitter.h.
+//
+// Diagnostic definitions are decentralized because placing all diagnostic
+// definitions centrally is expected to create a compilation bottleneck
+// long-term, and we also see value to keeping diagnostic format strings close
+// to the consuming code.
+enum class DiagnosticKind : int32_t {
+#define DIAGNOSTIC_KIND(DiagnosticName) DiagnosticName,
+#include "toolchain/diagnostics/diagnostic_registry.def"
+};
+
+auto operator<<(llvm::raw_ostream& out, DiagnosticKind kind)
+    -> llvm::raw_ostream&;
+
+inline auto operator<<(std::ostream& out, DiagnosticKind kind)
+    -> std::ostream& {
+  llvm::raw_os_ostream(out) << kind;
+  return out;
+}
+
+}  // namespace Carbon
+
+#endif  // TOOLCHAIN_DIAGNOSTICS_DIAGNOSTIC_KIND_H_

+ 22 - 0
toolchain/diagnostics/diagnostic_kind_test.cpp

@@ -0,0 +1,22 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#include "toolchain/diagnostics/diagnostic_kind.h"
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#include "llvm/Support/raw_ostream.h"
+
+namespace Carbon::Testing {
+namespace {
+
+TEST(DiagnosticKindTest, Name) {
+  std::string buffer;
+  llvm::raw_string_ostream(buffer) << DiagnosticKind::TestDiagnostic;
+  EXPECT_EQ(buffer, "TestDiagnostic");
+}
+
+}  // namespace
+}  // namespace Carbon::Testing

+ 76 - 0
toolchain/diagnostics/diagnostic_registry.def

@@ -0,0 +1,76 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// Note that this is an X-macro header.
+//
+// It does not use `#include` guards, and instead is designed to be `#include`ed
+// after some set of x-macros are defined in order for its inclusion to expand
+// to the desired output.
+//
+// The viable X-macros to define prior to including the header are:
+//
+// - `DIAGNOSTIC_KIND`
+
+// ============================================================================
+// Lexer diagnostics
+// ============================================================================
+
+DIAGNOSTIC_KIND(BinaryRealLiteral)
+DIAGNOSTIC_KIND(ContentBeforeStringTerminator)
+DIAGNOSTIC_KIND(DecimalEscapeSequence)
+DIAGNOSTIC_KIND(EmptyDigitSequence)
+DIAGNOSTIC_KIND(HexadecimalEscapeMissingDigits)
+DIAGNOSTIC_KIND(InvalidDigit)
+DIAGNOSTIC_KIND(InvalidDigitSeparator)
+DIAGNOSTIC_KIND(InvalidHorizontalWhitespaceInString)
+DIAGNOSTIC_KIND(IrregularDigitSeparators)
+DIAGNOSTIC_KIND(MismatchedClosing)
+DIAGNOSTIC_KIND(MismatchedIndentInString)
+DIAGNOSTIC_KIND(NoWhitespaceAfterCommentIntroducer)
+DIAGNOSTIC_KIND(TooManyDigits)
+DIAGNOSTIC_KIND(TrailingComment)
+DIAGNOSTIC_KIND(UnicodeEscapeMissingBracedDigits)
+DIAGNOSTIC_KIND(UnicodeEscapeSurrogate)
+DIAGNOSTIC_KIND(UnicodeEscapeTooLarge)
+DIAGNOSTIC_KIND(UnknownBaseSpecifier)
+DIAGNOSTIC_KIND(UnknownEscapeSequence)
+DIAGNOSTIC_KIND(UnmatchedClosing)
+DIAGNOSTIC_KIND(UnrecognizedCharacters)
+DIAGNOSTIC_KIND(UnterminatedString)
+DIAGNOSTIC_KIND(WrongRealLiteralExponent)
+
+// ============================================================================
+// Parser diagnostics
+// ============================================================================
+
+DIAGNOSTIC_KIND(BinaryOperatorRequiresWhitespace)
+DIAGNOSTIC_KIND(ExpectedCloseParen)
+DIAGNOSTIC_KIND(ExpectedCodeBlock)
+DIAGNOSTIC_KIND(ExpectedExpression)
+DIAGNOSTIC_KIND(ExpectedFunctionBodyOrSemi)
+DIAGNOSTIC_KIND(ExpectedFunctionName)
+DIAGNOSTIC_KIND(ExpectedFunctionParams)
+DIAGNOSTIC_KIND(ExpectedIdentifierAfterDot)
+DIAGNOSTIC_KIND(ExpectedParameterName)
+DIAGNOSTIC_KIND(ExpectedParenAfter)
+DIAGNOSTIC_KIND(ExpectedSemiAfter)
+DIAGNOSTIC_KIND(ExpectedSemiAfterExpression)
+DIAGNOSTIC_KIND(ExpectedStructLiteralField)
+DIAGNOSTIC_KIND(ExpectedVariableName)
+DIAGNOSTIC_KIND(OperatorRequiresParentheses)
+DIAGNOSTIC_KIND(StackLimitExceeded)
+DIAGNOSTIC_KIND(UnaryOperatorHasWhitespace)
+DIAGNOSTIC_KIND(UnaryOperatorRequiresWhitespace)
+DIAGNOSTIC_KIND(UnexpectedTokenAfterListElement)
+DIAGNOSTIC_KIND(UnexpectedTokenInCodeBlock)
+DIAGNOSTIC_KIND(UnrecognizedDeclaration)
+
+// ============================================================================
+// Other diagnostics
+// ============================================================================
+
+// TestDiagnostic is only for unit tests.
+DIAGNOSTIC_KIND(TestDiagnostic)
+
+#undef DIAGNOSTIC_KIND

+ 9 - 5
toolchain/diagnostics/mocks.cpp

@@ -7,12 +7,16 @@
 namespace Carbon {
 
 void PrintTo(const Diagnostic& diagnostic, std::ostream* os) {
-  *os << "Diagnostic{"
-      << (diagnostic.level == Diagnostic::Level::Error ? "Error" : "Warning")
-      << ", " << diagnostic.location.file_name << ":"
+  *os << "Diagnostic{" << diagnostic.kind << ", ";
+  PrintTo(diagnostic.level, os);
+  *os << ", " << diagnostic.location.file_name << ":"
       << diagnostic.location.line_number << ":"
-      << diagnostic.location.column_number << ", "
-      << diagnostic.short_name.str() << ", " << diagnostic.message << "}";
+      << diagnostic.location.column_number << ", \""
+      << diagnostic.format_fn(diagnostic) << "\"}";
+}
+
+void PrintTo(DiagnosticLevel level, std::ostream* os) {
+  *os << (level == DiagnosticLevel::Error ? "Error" : "Warning");
 }
 
 }  // namespace Carbon

+ 22 - 30
toolchain/diagnostics/mocks.h

@@ -17,46 +17,38 @@ class MockDiagnosticConsumer : public DiagnosticConsumer {
               (override));
 };
 
-// Matcher `DiagnosticAt` matches the location of a diagnostic.
-MATCHER_P2(DiagnosticAt, line, column, "") {
+MATCHER_P(IsDiagnosticMessage, matcher, "") {
   const Diagnostic& diag = arg;
-  const Diagnostic::Location& loc = diag.location;
-  if (loc.line_number != line) {
-    *result_listener << "\nExpected diagnostic on line " << line
-                     << " but diagnostic is on line " << loc.line_number << ".";
-    return false;
-  }
-  if (loc.column_number != column) {
-    *result_listener << "\nExpected diagnostic on column " << column
-                     << " but diagnostic is on column " << loc.column_number
-                     << ".";
-    return false;
-  }
-  return true;
+  return testing::ExplainMatchResult(matcher, diag.format_fn(diag),
+                                     result_listener);
 }
 
-inline auto DiagnosticLevel(Diagnostic::Level level) -> auto {
-  return testing::Field(&Diagnostic::level, level);
-}
-
-template <typename Matcher>
-auto DiagnosticMessage(Matcher&& inner_matcher) -> auto {
-  return testing::Field(&Diagnostic::message,
-                        std::forward<Matcher&&>(inner_matcher));
-}
-
-template <typename Matcher>
-auto DiagnosticShortName(Matcher&& inner_matcher) -> auto {
-  return testing::Field(&Diagnostic::short_name,
-                        std::forward<Matcher&&>(inner_matcher));
+inline auto IsDiagnostic(testing::Matcher<DiagnosticKind> kind,
+                         testing::Matcher<DiagnosticLevel> level,
+                         testing::Matcher<int> line_number,
+                         testing::Matcher<int> column_number,
+                         testing::Matcher<std::string> message) {
+  return testing::AllOf(
+      testing::Field("kind", &Diagnostic::kind, kind),
+      testing::Field("level", &Diagnostic::level, level),
+      testing::Field(
+          &Diagnostic::location,
+          testing::AllOf(
+              testing::Field("line_number", &DiagnosticLocation::line_number,
+                             line_number),
+              testing::Field("column_number",
+                             &DiagnosticLocation::column_number,
+                             column_number))),
+      IsDiagnosticMessage(message));
 }
 
 }  // namespace Carbon::Testing
 
 namespace Carbon {
 
-// Printing helper for tests.
+// Printing helpers for tests.
 void PrintTo(const Diagnostic& diagnostic, std::ostream* os);
+void PrintTo(DiagnosticLevel level, std::ostream* os);
 
 }  // namespace Carbon
 

+ 1 - 1
toolchain/diagnostics/null_diagnostics.h

@@ -13,7 +13,7 @@ template <typename LocationT>
 inline auto NullDiagnosticLocationTranslator()
     -> DiagnosticLocationTranslator<LocationT>& {
   struct Translator : DiagnosticLocationTranslator<LocationT> {
-    auto GetLocation(LocationT /*loc*/) -> Diagnostic::Location override {
+    auto GetLocation(LocationT /*loc*/) -> DiagnosticLocation override {
       return {};
     }
   };

+ 19 - 44
toolchain/diagnostics/sorting_diagnostic_consumer_test.cpp

@@ -17,68 +17,43 @@ namespace {
 
 using ::testing::InSequence;
 
-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.
-  static constexpr llvm::StringLiteral Message = "{0}";
-
-  auto Format() -> std::string {
-    // Work around a bug in Clang's unused const variable warning by marking it
-    // used here with a no-op.
-    static_cast<void>(ShortName);
-
-    return llvm::formatv(Message.data(), message).str();
-  }
-
-  std::string message;
-};
+CARBON_DIAGNOSTIC(TestDiagnostic, Error, "{0}", llvm::StringRef);
 
 struct FakeDiagnosticLocationTranslator
-    : DiagnosticLocationTranslator<Diagnostic::Location> {
-  auto GetLocation(Diagnostic::Location loc) -> Diagnostic::Location override {
+    : DiagnosticLocationTranslator<DiagnosticLocation> {
+  auto GetLocation(DiagnosticLocation loc) -> DiagnosticLocation override {
     return loc;
   }
 };
 
-// Produces a location for the given line and column.
-static auto MakeLoc(int line, int col) -> Diagnostic::Location {
-  return {.file_name = "test", .line_number = line, .column_number = col};
-}
-
 TEST(SortedDiagnosticEmitterTest, SortErrors) {
   FakeDiagnosticLocationTranslator translator;
   Testing::MockDiagnosticConsumer consumer;
   SortingDiagnosticConsumer sorting_consumer(consumer);
-  DiagnosticEmitter<Diagnostic::Location> emitter(translator, sorting_consumer);
+  DiagnosticEmitter<DiagnosticLocation> emitter(translator, sorting_consumer);
 
-  emitter.EmitError<FakeDiagnostic>(MakeLoc(2, 1), {.message = "M1"});
-  emitter.EmitError<FakeDiagnostic>(MakeLoc(1, 1), {.message = "M2"});
-  emitter.EmitError<FakeDiagnostic>(MakeLoc(1, 3), {.message = "M3"});
-  emitter.EmitError<FakeDiagnostic>(MakeLoc(3, 4), {.message = "M4"});
-  emitter.EmitError<FakeDiagnostic>(MakeLoc(3, 2), {.message = "M5"});
+  emitter.Emit({"f", 2, 1}, TestDiagnostic, "M1");
+  emitter.Emit({"f", 1, 1}, TestDiagnostic, "M2");
+  emitter.Emit({"f", 1, 3}, TestDiagnostic, "M3");
+  emitter.Emit({"f", 3, 4}, TestDiagnostic, "M4");
+  emitter.Emit({"f", 3, 2}, TestDiagnostic, "M5");
 
   InSequence s;
   EXPECT_CALL(consumer, HandleDiagnostic(
-                            AllOf(DiagnosticLevel(Diagnostic::Error),
-                                  DiagnosticAt(1, 1), DiagnosticMessage("M2"),
-                                  DiagnosticShortName("fake-diagnostic"))));
+                            IsDiagnostic(DiagnosticKind::TestDiagnostic,
+                                         DiagnosticLevel::Error, 1, 1, "M2")));
   EXPECT_CALL(consumer, HandleDiagnostic(
-                            AllOf(DiagnosticLevel(Diagnostic::Error),
-                                  DiagnosticAt(1, 3), DiagnosticMessage("M3"),
-                                  DiagnosticShortName("fake-diagnostic"))));
+                            IsDiagnostic(DiagnosticKind::TestDiagnostic,
+                                         DiagnosticLevel::Error, 1, 3, "M3")));
   EXPECT_CALL(consumer, HandleDiagnostic(
-                            AllOf(DiagnosticLevel(Diagnostic::Error),
-                                  DiagnosticAt(2, 1), DiagnosticMessage("M1"),
-                                  DiagnosticShortName("fake-diagnostic"))));
+                            IsDiagnostic(DiagnosticKind::TestDiagnostic,
+                                         DiagnosticLevel::Error, 2, 1, "M1")));
   EXPECT_CALL(consumer, HandleDiagnostic(
-                            AllOf(DiagnosticLevel(Diagnostic::Error),
-                                  DiagnosticAt(3, 2), DiagnosticMessage("M5"),
-                                  DiagnosticShortName("fake-diagnostic"))));
+                            IsDiagnostic(DiagnosticKind::TestDiagnostic,
+                                         DiagnosticLevel::Error, 3, 2, "M5")));
   EXPECT_CALL(consumer, HandleDiagnostic(
-                            AllOf(DiagnosticLevel(Diagnostic::Error),
-                                  DiagnosticAt(3, 4), DiagnosticMessage("M4"),
-                                  DiagnosticShortName("fake-diagnostic"))));
+                            IsDiagnostic(DiagnosticKind::TestDiagnostic,
+                                         DiagnosticLevel::Error, 3, 4, "M4")));
   sorting_consumer.Flush();
 }
 

+ 1 - 0
toolchain/lexer/BUILD

@@ -13,6 +13,7 @@ cc_library(
     textual_hdrs = ["token_registry.def"],
     deps = [
         "//common:check",
+        "//common:ostream",
         "@llvm-project//llvm:Support",
     ],
 )

+ 6 - 19
toolchain/lexer/lex_helpers.cpp

@@ -8,23 +8,6 @@
 
 namespace Carbon {
 
-namespace {
-struct TooManyDigits : DiagnosticBase<TooManyDigits> {
-  static constexpr llvm::StringLiteral ShortName = "syntax-invalid-number";
-
-  auto Format() -> std::string {
-    return llvm::formatv(
-               "Found a sequence of {0} digits, which is greater than the "
-               "limit of {1}.",
-               count, limit)
-        .str();
-  }
-
-  size_t count;
-  size_t limit;
-};
-}  // namespace
-
 auto CanLexInteger(DiagnosticEmitter<const char*>& emitter,
                    llvm::StringRef text) -> bool {
   // llvm::getAsInteger is used for parsing, but it's quadratic and visibly slow
@@ -37,8 +20,12 @@ auto CanLexInteger(DiagnosticEmitter<const char*>& emitter,
   // is far above the threshold for normal integers.
   constexpr size_t DigitLimit = 1000;
   if (text.size() > DigitLimit) {
-    emitter.EmitError<TooManyDigits>(
-        text.begin(), {.count = text.size(), .limit = DigitLimit});
+    CARBON_DIAGNOSTIC(
+        TooManyDigits, Error,
+        "Found a sequence of {0} digits, which is greater than the "
+        "limit of {1}.",
+        size_t, size_t);
+    emitter.Emit(text.begin(), TooManyDigits, text.size(), DigitLimit);
     return false;
   }
   return true;

+ 30 - 83
toolchain/lexer/numeric_literal.cpp

@@ -14,8 +14,8 @@
 
 namespace Carbon {
 
-// Adapts radix for use with formatv.
-auto operator<<(llvm::raw_ostream& out, LexedNumericLiteral::Radix radix)
+// Adapts Radix for use with formatv.
+static auto operator<<(llvm::raw_ostream& out, LexedNumericLiteral::Radix radix)
     -> llvm::raw_ostream& {
   switch (radix) {
     case LexedNumericLiteral::Radix::Binary:
@@ -31,75 +31,24 @@ auto operator<<(llvm::raw_ostream& out, LexedNumericLiteral::Radix radix)
   return out;
 }
 
-namespace {
-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 : DiagnosticBase<InvalidDigit> {
-  static constexpr llvm::StringLiteral ShortName = "syntax-invalid-number";
-
-  auto Format() -> std::string {
-    return llvm::formatv("Invalid digit '{0}' in {1} numeric literal.", digit,
-                         radix)
-        .str();
-  }
-
-  char digit;
-  LexedNumericLiteral::Radix radix;
-};
-
-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 : DiagnosticBase<IrregularDigitSeparators> {
-  static constexpr llvm::StringLiteral ShortName =
-      "syntax-irregular-digit-separators";
-
-  auto Format() -> std::string {
-    CHECK((radix == LexedNumericLiteral::Radix::Decimal ||
-           radix == LexedNumericLiteral::Radix::Hexadecimal))
-        << "unexpected radix: " << radix;
-    return llvm::formatv(
-               "Digit separators in {0} number should appear every {1} "
-               "characters from the right.",
-               radix,
-               (radix == LexedNumericLiteral::Radix::Decimal ? "3" : "4"))
-        .str();
-  }
-
-  LexedNumericLiteral::Radix radix;
-};
-
-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 : DiagnosticBase<BinaryRealLiteral> {
-  static constexpr llvm::StringLiteral ShortName = "syntax-invalid-number";
-  static constexpr llvm::StringLiteral Message =
-      "Binary real number literals are not supported.";
-};
-
-struct WrongRealLiteralExponent : DiagnosticBase<WrongRealLiteralExponent> {
-  static constexpr llvm::StringLiteral ShortName = "syntax-invalid-number";
-
-  auto Format() -> std::string {
-    return llvm::formatv("Expected '{0}' to introduce exponent.", expected)
-        .str();
-  }
-
-  char expected;
-};
-
-}  // namespace
+CARBON_DIAGNOSTIC(InvalidDigitSeparator, Error,
+                  "Misplaced digit separator in numeric literal.");
+CARBON_DIAGNOSTIC(InvalidDigit, Error,
+                  "Invalid digit '{0}' in {1} numeric literal.", char,
+                  LexedNumericLiteral::Radix);
+CARBON_DIAGNOSTIC(EmptyDigitSequence, Error,
+                  "Empty digit sequence in numeric literal.");
+CARBON_DIAGNOSTIC(
+    IrregularDigitSeparators, Error,
+    "Digit separators in {0} number should appear every {1} characters "
+    "from the right.",
+    LexedNumericLiteral::Radix, int);
+CARBON_DIAGNOSTIC(UnknownBaseSpecifier, Error,
+                  "Unknown base specifier in numeric literal.");
+CARBON_DIAGNOSTIC(BinaryRealLiteral, Error,
+                  "Binary real number literals are not supported.");
+CARBON_DIAGNOSTIC(WrongRealLiteralExponent, Error,
+                  "Expected '{0}' to introduce exponent.", char);
 
 auto LexedNumericLiteral::Lex(llvm::StringRef source_text)
     -> llvm::Optional<LexedNumericLiteral> {
@@ -368,19 +317,18 @@ auto LexedNumericLiteral::Parser::CheckDigitSequence(
       // next to another digit separator, or at the end.
       if (!allow_digit_separators || i == 0 || text[i - 1] == '_' ||
           i + 1 == n) {
-        emitter_.EmitError<InvalidDigitSeparator>(text.begin() + i);
+        emitter_.Emit(text.begin() + 1, InvalidDigitSeparator);
       }
       ++num_digit_separators;
       continue;
     }
 
-    emitter_.EmitError<InvalidDigit>(text.begin() + i,
-                                     {.digit = c, .radix = radix});
+    emitter_.Emit(text.begin() + i, InvalidDigit, c, radix);
     return {.ok = false};
   }
 
   if (num_digit_separators == static_cast<int>(text.size())) {
-    emitter_.EmitError<EmptyDigitSequence>(text.begin());
+    emitter_.Emit(text.begin(), EmptyDigitSequence);
     return {.ok = false};
   }
 
@@ -410,8 +358,8 @@ auto LexedNumericLiteral::Parser::CheckDigitSeparatorPlacement(
   }
 
   auto diagnose_irregular_digit_separators = [&]() {
-    emitter_.EmitError<IrregularDigitSeparators>(text.begin(),
-                                                 {.radix = radix});
+    emitter_.Emit(text.begin(), IrregularDigitSeparators, radix,
+                  radix == Radix::Decimal ? 3 : 4);
   };
 
   // For decimal and hexadecimal digit sequences, digit separators must form
@@ -439,7 +387,7 @@ auto LexedNumericLiteral::Parser::CheckDigitSeparatorPlacement(
 auto LexedNumericLiteral::Parser::CheckLeadingZero() -> bool {
   if (radix_ == Radix::Decimal && int_part_.startswith("0") &&
       int_part_ != "0") {
-    emitter_.EmitError<UnknownBaseSpecifier>(int_part_.begin());
+    emitter_.Emit(int_part_.begin(), UnknownBaseSpecifier);
     return false;
   }
   return true;
@@ -460,8 +408,8 @@ auto LexedNumericLiteral::Parser::CheckFractionalPart() -> bool {
   }
 
   if (radix_ == Radix::Binary) {
-    emitter_.EmitError<BinaryRealLiteral>(literal_.text_.begin() +
-                                          literal_.radix_point_);
+    emitter_.Emit(literal_.text_.begin() + literal_.radix_point_,
+                  BinaryRealLiteral);
     // Carry on and parse the binary real literal anyway.
   }
 
@@ -481,9 +429,8 @@ auto LexedNumericLiteral::Parser::CheckExponentPart() -> bool {
 
   char expected_exponent_kind = (radix_ == Radix::Decimal ? 'e' : 'p');
   if (literal_.text_[literal_.exponent_] != expected_exponent_kind) {
-    emitter_.EmitError<WrongRealLiteralExponent>(
-        literal_.text_.begin() + literal_.exponent_,
-        {.expected = expected_exponent_kind});
+    emitter_.Emit(literal_.text_.begin() + literal_.exponent_,
+                  WrongRealLiteralExponent, expected_exponent_kind);
     return false;
   }
 

+ 39 - 77
toolchain/lexer/string_literal.cpp

@@ -17,72 +17,36 @@ namespace Carbon {
 
 using LexerDiagnosticEmitter = DiagnosticEmitter<const char*>;
 
-struct 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 : 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 : 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
-    : 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 "
-      "uppercase hexadecimal digits, for example `\\u{70AD}`.";
-};
-
-struct 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 : 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 : DiagnosticBase<UnknownEscapeSequence> {
-  static constexpr llvm::StringLiteral ShortName = "syntax-invalid-string";
-  static constexpr const char* Message = "Unrecognized escape sequence `{0}`.";
-
-  auto Format() -> std::string { return llvm::formatv(Message, first).str(); }
-
-  char first;
-};
-
-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 "
-      "string literal.";
-};
-
-struct 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 "
-      "sequence in a string literal.";
-};
+CARBON_DIAGNOSTIC(
+    ContentBeforeStringTerminator, Error,
+    "Only whitespace is permitted before the closing `\"\"\"` of a "
+    "multi-line string.");
+CARBON_DIAGNOSTIC(
+    UnicodeEscapeTooLarge, Error,
+    "Code point specified by `\\u{{...}}` escape is greater than 0x10FFFF.");
+CARBON_DIAGNOSTIC(
+    UnicodeEscapeSurrogate, Error,
+    "Code point specified by `\\u{{...}}` escape is a surrogate character.");
+CARBON_DIAGNOSTIC(
+    UnicodeEscapeMissingBracedDigits, Error,
+    "Escape sequence `\\u` must be followed by a braced sequence of "
+    "uppercase hexadecimal digits, for example `\\u{{70AD}}`.");
+CARBON_DIAGNOSTIC(HexadecimalEscapeMissingDigits, Error,
+                  "Escape sequence `\\x` must be followed by two "
+                  "uppercase hexadecimal digits, for example `\\x0F`.");
+CARBON_DIAGNOSTIC(
+    DecimalEscapeSequence, Error,
+    "Decimal digit follows `\\0` escape sequence. Use `\\x00` instead "
+    "of `\\0` if the next character is a digit.");
+CARBON_DIAGNOSTIC(UnknownEscapeSequence, Error,
+                  "Unrecognized escape sequence `{0}`.", char);
+CARBON_DIAGNOSTIC(MismatchedIndentInString, Error,
+                  "Indentation does not match that of the closing \"\"\" in "
+                  "multi-line string literal.");
+CARBON_DIAGNOSTIC(
+    InvalidHorizontalWhitespaceInString, Error,
+    "Whitespace other than plain space must be expressed with an escape "
+    "sequence in a string literal.");
 
 static constexpr char MultiLineIndicator[] = R"(""")";
 
@@ -218,7 +182,7 @@ static auto CheckIndent(LexerDiagnosticEmitter& emitter, llvm::StringRef text,
   // The last line is not permitted to contain any content after its
   // indentation.
   if (indent.end() != content.end()) {
-    emitter.EmitError<ContentBeforeStringTerminator>(indent.end());
+    emitter.Emit(indent.end(), ContentBeforeStringTerminator);
   }
 
   return indent;
@@ -233,12 +197,12 @@ static auto ExpandUnicodeEscapeSequence(LexerDiagnosticEmitter& emitter,
     return false;
   }
   if (digits.getAsInteger(16, code_point) || code_point > 0x10FFFF) {
-    emitter.EmitError<UnicodeEscapeTooLarge>(digits.begin());
+    emitter.Emit(digits.begin(), UnicodeEscapeTooLarge);
     return false;
   }
 
   if (code_point >= 0xD800 && code_point < 0xE000) {
-    emitter.EmitError<UnicodeEscapeSurrogate>(digits.begin());
+    emitter.Emit(digits.begin(), UnicodeEscapeSurrogate);
     return false;
   }
 
@@ -291,7 +255,7 @@ static auto ExpandAndConsumeEscapeSequence(LexerDiagnosticEmitter& emitter,
     case '0':
       result += '\0';
       if (!content.empty() && IsDecimalDigit(content.front())) {
-        emitter.EmitError<DecimalEscapeSequence>(content.begin());
+        emitter.Emit(content.begin(), DecimalEscapeSequence);
         return;
       }
       return;
@@ -303,7 +267,7 @@ static auto ExpandAndConsumeEscapeSequence(LexerDiagnosticEmitter& emitter,
         content = content.drop_front(2);
         return;
       }
-      emitter.EmitError<HexadecimalEscapeMissingDigits>(content.begin());
+      emitter.Emit(content.begin(), HexadecimalEscapeMissingDigits);
       break;
     case 'u': {
       llvm::StringRef remaining = content;
@@ -318,12 +282,11 @@ static auto ExpandAndConsumeEscapeSequence(LexerDiagnosticEmitter& emitter,
           return;
         }
       }
-      emitter.EmitError<UnicodeEscapeMissingBracedDigits>(content.begin());
+      emitter.Emit(content.begin(), UnicodeEscapeMissingBracedDigits);
       break;
     }
     default:
-      emitter.EmitError<UnknownEscapeSequence>(content.begin() - 1,
-                                               {.first = first});
+      emitter.Emit(content.begin() - 1, UnknownEscapeSequence, first);
       break;
   }
 
@@ -352,7 +315,7 @@ static auto ExpandEscapeSequencesAndRemoveIndent(
       const char* line_start = contents.begin();
       contents = contents.drop_while(IsHorizontalWhitespace);
       if (!contents.startswith("\n")) {
-        emitter.EmitError<MismatchedIndentInString>(line_start);
+        emitter.Emit(line_start, MismatchedIndentInString);
       }
     }
 
@@ -391,8 +354,7 @@ static auto ExpandEscapeSequencesAndRemoveIndent(
             contents[after_space] != '\n') {
           // TODO: Include the source range of the whitespace up to
           // `contents.begin() + after_space` in the diagnostic.
-          emitter.EmitError<InvalidHorizontalWhitespaceInString>(
-              contents.begin());
+          emitter.Emit(contents.begin(), InvalidHorizontalWhitespaceInString);
           // Include the whitespace in the string contents for error recovery.
           result += contents.substr(0, after_space);
         }

+ 3 - 9
toolchain/lexer/test_helpers.h

@@ -28,32 +28,26 @@ class SingleTokenDiagnosticTranslator
   explicit SingleTokenDiagnosticTranslator(llvm::StringRef token)
       : token_(token) {}
 
-  auto GetLocation(const char* pos) -> Diagnostic::Location override {
+  auto GetLocation(const char* pos) -> DiagnosticLocation override {
     CHECK(StringRefContainsPointer(token_, pos))
         << "invalid diagnostic location";
     llvm::StringRef prefix = token_.take_front(pos - token_.begin());
     auto [before_last_newline, this_line] = prefix.rsplit('\n');
     if (before_last_newline.size() == prefix.size()) {
       // On first line.
-      return {.file_name = SynthesizeFilename(),
-              .line_number = 1,
+      return {.line_number = 1,
               .column_number = static_cast<int32_t>(pos - token_.begin() + 1)};
     } else {
       // On second or subsequent lines. Note that the line number here is 2
       // more than the number of newlines because `rsplit` removed one newline
       // and `line_number` is 1-based.
-      return {.file_name = SynthesizeFilename(),
-              .line_number =
+      return {.line_number =
                   static_cast<int32_t>(before_last_newline.count('\n') + 2),
               .column_number = static_cast<int32_t>(this_line.size() + 1)};
     }
   }
 
  private:
-  [[nodiscard]] auto SynthesizeFilename() const -> std::string {
-    return llvm::formatv("`{0}`", token_);
-  }
-
   llvm::StringRef token_;
 };
 

+ 4 - 0
toolchain/lexer/token_kind.h

@@ -9,6 +9,7 @@
 #include <initializer_list>
 #include <iterator>
 
+#include "common/ostream.h"
 #include "llvm/ADT/StringRef.h"
 
 namespace Carbon {
@@ -99,6 +100,9 @@ class TokenKind {
   // NOLINTNEXTLINE(google-explicit-constructor)
   constexpr operator KindEnum() const { return kind_value_; }
 
+  // Prints the TokenKind, typically for diagnostics.
+  void Print(llvm::raw_ostream& out) const { out << GetFixedSpelling(); }
+
  private:
   constexpr explicit TokenKind(KindEnum kind_value) : kind_value_(kind_value) {}
 

+ 20 - 46
toolchain/lexer/tokenized_buffer.cpp

@@ -27,43 +27,17 @@
 
 namespace Carbon {
 
-struct TrailingComment : DiagnosticBase<TrailingComment> {
-  static constexpr llvm::StringLiteral ShortName = "syntax-comments";
-  static constexpr llvm::StringLiteral Message =
-      "Trailing comments are not permitted.";
-};
-
-struct NoWhitespaceAfterCommentIntroducer
-    : DiagnosticBase<NoWhitespaceAfterCommentIntroducer> {
-  static constexpr llvm::StringLiteral ShortName = "syntax-comments";
-  static constexpr llvm::StringLiteral Message =
-      "Whitespace is required after '//'.";
-};
-
-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 : 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 : DiagnosticBase<UnrecognizedCharacters> {
-  static constexpr llvm::StringLiteral ShortName =
-      "syntax-unrecognized-characters";
-  static constexpr llvm::StringLiteral Message =
-      "Encountered unrecognized characters while parsing.";
-};
-
-struct UnterminatedString : DiagnosticBase<UnterminatedString> {
-  static constexpr llvm::StringLiteral ShortName = "syntax-string-terminator";
-  static constexpr llvm::StringLiteral Message =
-      "String is missing a terminator.";
-};
+CARBON_DIAGNOSTIC(TrailingComment, Error,
+                  "Trailing comments are not permitted.");
+CARBON_DIAGNOSTIC(NoWhitespaceAfterCommentIntroducer, Error,
+                  "Whitespace is required after '//'.");
+CARBON_DIAGNOSTIC(UnmatchedClosing, Error,
+                  "Closing symbol without a corresponding opening symbol.");
+CARBON_DIAGNOSTIC(MismatchedClosing, Error,
+                  "Closing symbol does not match most recent opening symbol.");
+CARBON_DIAGNOSTIC(UnrecognizedCharacters, Error,
+                  "Encountered unrecognized characters while parsing.");
+CARBON_DIAGNOSTIC(UnterminatedString, Error, "String is missing a terminator.");
 
 // TODO: Move Overload and VariantMatch somewhere more central.
 
@@ -157,12 +131,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>(source_text.begin());
+          emitter_.Emit(source_text.begin(), TrailingComment);
         }
         // The introducer '//' must be followed by whitespace or EOF.
         if (source_text.size() > 2 && !IsSpace(source_text[2])) {
-          emitter_.EmitError<NoWhitespaceAfterCommentIntroducer>(
-              source_text.begin() + 2);
+          emitter_.Emit(source_text.begin() + 2,
+                        NoWhitespaceAfterCommentIntroducer);
         }
         while (!source_text.empty() && source_text.front() != '\n') {
           ++current_column_;
@@ -311,7 +285,7 @@ class TokenizedBuffer::Lexer {
           literal->ComputeValue(emitter_));
       return token;
     } else {
-      emitter_.EmitError<UnterminatedString>(literal->text().begin());
+      emitter_.Emit(literal->text().begin(), UnterminatedString);
       return buffer_.AddToken({.kind = TokenKind::Error(),
                                .token_line = string_line,
                                .column = string_column,
@@ -361,7 +335,7 @@ class TokenizedBuffer::Lexer {
       closing_token_info.kind = TokenKind::Error();
       closing_token_info.error_length = kind.GetFixedSpelling().size();
 
-      emitter_.EmitError<UnmatchedClosing>(location);
+      emitter_.Emit(location, UnmatchedClosing);
       // Note that this still returns true as we do consume a symbol.
       return token;
     }
@@ -438,7 +412,7 @@ class TokenizedBuffer::Lexer {
       }
 
       open_groups_.pop_back();
-      token_emitter_.EmitError<MismatchedClosing>(opening_token);
+      token_emitter_.Emit(opening_token, MismatchedClosing);
 
       CHECK(!buffer_.tokens().empty()) << "Must have a prior opening token!";
       Token prev_token = buffer_.tokens().end()[-1];
@@ -536,7 +510,7 @@ class TokenizedBuffer::Lexer {
          .token_line = current_line_,
          .column = current_column_,
          .error_length = static_cast<int32_t>(error_text.size())});
-    emitter_.EmitError<UnrecognizedCharacters>(error_text.begin());
+    emitter_.Emit(error_text.begin(), UnrecognizedCharacters);
 
     current_column_ += error_text.size();
     source_text = source_text.drop_front(error_text.size());
@@ -909,7 +883,7 @@ auto TokenizedBuffer::TokenIterator::Print(llvm::raw_ostream& output) const
 }
 
 auto TokenizedBuffer::SourceBufferLocationTranslator::GetLocation(
-    const char* loc) -> Diagnostic::Location {
+    const char* loc) -> DiagnosticLocation {
   CHECK(StringRefContainsPointer(buffer_->source_->text(), loc))
       << "location not within buffer";
   int64_t offset = loc - buffer_->source_->text().begin();
@@ -952,7 +926,7 @@ auto TokenizedBuffer::SourceBufferLocationTranslator::GetLocation(
 }
 
 auto TokenizedBuffer::TokenLocationTranslator::GetLocation(Token token)
-    -> Diagnostic::Location {
+    -> DiagnosticLocation {
   // Map the token location into a position within the source buffer.
   auto& token_info = buffer_->GetTokenInfo(token);
   auto& line_info = buffer_->GetLineInfo(token_info.token_line);

+ 2 - 2
toolchain/lexer/tokenized_buffer.h

@@ -257,7 +257,7 @@ class TokenizedBuffer {
           last_line_lexed_to_column_(last_line_lexed_to_column) {}
 
     // Map the given token into a diagnostic location.
-    auto GetLocation(Token token) -> Diagnostic::Location override;
+    auto GetLocation(Token token) -> DiagnosticLocation override;
 
    private:
     TokenizedBuffer* buffer_;
@@ -382,7 +382,7 @@ class TokenizedBuffer {
 
     // Map the given position within the source buffer into a diagnostic
     // location.
-    auto GetLocation(const char* loc) -> Diagnostic::Location override;
+    auto GetLocation(const char* loc) -> DiagnosticLocation override;
 
    private:
     TokenizedBuffer* buffer_;

+ 37 - 44
toolchain/lexer/tokenized_buffer_test.cpp

@@ -24,6 +24,7 @@
 namespace Carbon::Testing {
 namespace {
 
+using ::testing::_;
 using ::testing::ElementsAre;
 using ::testing::Eq;
 using ::testing::HasSubstr;
@@ -934,25 +935,26 @@ TEST_F(LexerTest, TypeLiterals) {
 
 TEST_F(LexerTest, TypeLiteralTooManyDigits) {
   std::string code = "i";
-  code.append(10000, '9');
+  constexpr int Count = 10000;
+  code.append(Count, '9');
 
   Testing::MockDiagnosticConsumer consumer;
-  EXPECT_CALL(
-      consumer,
-      HandleDiagnostic(AllOf(
-          DiagnosticAt(1, 2),
-          DiagnosticMessage(HasSubstr("Found a sequence of 10000 digits")))));
+  EXPECT_CALL(consumer,
+              HandleDiagnostic(IsDiagnostic(
+                  DiagnosticKind::TooManyDigits, DiagnosticLevel::Error, 1, 2,
+                  HasSubstr(llvm::formatv(" {0} ", Count)))));
   auto buffer = Lex(code, consumer);
   EXPECT_TRUE(buffer.has_errors());
-  ASSERT_THAT(buffer,
-              HasTokens(llvm::ArrayRef<ExpectedToken>{
-                  {.kind = TokenKind::Error(),
-                   .line = 1,
-                   .column = 1,
-                   .indent_column = 1,
-                   .text = {code}},
-                  {.kind = TokenKind::EndOfFile(), .line = 1, .column = 10002},
-              }));
+  ASSERT_THAT(
+      buffer,
+      HasTokens(llvm::ArrayRef<ExpectedToken>{
+          {.kind = TokenKind::Error(),
+           .line = 1,
+           .column = 1,
+           .indent_column = 1,
+           .text = {code}},
+          {.kind = TokenKind::EndOfFile(), .line = 1, .column = Count + 2},
+      }));
 }
 
 TEST_F(LexerTest, DiagnosticTrailingComment) {
@@ -962,66 +964,57 @@ TEST_F(LexerTest, DiagnosticTrailingComment) {
   )";
 
   Testing::MockDiagnosticConsumer consumer;
-  EXPECT_CALL(consumer, HandleDiagnostic(AllOf(
-                            DiagnosticAt(3, 19),
-                            DiagnosticMessage(HasSubstr("Trailing comment")))));
+  EXPECT_CALL(consumer,
+              HandleDiagnostic(IsDiagnostic(DiagnosticKind::TrailingComment,
+                                            DiagnosticLevel::Error, 3, 19, _)));
   Lex(testcase, consumer);
 }
 
 TEST_F(LexerTest, DiagnosticWhitespace) {
   Testing::MockDiagnosticConsumer consumer;
-  EXPECT_CALL(consumer,
-              HandleDiagnostic(AllOf(
-                  DiagnosticAt(1, 3),
-                  DiagnosticMessage(HasSubstr("Whitespace is required")))));
+  EXPECT_CALL(consumer, HandleDiagnostic(IsDiagnostic(
+                            DiagnosticKind::NoWhitespaceAfterCommentIntroducer,
+                            DiagnosticLevel::Error, 1, 3, _)));
   Lex("//no space after comment", consumer);
 }
 
 TEST_F(LexerTest, DiagnosticUnrecognizedEscape) {
   Testing::MockDiagnosticConsumer consumer;
-  EXPECT_CALL(
-      consumer,
-      HandleDiagnostic(AllOf(
-          DiagnosticAt(1, 8),
-          DiagnosticMessage(HasSubstr("Unrecognized escape sequence `b`")))));
+  EXPECT_CALL(consumer, HandleDiagnostic(IsDiagnostic(
+                            DiagnosticKind::UnknownEscapeSequence,
+                            DiagnosticLevel::Error, 1, 8, HasSubstr("`b`"))));
   Lex(R"("hello\bworld")", consumer);
 }
 
 TEST_F(LexerTest, DiagnosticBadHex) {
   Testing::MockDiagnosticConsumer consumer;
-  EXPECT_CALL(
-      consumer,
-      HandleDiagnostic(AllOf(
-          DiagnosticAt(1, 9),
-          DiagnosticMessage(HasSubstr("two uppercase hexadecimal digits")))));
+  EXPECT_CALL(consumer, HandleDiagnostic(IsDiagnostic(
+                            DiagnosticKind::HexadecimalEscapeMissingDigits,
+                            DiagnosticLevel::Error, 1, 9, _)));
   Lex(R"("hello\xabworld")", consumer);
 }
 
 TEST_F(LexerTest, DiagnosticInvalidDigit) {
   Testing::MockDiagnosticConsumer consumer;
-  EXPECT_CALL(
-      consumer,
-      HandleDiagnostic(AllOf(
-          DiagnosticAt(1, 6),
-          DiagnosticMessage(HasSubstr("Invalid digit 'a' in hexadecimal")))));
+  EXPECT_CALL(consumer, HandleDiagnostic(IsDiagnostic(
+                            DiagnosticKind::InvalidDigit,
+                            DiagnosticLevel::Error, 1, 6, HasSubstr("'a'"))));
   Lex("0x123abc", consumer);
 }
 
 TEST_F(LexerTest, DiagnosticMissingTerminator) {
   Testing::MockDiagnosticConsumer consumer;
   EXPECT_CALL(consumer,
-              HandleDiagnostic(
-                  AllOf(DiagnosticAt(1, 1),
-                        DiagnosticMessage(HasSubstr("missing a terminator")))));
+              HandleDiagnostic(IsDiagnostic(DiagnosticKind::UnterminatedString,
+                                            DiagnosticLevel::Error, 1, 1, _)));
   Lex(R"(#" ")", consumer);
 }
 
 TEST_F(LexerTest, DiagnosticUnrecognizedChar) {
   Testing::MockDiagnosticConsumer consumer;
-  EXPECT_CALL(consumer,
-              HandleDiagnostic(AllOf(
-                  DiagnosticAt(1, 1),
-                  DiagnosticMessage(HasSubstr("unrecognized character")))));
+  EXPECT_CALL(consumer, HandleDiagnostic(
+                            IsDiagnostic(DiagnosticKind::UnrecognizedCharacters,
+                                         DiagnosticLevel::Error, 1, 1, _)));
   Lex("\b", consumer);
 }
 

+ 22 - 21
toolchain/parser/parse_tree_test.cpp

@@ -1120,36 +1120,37 @@ TEST_F(ParseTreeTest, StructErrors) {
   };
   Testcase testcases[] = {
       {"var x: {i32} = {};",
-       DiagnosticMessage("Expected `.field: type` or `.field = value`.")},
+       IsDiagnosticMessage("Expected `.field: type` or `.field = value`.")},
       {"var x: {a} = {};",
-       DiagnosticMessage("Expected `.field: type` or `.field = value`.")},
+       IsDiagnosticMessage("Expected `.field: type` or `.field = value`.")},
       {"var x: {a:} = {};",
-       DiagnosticMessage("Expected `.field: type` or `.field = value`.")},
+       IsDiagnosticMessage("Expected `.field: type` or `.field = value`.")},
       {"var x: {a=} = {};",
-       DiagnosticMessage("Expected `.field: type` or `.field = value`.")},
-      {"var x: {.} = {};", DiagnosticMessage("Expected identifier after `.`.")},
+       IsDiagnosticMessage("Expected `.field: type` or `.field = value`.")},
+      {"var x: {.} = {};",
+       IsDiagnosticMessage("Expected identifier after `.`.")},
       {"var x: {.\"hello\" = 0, .y = 4} = {};",
-       DiagnosticMessage("Expected identifier after `.`.")},
+       IsDiagnosticMessage("Expected identifier after `.`.")},
       {"var x: {.\"hello\": i32, .y: i32} = {};",
-       DiagnosticMessage("Expected identifier after `.`.")},
+       IsDiagnosticMessage("Expected identifier after `.`.")},
       {"var x: {.a} = {};",
-       DiagnosticMessage("Expected `.field: type` or `.field = value`.")},
-      {"var x: {.a:} = {};", DiagnosticMessage("Expected expression.")},
-      {"var x: {.a=} = {};", DiagnosticMessage("Expected expression.")},
+       IsDiagnosticMessage("Expected `.field: type` or `.field = value`.")},
+      {"var x: {.a:} = {};", IsDiagnosticMessage("Expected expression.")},
+      {"var x: {.a=} = {};", IsDiagnosticMessage("Expected expression.")},
       {"var x: {.a: i32, .b = 0} = {};",
-       DiagnosticMessage("Expected `.field: type`.")},
+       IsDiagnosticMessage("Expected `.field: type`.")},
       {"var x: {.a = 0, b: i32} = {};",
-       DiagnosticMessage("Expected `.field = value`.")},
+       IsDiagnosticMessage("Expected `.field = value`.")},
       {"var x: {,} = {};",
-       DiagnosticMessage("Expected `.field: type` or `.field = value`.")},
+       IsDiagnosticMessage("Expected `.field: type` or `.field = value`.")},
       {"var x: {.a: i32,,} = {};",
-       DiagnosticMessage("Expected `.field: type`.")},
+       IsDiagnosticMessage("Expected `.field: type`.")},
       {"var x: {.a = 0,,} = {};",
-       DiagnosticMessage("Expected `.field = value`.")},
+       IsDiagnosticMessage("Expected `.field = value`.")},
       {"var x: {.a: i32 banana} = {.a = 0};",
-       DiagnosticMessage("Expected `,` or `}`.")},
+       IsDiagnosticMessage("Expected `,` or `}`.")},
       {"var x: {.a: i32} = {.a = 0 banana};",
-       DiagnosticMessage("Expected `,` or `}`.")},
+       IsDiagnosticMessage("Expected `,` or `}`.")},
   };
 
   for (const Testcase& testcase : testcases) {
@@ -1268,10 +1269,10 @@ TEST_F(ParseTreeTest, RecursionLimit) {
   // Recursion might be exceeded multiple times due to quirks in parse tree
   // handling; we only need to be sure it's hit at least once for test
   // correctness.
-  EXPECT_CALL(
-      consumer,
-      HandleDiagnostic(DiagnosticMessage(llvm::formatv(
-          "Exceeded recursion limit ({0})", ParseTree::StackDepthLimit))))
+  EXPECT_CALL(consumer, HandleDiagnostic(IsDiagnosticMessage(
+                            llvm::formatv("Exceeded recursion limit ({0})",
+                                          ParseTree::StackDepthLimit)
+                                .str())))
       .Times(AtLeast(1));
   ParseTree tree = ParseTree::Parse(tokens, consumer);
   EXPECT_TRUE(tree.has_errors());

+ 99 - 218
toolchain/parser/parser_impl.cpp

@@ -17,14 +17,8 @@
 
 namespace Carbon {
 
-struct StackLimitExceeded : DiagnosticBase<StackLimitExceeded> {
-  static constexpr llvm::StringLiteral ShortName = "syntax-error";
-
-  static auto Format() -> std::string {
-    return llvm::formatv("Exceeded recursion limit ({0})",
-                         ParseTree::StackDepthLimit);
-  }
-};
+CARBON_DIAGNOSTIC(StackLimitExceeded, Error, "Exceeded recursion limit ({0})",
+                  int);
 
 // Manages the parser's stack depth, particularly decrementing on destruction.
 // This should only be instantiated through RETURN_IF_STACK_LIMITED.
@@ -37,7 +31,8 @@ class ParseTree::Parser::ScopedStackStep {
 
   auto VerifyUnderLimit() -> bool {
     if (parser_->stack_depth_ >= StackDepthLimit) {
-      parser_->emitter_.EmitError<StackLimitExceeded>(*parser_->position_);
+      parser_->emitter_.Emit(*parser_->position_, StackLimitExceeded,
+                             ParseTree::StackDepthLimit);
       return false;
     }
     return true;
@@ -55,187 +50,69 @@ class ParseTree::Parser::ScopedStackStep {
     return (error_return_expr);                    \
   }
 
-struct UnexpectedTokenInCodeBlock : DiagnosticBase<UnexpectedTokenInCodeBlock> {
-  static constexpr llvm::StringLiteral ShortName = "syntax-error";
-  static constexpr llvm::StringLiteral Message =
-      "Unexpected token in code block.";
-};
-
-struct ExpectedFunctionName : DiagnosticBase<ExpectedFunctionName> {
-  static constexpr llvm::StringLiteral ShortName = "syntax-error";
-  static constexpr llvm::StringLiteral Message =
-      "Expected function name after `fn` keyword.";
-};
-
-struct ExpectedFunctionParams : DiagnosticBase<ExpectedFunctionParams> {
-  static constexpr llvm::StringLiteral ShortName = "syntax-error";
-  static constexpr llvm::StringLiteral Message =
-      "Expected `(` after function name.";
-};
-
-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 : DiagnosticBase<ExpectedVariableName> {
-  static constexpr llvm::StringLiteral ShortName = "syntax-error";
-  static constexpr llvm::StringLiteral Message =
-      "Expected pattern in `var` declaration.";
-};
-
-struct ExpectedParameterName : DiagnosticBase<ExpectedParameterName> {
-  static constexpr llvm::StringLiteral ShortName = "syntax-error";
-  static constexpr llvm::StringLiteral Message =
-      "Expected parameter declaration.";
-};
-
-struct ExpectedStructLiteralField : DiagnosticBase<ExpectedStructLiteralField> {
-  static constexpr llvm::StringLiteral ShortName = "syntax-error";
-
-  auto Format() -> std::string {
-    std::string result = "Expected ";
-    if (can_be_type) {
-      result += "`.field: type`";
-    }
-    if (can_be_type && can_be_value) {
-      result += " or ";
-    }
-    if (can_be_value) {
-      result += "`.field = value`";
-    }
-    result += ".";
-    return result;
-  }
-
-  bool can_be_type;
-  bool can_be_value;
+// A relative location for characters in errors.
+enum class RelativeLocation : int8_t {
+  Around,
+  After,
+  Before,
 };
 
-struct UnrecognizedDeclaration : DiagnosticBase<UnrecognizedDeclaration> {
-  static constexpr llvm::StringLiteral ShortName = "syntax-error";
-  static constexpr llvm::StringLiteral Message =
-      "Unrecognized declaration introducer.";
-};
-
-struct ExpectedCodeBlock : DiagnosticBase<ExpectedCodeBlock> {
-  static constexpr llvm::StringLiteral ShortName = "syntax-error";
-  static constexpr llvm::StringLiteral Message = "Expected braced code block.";
-};
-
-struct ExpectedExpression : DiagnosticBase<ExpectedExpression> {
-  static constexpr llvm::StringLiteral ShortName = "syntax-error";
-  static constexpr llvm::StringLiteral Message = "Expected expression.";
-};
-
-struct ExpectedParenAfter : DiagnosticBase<ExpectedParenAfter> {
-  static constexpr llvm::StringLiteral ShortName = "syntax-error";
-  static constexpr const char* Message = "Expected `(` after `{0}`.";
-
-  auto Format() -> std::string {
-    return llvm::formatv(Message, introducer.GetFixedSpelling()).str();
-  }
-
-  TokenKind introducer;
-};
-
-struct ExpectedCloseParen : DiagnosticBase<ExpectedCloseParen> {
-  static constexpr llvm::StringLiteral ShortName = "syntax-error";
-  static constexpr llvm::StringLiteral Message =
-      "Unexpected tokens before `)`.";
-
-  // TODO: Include the location of the matching open paren in the diagnostic.
-  TokenizedBuffer::Token open_paren;
-};
-
-struct ExpectedSemiAfterExpression
-    : DiagnosticBase<ExpectedSemiAfterExpression> {
-  static constexpr llvm::StringLiteral ShortName = "syntax-error";
-  static constexpr llvm::StringLiteral Message =
-      "Expected `;` after expression.";
-};
-
-struct ExpectedSemiAfter : DiagnosticBase<ExpectedSemiAfter> {
-  static constexpr llvm::StringLiteral ShortName = "syntax-error";
-  static constexpr const char* Message = "Expected `;` after `{0}`.";
-
-  auto Format() -> std::string {
-    return llvm::formatv(Message, preceding.GetFixedSpelling()).str();
-  }
-
-  TokenKind preceding;
-};
-
-struct ExpectedIdentifierAfterDot : DiagnosticBase<ExpectedIdentifierAfterDot> {
-  static constexpr llvm::StringLiteral ShortName = "syntax-error";
-  static constexpr llvm::StringLiteral Message =
-      "Expected identifier after `.`.";
-};
-
-struct UnexpectedTokenAfterListElement
-    : DiagnosticBase<UnexpectedTokenAfterListElement> {
-  static constexpr llvm::StringLiteral ShortName = "syntax-error";
-  static constexpr const char* Message = "Expected `,` or `{0}`.";
-
-  auto Format() -> std::string {
-    return llvm::formatv(Message, close.GetFixedSpelling()).str();
-  }
-
-  TokenKind close;
-};
-
-struct BinaryOperatorRequiresWhitespace
-    : DiagnosticBase<BinaryOperatorRequiresWhitespace> {
-  static constexpr llvm::StringLiteral ShortName = "syntax-error";
-  static constexpr const char* Message =
-      "Whitespace missing {0} binary operator.";
-
-  auto Format() -> std::string {
-    const char* position = "around";
-    if (has_leading_space) {
-      position = "after";
-    } else if (has_trailing_space) {
-      position = "before";
-    }
-    return llvm::formatv(Message, position);
-  }
-
-  bool has_leading_space;
-  bool has_trailing_space;
-};
-
-struct UnaryOperatorHasWhitespace : DiagnosticBase<UnaryOperatorHasWhitespace> {
-  static constexpr llvm::StringLiteral ShortName = "syntax-error";
-  static constexpr const char* Message =
-      "Whitespace is not allowed {0} this unary operator.";
-
-  auto Format() -> std::string {
-    return llvm::formatv(Message, prefix ? "after" : "before");
-  }
-
-  bool prefix;
-};
-
-struct UnaryOperatorRequiresWhitespace
-    : DiagnosticBase<UnaryOperatorRequiresWhitespace> {
-  static constexpr llvm::StringLiteral ShortName = "syntax-error";
-  static constexpr const char* Message =
-      "Whitespace is required {0} this unary operator.";
-
-  auto Format() -> std::string {
-    return llvm::formatv(Message, prefix ? "before" : "after");
+// Adapts RelativeLocation for use with formatv.
+static auto operator<<(llvm::raw_ostream& out, RelativeLocation loc)
+    -> llvm::raw_ostream& {
+  switch (loc) {
+    case RelativeLocation::Around:
+      out << "around";
+      break;
+    case RelativeLocation::After:
+      out << "after";
+      break;
+    case RelativeLocation::Before:
+      out << "before";
+      break;
   }
+  return out;
+}
 
-  bool prefix;
-};
-
-struct OperatorRequiresParentheses
-    : DiagnosticBase<OperatorRequiresParentheses> {
-  static constexpr llvm::StringLiteral ShortName = "syntax-error";
-  static constexpr llvm::StringLiteral Message =
-      "Parentheses are required to disambiguate operator precedence.";
-};
+CARBON_DIAGNOSTIC(ExpectedFunctionName, Error,
+                  "Expected function name after `fn` keyword.");
+CARBON_DIAGNOSTIC(ExpectedFunctionParams, Error,
+                  "Expected `(` after function name.");
+CARBON_DIAGNOSTIC(
+    ExpectedFunctionBodyOrSemi, Error,
+    "Expected function definition or `;` after function declaration.");
+CARBON_DIAGNOSTIC(ExpectedVariableName, Error,
+                  "Expected pattern in `var` declaration.");
+CARBON_DIAGNOSTIC(ExpectedParameterName, Error,
+                  "Expected parameter declaration.");
+CARBON_DIAGNOSTIC(ExpectedStructLiteralField, Error, "Expected {0}{1}{2}.",
+                  llvm::StringRef, llvm::StringRef, llvm::StringRef);
+CARBON_DIAGNOSTIC(UnrecognizedDeclaration, Error,
+                  "Unrecognized declaration introducer.");
+CARBON_DIAGNOSTIC(ExpectedCodeBlock, Error, "Expected braced code block.");
+CARBON_DIAGNOSTIC(ExpectedExpression, Error, "Expected expression.");
+CARBON_DIAGNOSTIC(ExpectedParenAfter, Error, "Expected `(` after `{0}`.",
+                  TokenKind);
+CARBON_DIAGNOSTIC(ExpectedCloseParen, Error, "Unexpected tokens before `)`.");
+CARBON_DIAGNOSTIC(ExpectedSemiAfterExpression, Error,
+                  "Expected `;` after expression.");
+CARBON_DIAGNOSTIC(ExpectedSemiAfter, Error, "Expected `;` after `{0}`.",
+                  TokenKind);
+CARBON_DIAGNOSTIC(ExpectedIdentifierAfterDot, Error,
+                  "Expected identifier after `.`.");
+CARBON_DIAGNOSTIC(UnexpectedTokenAfterListElement, Error,
+                  "Expected `,` or `{0}`.", TokenKind);
+CARBON_DIAGNOSTIC(BinaryOperatorRequiresWhitespace, Error,
+                  "Whitespace missing {0} binary operator.", RelativeLocation);
+CARBON_DIAGNOSTIC(UnaryOperatorHasWhitespace, Error,
+                  "Whitespace is not allowed {0} this unary operator.",
+                  RelativeLocation);
+CARBON_DIAGNOSTIC(UnaryOperatorRequiresWhitespace, Error,
+                  "Whitespace is required {0} this unary operator.",
+                  RelativeLocation);
+CARBON_DIAGNOSTIC(
+    OperatorRequiresParentheses, Error,
+    "Parentheses are required to disambiguate operator precedence.");
 
 ParseTree::Parser::Parser(ParseTree& tree_arg, TokenizedBuffer& tokens_arg,
                           TokenDiagnosticEmitter& emitter)
@@ -444,8 +321,8 @@ auto ParseTree::Parser::ParseCloseParen(TokenizedBuffer::Token open_paren,
     return close_paren;
   }
 
-  emitter_.EmitError<ExpectedCloseParen>(*position_,
-                                         {.open_paren = open_paren});
+  // TODO: Include the location of the matching open_paren in the diagnostic.
+  emitter_.Emit(*position_, ExpectedCloseParen);
   SkipTo(tokens_.GetMatchedClosingToken(open_paren));
   AddLeafNode(kind, Consume(TokenKind::CloseParen()));
   return llvm::None;
@@ -477,8 +354,7 @@ auto ParseTree::Parser::ParseList(TokenKind open, TokenKind close,
 
       if (!NextTokenIsOneOf({close, TokenKind::Comma()})) {
         if (!element_error) {
-          emitter_.EmitError<UnexpectedTokenAfterListElement>(*position_,
-                                                              {.close = close});
+          emitter_.Emit(*position_, UnexpectedTokenAfterListElement, close);
         }
         has_errors = true;
 
@@ -521,11 +397,11 @@ auto ParseTree::Parser::ParsePattern(PatternKind kind) -> llvm::Optional<Node> {
 
   switch (kind) {
     case PatternKind::Parameter:
-      emitter_.EmitError<ExpectedParameterName>(*position_);
+      emitter_.Emit(*position_, ExpectedParameterName);
       break;
 
     case PatternKind::Variable:
-      emitter_.EmitError<ExpectedVariableName>(*position_);
+      emitter_.Emit(*position_, ExpectedVariableName);
       break;
   }
 
@@ -570,7 +446,7 @@ auto ParseTree::Parser::ParseCodeBlock() -> llvm::Optional<Node> {
       ConsumeIf(TokenKind::OpenCurlyBrace());
   if (!maybe_open_curly) {
     // Recover by parsing a single statement.
-    emitter_.EmitError<ExpectedCodeBlock>(*position_);
+    emitter_.Emit(*position_, ExpectedCodeBlock);
     return ParseStatement();
   }
   TokenizedBuffer::Token open_curly = *maybe_open_curly;
@@ -617,7 +493,7 @@ auto ParseTree::Parser::ParseFunctionDeclaration() -> Node {
   auto name_n = ConsumeAndAddLeafNodeIf(TokenKind::Identifier(),
                                         ParseNodeKind::DeclaredName());
   if (!name_n) {
-    emitter_.EmitError<ExpectedFunctionName>(*position_);
+    emitter_.Emit(*position_, ExpectedFunctionName);
     // FIXME: We could change the lexer to allow us to synthesize certain
     // kinds of tokens and try to "recover" here, but unclear that this is
     // really useful.
@@ -627,7 +503,7 @@ auto ParseTree::Parser::ParseFunctionDeclaration() -> Node {
 
   TokenizedBuffer::Token open_paren = *position_;
   if (tokens_.GetKind(open_paren) != TokenKind::OpenParen()) {
-    emitter_.EmitError<ExpectedFunctionParams>(open_paren);
+    emitter_.Emit(open_paren, ExpectedFunctionParams);
     SkipPastLikelyEnd(function_intro_token, handle_semi_in_error_recovery);
     return add_error_function_node();
   }
@@ -648,7 +524,7 @@ auto ParseTree::Parser::ParseFunctionDeclaration() -> Node {
     }
   } else if (!ConsumeAndAddLeafNodeIf(TokenKind::Semi(),
                                       ParseNodeKind::DeclarationEnd())) {
-    emitter_.EmitError<ExpectedFunctionBodyOrSemi>(*position_);
+    emitter_.Emit(*position_, ExpectedFunctionBodyOrSemi);
     if (tokens_.GetLine(*position_) == tokens_.GetLine(close_paren)) {
       // Only need to skip if we've not already found a new line.
       SkipPastLikelyEnd(function_intro_token, handle_semi_in_error_recovery);
@@ -688,7 +564,7 @@ auto ParseTree::Parser::ParseVariableDeclaration() -> Node {
   auto semi = ConsumeAndAddLeafNodeIf(TokenKind::Semi(),
                                       ParseNodeKind::DeclarationEnd());
   if (!semi) {
-    emitter_.EmitError<ExpectedSemiAfterExpression>(*position_);
+    emitter_.Emit(*position_, ExpectedSemiAfterExpression);
     SkipPastLikelyEnd(var_token, [&](TokenizedBuffer::Token semi) {
       return AddLeafNode(ParseNodeKind::DeclarationEnd(), semi);
     });
@@ -720,7 +596,7 @@ auto ParseTree::Parser::ParseDeclaration() -> llvm::Optional<Node> {
   }
 
   // We didn't recognize an introducer for a valid declaration.
-  emitter_.EmitError<UnrecognizedDeclaration>(*position_);
+  emitter_.Emit(*position_, UnrecognizedDeclaration);
 
   // Skip forward past any end of a declaration we simply didn't understand so
   // that we can find the start of the next declaration or the end of a scope.
@@ -781,9 +657,12 @@ auto ParseTree::Parser::ParseBraceExpression() -> llvm::Optional<Node> {
         auto start_elem = GetSubtreeStartPosition();
 
         auto diagnose_invalid_syntax = [&] {
-          emitter_.EmitError<ExpectedStructLiteralField>(
-              *position_,
-              {.can_be_type = kind != Value, .can_be_value = kind != Type});
+          bool can_be_type = kind != Value;
+          bool can_be_value = kind != Type;
+          emitter_.Emit(*position_, ExpectedStructLiteralField,
+                        can_be_type ? "`.field: type`" : "",
+                        (can_be_type && can_be_value) ? " or " : "",
+                        can_be_value ? "`.field = value`" : "");
           return llvm::None;
         };
 
@@ -857,7 +736,7 @@ auto ParseTree::Parser::ParsePrimaryExpression() -> llvm::Optional<Node> {
       return ParseBraceExpression();
 
     default:
-      emitter_.EmitError<ExpectedExpression>(*position_);
+      emitter_.Emit(*position_, ExpectedExpression);
       return llvm::None;
   }
 
@@ -874,7 +753,7 @@ auto ParseTree::Parser::ParseDesignatorExpression(SubtreeStart start,
   if (name) {
     AddLeafNode(ParseNodeKind::DesignatedName(), *name);
   } else {
-    emitter_.EmitError<ExpectedIdentifierAfterDot>(*position_);
+    emitter_.Emit(*position_, ExpectedIdentifierAfterDot);
     // If we see a keyword, assume it was intended to be the designated name.
     // TODO: Should keywords be valid in designators?
     if (NextTokenKind().IsKeyword()) {
@@ -997,10 +876,12 @@ auto ParseTree::Parser::DiagnoseOperatorFixity(OperatorFixity fixity) -> void {
   if (fixity == OperatorFixity::Infix) {
     // Infix operators must satisfy the infix operator rules.
     if (!is_valid_as_infix) {
-      emitter_.EmitError<BinaryOperatorRequiresWhitespace>(
-          *position_,
-          {.has_leading_space = tokens_.HasLeadingWhitespace(*position_),
-           .has_trailing_space = tokens_.HasTrailingWhitespace(*position_)});
+      emitter_.Emit(*position_, BinaryOperatorRequiresWhitespace,
+                    tokens_.HasLeadingWhitespace(*position_)
+                        ? RelativeLocation::After
+                        : (tokens_.HasTrailingWhitespace(*position_)
+                               ? RelativeLocation::Before
+                               : RelativeLocation::Around));
     }
   } else {
     bool prefix = fixity == OperatorFixity::Prefix;
@@ -1010,13 +891,15 @@ auto ParseTree::Parser::DiagnoseOperatorFixity(OperatorFixity fixity) -> void {
     if (NextTokenKind().IsSymbol() &&
         (prefix ? tokens_.HasTrailingWhitespace(*position_)
                 : tokens_.HasLeadingWhitespace(*position_))) {
-      emitter_.EmitError<UnaryOperatorHasWhitespace>(*position_,
-                                                     {.prefix = prefix});
+      emitter_.Emit(
+          *position_, UnaryOperatorHasWhitespace,
+          prefix ? RelativeLocation::After : RelativeLocation::Before);
     }
     // Pre/postfix operators must not satisfy the infix operator rules.
     if (is_valid_as_infix) {
-      emitter_.EmitError<UnaryOperatorRequiresWhitespace>(*position_,
-                                                          {.prefix = prefix});
+      emitter_.Emit(
+          *position_, UnaryOperatorRequiresWhitespace,
+          prefix ? RelativeLocation::Before : RelativeLocation::After);
     }
   }
 }
@@ -1063,7 +946,7 @@ auto ParseTree::Parser::ParseOperatorExpression(
         OperatorPriority::RightFirst) {
       // The precedence rules don't permit this prefix operator in this
       // context. Diagnose this, but carry on and parse it anyway.
-      emitter_.EmitError<OperatorRequiresParentheses>(*position_);
+      emitter_.Emit(*position_, OperatorRequiresParentheses);
     } else {
       // Check that this operator follows the proper whitespace rules.
       DiagnoseOperatorFixity(OperatorFixity::Prefix);
@@ -1096,7 +979,7 @@ auto ParseTree::Parser::ParseOperatorExpression(
       // Either the LHS operator and this operator are ambiguous, or the
       // LHS operaor is a unary operator that can't be nested within
       // this operator. Either way, parentheses are required.
-      emitter_.EmitError<OperatorRequiresParentheses>(*position_);
+      emitter_.Emit(*position_, OperatorRequiresParentheses);
       lhs = llvm::None;
     } else {
       DiagnoseOperatorFixity(is_binary ? OperatorFixity::Infix
@@ -1142,7 +1025,7 @@ auto ParseTree::Parser::ParseExpressionStatement() -> llvm::Optional<Node> {
   }
 
   if (!has_errors) {
-    emitter_.EmitError<ExpectedSemiAfterExpression>(*position_);
+    emitter_.Emit(*position_, ExpectedSemiAfterExpression);
   }
 
   if (auto recovery_node =
@@ -1164,8 +1047,7 @@ auto ParseTree::Parser::ParseParenCondition(TokenKind introducer)
   auto start = GetSubtreeStartPosition();
   auto open_paren = ConsumeIf(TokenKind::OpenParen());
   if (!open_paren) {
-    emitter_.EmitError<ExpectedParenAfter>(*position_,
-                                           {.introducer = introducer});
+    emitter_.Emit(*position_, ExpectedParenAfter, introducer);
   }
 
   auto expr = ParseExpression();
@@ -1231,8 +1113,7 @@ auto ParseTree::Parser::ParseKeywordStatement(ParseNodeKind kind,
   auto semi =
       ConsumeAndAddLeafNodeIf(TokenKind::Semi(), ParseNodeKind::StatementEnd());
   if (!semi) {
-    emitter_.EmitError<ExpectedSemiAfter>(*position_,
-                                          {.preceding = keyword_kind});
+    emitter_.Emit(*position_, ExpectedSemiAfter, keyword_kind);
     // FIXME: Try to skip to a semicolon to recover.
   }
   return AddNode(kind, keyword, start, /*has_error=*/!semi || arg_error);