Switch from manually tracking whether we've emitted any errors to asking the diagnostics machinery. (#406)

diagnostics/diagnostic_emitter.h

@@ -23,6 +23,13 @@ namespace Carbon {
 // 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,
+  };
+
   struct Location {
     // Name of the file or buffer that this diagnostic refers to.
     std::string file_name;
@@ -32,6 +39,7 @@ struct Diagnostic {
     int32_t column_number;
   };
 
+  Level level;
   Location location;
   llvm::StringRef short_name;
   std::string message;
@@ -84,9 +92,10 @@ class DiagnosticEmitter {
   auto EmitError(LocationT location, DiagnosticT diag) -> void {
     // TODO: Encode the diagnostic kind in the Diagnostic object rather than
     // hardcoding an "error: " prefix.
-    consumer_->HandleDiagnostic({.location = translator_->GetLocation(location),
+    consumer_->HandleDiagnostic({.level = Diagnostic::Error,
+                                 .location = translator_->GetLocation(location),
                                  .short_name = DiagnosticT::ShortName,
-                                 .message = "error: " + diag.Format()});
+                                 .message = diag.Format()});
   }
 
   // Emits a stateless error unconditionally.
@@ -107,9 +116,10 @@ class DiagnosticEmitter {
       // TODO: Encode the diagnostic kind in the Diagnostic object rather than
       // hardcoding a "warning: " prefix.
       consumer_->HandleDiagnostic(
-          {.location = translator_->GetLocation(location),
+          {.level = Diagnostic::Warning,
+           .location = translator_->GetLocation(location),
            .short_name = DiagnosticT::ShortName,
-           .message = "warning: " + diag.Format()});
+           .message = diag.Format()});
     }
   }
 
@@ -139,6 +149,29 @@ 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 {
+ public:
+  ErrorTrackingDiagnosticConsumer(DiagnosticConsumer& next_consumer)
+      : next_consumer(&next_consumer) {}
+
+  auto HandleDiagnostic(const Diagnostic& diagnostic) -> void override {
+    seen_error |= diagnostic.level == Diagnostic::Error;
+    next_consumer->HandleDiagnostic(diagnostic);
+  }
+
+  // Returns whether we've seen an error since the last reset.
+  auto SeenError() const -> bool { return seen_error; }
+
+  // Reset whether we've seen an error.
+  auto Reset() -> void { seen_error = false; }
+
+ private:
+  DiagnosticConsumer* next_consumer;
+  bool seen_error = false;
+};
+
 }  // namespace Carbon
 
 #endif  // DIAGNOSTICS_DIAGNOSTICEMITTER_H_

diagnostics/diagnostic_emitter_test.cpp

