diff --git a/toolchain/diagnostics/BUILD b/toolchain/diagnostics/BUILD index b63a8e2ef151..0fd28b1fd62f 100644 --- a/toolchain/diagnostics/BUILD +++ b/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", - ], -) diff --git a/toolchain/diagnostics/diagnostic_emitter.h b/toolchain/diagnostics/diagnostic_emitter.h index 64149dc8cfd7..ff84435dfd01 100644 --- a/toolchain/diagnostics/diagnostic_emitter.h +++ b/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 #include @@ -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; - Level level; - Location location; - llvm::StringRef short_name; - std::string message; + // 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 format_args; + + // Returns the formatted string. By default, this uses llvm::formatv. + std::function 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 { -// static constexpr llvm::StringLiteral ShortName = "short-name"; -// static constexpr llvm::StringLiteral Message = "A message."; -// }; -// -// emitter.EmitError(location); -// -// A complex child may provide an alternate `Format` implementation: -// -// struct MyComplexError : DiagnosticBase { -// static constexpr llvm::StringLiteral ShortName = "short-name"; -// -// auto Format() -> std::string { return llvm::formatv("See {0}.", ref); } -// -// std::string ref; -// }; -// -// emitter.EmitError(location, {.ref = "ref"; }); -template +namespace Internal { + +// Use the DIAGNOSTIC macro to instantiate this. +// This stores static information about a diagnostic category. +template 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()); + }; + + // 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 + inline auto FormatFnImpl(const Diagnostic& diagnostic, + std::index_sequence /*indices*/) const + -> std::string { + assert(diagnostic.format_args.size() == sizeof...(Args)); + return llvm::formatv(diagnostic.format.data(), + llvm::any_cast(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 , 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 - auto EmitError(LocationT location) - -> std::enable_if_t> { - EmitError(location, {}); - } - - // Emits a warning if `F` returns true. `F` may or may not be called if the - // warning is disabled. - template - auto EmitWarningIf(LocationT location, - llvm::function_ref 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 + void Emit(LocationT location, + const Internal::DiagnosticBase& 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) { + 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_ diff --git a/toolchain/diagnostics/diagnostic_emitter_test.cpp b/toolchain/diagnostics/diagnostic_emitter_test.cpp index 1b36d2305b52..c4d2fae4c0b8 100644 --- a/toolchain/diagnostics/diagnostic_emitter_test.cpp +++ b/toolchain/diagnostics/diagnostic_emitter_test.cpp @@ -14,75 +14,48 @@ namespace Carbon::Testing { namespace { -struct FakeDiagnostic : DiagnosticBase { - 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(ShortName); - - return llvm::formatv(Message.data(), message).str(); - } - - std::string message; -}; - struct FakeDiagnosticLocationTranslator : DiagnosticLocationTranslator { - 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 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 emitter_; +}; - emitter.EmitError(1, {.message = "M1"}); - emitter.EmitError(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 reported; +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); +} - FakeDiagnosticLocationTranslator translator; - Testing::MockDiagnosticConsumer consumer; - DiagnosticEmitter 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")))); - - emitter.EmitWarningIf(3, [](FakeDiagnostic& diagnostic) { - diagnostic.message = "M1"; - return true; - }); - emitter.EmitWarningIf(4, [](FakeDiagnostic& diagnostic) { - diagnostic.message = "M2"; - return false; - }); - emitter.EmitWarningIf(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 diff --git a/toolchain/diagnostics/diagnostic_kind.cpp b/toolchain/diagnostics/diagnostic_kind.cpp new file mode 100644 index 000000000000..9eadb828f752 --- /dev/null +++ b/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(kind)]; + return out; +} + +} // namespace Carbon diff --git a/toolchain/diagnostics/diagnostic_kind.h b/toolchain/diagnostics/diagnostic_kind.h new file mode 100644 index 000000000000..f767fb398813 --- /dev/null +++ b/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_ diff --git a/toolchain/diagnostics/diagnostic_kind_test.cpp b/toolchain/diagnostics/diagnostic_kind_test.cpp new file mode 100644 index 000000000000..a958e348e39e --- /dev/null +++ b/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 +#include + +#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 diff --git a/toolchain/diagnostics/diagnostic_registry.def b/toolchain/diagnostics/diagnostic_registry.def new file mode 100644 index 000000000000..9d3fa586e8ea --- /dev/null +++ b/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 diff --git a/toolchain/diagnostics/mocks.cpp b/toolchain/diagnostics/mocks.cpp index 96185d46c48d..76dfdc71278c 100644 --- a/toolchain/diagnostics/mocks.cpp +++ b/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 diff --git a/toolchain/diagnostics/mocks.h b/toolchain/diagnostics/mocks.h index 1118f4bbbf5d..fbfc89cf1814 100644 --- a/toolchain/diagnostics/mocks.h +++ b/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 -auto DiagnosticMessage(Matcher&& inner_matcher) -> auto { - return testing::Field(&Diagnostic::message, - std::forward(inner_matcher)); -} - -template -auto DiagnosticShortName(Matcher&& inner_matcher) -> auto { - return testing::Field(&Diagnostic::short_name, - std::forward(inner_matcher)); +inline auto IsDiagnostic(testing::Matcher kind, + testing::Matcher level, + testing::Matcher line_number, + testing::Matcher column_number, + testing::Matcher 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 diff --git a/toolchain/diagnostics/null_diagnostics.h b/toolchain/diagnostics/null_diagnostics.h index 9348a06e8139..5ff3bcf106e3 100644 --- a/toolchain/diagnostics/null_diagnostics.h +++ b/toolchain/diagnostics/null_diagnostics.h @@ -13,7 +13,7 @@ template inline auto NullDiagnosticLocationTranslator() -> DiagnosticLocationTranslator& { struct Translator : DiagnosticLocationTranslator { - auto GetLocation(LocationT /*loc*/) -> Diagnostic::Location override { + auto GetLocation(LocationT /*loc*/) -> DiagnosticLocation override { return {}; } }; diff --git a/toolchain/diagnostics/sorting_diagnostic_consumer_test.cpp b/toolchain/diagnostics/sorting_diagnostic_consumer_test.cpp index 99057bc60266..92fce660b028 100644 --- a/toolchain/diagnostics/sorting_diagnostic_consumer_test.cpp +++ b/toolchain/diagnostics/sorting_diagnostic_consumer_test.cpp @@ -17,68 +17,43 @@ namespace { using ::testing::InSequence; -struct FakeDiagnostic : DiagnosticBase { - 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(ShortName); - - return llvm::formatv(Message.data(), message).str(); - } - - std::string message; -}; +CARBON_DIAGNOSTIC(TestDiagnostic, Error, "{0}", llvm::StringRef); struct FakeDiagnosticLocationTranslator - : DiagnosticLocationTranslator { - auto GetLocation(Diagnostic::Location loc) -> Diagnostic::Location override { + : DiagnosticLocationTranslator { + 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 emitter(translator, sorting_consumer); + DiagnosticEmitter emitter(translator, sorting_consumer); - emitter.EmitError(MakeLoc(2, 1), {.message = "M1"}); - emitter.EmitError(MakeLoc(1, 1), {.message = "M2"}); - emitter.EmitError(MakeLoc(1, 3), {.message = "M3"}); - emitter.EmitError(MakeLoc(3, 4), {.message = "M4"}); - emitter.EmitError(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(); } diff --git a/toolchain/lexer/BUILD b/toolchain/lexer/BUILD index 0768844159df..85f910a21547 100644 --- a/toolchain/lexer/BUILD +++ b/toolchain/lexer/BUILD @@ -13,6 +13,7 @@ cc_library( textual_hdrs = ["token_registry.def"], deps = [ "//common:check", + "//common:ostream", "@llvm-project//llvm:Support", ], ) diff --git a/toolchain/lexer/lex_helpers.cpp b/toolchain/lexer/lex_helpers.cpp index f8923d7c4dd2..07420e68305f 100644 --- a/toolchain/lexer/lex_helpers.cpp +++ b/toolchain/lexer/lex_helpers.cpp @@ -8,23 +8,6 @@ namespace Carbon { -namespace { -struct TooManyDigits : DiagnosticBase { - 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& 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& emitter, // is far above the threshold for normal integers. constexpr size_t DigitLimit = 1000; if (text.size() > DigitLimit) { - emitter.EmitError( - 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; diff --git a/toolchain/lexer/numeric_literal.cpp b/toolchain/lexer/numeric_literal.cpp index d6ac89010e99..45297aa0f8e3 100644 --- a/toolchain/lexer/numeric_literal.cpp +++ b/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 { - static constexpr llvm::StringLiteral ShortName = "syntax-invalid-number"; - static constexpr llvm::StringLiteral Message = - "Empty digit sequence in numeric literal."; -}; - -struct InvalidDigit : DiagnosticBase { - 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 { - static constexpr llvm::StringLiteral ShortName = "syntax-invalid-number"; - static constexpr llvm::StringLiteral Message = - "Misplaced digit separator in numeric literal."; -}; - -struct IrregularDigitSeparators : DiagnosticBase { - 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 { - static constexpr llvm::StringLiteral ShortName = "syntax-invalid-number"; - static constexpr llvm::StringLiteral Message = - "Unknown base specifier in numeric literal."; -}; - -struct BinaryRealLiteral : DiagnosticBase { - static constexpr llvm::StringLiteral ShortName = "syntax-invalid-number"; - static constexpr llvm::StringLiteral Message = - "Binary real number literals are not supported."; -}; - -struct WrongRealLiteralExponent : DiagnosticBase { - 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 { @@ -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(text.begin() + i); + emitter_.Emit(text.begin() + 1, InvalidDigitSeparator); } ++num_digit_separators; continue; } - emitter_.EmitError(text.begin() + i, - {.digit = c, .radix = radix}); + emitter_.Emit(text.begin() + i, InvalidDigit, c, radix); return {.ok = false}; } if (num_digit_separators == static_cast(text.size())) { - emitter_.EmitError(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(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(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(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( - literal_.text_.begin() + literal_.exponent_, - {.expected = expected_exponent_kind}); + emitter_.Emit(literal_.text_.begin() + literal_.exponent_, + WrongRealLiteralExponent, expected_exponent_kind); return false; } diff --git a/toolchain/lexer/string_literal.cpp b/toolchain/lexer/string_literal.cpp index f443fe47c93f..a9818c87d854 100644 --- a/toolchain/lexer/string_literal.cpp +++ b/toolchain/lexer/string_literal.cpp @@ -17,72 +17,36 @@ namespace Carbon { using LexerDiagnosticEmitter = DiagnosticEmitter; -struct ContentBeforeStringTerminator - : DiagnosticBase { - 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 { - 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 { - 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 { - 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 { - 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 { - 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 { - 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 { - 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 { - 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(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(digits.begin()); + emitter.Emit(digits.begin(), UnicodeEscapeTooLarge); return false; } if (code_point >= 0xD800 && code_point < 0xE000) { - emitter.EmitError(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(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(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(content.begin()); + emitter.Emit(content.begin(), UnicodeEscapeMissingBracedDigits); break; } default: - emitter.EmitError(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(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( - contents.begin()); + emitter.Emit(contents.begin(), InvalidHorizontalWhitespaceInString); // Include the whitespace in the string contents for error recovery. result += contents.substr(0, after_space); } diff --git a/toolchain/lexer/test_helpers.h b/toolchain/lexer/test_helpers.h index f47abc612b89..5258ede68156 100644 --- a/toolchain/lexer/test_helpers.h +++ b/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(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(before_last_newline.count('\n') + 2), .column_number = static_cast(this_line.size() + 1)}; } } private: - [[nodiscard]] auto SynthesizeFilename() const -> std::string { - return llvm::formatv("`{0}`", token_); - } - llvm::StringRef token_; }; diff --git a/toolchain/lexer/token_kind.h b/toolchain/lexer/token_kind.h index a0c0bf6f1a67..d48af233381a 100644 --- a/toolchain/lexer/token_kind.h +++ b/toolchain/lexer/token_kind.h @@ -9,6 +9,7 @@ #include #include +#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) {} diff --git a/toolchain/lexer/tokenized_buffer.cpp b/toolchain/lexer/tokenized_buffer.cpp index ebff79ca599c..8c777e84e9a1 100644 --- a/toolchain/lexer/tokenized_buffer.cpp +++ b/toolchain/lexer/tokenized_buffer.cpp @@ -27,43 +27,17 @@ namespace Carbon { -struct TrailingComment : DiagnosticBase { - static constexpr llvm::StringLiteral ShortName = "syntax-comments"; - static constexpr llvm::StringLiteral Message = - "Trailing comments are not permitted."; -}; - -struct NoWhitespaceAfterCommentIntroducer - : DiagnosticBase { - static constexpr llvm::StringLiteral ShortName = "syntax-comments"; - static constexpr llvm::StringLiteral Message = - "Whitespace is required after '//'."; -}; - -struct UnmatchedClosing : DiagnosticBase { - static constexpr llvm::StringLiteral ShortName = "syntax-balanced-delimiters"; - static constexpr llvm::StringLiteral Message = - "Closing symbol without a corresponding opening symbol."; -}; - -struct MismatchedClosing : DiagnosticBase { - 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 { - static constexpr llvm::StringLiteral ShortName = - "syntax-unrecognized-characters"; - static constexpr llvm::StringLiteral Message = - "Encountered unrecognized characters while parsing."; -}; - -struct UnterminatedString : DiagnosticBase { - 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(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( - 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(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(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(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(error_text.size())}); - emitter_.EmitError(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); diff --git a/toolchain/lexer/tokenized_buffer.h b/toolchain/lexer/tokenized_buffer.h index 25ff8060795d..63dc9f681da1 100644 --- a/toolchain/lexer/tokenized_buffer.h +++ b/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_; diff --git a/toolchain/lexer/tokenized_buffer_test.cpp b/toolchain/lexer/tokenized_buffer_test.cpp index d2d8ac908455..d8131f29a9f9 100644 --- a/toolchain/lexer/tokenized_buffer_test.cpp +++ b/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{ - {.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{ + {.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); } diff --git a/toolchain/parser/parse_tree_test.cpp b/toolchain/parser/parse_tree_test.cpp index ba5c19e4d507..d71156669520 100644 --- a/toolchain/parser/parse_tree_test.cpp +++ b/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()); diff --git a/toolchain/parser/parser_impl.cpp b/toolchain/parser/parser_impl.cpp index 5001d972ee65..4c98fe4d3388 100644 --- a/toolchain/parser/parser_impl.cpp +++ b/toolchain/parser/parser_impl.cpp @@ -17,14 +17,8 @@ namespace Carbon { -struct StackLimitExceeded : DiagnosticBase { - 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(*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 { - static constexpr llvm::StringLiteral ShortName = "syntax-error"; - static constexpr llvm::StringLiteral Message = - "Unexpected token in code block."; +// A relative location for characters in errors. +enum class RelativeLocation : int8_t { + Around, + After, + Before, }; -struct ExpectedFunctionName : DiagnosticBase { - static constexpr llvm::StringLiteral ShortName = "syntax-error"; - static constexpr llvm::StringLiteral Message = - "Expected function name after `fn` keyword."; -}; - -struct ExpectedFunctionParams : DiagnosticBase { - static constexpr llvm::StringLiteral ShortName = "syntax-error"; - static constexpr llvm::StringLiteral Message = - "Expected `(` after function name."; -}; - -struct ExpectedFunctionBodyOrSemi : DiagnosticBase { - static constexpr llvm::StringLiteral ShortName = "syntax-error"; - static constexpr llvm::StringLiteral Message = - "Expected function definition or `;` after function declaration."; -}; - -struct ExpectedVariableName : DiagnosticBase { - static constexpr llvm::StringLiteral ShortName = "syntax-error"; - static constexpr llvm::StringLiteral Message = - "Expected pattern in `var` declaration."; -}; - -struct ExpectedParameterName : DiagnosticBase { - static constexpr llvm::StringLiteral ShortName = "syntax-error"; - static constexpr llvm::StringLiteral Message = - "Expected parameter declaration."; -}; - -struct ExpectedStructLiteralField : DiagnosticBase { - 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; +// 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 can_be_type; - bool can_be_value; -}; - -struct UnrecognizedDeclaration : DiagnosticBase { - static constexpr llvm::StringLiteral ShortName = "syntax-error"; - static constexpr llvm::StringLiteral Message = - "Unrecognized declaration introducer."; -}; - -struct ExpectedCodeBlock : DiagnosticBase { - static constexpr llvm::StringLiteral ShortName = "syntax-error"; - static constexpr llvm::StringLiteral Message = "Expected braced code block."; -}; - -struct ExpectedExpression : DiagnosticBase { - static constexpr llvm::StringLiteral ShortName = "syntax-error"; - static constexpr llvm::StringLiteral Message = "Expected expression."; -}; - -struct ExpectedParenAfter : DiagnosticBase { - 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 { - 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 { - static constexpr llvm::StringLiteral ShortName = "syntax-error"; - static constexpr llvm::StringLiteral Message = - "Expected `;` after expression."; -}; - -struct ExpectedSemiAfter : DiagnosticBase { - 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 { - static constexpr llvm::StringLiteral ShortName = "syntax-error"; - static constexpr llvm::StringLiteral Message = - "Expected identifier after `.`."; -}; - -struct UnexpectedTokenAfterListElement - : DiagnosticBase { - 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 { - 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 { - 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 { - 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"); - } - - bool prefix; -}; - -struct OperatorRequiresParentheses - : DiagnosticBase { - 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(*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(*position_, - {.close = close}); + emitter_.Emit(*position_, UnexpectedTokenAfterListElement, close); } has_errors = true; @@ -521,11 +397,11 @@ auto ParseTree::Parser::ParsePattern(PatternKind kind) -> llvm::Optional { switch (kind) { case PatternKind::Parameter: - emitter_.EmitError(*position_); + emitter_.Emit(*position_, ExpectedParameterName); break; case PatternKind::Variable: - emitter_.EmitError(*position_); + emitter_.Emit(*position_, ExpectedVariableName); break; } @@ -570,7 +446,7 @@ auto ParseTree::Parser::ParseCodeBlock() -> llvm::Optional { ConsumeIf(TokenKind::OpenCurlyBrace()); if (!maybe_open_curly) { // Recover by parsing a single statement. - emitter_.EmitError(*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(*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(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(*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(*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 { } // We didn't recognize an introducer for a valid declaration. - emitter_.EmitError(*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 { auto start_elem = GetSubtreeStartPosition(); auto diagnose_invalid_syntax = [&] { - emitter_.EmitError( - *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 { return ParseBraceExpression(); default: - emitter_.EmitError(*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(*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( - *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(*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(*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(*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(*position_); + emitter_.Emit(*position_, OperatorRequiresParentheses); lhs = llvm::None; } else { DiagnoseOperatorFixity(is_binary ? OperatorFixity::Infix @@ -1142,7 +1025,7 @@ auto ParseTree::Parser::ParseExpressionStatement() -> llvm::Optional { } if (!has_errors) { - emitter_.EmitError(*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(*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(*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);