diff --git a/common/error.h b/common/error.h index 403befc461dd..1752c242408e 100644 --- a/common/error.h +++ b/common/error.h @@ -23,19 +23,48 @@ struct Success {}; class [[nodiscard]] Error { public: // Represents an error state. - explicit Error(llvm::Twine message) : message_(message.str()) { + explicit Error(llvm::Twine prefix, llvm::Twine location, llvm::Twine message) + : prefix_(prefix.str()), + location_(location.str()), + message_(message.str()) { CARBON_CHECK(!message_.empty()) << "Errors must have a message."; } - Error(Error&& other) noexcept : message_(std::move(other.message_)) {} + // Represents an error with no associated prefix or location. + // TODO: Consider using two different types. + explicit Error(llvm::Twine message) : Error("", "", message) {} - // Prints the error string. Note this marks as used. - void Print(llvm::raw_ostream& out) const { out << message(); } + Error(Error&& other) noexcept + : prefix_(std::move(other.prefix_)), + location_(std::move(other.location_)), + message_(std::move(other.message_)) {} + + // Prints the error string. + void Print(llvm::raw_ostream& out) const { + if (!prefix().empty()) { + out << prefix() << ": "; + } + if (!location().empty()) { + out << location() << ": "; + } + out << message(); + } + + // Returns the prefix to prepend to the error, such as "ERROR". + auto prefix() const -> const std::string& { return prefix_; } + + // Returns a string describing the location of the error, such as + // "file.cc:123". + auto location() const -> const std::string& { return location_; } // Returns the error message. auto message() const -> const std::string& { return message_; } private: + // A prefix, indicating the kind of error. + std::string prefix_; + // The location associated with the error. + std::string location_; // The error message. std::string message_; }; @@ -109,7 +138,12 @@ class [[nodiscard]] ErrorOr { // `Error` and `ErrorOr`. class ErrorBuilder { public: - ErrorBuilder() : out_(std::make_unique(message_)) {} + explicit ErrorBuilder(std::string prefix, std::string location) + : prefix_(std::move(prefix)), + location_(std::move(location)), + out_(std::make_unique(message_)) {} + + explicit ErrorBuilder() : ErrorBuilder("", "") {} // Accumulates string message. template @@ -119,15 +153,17 @@ class ErrorBuilder { } // NOLINTNEXTLINE(google-explicit-constructor): Implicit cast for returns. - operator Error() { return Error(message_); } + operator Error() { return Error(prefix_, location_, message_); } template // NOLINTNEXTLINE(google-explicit-constructor): Implicit cast for returns. operator ErrorOr() { - return Error(message_); + return Error(prefix_, location_, message_); } private: + std::string prefix_; + std::string location_; std::string message_; // Use a pointer to allow move construction. std::unique_ptr out_; diff --git a/explorer/common/error_builders.h b/explorer/common/error_builders.h index 17ad3c243c24..9a31f098ee82 100644 --- a/explorer/common/error_builders.h +++ b/explorer/common/error_builders.h @@ -22,21 +22,15 @@ namespace Carbon { // provided as a fallback for cases that don't fit those classifications. inline auto CompilationError(SourceLocation loc) -> ErrorBuilder { - ErrorBuilder builder; - (void)(builder << "COMPILATION ERROR: " << loc << ": "); - return builder; + return ErrorBuilder("COMPILATION ERROR", loc.ToString()); } inline auto ProgramError(SourceLocation loc) -> ErrorBuilder { - ErrorBuilder builder; - (void)(builder << "PROGRAM ERROR: " << loc << ": "); - return builder; + return ErrorBuilder("PROGRAM ERROR", loc.ToString()); } inline auto RuntimeError(SourceLocation loc) -> ErrorBuilder { - ErrorBuilder builder; - (void)(builder << "RUNTIME ERROR: " << loc << ": "); - return builder; + return ErrorBuilder("RUNTIME ERROR", loc.ToString()); } } // namespace Carbon diff --git a/explorer/common/error_builders_test.cpp b/explorer/common/error_builders_test.cpp index f4ac9fd50dc7..a404f65248b1 100644 --- a/explorer/common/error_builders_test.cpp +++ b/explorer/common/error_builders_test.cpp @@ -11,19 +11,35 @@ namespace Carbon::Testing { namespace { +auto ToString(const Error& err) -> std::string { + std::string result; + llvm::raw_string_ostream out(result); + err.Print(out); + return result; +} + TEST(ErrorBuildersTest, CompilationError) { Error err = CompilationError(SourceLocation("x", 1)) << "test"; - EXPECT_EQ(err.message(), "COMPILATION ERROR: x:1: test"); + EXPECT_EQ(err.prefix(), "COMPILATION ERROR"); + EXPECT_EQ(err.location(), "x:1"); + EXPECT_EQ(err.message(), "test"); + EXPECT_EQ(ToString(err), "COMPILATION ERROR: x:1: test"); } TEST(ErrorBuildersTest, ProgramError) { Error err = ProgramError(SourceLocation("x", 1)) << "test"; - EXPECT_EQ(err.message(), "PROGRAM ERROR: x:1: test"); + EXPECT_EQ(err.prefix(), "PROGRAM ERROR"); + EXPECT_EQ(err.location(), "x:1"); + EXPECT_EQ(err.message(), "test"); + EXPECT_EQ(ToString(err), "PROGRAM ERROR: x:1: test"); } TEST(ErrorBuildersTest, RuntimeError) { Error err = RuntimeError(SourceLocation("x", 1)) << "test"; - EXPECT_EQ(err.message(), "RUNTIME ERROR: x:1: test"); + EXPECT_EQ(err.prefix(), "RUNTIME ERROR"); + EXPECT_EQ(err.location(), "x:1"); + EXPECT_EQ(err.message(), "test"); + EXPECT_EQ(ToString(err), "RUNTIME ERROR: x:1: test"); } } // namespace diff --git a/explorer/common/source_location.h b/explorer/common/source_location.h index ec9fc26b7c9a..2e11e67dcfd1 100644 --- a/explorer/common/source_location.h +++ b/explorer/common/source_location.h @@ -33,6 +33,12 @@ class SourceLocation { void Print(llvm::raw_ostream& out) const { out << filename_ << ":" << line_num_; } + auto ToString() const -> std::string { + std::string result; + llvm::raw_string_ostream out(result); + Print(out); + return result; + } LLVM_DUMP_METHOD void Dump() const { Print(llvm::errs()); } private: diff --git a/explorer/main.cpp b/explorer/main.cpp index 745aefa6e781..b5d0dd37975a 100644 --- a/explorer/main.cpp +++ b/explorer/main.cpp @@ -89,7 +89,7 @@ static auto Main(llvm::StringRef default_prelude_file, int argc, char* argv[]) auto ExplorerMain(llvm::StringRef default_prelude_file, int argc, char** argv) -> int { if (auto result = Main(default_prelude_file, argc, argv); !result.ok()) { - llvm::errs() << result.error().message() << "\n"; + llvm::errs() << result.error() << "\n"; return EXIT_FAILURE; } return EXIT_SUCCESS; diff --git a/explorer/syntax/lexer.lpp b/explorer/syntax/lexer.lpp index e18e040f9886..0b5b334d7662 100644 --- a/explorer/syntax/lexer.lpp +++ b/explorer/syntax/lexer.lpp @@ -249,7 +249,7 @@ operand_start [(A-Za-z0-9_\"] if (intrinsic.ok()) { return CARBON_ARG_TOKEN(intrinsic_identifier, *intrinsic); } else { - return context.RecordSyntaxError(intrinsic.error().message()); + return context.RecordSyntaxError(std::move(intrinsic).error()); } } diff --git a/explorer/syntax/parse.cpp b/explorer/syntax/parse.cpp index ef065afa19f9..81cdc9939c7f 100644 --- a/explorer/syntax/parse.cpp +++ b/explorer/syntax/parse.cpp @@ -29,10 +29,11 @@ static auto ParseImpl(yyscan_t scanner, Nonnull arena, } if (auto syntax_error_code = parser(); syntax_error_code != 0) { - const std::string error_message = context.error_messages().empty() - ? "Unknown parser error" - : context.error_messages()[0]; - return Error(error_message); + auto errors = context.take_errors(); + if (errors.empty()) { + return Error("Unknown parser erroor"); + } + return std::move(errors.front()); } // Return parse results. diff --git a/explorer/syntax/parse_and_lex_context.cpp b/explorer/syntax/parse_and_lex_context.cpp index d2ec69766e6a..a75b16eb89f9 100644 --- a/explorer/syntax/parse_and_lex_context.cpp +++ b/explorer/syntax/parse_and_lex_context.cpp @@ -4,23 +4,20 @@ #include "explorer/syntax/parse_and_lex_context.h" +#include "explorer/common/error_builders.h" + namespace Carbon { -auto ParseAndLexContext::RecordSyntaxError(const std::string& message, - bool prefix_with_newline) - -> Parser::symbol_type { - // Optionally adds a newline in trace mode because trace prints an incomplete - // line "Reading a token: " which can prevent LIT from finding expected - // patterns. - // TODO: support formatting of `SourceLocation` instances with formatv(). - std::string full_message; - llvm::raw_string_ostream(full_message) - << (prefix_with_newline && parser_debug() ? "\n" : "") - << "COMPILATION ERROR: " << source_loc() << ": " << message; - error_messages_.push_back(full_message); +auto ParseAndLexContext::RecordSyntaxError(Error error) -> Parser::symbol_type { + errors_.push_back(std::move(error)); // TODO: use `YYerror` token once bison is upgraded to at least 3.5. return Parser::make_END_OF_FILE(current_token_position); } +auto ParseAndLexContext::RecordSyntaxError(const std::string& message) + -> Parser::symbol_type { + return RecordSyntaxError(CompilationError(source_loc()) << message); +} + } // namespace Carbon diff --git a/explorer/syntax/parse_and_lex_context.h b/explorer/syntax/parse_and_lex_context.h index 358a492672f6..446a6a0affbd 100644 --- a/explorer/syntax/parse_and_lex_context.h +++ b/explorer/syntax/parse_and_lex_context.h @@ -21,11 +21,10 @@ class ParseAndLexContext { bool parser_debug) : input_file_name_(input_file_name), parser_debug_(parser_debug) {} - // Formats ands records a lexer error. Returns an error token as a - // convenience. - auto RecordSyntaxError(const std::string& message, - bool prefix_with_newline = false) - -> Parser::symbol_type; + // Formats ands records a lexing oor parsing error. Returns an error token as + // a convenience. + auto RecordSyntaxError(Error error) -> Parser::symbol_type; + auto RecordSyntaxError(const std::string& message) -> Parser::symbol_type; auto source_loc() const -> SourceLocation { return SourceLocation(input_file_name_, @@ -37,8 +36,10 @@ class ParseAndLexContext { // The source range of the token being (or just) lex'd. location current_token_position; - auto error_messages() const -> const std::vector { - return error_messages_; + auto take_errors() -> std::vector { + std::vector errors = std::move(errors_); + errors_.clear(); + return errors; } private: @@ -48,7 +49,7 @@ class ParseAndLexContext { bool parser_debug_; - std::vector error_messages_; + std::vector errors_; }; } // namespace Carbon diff --git a/explorer/syntax/parser.ypp b/explorer/syntax/parser.ypp index 79e9357baee9..5ef6940a4c55 100644 --- a/explorer/syntax/parser.ypp +++ b/explorer/syntax/parser.ypp @@ -689,7 +689,7 @@ non_expression_pattern: if (alternative_pattern.ok()) { $$ = *alternative_pattern; } else { - context.RecordSyntaxError(alternative_pattern.error().message()); + context.RecordSyntaxError(std::move(alternative_pattern).error()); YYERROR; } } @@ -923,7 +923,7 @@ function_declaration: if (fn.ok()) { $$ = *fn; } else { - context.RecordSyntaxError(fn.error().message()); + context.RecordSyntaxError(std::move(fn).error()); YYERROR; } } @@ -934,7 +934,7 @@ function_declaration: if (fn.ok()) { $$ = *fn; } else { - context.RecordSyntaxError(fn.error().message()); + context.RecordSyntaxError(std::move(fn).error()); YYERROR; } } @@ -1025,7 +1025,7 @@ declaration: if (impl.ok()) { $$ = *impl; } else { - context.RecordSyntaxError(impl.error().message()); + context.RecordSyntaxError(std::move(impl).error()); YYERROR; } } diff --git a/explorer/syntax/prelude.cpp b/explorer/syntax/prelude.cpp index c0c943079b8e..8e62c215d289 100644 --- a/explorer/syntax/prelude.cpp +++ b/explorer/syntax/prelude.cpp @@ -15,8 +15,8 @@ void AddPrelude(std::string_view prelude_file_name, Nonnull arena, if (!parse_result.ok()) { // Try again with tracing, to help diagnose the problem. ErrorOr trace_parse_result = Parse(arena, prelude_file_name, true); - CARBON_FATAL() << "Failed to parse prelude: " - << trace_parse_result.error().message(); + CARBON_FATAL() << "Failed to parse prelude:\n" + << trace_parse_result.error(); } const auto& prelude = *parse_result; declarations->insert(declarations->begin(), prelude.declarations.begin(), diff --git a/explorer/testdata/addr/fail-method-me-misspelled.carbon b/explorer/testdata/addr/fail-method-me-misspelled.carbon new file mode 100644 index 000000000000..eb6f0a563e5b --- /dev/null +++ b/explorer/testdata/addr/fail-method-me-misspelled.carbon @@ -0,0 +1,18 @@ +// 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 +// +// RUN: %{not} %{explorer} %s 2>&1 | \ +// RUN: %{FileCheck} --match-full-lines --allow-unused-prefixes=false %s +// RUN: %{not} %{explorer} --parser_debug --trace_file=- %s 2>&1 | \ +// RUN: %{FileCheck} --match-full-lines --allow-unused-prefixes %s +// AUTOUPDATE: %{explorer} %s + +package ExplorerTest api; + +class C { + // CHECK: COMPILATION ERROR: {{.*}}/explorer/testdata/addr/fail-method-me-misspelled.carbon:[[@LINE+1]]: illegal binding pattern in implicit parameter list + fn F[addr mew: Self*]() {} +} + +fn Main() -> i32 { return 0; } diff --git a/explorer/testdata/as/fail_no_conversion.carbon b/explorer/testdata/as/fail_no_conversion.carbon index 3a4ad3318e5e..6121e783f7de 100644 --- a/explorer/testdata/as/fail_no_conversion.carbon +++ b/explorer/testdata/as/fail_no_conversion.carbon @@ -15,6 +15,6 @@ class A { var n: i32; } fn Main() -> i32 { var a: A = {.n = 5}; // CHECK: COMPILATION ERROR: {{.*}}/explorer/testdata/as/fail_no_conversion.carbon:[[@LINE+2]]: type error in `as`: `class A` is not explicitly convertible to `i32`: - // CHECK: COMPILATION ERROR: {{.*}}/explorer/testdata/as/fail_no_conversion.carbon:[[@LINE+1]]: could not find implementation of interface As(T = i32) for class A + // CHECK: could not find implementation of interface As(T = i32) for class A return a as i32; } diff --git a/explorer/testdata/basic_syntax/fail_unknown_intrinsic.carbon b/explorer/testdata/basic_syntax/fail_unknown_intrinsic.carbon new file mode 100644 index 000000000000..c5724dab6c6f --- /dev/null +++ b/explorer/testdata/basic_syntax/fail_unknown_intrinsic.carbon @@ -0,0 +1,16 @@ +// 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 +// +// RUN: %{not} %{explorer} %s 2>&1 | \ +// RUN: %{FileCheck} --match-full-lines --allow-unused-prefixes=false %s +// RUN: %{not} %{explorer} --parser_debug --trace_file=- %s 2>&1 | \ +// RUN: %{FileCheck} --match-full-lines --allow-unused-prefixes %s +// AUTOUPDATE: %{explorer} %s + +package ExplorerTest api; + +fn Main() -> i32 { + // CHECK: COMPILATION ERROR: {{.*}}/explorer/testdata/basic_syntax/fail_unknown_intrinsic.carbon:[[@LINE+1]]: Unknown intrinsic 'nonexistent' + return __intrinsic_nonexistent(); +} diff --git a/explorer/testdata/generic_function/fail_missing_exclam.carbon b/explorer/testdata/generic_function/fail_missing_exclam.carbon new file mode 100644 index 000000000000..6e08c8bd963a --- /dev/null +++ b/explorer/testdata/generic_function/fail_missing_exclam.carbon @@ -0,0 +1,18 @@ +// 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 +// +// RUN: %{not} %{explorer} %s 2>&1 | \ +// RUN: %{FileCheck} --match-full-lines --allow-unused-prefixes=false %s +// RUN: %{not} %{explorer} --parser_debug --trace_file=- %s 2>&1 | \ +// RUN: %{FileCheck} --match-full-lines --allow-unused-prefixes %s +// AUTOUPDATE: %{explorer} %s + +package ExplorerTest api; + +// CHECK: COMPILATION ERROR: {{.*}}/explorer/testdata/generic_function/fail_missing_exclam.carbon:[[@LINE+1]]: illegal binding pattern in implicit parameter list +fn F[T: Type](); + +fn Main() -> i32 { + return 0; +} diff --git a/explorer/testdata/match/fail_not_alternative.carbon b/explorer/testdata/match/fail_not_alternative.carbon new file mode 100644 index 000000000000..36b7536d8433 --- /dev/null +++ b/explorer/testdata/match/fail_not_alternative.carbon @@ -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 +// +// RUN: %{not} %{explorer} %s 2>&1 | \ +// RUN: %{FileCheck} --match-full-lines --allow-unused-prefixes=false %s +// RUN: %{not} %{explorer} --parser_debug --trace_file=- %s 2>&1 | \ +// RUN: %{FileCheck} --match-full-lines --allow-unused-prefixes %s +// AUTOUPDATE: %{explorer} %s +// CHECK: PROGRAM ERROR: {{.*}}/explorer/testdata/match/fail_not_alternative.carbon:17: Alternative pattern must have the form of a field access. + +package ExplorerTest api; + +fn Main() -> i32 { + var x: i32 = 0; + match (x) { + case i32(n: i32) => { + return 1; + } + } + return 0; +}