Просмотр исходного кода

Merge type of diagnostic and its substitutions. (#366)

Richard Smith 5 лет назад
Родитель
Сommit
62ae0e08cf
3 измененных файлов с 44 добавлено и 59 удалено
  1. 11 16
      diagnostics/diagnostic_emitter.h
  2. 16 20
      diagnostics/diagnostic_emitter_test.cpp
  3. 17 23
      lexer/numeric_literal.cpp

+ 11 - 16
diagnostics/diagnostic_emitter.h

@@ -37,30 +37,27 @@ class DiagnosticEmitter {
       : callback_(std::move(callback)) {}
   ~DiagnosticEmitter() = default;
 
-  // Emits an error unconditionally after applying the provided `substitutions`.
+  // Emits an error unconditionally.
   template <typename DiagnosticT>
-  void EmitError(typename DiagnosticT::Substitutions substitutions) {
-    callback_({.short_name = DiagnosticT::ShortName,
-               .message = DiagnosticT::Format(substitutions)});
+  auto EmitError(DiagnosticT diag) -> void {
+    callback_({.short_name = DiagnosticT::ShortName, .message = diag.Format()});
   }
 
-  // Emits an error unconditionally when there are no substitutions.
+  // Emits a stateless error unconditionally.
   template <typename DiagnosticT>
-  std::enable_if_t<std::is_empty_v<typename DiagnosticT::Substitutions>>
-  EmitError() {
+  auto EmitError() -> std::enable_if_t<std::is_empty_v<DiagnosticT>> {
     EmitError<DiagnosticT>({});
   }
 
   // Emits a warning if `F` returns true.  `F` may or may not be called if the
   // warning is disabled.
   template <typename DiagnosticT>
-  void EmitWarningIf(
-      llvm::function_ref<bool(typename DiagnosticT::Substitutions&)> f) {
+  auto EmitWarningIf(llvm::function_ref<bool(DiagnosticT&)> f) -> void {
     // TODO(kfm): check if this warning is enabled
-    typename DiagnosticT::Substitutions substitutions;
-    if (f(substitutions)) {
-      callback_({.short_name = DiagnosticT::ShortName,
-                 .message = DiagnosticT::Format(substitutions)});
+    DiagnosticT diag;
+    if (f(diag)) {
+      callback_(
+          {.short_name = DiagnosticT::ShortName, .message = diag.Format()});
     }
   }
 
@@ -83,9 +80,7 @@ inline auto NullDiagnosticEmitter() -> DiagnosticEmitter& {
 template <typename Derived>
 struct SimpleDiagnostic {
   struct Substitutions {};
-  static auto Format(const Substitutions&) -> std::string {
-    return Derived::Message.str();
-  }
+  static auto Format() -> std::string { return Derived::Message.str(); }
 };
 
 }  // namespace Carbon

+ 16 - 20
diagnostics/diagnostic_emitter_test.cpp

@@ -21,15 +21,14 @@ struct FakeDiagnostic {
   // selection of the message.
   static constexpr llvm::StringLiteral Message = "{0}";
 
-  struct Substitutions {
-    std::string message;
-  };
-  static auto Format(const Substitutions& substitutions) -> std::string {
+  std::string message;
+
+  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(), substitutions.message).str();
+    return llvm::formatv(Message.data(), message).str();
   }
 };
 
@@ -55,21 +54,18 @@ TEST(DiagTest, EmitWarnings) {
     reported.push_back(diagnostic.message);
   });
 
-  emitter.EmitWarningIf<FakeDiagnostic>(
-      [](FakeDiagnostic::Substitutions& diagnostic) {
-        diagnostic.message = "M1";
-        return true;
-      });
-  emitter.EmitWarningIf<FakeDiagnostic>(
-      [](FakeDiagnostic::Substitutions& diagnostic) {
-        diagnostic.message = "M2";
-        return false;
-      });
-  emitter.EmitWarningIf<FakeDiagnostic>(
-      [](FakeDiagnostic::Substitutions& diagnostic) {
-        diagnostic.message = "M3";
-        return true;
-      });
+  emitter.EmitWarningIf<FakeDiagnostic>([](FakeDiagnostic& diagnostic) {
+    diagnostic.message = "M1";
+    return true;
+  });
+  emitter.EmitWarningIf<FakeDiagnostic>([](FakeDiagnostic& diagnostic) {
+    diagnostic.message = "M2";
+    return false;
+  });
+  emitter.EmitWarningIf<FakeDiagnostic>([](FakeDiagnostic& diagnostic) {
+    diagnostic.message = "M3";
+    return true;
+  });
 
   EXPECT_THAT(reported, ElementsAre("M1", "M3"));
 }

+ 17 - 23
lexer/numeric_literal.cpp

@@ -21,16 +21,13 @@ struct EmptyDigitSequence : SimpleDiagnostic<EmptyDigitSequence> {
 struct InvalidDigit {
   static constexpr llvm::StringLiteral ShortName = "syntax-invalid-number";
 
-  struct Substitutions {
-    char digit;
-    int radix;
-  };
-  static auto Format(const Substitutions& subst) -> std::string {
-    return llvm::formatv("Invalid digit '{0}' in {1} numeric literal.",
-                         subst.digit,
-                         (subst.radix == 2    ? "binary"
-                          : subst.radix == 16 ? "hexadecimal"
-                                              : "decimal"))
+  char digit;
+  int radix;
+
+  auto Format() -> std::string {
+    return llvm::formatv("Invalid digit '{0}' in {1} numeric literal.", digit,
+                         (radix == 2 ? "binary"
+                                     : radix == 16 ? "hexadecimal" : "decimal"))
         .str();
   }
 };
@@ -45,16 +42,15 @@ struct IrregularDigitSeparators {
   static constexpr llvm::StringLiteral ShortName =
       "syntax-irregular-digit-separators";
 
-  struct Substitutions {
-    int radix;
-  };
-  static auto Format(const Substitutions& subst) -> std::string {
-    assert((subst.radix == 10 || subst.radix == 16) && "unexpected radix");
+  int radix;
+
+  auto Format() -> std::string {
+    assert((radix == 10 || radix == 16) && "unexpected radix");
     return llvm::formatv(
                "Digit separators in {0} number should appear every {1} "
                "characters from the right.",
-               (subst.radix == 10 ? "decimal" : "hexadecimal"),
-               (subst.radix == 10 ? "3" : "4"))
+               (radix == 10 ? "decimal" : "hexadecimal"),
+               (radix == 10 ? "3" : "4"))
         .str();
   }
 };
@@ -74,12 +70,10 @@ struct BinaryRealLiteral : SimpleDiagnostic<BinaryRealLiteral> {
 struct WrongRealLiteralExponent {
   static constexpr llvm::StringLiteral ShortName = "syntax-invalid-number";
 
-  struct Substitutions {
-    char expected;
-  };
-  static auto Format(const Substitutions& subst) -> std::string {
-    return llvm::formatv("Expected '{0}' to introduce exponent.",
-                         subst.expected)
+  char expected;
+
+  auto Format() -> std::string {
+    return llvm::formatv("Expected '{0}' to introduce exponent.", expected)
         .str();
   }
 };