@@ -14,6 +14,7 @@ namespace Carbon {
 namespace {
 
 using Testing::DiagnosticAt;
+using Testing::DiagnosticLevel;
 using Testing::DiagnosticMessage;
 using Testing::DiagnosticShortName;
 using ::testing::ElementsAre;
@@ -47,12 +48,14 @@ TEST(DiagTest, EmitErrors) {
   Testing::MockDiagnosticConsumer consumer;
   DiagnosticEmitter<int> emitter(translator, consumer);
 
-  EXPECT_CALL(consumer, HandleDiagnostic(AllOf(
-                            DiagnosticAt(1, 1), DiagnosticMessage("error: M1"),
-                            DiagnosticShortName("fake-diagnostic"))));
-  EXPECT_CALL(consumer, HandleDiagnostic(AllOf(
-                            DiagnosticAt(1, 2), DiagnosticMessage("error: M2"),
-                            DiagnosticShortName("fake-diagnostic"))));
+  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"))));
 
   emitter.EmitError<FakeDiagnostic>(1, {.message = "M1"});
   emitter.EmitError<FakeDiagnostic>(2, {.message = "M2"});
@@ -65,14 +68,14 @@ TEST(DiagTest, EmitWarnings) {
   Testing::MockDiagnosticConsumer consumer;
   DiagnosticEmitter<int> emitter(translator, consumer);
 
-  EXPECT_CALL(consumer,
-              HandleDiagnostic(AllOf(DiagnosticAt(1, 3),
-                                     DiagnosticMessage("warning: M1"),
-                                     DiagnosticShortName("fake-diagnostic"))));
-  EXPECT_CALL(consumer,
-              HandleDiagnostic(AllOf(DiagnosticAt(1, 5),
-                                     DiagnosticMessage("warning: M3"),
-                                     DiagnosticShortName("fake-diagnostic"))));
+  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"))));
 
   emitter.EmitWarningIf<FakeDiagnostic>(3, [](FakeDiagnostic& diagnostic) {
     diagnostic.message = "M1";

diagnostics/mocks.h

@@ -35,6 +35,10 @@ MATCHER_P2(DiagnosticAt, line, column, "") {
   return true;
 }
 
+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,

lexer/numeric_literal.cpp

@@ -166,13 +166,9 @@ LexedNumericLiteral::Parser::Parser(DiagnosticEmitter<const char*>& emitter,
 
 // Check that the numeric literal token is syntactically valid and meaningful,
 // and diagnose if not.
-auto LexedNumericLiteral::Parser::Check() -> CheckResult {
-  if (!CheckLeadingZero() || !CheckIntPart() || !CheckFractionalPart() ||
-      !CheckExponentPart()) {
-    return UnrecoverableError;
-  }
-
-  return recovered_from_error ? RecoverableError : Valid;
+auto LexedNumericLiteral::Parser::Check() -> bool {
+  return CheckLeadingZero() && CheckIntPart() && CheckFractionalPart() &&
+         CheckExponentPart();
 }
 
 // Parse a string that is known to be a valid base-radix integer into an
@@ -280,7 +276,6 @@ auto LexedNumericLiteral::Parser::CheckDigitSequence(
       if (!allow_digit_separators || i == 0 || text[i - 1] == '_' ||
           i + 1 == n) {
         emitter.EmitError<InvalidDigitSeparator>(text.begin() + i);
-        recovered_from_error = true;
       }
       ++num_digit_separators;
       continue;
@@ -322,7 +317,6 @@ auto LexedNumericLiteral::Parser::CheckDigitSeparatorPlacement(
 
   auto diagnose_irregular_digit_separators = [&]() {
     emitter.EmitError<IrregularDigitSeparators>(text.begin(), {.radix = radix});
-    recovered_from_error = true;
   };
 
   // For decimal and hexadecimal digit sequences, digit separators must form
@@ -372,7 +366,6 @@ auto LexedNumericLiteral::Parser::CheckFractionalPart() -> bool {
   if (radix == 2) {
     emitter.EmitError<BinaryRealLiteral>(literal.text.begin() +
                                          literal.radix_point);
-    recovered_from_error = true;
     // Carry on and parse the binary real literal anyway.
   }
 

lexer/numeric_literal.h

@@ -57,19 +57,11 @@ class LexedNumericLiteral::Parser {
     return literal.radix_point == static_cast<int>(literal.text.size());
   }
 
-  enum CheckResult {
-    // The token is valid.
-    Valid,
-    // The token is invalid, but we've diagnosed and recovered from the error.
-    RecoverableError,
-    // The token is invalid, and we've diagnosed, but we can't assign meaning
-    // to it.
-    UnrecoverableError,
-  };
-
   // Check that the numeric literal token is syntactically valid and
-  // meaningful, and diagnose if not.
-  auto Check() -> CheckResult;
+  // meaningful, and diagnose if not. Returns `true` if the token was
+  // sufficiently valid that we could determine its meaning. If `false` is
+  // returned, a diagnostic has already been issued.
+  auto Check() -> bool;
 
   // Get the radix of this token. One of 2, 10, or 16.
   auto GetRadix() -> int { return radix; }
@@ -119,9 +111,6 @@ class LexedNumericLiteral::Parser {
 
   // True if we found a `-` before `exponent_part`.
   bool exponent_is_negative = false;
-
-  // True if we produced an error but recovered.
-  bool recovered_from_error = false;
 };
 
 }  // namespace Carbon

lexer/numeric_literal_fuzzer.cpp

@@ -24,7 +24,7 @@ extern "C" int LLVMFuzzerTestOneInput(const unsigned char* data,
 
   LexedNumericLiteral::Parser parser(NullDiagnosticEmitter<const char*>(),
                                      *token);
-  if (parser.Check() == LexedNumericLiteral::Parser::UnrecoverableError) {
+  if (!parser.Check()) {
     // Lexically OK, but token is meaningless.
     return 0;
   }

lexer/numeric_literal_test.cpp

@@ -17,6 +17,9 @@ namespace Carbon {
 namespace {
 
 struct NumericLiteralTest : ::testing::Test {
+  NumericLiteralTest() : error_tracker(ConsoleDiagnosticConsumer()) {}
+
+  ErrorTrackingDiagnosticConsumer error_tracker;
   std::vector<std::unique_ptr<Testing::SingleTokenDiagnosticTranslator>>
       translators;
   std::vector<std::unique_ptr<DiagnosticEmitter<const char*>>> emitters;
@@ -32,7 +35,7 @@ struct NumericLiteralTest : ::testing::Test {
     translators.push_back(
         std::make_unique<Testing::SingleTokenDiagnosticTranslator>(text));
     emitters.push_back(std::make_unique<DiagnosticEmitter<const char*>>(
-        *translators.back(), ConsoleDiagnosticConsumer()));
+        *translators.back(), error_tracker));
     return LexedNumericLiteral::Parser(*emitters.back(), Lex(text));
   }
 };
@@ -50,8 +53,10 @@ TEST_F(NumericLiteralTest, HandlesIntegerLiteral) {
       {.token = "1_234_567", .value = 1'234'567, .radix = 10},
   };
   for (Testcase testcase : testcases) {
+    error_tracker.Reset();
     auto parser = Parse(testcase.token);
-    EXPECT_EQ(parser.Check(), parser.Valid) << testcase.token;
+    EXPECT_TRUE(parser.Check()) << testcase.token;
+    EXPECT_FALSE(error_tracker.SeenError()) << testcase.token;
     EXPECT_EQ(parser.IsInteger(), true);
     EXPECT_EQ(parser.GetMantissa().getZExtValue(), testcase.value);
     EXPECT_EQ(parser.GetExponent().getSExtValue(), 0);
@@ -75,8 +80,10 @@ TEST_F(NumericLiteralTest, ValidatesBaseSpecifier) {
       "0b0000000",
   };
   for (llvm::StringLiteral literal : valid) {
+    error_tracker.Reset();
     auto parser = Parse(literal);
-    EXPECT_EQ(parser.Check(), parser.Valid) << literal;
+    EXPECT_TRUE(parser.Check()) << literal;
+    EXPECT_FALSE(error_tracker.SeenError()) << literal;
   }
 
   llvm::StringLiteral invalid[] = {
@@ -86,8 +93,10 @@ TEST_F(NumericLiteralTest, ValidatesBaseSpecifier) {
       "0x_", "0b_",
   };
   for (llvm::StringLiteral literal : invalid) {
+    error_tracker.Reset();
     auto parser = Parse(literal);
-    EXPECT_EQ(parser.Check(), parser.UnrecoverableError) << literal;
+    EXPECT_FALSE(parser.Check()) << literal;
+    EXPECT_TRUE(error_tracker.SeenError()) << literal;
   }
 }
 
@@ -108,8 +117,10 @@ TEST_F(NumericLiteralTest, ValidatesIntegerDigitSeparators) {
       "0b111_0000",
   };
   for (llvm::StringLiteral literal : valid) {
+    error_tracker.Reset();
     auto parser = Parse(literal);
-    EXPECT_EQ(parser.Check(), parser.Valid) << literal;
+    EXPECT_TRUE(parser.Check()) << literal;
+    EXPECT_FALSE(error_tracker.SeenError()) << literal;
   }
 
   llvm::StringLiteral invalid[] = {
@@ -134,8 +145,10 @@ TEST_F(NumericLiteralTest, ValidatesIntegerDigitSeparators) {
       "0b1_01_01_",
   };
   for (llvm::StringLiteral literal : invalid) {
+    error_tracker.Reset();
     auto parser = Parse(literal);
-    EXPECT_EQ(parser.Check(), parser.RecoverableError) << literal;
+    EXPECT_TRUE(parser.Check()) << literal;
+    EXPECT_TRUE(error_tracker.SeenError()) << literal;
   }
 }
 
@@ -186,10 +199,10 @@ TEST_F(NumericLiteralTest, HandlesRealLiteral) {
        .radix = 2},
   };
   for (Testcase testcase : testcases) {
+    error_tracker.Reset();
     auto parser = Parse(testcase.token);
-    EXPECT_EQ(parser.Check(),
-              testcase.radix == 2 ? parser.RecoverableError : parser.Valid)
-        << testcase.token;
+    EXPECT_TRUE(parser.Check()) << testcase.token;
+    EXPECT_EQ(error_tracker.SeenError(), testcase.radix == 2) << testcase.token;
     EXPECT_EQ(parser.IsInteger(), false);
     EXPECT_EQ(parser.GetMantissa().getZExtValue(), testcase.mantissa);
     EXPECT_EQ(parser.GetExponent().getSExtValue(), testcase.exponent);
@@ -199,8 +212,10 @@ TEST_F(NumericLiteralTest, HandlesRealLiteral) {
 
 TEST_F(NumericLiteralTest, HandlesRealLiteralOverflow) {
   llvm::StringLiteral input = "0x1.000001p-9223372036854775800";
+  error_tracker.Reset();
   auto parser = Parse(input);
-  EXPECT_EQ(parser.Check(), parser.Valid);
+  EXPECT_TRUE(parser.Check());
+  EXPECT_FALSE(error_tracker.SeenError());
   EXPECT_EQ(parser.GetMantissa(), 0x1000001);
   EXPECT_EQ((parser.GetExponent() + 9223372036854775800).getSExtValue(), -24);
   EXPECT_EQ(parser.GetRadix(), 16);
@@ -213,8 +228,10 @@ TEST_F(NumericLiteralTest, ValidatesRealLiterals) {
       "123.4e56_78", "0x12_34.5", "0x12.3_4",  "0x12.34p5_6",
   };
   for (llvm::StringLiteral literal : invalid_digit_separators) {
+    error_tracker.Reset();
     auto parser = Parse(literal);
-    EXPECT_EQ(parser.Check(), parser.RecoverableError) << literal;
+    EXPECT_TRUE(parser.Check()) << literal;
+    EXPECT_TRUE(error_tracker.SeenError()) << literal;
   }
 
   llvm::StringLiteral invalid[] = {
@@ -263,8 +280,10 @@ TEST_F(NumericLiteralTest, ValidatesRealLiterals) {
       "0x0.0p-A",
   };
   for (llvm::StringLiteral literal : invalid) {
+    error_tracker.Reset();
     auto parser = Parse(literal);
-    EXPECT_EQ(parser.Check(), parser.UnrecoverableError) << literal;
+    EXPECT_FALSE(parser.Check()) << literal;
+    EXPECT_TRUE(error_tracker.SeenError()) << literal;
   }
 }
 

lexer/string_literal.cpp

@@ -168,32 +168,22 @@ static auto ComputeIndentOfFinalLine(llvm::StringRef text) -> llvm::StringRef {
   llvm_unreachable("Given text is required to contain a newline.");
 }
 
-namespace {
-// The leading whitespace in a multi-line string literal.
-struct Indent {
-  llvm::StringRef indent;
-  bool has_errors;
-};
-}  // namespace
-
 // Check the literal is indented properly, if it's a multi-line litera.
 // Find the leading whitespace that should be removed from each line of a
 // multi-line string literal.
 static auto CheckIndent(LexerDiagnosticEmitter& emitter, llvm::StringRef text,
-                        llvm::StringRef content) -> Indent {
+                        llvm::StringRef content) -> llvm::StringRef {
   // Find the leading horizontal whitespace on the final line of this literal.
   // Note that for an empty literal, this might not be inside the content.
   llvm::StringRef indent = ComputeIndentOfFinalLine(text);
-  bool has_errors = false;
 
   // The last line is not permitted to contain any content after its
   // indentation.
   if (indent.end() != content.end()) {
     emitter.EmitError<ContentBeforeStringTerminator>(indent.end());
-    has_errors = true;
   }
 
-  return {.indent = indent, .has_errors = has_errors};
+  return indent;
 }
 
 // Expand a `\u{HHHHHH}` escape sequence into a sequence of UTF-8 code units.
@@ -233,7 +223,7 @@ static auto ExpandUnicodeEscapeSequence(LexerDiagnosticEmitter& emitter,
 // `\n`), and will be updated to remove the leading escape sequence.
 static auto ExpandAndConsumeEscapeSequence(LexerDiagnosticEmitter& emitter,
                                            llvm::StringRef& content,
-                                           std::string& result) -> bool {
+                                           std::string& result) -> void {
   assert(!content.empty() && "should have escaped closing delimiter");
   char first = content.front();
   content = content.drop_front(1);
@@ -241,36 +231,36 @@ static auto ExpandAndConsumeEscapeSequence(LexerDiagnosticEmitter& emitter,
   switch (first) {
     case 't':
       result += '\t';
-      return true;
+      return;
     case 'n':
       result += '\n';
-      return true;
+      return;
     case 'r':
       result += '\r';
-      return true;
+      return;
     case '"':
       result += '"';
-      return true;
+      return;
     case '\'':
       result += '\'';
-      return true;
+      return;
     case '\\':
       result += '\\';
-      return true;
+      return;
     case '0':
       result += '\0';
       if (!content.empty() && IsDecimalDigit(content.front())) {
         emitter.EmitError<DecimalEscapeSequence>(content.begin());
-        return false;
+        return;
       }
-      return true;
+      return;
     case 'x':
       if (content.size() >= 2 && IsUpperHexDigit(content[0]) &&
           IsUpperHexDigit(content[1])) {
         result +=
             static_cast<char>(llvm::hexFromNibbles(content[0], content[1]));
         content = content.drop_front(2);
-        return true;
+        return;
       }
       emitter.EmitError<HexadecimalEscapeMissingDigits>(content.begin());
       break;
@@ -284,7 +274,7 @@ static auto ExpandAndConsumeEscapeSequence(LexerDiagnosticEmitter& emitter,
             break;
           }
           content = remaining;
-          return true;
+          return;
         }
       }
       emitter.EmitError<UnicodeEscapeMissingBracedDigits>(content.begin());
@@ -300,16 +290,14 @@ static auto ExpandAndConsumeEscapeSequence(LexerDiagnosticEmitter& emitter,
   // issued a diagnostic. For error recovery purposes, expand this escape
   // sequence to itself, dropping the introducer (for example, `\q` -> `q`).
   result += first;
-  return false;
 }
 
 // Expand any escape sequences in the given string literal.
 static auto ExpandEscapeSequencesAndRemoveIndent(
     LexerDiagnosticEmitter& emitter, llvm::StringRef contents, int hash_level,
-    llvm::StringRef indent) -> LexedStringLiteral::ExpandedValue {
+    llvm::StringRef indent) -> std::string {
   std::string result;
   result.reserve(contents.size());
-  bool has_errors = false;
 
   llvm::SmallString<16> escape("\\");
   escape.resize(1 + hash_level, '#');
@@ -324,7 +312,6 @@ static auto ExpandEscapeSequencesAndRemoveIndent(
       contents = contents.drop_while(IsHorizontalWhitespace);
       if (!contents.startswith("\n")) {
         emitter.EmitError<MismatchedIndentInString>(line_start);
-        has_errors = true;
       }
     }
 
@@ -335,7 +322,7 @@ static auto ExpandEscapeSequencesAndRemoveIndent(
       contents = contents.substr(end_of_regular_text);
 
       if (contents.empty()) {
-        return {.result = result, .has_errors = has_errors};
+        return result;
       }
 
       if (contents.consume_front("\n")) {
@@ -364,20 +351,17 @@ static auto ExpandEscapeSequencesAndRemoveIndent(
       }
 
       // Handle this escape sequence.
-      if (!ExpandAndConsumeEscapeSequence(emitter, contents, result)) {
-        has_errors = true;
-      }
+      ExpandAndConsumeEscapeSequence(emitter, contents, result);
     }
   }
 }
 
 auto LexedStringLiteral::ComputeValue(LexerDiagnosticEmitter& emitter) const
-    -> ExpandedValue {
-  auto indent = multi_line ? CheckIndent(emitter, text, content) : Indent();
-  auto result = ExpandEscapeSequencesAndRemoveIndent(emitter, content,
-                                                     hash_level, indent.indent);
-  result.has_errors |= indent.has_errors;
-  return result;
+    -> std::string {
+  llvm::StringRef indent =
+      multi_line ? CheckIndent(emitter, text, content) : llvm::StringRef();
+  return ExpandEscapeSequencesAndRemoveIndent(emitter, content, hash_level,
+                                              indent);
 }
 
 }  // namespace Carbon

lexer/string_literal.h

@@ -23,16 +23,10 @@ class LexedStringLiteral {
   static auto Lex(llvm::StringRef source_text)
       -> llvm::Optional<LexedStringLiteral>;
 
-  // The result of expanding escape sequences in a string literal.
-  struct ExpandedValue {
-    std::string result;
-    bool has_errors;
-  };
-
   // Expand any escape sequences in the given string literal and compute the
-  // resulting value.
+  // resulting value. This handles error recovery internally and cannot fail.
   auto ComputeValue(DiagnosticEmitter<const char*>& emitter) const
-      -> ExpandedValue;
+      -> std::string;
 
  private:
   LexedStringLiteral(llvm::StringRef text, llvm::StringRef content,

lexer/string_literal_test.cpp

@@ -13,6 +13,10 @@ namespace Carbon {
 namespace {
 
 struct StringLiteralTest : ::testing::Test {
+  StringLiteralTest() : error_tracker(ConsoleDiagnosticConsumer()) {}
+
+  ErrorTrackingDiagnosticConsumer error_tracker;
+
   auto Lex(llvm::StringRef text) -> LexedStringLiteral {
     llvm::Optional<LexedStringLiteral> result = LexedStringLiteral::Lex(text);
     assert(result);
@@ -20,11 +24,10 @@ struct StringLiteralTest : ::testing::Test {
     return *result;
   }
 
-  auto Parse(llvm::StringRef text) -> LexedStringLiteral::ExpandedValue {
+  auto Parse(llvm::StringRef text) -> std::string {
     LexedStringLiteral token = Lex(text);
     Testing::SingleTokenDiagnosticTranslator translator(text);
-    DiagnosticEmitter<const char*> emitter(translator,
-                                           ConsoleDiagnosticConsumer());
+    DiagnosticEmitter<const char*> emitter(translator, error_tracker);
     return token.ComputeValue(emitter);
   }
 };
@@ -182,9 +185,10 @@ TEST_F(StringLiteralTest, StringLiteralContents) {
   };
 
   for (auto [test, contents] : testcases) {
+    error_tracker.Reset();
     auto value = Parse(test.trim());
-    EXPECT_FALSE(value.has_errors) << "`" << test << "`";
-    EXPECT_EQ(value.result, contents);
+    EXPECT_FALSE(error_tracker.SeenError()) << "`" << test << "`";
+    EXPECT_EQ(value, contents);
   }
 }
 
@@ -205,9 +209,10 @@ TEST_F(StringLiteralTest, StringLiteralBadIndent) {
   };
 
   for (auto [test, contents] : testcases) {
+    error_tracker.Reset();
     auto value = Parse(test);
-    EXPECT_TRUE(value.has_errors) << "`" << test << "`";
-    EXPECT_EQ(value.result, contents);
+    EXPECT_TRUE(error_tracker.SeenError()) << "`" << test << "`";
+    EXPECT_EQ(value, contents);
   }
 }
 
@@ -253,8 +258,9 @@ TEST_F(StringLiteralTest, StringLiteralBadEscapeSequence) {
   };
 
   for (llvm::StringLiteral test : testcases) {
+    error_tracker.Reset();
     auto value = Parse(test);
-    EXPECT_TRUE(value.has_errors) << "`" << test << "`";
+    EXPECT_TRUE(error_tracker.SeenError()) << "`" << test << "`";
     // TODO: Test value produced by error recovery.
   }
 }

lexer/tokenized_buffer.cpp

@@ -132,13 +132,11 @@ class TokenizedBuffer::Lexer {
         // Any comment must be the only non-whitespace on the line.
         if (set_indent) {
           emitter.EmitError<TrailingComment>(source_text.begin());
-          buffer.has_errors = true;
         }
         // The introducer '//' must be followed by whitespace or EOF.
         if (source_text.size() > 2 && !IsSpace(source_text[2])) {
           emitter.EmitError<NoWhitespaceAfterCommentIntroducer>(
               source_text.begin() + 2);
-          buffer.has_errors = true;
         }
         while (!source_text.empty() && source_text.front() != '\n') {
           ++current_column;
@@ -207,24 +205,14 @@ class TokenizedBuffer::Lexer {
 
     LexedNumericLiteral::Parser literal_parser(emitter, *literal);
 
-    switch (literal_parser.Check()) {
-      case LexedNumericLiteral::Parser::UnrecoverableError: {
-        auto token = buffer.AddToken({
-            .kind = TokenKind::Error(),
-            .token_line = current_line,
-            .column = int_column,
-            .error_length = token_size,
-        });
-        buffer.has_errors = true;
-        return token;
-      }
-
-      case LexedNumericLiteral::Parser::RecoverableError:
-        buffer.has_errors = true;
-        break;
-
-      case LexedNumericLiteral::Parser::Valid:
-        break;
+    if (!literal_parser.Check()) {
+      auto token = buffer.AddToken({
+          .kind = TokenKind::Error(),
+          .token_line = current_line,
+          .column = int_column,
+          .error_length = token_size,
+      });
+      return token;
     }
 
     if (literal_parser.IsInteger()) {
@@ -281,16 +269,12 @@ class TokenizedBuffer::Lexer {
       }
     }
 
-    // Determine string literal value.
-    auto expanded = literal->ComputeValue(emitter);
-    buffer.has_errors |= expanded.has_errors;
-
     auto token = buffer.AddToken({.kind = TokenKind::StringLiteral(),
                                   .token_line = string_line,
                                   .column = string_column});
     buffer.GetTokenInfo(token).literal_index =
         buffer.literal_string_storage.size();
-    buffer.literal_string_storage.push_back(std::move(expanded.result));
+    buffer.literal_string_storage.push_back(literal->ComputeValue(emitter));
     return token;
   }
 
@@ -335,7 +319,6 @@ class TokenizedBuffer::Lexer {
     if (open_groups.empty()) {
       closing_token_info.kind = TokenKind::Error();
       closing_token_info.error_length = kind.GetFixedSpelling().size();
-      buffer.has_errors = true;
 
       emitter.EmitError<UnmatchedClosing>(location);
       // Note that this still returns true as we do consume a symbol.
@@ -365,7 +348,6 @@ class TokenizedBuffer::Lexer {
       }
 
       open_groups.pop_back();
-      buffer.has_errors = true;
       token_emitter.EmitError<MismatchedClosing>(opening_token);
 
       // TODO: do a smarter backwards scan for where to put the closing
@@ -460,7 +442,6 @@ class TokenizedBuffer::Lexer {
 
     current_column += error_text.size();
     source_text = source_text.drop_front(error_text.size());
-    buffer.has_errors = true;
     return token;
   }
 
@@ -474,7 +455,8 @@ class TokenizedBuffer::Lexer {
 auto TokenizedBuffer::Lex(SourceBuffer& source, DiagnosticConsumer& consumer)
     -> TokenizedBuffer {
   TokenizedBuffer buffer(source);
-  Lexer lexer(buffer, consumer);
+  ErrorTrackingDiagnosticConsumer error_tracking_consumer(consumer);
+  Lexer lexer(buffer, error_tracking_consumer);
 
   llvm::StringRef source_text = source.Text();
   while (lexer.SkipWhitespace(source_text)) {
@@ -498,6 +480,11 @@ auto TokenizedBuffer::Lex(SourceBuffer& source, DiagnosticConsumer& consumer)
 
   lexer.CloseInvalidOpenGroups(TokenKind::Error());
   lexer.AddEndOfFileToken();
+
+  if (error_tracking_consumer.SeenError()) {
+    buffer.has_errors = true;
+  }
+
   return buffer;
 }