From 87e58f5db328bc2512bde030b788de4e247bb4fb Mon Sep 17 00:00:00 2001 From: Jon Meow Date: Wed, 13 Apr 2022 09:05:21 -0700 Subject: [PATCH] Switch executable semantics to use ErrorBuilder directly and relocate macros (#1184) --- common/error.h | 53 +++++++++ common/error_test.cpp | 48 ++++++++ executable_semantics/README.md | 4 +- executable_semantics/ast/BUILD | 35 ++---- executable_semantics/ast/ast_node.h | 2 +- executable_semantics/ast/declaration.cpp | 4 +- executable_semantics/ast/declaration.h | 2 +- executable_semantics/ast/expression.cpp | 5 +- executable_semantics/ast/expression.h | 2 +- executable_semantics/ast/paren_contents.h | 4 +- executable_semantics/ast/pattern.cpp | 4 +- executable_semantics/ast/pattern.h | 2 +- executable_semantics/ast/return_term.h | 2 +- executable_semantics/ast/statement.h | 2 +- executable_semantics/ast/static_scope.cpp | 9 +- executable_semantics/ast/static_scope.h | 2 +- executable_semantics/common/BUILD | 26 ++-- executable_semantics/common/error.h | 109 ----------------- executable_semantics/common/error_builders.h | 44 +++++++ .../common/error_builders_test.cpp | 30 +++++ executable_semantics/common/error_test.cpp | 101 ---------------- .../{ast => common}/source_location.h | 6 +- executable_semantics/fuzzing/BUILD | 2 +- executable_semantics/fuzzing/fuzzverter.cpp | 2 +- executable_semantics/interpreter/BUILD | 12 +- .../interpreter/action_stack.cpp | 2 +- executable_semantics/interpreter/heap.cpp | 4 +- executable_semantics/interpreter/heap.h | 2 +- .../interpreter/impl_scope.cpp | 12 +- .../interpreter/interpreter.cpp | 6 +- .../interpreter/resolve_control_flow.cpp | 12 +- .../interpreter/resolve_names.cpp | 5 +- .../interpreter/type_checker.cpp | 112 +++++++++--------- executable_semantics/interpreter/value.cpp | 23 ++-- executable_semantics/syntax/BUILD | 2 +- executable_semantics/syntax/parse.cpp | 9 +- .../testdata/array/fail_index.carbon | 2 +- 37 files changed, 327 insertions(+), 376 deletions(-) delete mode 100644 executable_semantics/common/error.h create mode 100644 executable_semantics/common/error_builders.h create mode 100644 executable_semantics/common/error_builders_test.cpp delete mode 100644 executable_semantics/common/error_test.cpp rename executable_semantics/{ast => common}/source_location.h (88%) diff --git a/common/error.h b/common/error.h index 7b206a3456ed..5dea8ecb7b23 100644 --- a/common/error.h +++ b/common/error.h @@ -104,6 +104,59 @@ class [[nodiscard]] ErrorOr { std::variant val_; }; +// A helper class for accumulating error message and converting to +// `Error` and `ErrorOr`. +class ErrorBuilder { + public: + ErrorBuilder() : out_(std::make_unique(message_)) {} + + // Accumulates string message. + template + [[nodiscard]] auto operator<<(const T& message) -> ErrorBuilder& { + *out_ << message; + return *this; + } + + // NOLINTNEXTLINE(google-explicit-constructor): Implicit cast for returns. + operator Error() { return Error(message_); } + + template + // NOLINTNEXTLINE(google-explicit-constructor): Implicit cast for returns. + operator ErrorOr() { + return Error(message_); + } + + private: + std::string message_; + // Use a pointer to allow move construction. + std::unique_ptr out_; +}; + } // namespace Carbon +// Macro hackery to get a unique variable name. +#define MAKE_UNIQUE_NAME_IMPL(a, b, c) a##b##c +#define MAKE_UNIQUE_NAME(a, b, c) MAKE_UNIQUE_NAME_IMPL(a, b, c) + +#define RETURN_IF_ERROR_IMPL(unique_name, expr) \ + if (auto unique_name = (expr); /* NOLINT(bugprone-macro-parentheses) */ \ + !(unique_name).ok()) { \ + return std::move(unique_name).error(); \ + } + +#define RETURN_IF_ERROR(expr) \ + RETURN_IF_ERROR_IMPL( \ + MAKE_UNIQUE_NAME(_llvm_error_line, __LINE__, __COUNTER__), expr) + +#define ASSIGN_OR_RETURN_IMPL(unique_name, var, expr) \ + auto unique_name = (expr); /* NOLINT(bugprone-macro-parentheses) */ \ + if (!(unique_name).ok()) { \ + return std::move(unique_name).error(); \ + } \ + var = std::move(*(unique_name)); /* NOLINT(bugprone-macro-parentheses) */ + +#define ASSIGN_OR_RETURN(var, expr) \ + ASSIGN_OR_RETURN_IMPL( \ + MAKE_UNIQUE_NAME(_llvm_expected_line, __LINE__, __COUNTER__), var, expr) + #endif // COMMON_ERROR_H_ diff --git a/common/error_test.cpp b/common/error_test.cpp index df93c6b251b4..c1b976dae442 100644 --- a/common/error_test.cpp +++ b/common/error_test.cpp @@ -49,5 +49,53 @@ TEST(ErrorTest, IndirectErrorOrSuccess) { EXPECT_TRUE(IndirectErrorOrSuccessTest().ok()); } +TEST(ErrorTest, ReturnIfErrorNoError) { + auto result = []() -> ErrorOr { + RETURN_IF_ERROR(ErrorOr(Success())); + RETURN_IF_ERROR(ErrorOr(Success())); + return Success(); + }(); + EXPECT_TRUE(result.ok()); +} + +TEST(ErrorTest, ReturnIfErrorHasError) { + auto result = []() -> ErrorOr { + RETURN_IF_ERROR(ErrorOr(Success())); + RETURN_IF_ERROR(ErrorOr(Error("error"))); + return Success(); + }(); + ASSERT_FALSE(result.ok()); + EXPECT_EQ(result.error().message(), "error"); +} + +TEST(ErrorTest, AssignOrReturnNoError) { + auto result = []() -> ErrorOr { + ASSIGN_OR_RETURN(int a, ErrorOr(1)); + ASSIGN_OR_RETURN(const int b, ErrorOr(2)); + int c = 0; + ASSIGN_OR_RETURN(c, ErrorOr(3)); + return a + b + c; + }(); + ASSERT_TRUE(result.ok()); + EXPECT_EQ(6, *result); +} + +TEST(ErrorTest, AssignOrReturnHasDirectError) { + auto result = []() -> ErrorOr { + RETURN_IF_ERROR(ErrorOr(Error("error"))); + return 0; + }(); + ASSERT_FALSE(result.ok()); +} + +TEST(ErrorTest, AssignOrReturnHasErrorInExpected) { + auto result = []() -> ErrorOr { + ASSIGN_OR_RETURN(int a, ErrorOr(Error("error"))); + return a; + }(); + ASSERT_FALSE(result.ok()); + EXPECT_EQ(result.error().message(), "error"); +} + } // namespace } // namespace Carbon::Testing diff --git a/executable_semantics/README.md b/executable_semantics/README.md index f422d261a138..38826bba8def 100644 --- a/executable_semantics/README.md +++ b/executable_semantics/README.md @@ -54,8 +54,8 @@ lifetime groups in the future. For simplicity, `executable_semantics` generally treats all errors as fatal. Errors caused by bugs in the user-provided Carbon code should be reported with -the macros in [`error.h`](common/error.h). Errors caused by bugs in -`executable_semantics` itself should be reported with +the error builders in [`error_builders.h`](common/error_builders.h). Errors +caused by bugs in `executable_semantics` itself should be reported with [`CHECK` or `FATAL`](../common/check.h). ## Example Programs (Regression Tests) diff --git a/executable_semantics/ast/BUILD b/executable_semantics/ast/BUILD index ea254ae4ac6a..6a62fa02a82a 100644 --- a/executable_semantics/ast/BUILD +++ b/executable_semantics/ast/BUILD @@ -22,7 +22,7 @@ cc_library( "ast_rtti.h", ], deps = [ - ":source_location", + "//executable_semantics/common:source_location", "@llvm-project//llvm:Support", ], ) @@ -77,11 +77,11 @@ cc_library( deps = [ ":ast_node", ":pattern", - ":source_location", ":value_category", "//common:check", "//common:ostream", "//executable_semantics/common:nonnull", + "//executable_semantics/common:source_location", "@llvm-project//llvm:Support", ], ) @@ -97,12 +97,12 @@ cc_library( ":impl_binding", ":pattern", ":return_term", - ":source_location", ":statement", ":static_scope", ":value_category", "//common:ostream", "//executable_semantics/common:nonnull", + "//executable_semantics/common:source_location", "@llvm-project//llvm:Support", ], ) @@ -112,10 +112,10 @@ cc_library( hdrs = ["return_term.h"], deps = [ ":expression", - ":source_location", "//common:check", "//common:ostream", "//executable_semantics/common:nonnull", + "//executable_semantics/common:source_location", "@llvm-project//llvm:Support", ], ) @@ -127,13 +127,13 @@ cc_library( deps = [ ":ast_node", ":paren_contents", - ":source_location", ":static_scope", ":value_category", "//common:indirect_value", "//common:ostream", "//executable_semantics/common:arena", - "//executable_semantics/common:error", + "//executable_semantics/common:error_builders", + "//executable_semantics/common:source_location", "@llvm-project//llvm:Support", ], ) @@ -159,8 +159,8 @@ cc_library( name = "paren_contents", hdrs = ["paren_contents.h"], deps = [ - ":source_location", - "//executable_semantics/common:error", + "//executable_semantics/common:error_builders", + "//executable_semantics/common:source_location", ], ) @@ -171,12 +171,12 @@ cc_library( deps = [ ":ast_node", ":expression", - ":source_location", ":static_scope", ":value_category", "//common:ostream", "//executable_semantics/common:arena", - "//executable_semantics/common:error", + "//executable_semantics/common:error_builders", + "//executable_semantics/common:source_location", "@llvm-project//llvm:Support", ], ) @@ -200,25 +200,16 @@ cc_library( hdrs = ["static_scope.h"], deps = [ ":ast_node", - ":source_location", ":value_category", "//common:check", "//common:error", - "//executable_semantics/common:error", + "//executable_semantics/common:error_builders", "//executable_semantics/common:nonnull", + "//executable_semantics/common:source_location", "@llvm-project//llvm:Support", ], ) -cc_library( - name = "source_location", - hdrs = ["source_location.h"], - deps = [ - "//common:ostream", - "//executable_semantics/common:nonnull", - ], -) - cc_library( name = "statement", srcs = ["statement.cpp"], @@ -228,12 +219,12 @@ cc_library( ":expression", ":pattern", ":return_term", - ":source_location", ":static_scope", ":value_category", "//common:check", "//common:ostream", "//executable_semantics/common:arena", + "//executable_semantics/common:source_location", "@llvm-project//llvm:Support", ], ) diff --git a/executable_semantics/ast/ast_node.h b/executable_semantics/ast/ast_node.h index ed5fdfc4b2b3..8c0724d95a26 100644 --- a/executable_semantics/ast/ast_node.h +++ b/executable_semantics/ast/ast_node.h @@ -6,7 +6,7 @@ #define EXECUTABLE_SEMANTICS_AST_AST_NODE_H_ #include "executable_semantics/ast/ast_rtti.h" -#include "executable_semantics/ast/source_location.h" +#include "executable_semantics/common/source_location.h" #include "llvm/Support/Casting.h" namespace Carbon { diff --git a/executable_semantics/ast/declaration.cpp b/executable_semantics/ast/declaration.cpp index 7b4074fc5adf..575c8aac5e97 100644 --- a/executable_semantics/ast/declaration.cpp +++ b/executable_semantics/ast/declaration.cpp @@ -175,14 +175,14 @@ auto FunctionDeclaration::Create( case AstNodeKind::BindingPattern: { Nonnull bp = &cast(*param); if (me_pattern.has_value() || bp->name() != "me") { - return FATAL_COMPILATION_ERROR(source_loc) + return CompilationError(source_loc) << "illegal binding pattern in implicit parameter list"; } me_pattern = bp; break; } default: - return FATAL_COMPILATION_ERROR(source_loc) + return CompilationError(source_loc) << "illegal AST node in implicit parameter list"; } } diff --git a/executable_semantics/ast/declaration.h b/executable_semantics/ast/declaration.h index 34c1b66c8184..6f18c090db16 100644 --- a/executable_semantics/ast/declaration.h +++ b/executable_semantics/ast/declaration.h @@ -14,11 +14,11 @@ #include "executable_semantics/ast/impl_binding.h" #include "executable_semantics/ast/pattern.h" #include "executable_semantics/ast/return_term.h" -#include "executable_semantics/ast/source_location.h" #include "executable_semantics/ast/statement.h" #include "executable_semantics/ast/static_scope.h" #include "executable_semantics/ast/value_category.h" #include "executable_semantics/common/nonnull.h" +#include "executable_semantics/common/source_location.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/Support/Compiler.h" diff --git a/executable_semantics/ast/expression.cpp b/executable_semantics/ast/expression.cpp index c1ddc1cee728..4e62cf478e76 100644 --- a/executable_semantics/ast/expression.cpp +++ b/executable_semantics/ast/expression.cpp @@ -8,7 +8,7 @@ #include #include "executable_semantics/common/arena.h" -#include "executable_semantics/common/error.h" +#include "executable_semantics/common/error_builders.h" #include "llvm/ADT/StringExtras.h" #include "llvm/Support/Casting.h" #include "llvm/Support/raw_ostream.h" @@ -26,8 +26,7 @@ auto IntrinsicExpression::FindIntrinsic(std::string_view name, name.remove_prefix(std::strlen("__intrinsic_")); auto it = intrinsic_map.find(name); if (it == intrinsic_map.end()) { - return FATAL_COMPILATION_ERROR(source_loc) - << "Unknown intrinsic '" << name << "'"; + return CompilationError(source_loc) << "Unknown intrinsic '" << name << "'"; } return it->second; } diff --git a/executable_semantics/ast/expression.h b/executable_semantics/ast/expression.h index 4d9668d995bd..b8d6eb51b78b 100644 --- a/executable_semantics/ast/expression.h +++ b/executable_semantics/ast/expression.h @@ -14,10 +14,10 @@ #include "common/ostream.h" #include "executable_semantics/ast/ast_node.h" #include "executable_semantics/ast/paren_contents.h" -#include "executable_semantics/ast/source_location.h" #include "executable_semantics/ast/static_scope.h" #include "executable_semantics/ast/value_category.h" #include "executable_semantics/common/arena.h" +#include "executable_semantics/common/source_location.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/Support/Compiler.h" diff --git a/executable_semantics/ast/paren_contents.h b/executable_semantics/ast/paren_contents.h index 153222e2b643..0872bb82282e 100644 --- a/executable_semantics/ast/paren_contents.h +++ b/executable_semantics/ast/paren_contents.h @@ -9,8 +9,8 @@ #include #include -#include "executable_semantics/ast/source_location.h" -#include "executable_semantics/common/error.h" +#include "executable_semantics/common/error_builders.h" +#include "executable_semantics/common/source_location.h" namespace Carbon { diff --git a/executable_semantics/ast/pattern.cpp b/executable_semantics/ast/pattern.cpp index 41efd10a5385..bda4f4369850 100644 --- a/executable_semantics/ast/pattern.cpp +++ b/executable_semantics/ast/pattern.cpp @@ -9,7 +9,7 @@ #include "common/ostream.h" #include "executable_semantics/ast/expression.h" #include "executable_semantics/common/arena.h" -#include "executable_semantics/common/error.h" +#include "executable_semantics/common/error_builders.h" #include "llvm/ADT/StringExtras.h" #include "llvm/Support/Casting.h" @@ -151,7 +151,7 @@ auto TuplePatternFromParenContents(Nonnull arena, auto AlternativePattern::RequireFieldAccess(Nonnull alternative) -> ErrorOr> { if (alternative->kind() != ExpressionKind::FieldAccessExpression) { - return FATAL_PROGRAM_ERROR(alternative->source_loc()) + return ProgramError(alternative->source_loc()) << "Alternative pattern must have the form of a field access."; } return &cast(*alternative); diff --git a/executable_semantics/ast/pattern.h b/executable_semantics/ast/pattern.h index 3d909ec29499..5e183278c314 100644 --- a/executable_semantics/ast/pattern.h +++ b/executable_semantics/ast/pattern.h @@ -13,9 +13,9 @@ #include "executable_semantics/ast/ast_node.h" #include "executable_semantics/ast/ast_rtti.h" #include "executable_semantics/ast/expression.h" -#include "executable_semantics/ast/source_location.h" #include "executable_semantics/ast/static_scope.h" #include "executable_semantics/ast/value_category.h" +#include "executable_semantics/common/source_location.h" #include "llvm/ADT/ArrayRef.h" namespace Carbon { diff --git a/executable_semantics/ast/return_term.h b/executable_semantics/ast/return_term.h index e6cf613ac3e5..593d322e656a 100644 --- a/executable_semantics/ast/return_term.h +++ b/executable_semantics/ast/return_term.h @@ -11,8 +11,8 @@ #include "common/check.h" #include "common/ostream.h" #include "executable_semantics/ast/expression.h" -#include "executable_semantics/ast/source_location.h" #include "executable_semantics/common/nonnull.h" +#include "executable_semantics/common/source_location.h" namespace Carbon { diff --git a/executable_semantics/ast/statement.h b/executable_semantics/ast/statement.h index 1a7c3bfda964..432595f8c28a 100644 --- a/executable_semantics/ast/statement.h +++ b/executable_semantics/ast/statement.h @@ -13,10 +13,10 @@ #include "executable_semantics/ast/expression.h" #include "executable_semantics/ast/pattern.h" #include "executable_semantics/ast/return_term.h" -#include "executable_semantics/ast/source_location.h" #include "executable_semantics/ast/static_scope.h" #include "executable_semantics/ast/value_category.h" #include "executable_semantics/common/arena.h" +#include "executable_semantics/common/source_location.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/Support/Compiler.h" diff --git a/executable_semantics/ast/static_scope.cpp b/executable_semantics/ast/static_scope.cpp index da45837c18df..985919cfda54 100644 --- a/executable_semantics/ast/static_scope.cpp +++ b/executable_semantics/ast/static_scope.cpp @@ -4,7 +4,7 @@ #include "executable_semantics/ast/static_scope.h" -#include "executable_semantics/common/error.h" +#include "executable_semantics/common/error_builders.h" #include "llvm/Support/Error.h" namespace Carbon { @@ -13,7 +13,7 @@ auto StaticScope::Add(std::string name, ValueNodeView entity) -> ErrorOr { auto [it, success] = declared_names_.insert({name, entity}); if (!success && it->second != entity) { - return FATAL_COMPILATION_ERROR(entity.base().source_loc()) + return CompilationError(entity.base().source_loc()) << "Duplicate name `" << name << "` also found at " << it->second.base().source_loc(); } @@ -26,8 +26,7 @@ auto StaticScope::Resolve(const std::string& name, ASSIGN_OR_RETURN(std::optional result, TryResolve(name, source_loc)); if (!result) { - return FATAL_COMPILATION_ERROR(source_loc) - << "could not resolve '" << name << "'"; + return CompilationError(source_loc) << "could not resolve '" << name << "'"; } return *result; } @@ -45,7 +44,7 @@ auto StaticScope::TryResolve(const std::string& name, parent->TryResolve(name, source_loc)); if (parent_result.has_value() && result.has_value() && *parent_result != *result) { - return FATAL_COMPILATION_ERROR(source_loc) + return CompilationError(source_loc) << "'" << name << "' is ambiguous between " << result->base().source_loc() << " and " << parent_result->base().source_loc(); diff --git a/executable_semantics/ast/static_scope.h b/executable_semantics/ast/static_scope.h index b2ca222047f2..ea91025924b0 100644 --- a/executable_semantics/ast/static_scope.h +++ b/executable_semantics/ast/static_scope.h @@ -14,9 +14,9 @@ #include "common/check.h" #include "common/error.h" #include "executable_semantics/ast/ast_node.h" -#include "executable_semantics/ast/source_location.h" #include "executable_semantics/ast/value_category.h" #include "executable_semantics/common/nonnull.h" +#include "executable_semantics/common/source_location.h" #include "llvm/Support/Error.h" namespace Carbon { diff --git a/executable_semantics/common/BUILD b/executable_semantics/common/BUILD index 85677f60fd29..c3f7ea6ffe71 100644 --- a/executable_semantics/common/BUILD +++ b/executable_semantics/common/BUILD @@ -13,23 +13,21 @@ cc_library( ) cc_library( - name = "error", - hdrs = ["error.h"], + name = "error_builders", + hdrs = ["error_builders.h"], deps = [ - "//common:check", + ":source_location", "//common:error", - "//executable_semantics/ast:source_location", - "@llvm-project//llvm:Support", ], ) cc_test( - name = "error_test", - srcs = ["error_test.cpp"], + name = "error_builders_test", + srcs = ["error_builders_test.cpp"], deps = [ - ":error", + ":error_builders", + ":source_location", "@com_google_googletest//:gtest_main", - "@llvm-project//llvm:Support", ], ) @@ -38,6 +36,14 @@ cc_library( hdrs = ["nonnull.h"], deps = [ "//common:check", - "@llvm-project//llvm:Support", + ], +) + +cc_library( + name = "source_location", + hdrs = ["source_location.h"], + deps = [ + ":nonnull", + "//common:ostream", ], ) diff --git a/executable_semantics/common/error.h b/executable_semantics/common/error.h deleted file mode 100644 index 678b8bcca4d1..000000000000 --- a/executable_semantics/common/error.h +++ /dev/null @@ -1,109 +0,0 @@ -// 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 EXECUTABLE_SEMANTICS_COMMON_ERROR_H_ -#define EXECUTABLE_SEMANTICS_COMMON_ERROR_H_ - -#include - -#include "common/check.h" -#include "common/error.h" -#include "executable_semantics/ast/source_location.h" -#include "llvm/Support/Signals.h" -#include "llvm/Support/raw_ostream.h" - -namespace Carbon { - -// A helper class for accumulating error message and converting to -// `Carbon::Error`/`Carbon::ErrorOr`. -class ErrorBuilder { - public: - explicit ErrorBuilder(std::optional loc = std::nullopt) - : out_(message_) { - if (loc.has_value()) { - out_ << *loc << ": "; - } - } - - // Accumulates string message. - template - [[nodiscard]] auto operator<<(const T& message) -> ErrorBuilder& { - out_ << message; - return *this; - } - - // NOLINTNEXTLINE(google-explicit-constructor): Implicit cast for returns. - operator Error() { return Error(message_); } - - template - // NOLINTNEXTLINE(google-explicit-constructor): Implicit cast for returns. - operator ErrorOr() { - return Error(message_); - } - - std::string message_; - llvm::raw_string_ostream out_; -}; - -// Builds a Carbon::Error instance with the specified message. This should be -// used for non-recoverable errors with user input. -// -// For example: -// return FATAL_PROGRAM_ERROR(line_num) << "Line is bad!"; -// return FATAL_PROGRAM_ERROR_NO_LINE() << "Application is bad!"; -// -// Where possible, try to identify the error as a compilation or -// runtime error. Use CHECK/FATAL for internal errors. The generic program -// error option is provided as a fallback for cases that don't fit those -// classifications. -// -// TODO: replace below macro invocations with direct `return ErrorBuilder() << -// xx` calls. - -#define FATAL_PROGRAM_ERROR_NO_LINE() \ - Carbon::ErrorBuilder() << "PROGRAM ERROR: " - -#define FATAL_PROGRAM_ERROR(line) \ - FATAL_PROGRAM_ERROR_NO_LINE() << (line) << ": " - -#define FATAL_COMPILATION_ERROR_NO_LINE() \ - Carbon::ErrorBuilder() << "COMPILATION ERROR: " - -#define FATAL_COMPILATION_ERROR(line) \ - FATAL_COMPILATION_ERROR_NO_LINE() << (line) << ": " - -#define FATAL_RUNTIME_ERROR_NO_LINE() \ - Carbon::ErrorBuilder() << "RUNTIME ERROR: " - -#define FATAL_RUNTIME_ERROR(line) \ - FATAL_RUNTIME_ERROR_NO_LINE() << (line) << ": " - -// Macro hackery to get a unique variable name. -#define MAKE_UNIQUE_NAME_IMPL(a, b, c) a##b##c -#define MAKE_UNIQUE_NAME(a, b, c) MAKE_UNIQUE_NAME_IMPL(a, b, c) - -#define RETURN_IF_ERROR_IMPL(unique_name, expr) \ - if (auto unique_name = (expr); /* NOLINT(bugprone-macro-parentheses) */ \ - !(unique_name).ok()) { \ - return std::move(unique_name).error(); \ - } - -#define RETURN_IF_ERROR(expr) \ - RETURN_IF_ERROR_IMPL( \ - MAKE_UNIQUE_NAME(_llvm_error_line, __LINE__, __COUNTER__), expr) - -#define ASSIGN_OR_RETURN_IMPL(unique_name, var, expr) \ - auto unique_name = (expr); /* NOLINT(bugprone-macro-parentheses) */ \ - if (!(unique_name).ok()) { \ - return std::move(unique_name).error(); \ - } \ - var = std::move(*(unique_name)); /* NOLINT(bugprone-macro-parentheses) */ - -#define ASSIGN_OR_RETURN(var, expr) \ - ASSIGN_OR_RETURN_IMPL( \ - MAKE_UNIQUE_NAME(_llvm_expected_line, __LINE__, __COUNTER__), var, expr) - -} // namespace Carbon - -#endif // EXECUTABLE_SEMANTICS_COMMON_ERROR_H_ diff --git a/executable_semantics/common/error_builders.h b/executable_semantics/common/error_builders.h new file mode 100644 index 000000000000..5a140168bf50 --- /dev/null +++ b/executable_semantics/common/error_builders.h @@ -0,0 +1,44 @@ +// 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 EXECUTABLE_SEMANTICS_COMMON_ERROR_BUILDERS_H_ +#define EXECUTABLE_SEMANTICS_COMMON_ERROR_BUILDERS_H_ + +#include "common/error.h" +#include "executable_semantics/common/source_location.h" + +namespace Carbon { + +// Builds an Error instance with the specified message. This should be used for +// non-recoverable errors with user input. +// +// For example: +// return ProgramError(line_num) << "Line is bad!"; +// return ProgramError() << "Application is bad!"; +// +// Where possible, try to identify the error as a compilation or runtime error. +// Use CHECK/FATAL for internal errors. The generic program error option is +// 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; +} + +inline auto ProgramError(SourceLocation loc) -> ErrorBuilder { + ErrorBuilder builder; + (void)(builder << "PROGRAM ERROR: " << loc << ": "); + return builder; +} + +inline auto RuntimeError(SourceLocation loc) -> ErrorBuilder { + ErrorBuilder builder; + (void)(builder << "RUNTIME ERROR: " << loc << ": "); + return builder; +} + +} // namespace Carbon + +#endif // EXECUTABLE_SEMANTICS_COMMON_ERROR_BUILDERS_H_ diff --git a/executable_semantics/common/error_builders_test.cpp b/executable_semantics/common/error_builders_test.cpp new file mode 100644 index 000000000000..8aef7f970847 --- /dev/null +++ b/executable_semantics/common/error_builders_test.cpp @@ -0,0 +1,30 @@ +// 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 "executable_semantics/common/error_builders.h" + +#include + +#include "executable_semantics/common/source_location.h" + +namespace Carbon::Testing { +namespace { + +TEST(ErrorBuildersTest, CompilationError) { + Error err = CompilationError(SourceLocation("x", 1)) << "test"; + EXPECT_EQ(err.message(), "COMPILATION ERROR: x:1: test"); +} + +TEST(ErrorBuildersTest, ProgramError) { + Error err = ProgramError(SourceLocation("x", 1)) << "test"; + EXPECT_EQ(err.message(), "PROGRAM ERROR: x:1: test"); +} + +TEST(ErrorBuildersTest, RuntimeError) { + Error err = RuntimeError(SourceLocation("x", 1)) << "test"; + EXPECT_EQ(err.message(), "RUNTIME ERROR: x:1: test"); +} + +} // namespace +} // namespace Carbon::Testing diff --git a/executable_semantics/common/error_test.cpp b/executable_semantics/common/error_test.cpp deleted file mode 100644 index 4fddaa2c0cdc..000000000000 --- a/executable_semantics/common/error_test.cpp +++ /dev/null @@ -1,101 +0,0 @@ -// 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 "executable_semantics/common/error.h" - -#include - -namespace Carbon::Testing { -namespace { - -auto MakeSuccess() -> ErrorOr { return Success(); } - -auto MakeError(std::string_view message) -> ErrorOr { - return Error(message); -} - -auto MakeInt(int value) -> ErrorOr { return value; } - -auto MakeFailedInt(std::string_view message) -> ErrorOr { - return Error(message); -} - -auto ErrorToString(const Error& e) -> std::string { return e.message(); } - -template -auto ErrorToString(const ErrorOr& e) -> std::string { - return e.error().message(); -} - -TEST(ErrorTest, FatalProgramError) { - EXPECT_EQ(ErrorToString(FATAL_PROGRAM_ERROR_NO_LINE() << "test"), - "PROGRAM ERROR: test"); -} - -TEST(ErrorTest, FatalRuntimeError) { - EXPECT_EQ(ErrorToString(FATAL_RUNTIME_ERROR_NO_LINE() << "test"), - "RUNTIME ERROR: test"); -} - -TEST(ErrorTest, FatalCompilationError) { - EXPECT_EQ(ErrorToString(FATAL_COMPILATION_ERROR_NO_LINE() << "test"), - "COMPILATION ERROR: test"); -} - -TEST(ErrorTest, FatalProgramErrorLine) { - EXPECT_EQ(ErrorToString(FATAL_PROGRAM_ERROR(1) << "test"), - "PROGRAM ERROR: 1: test"); -} - -TEST(ErrorTest, ReturnIfErrorNoError) { - auto result = []() -> ErrorOr { - RETURN_IF_ERROR(MakeSuccess()); - RETURN_IF_ERROR(MakeSuccess()); - return Success(); - }(); - EXPECT_TRUE(result.ok()); -} - -TEST(ErrorTest, ReturnIfErrorHasError) { - auto result = []() -> ErrorOr { - RETURN_IF_ERROR(MakeSuccess()); - RETURN_IF_ERROR(MakeError("error")); - return Success(); - }(); - ASSERT_FALSE(result.ok()); - EXPECT_EQ(ErrorToString(result), "error"); -} - -TEST(ErrorTest, AssignOrReturnNoError) { - auto result = []() -> ErrorOr { - RETURN_IF_ERROR(MakeSuccess()); - ASSIGN_OR_RETURN(int a, MakeInt(1)); - ASSIGN_OR_RETURN(const int b, MakeInt(2)); - int c = 0; - ASSIGN_OR_RETURN(c, MakeInt(3)); - return a + b + c; - }(); - ASSERT_TRUE(result.ok()); - EXPECT_EQ(6, *result); -} - -TEST(ErrorTest, AssignOrReturnHasDirectError) { - auto result = []() -> ErrorOr { - RETURN_IF_ERROR(MakeError("error")); - return 0; - }(); - ASSERT_FALSE(result.ok()); -} - -TEST(ErrorTest, AssignOrReturnHasErrorInExpected) { - auto result = []() -> ErrorOr { - ASSIGN_OR_RETURN(int a, MakeFailedInt("error")); - return a; - }(); - ASSERT_FALSE(result.ok()); - EXPECT_EQ(ErrorToString(result), "error"); -} - -} // namespace -} // namespace Carbon::Testing diff --git a/executable_semantics/ast/source_location.h b/executable_semantics/common/source_location.h similarity index 88% rename from executable_semantics/ast/source_location.h rename to executable_semantics/common/source_location.h index ec0fa4e6e737..9f4c3dad30e3 100644 --- a/executable_semantics/ast/source_location.h +++ b/executable_semantics/common/source_location.h @@ -2,8 +2,8 @@ // Exceptions. See /LICENSE for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -#ifndef EXECUTABLE_SEMANTICS_AST_SOURCE_LOCATION_H_ -#define EXECUTABLE_SEMANTICS_AST_SOURCE_LOCATION_H_ +#ifndef EXECUTABLE_SEMANTICS_COMMON_SOURCE_LOCATION_H_ +#define EXECUTABLE_SEMANTICS_COMMON_SOURCE_LOCATION_H_ #include #include @@ -42,4 +42,4 @@ class SourceLocation { } // namespace Carbon -#endif // EXECUTABLE_SEMANTICS_AST_SOURCE_LOCATION_H_ +#endif // EXECUTABLE_SEMANTICS_COMMON_SOURCE_LOCATION_H_ diff --git a/executable_semantics/fuzzing/BUILD b/executable_semantics/fuzzing/BUILD index 315775bee1aa..93d8c6625dc5 100644 --- a/executable_semantics/fuzzing/BUILD +++ b/executable_semantics/fuzzing/BUILD @@ -58,7 +58,7 @@ cc_binary( ":fuzzer_util", "//common:error", "//common/fuzzing:carbon_cc_proto", - "//executable_semantics/common:error", + "//executable_semantics/common:error_builders", "//executable_semantics/common:nonnull", "//executable_semantics/syntax", "@com_google_protobuf//:protobuf_headers", diff --git a/executable_semantics/fuzzing/fuzzverter.cpp b/executable_semantics/fuzzing/fuzzverter.cpp index 1da75077e207..9e4ea5804014 100644 --- a/executable_semantics/fuzzing/fuzzverter.cpp +++ b/executable_semantics/fuzzing/fuzzverter.cpp @@ -19,7 +19,7 @@ #include "common/error.h" #include "common/fuzzing/carbon.pb.h" -#include "executable_semantics/common/error.h" +#include "executable_semantics/common/error_builders.h" #include "executable_semantics/fuzzing/ast_to_proto.h" #include "executable_semantics/fuzzing/fuzzer_util.h" #include "executable_semantics/syntax/parse.h" diff --git a/executable_semantics/interpreter/BUILD b/executable_semantics/interpreter/BUILD index 5cc22a7f34b0..76d67cf638ec 100644 --- a/executable_semantics/interpreter/BUILD +++ b/executable_semantics/interpreter/BUILD @@ -29,7 +29,7 @@ cc_library( "//executable_semantics/ast:pattern", "//executable_semantics/ast:statement", "//executable_semantics/common:arena", - "//executable_semantics/common:error", + "//executable_semantics/common:error_builders", "//executable_semantics/common:nonnull", "@llvm-project//llvm:Support", ], @@ -105,9 +105,9 @@ cc_library( ":address", ":heap_allocation_interface", "//common:ostream", - "//executable_semantics/ast:source_location", - "//executable_semantics/common:error", + "//executable_semantics/common:error_builders", "//executable_semantics/common:nonnull", + "//executable_semantics/common:source_location", "@llvm-project//llvm:Support", ], ) @@ -143,7 +143,7 @@ cc_library( "//executable_semantics/ast:expression", "//executable_semantics/ast:pattern", "//executable_semantics/common:arena", - "//executable_semantics/common:error", + "//executable_semantics/common:error_builders", "@llvm-project//llvm:Support", ], ) @@ -158,7 +158,7 @@ cc_library( "//executable_semantics/ast:declaration", "//executable_semantics/ast:return_term", "//executable_semantics/ast:statement", - "//executable_semantics/common:error", + "//executable_semantics/common:error_builders", "//executable_semantics/common:nonnull", "@llvm-project//llvm:Support", ], @@ -208,7 +208,7 @@ cc_library( "//executable_semantics/ast:expression", "//executable_semantics/ast:statement", "//executable_semantics/common:arena", - "//executable_semantics/common:error", + "//executable_semantics/common:error_builders", "//executable_semantics/common:nonnull", "@llvm-project//llvm:Support", ], diff --git a/executable_semantics/interpreter/action_stack.cpp b/executable_semantics/interpreter/action_stack.cpp index 662bb310be9c..643d4e779cb0 100644 --- a/executable_semantics/interpreter/action_stack.cpp +++ b/executable_semantics/interpreter/action_stack.cpp @@ -77,7 +77,7 @@ auto ActionStack::ValueOfNode(ValueNodeView value_node, } } // TODO: Move these errors to compile time and explain them more clearly. - return FATAL_RUNTIME_ERROR(source_loc) + return RuntimeError(source_loc) << "could not find `" << value_node.base() << "`"; } diff --git a/executable_semantics/interpreter/heap.cpp b/executable_semantics/interpreter/heap.cpp index 63f19174c679..796519b38f81 100644 --- a/executable_semantics/interpreter/heap.cpp +++ b/executable_semantics/interpreter/heap.cpp @@ -4,7 +4,7 @@ #include "executable_semantics/interpreter/heap.h" -#include "executable_semantics/common/error.h" +#include "executable_semantics/common/error_builders.h" #include "llvm/ADT/StringExtras.h" #include "llvm/Support/Error.h" @@ -40,7 +40,7 @@ auto Heap::Write(const Address& a, Nonnull v, auto Heap::CheckAlive(AllocationId allocation, SourceLocation source_loc) const -> ErrorOr { if (!alive_[allocation.index_]) { - return FATAL_RUNTIME_ERROR(source_loc) + return RuntimeError(source_loc) << "undefined behavior: access to dead value " << *values_[allocation.index_]; } diff --git a/executable_semantics/interpreter/heap.h b/executable_semantics/interpreter/heap.h index 60aa13ad537f..81d6a29a0491 100644 --- a/executable_semantics/interpreter/heap.h +++ b/executable_semantics/interpreter/heap.h @@ -8,8 +8,8 @@ #include #include "common/ostream.h" -#include "executable_semantics/ast/source_location.h" #include "executable_semantics/common/nonnull.h" +#include "executable_semantics/common/source_location.h" #include "executable_semantics/interpreter/address.h" #include "executable_semantics/interpreter/heap_allocation_interface.h" #include "executable_semantics/interpreter/value.h" diff --git a/executable_semantics/interpreter/impl_scope.cpp b/executable_semantics/interpreter/impl_scope.cpp index 6598e4979cce..8062c852668d 100644 --- a/executable_semantics/interpreter/impl_scope.cpp +++ b/executable_semantics/interpreter/impl_scope.cpp @@ -4,7 +4,7 @@ #include "executable_semantics/interpreter/impl_scope.h" -#include "executable_semantics/common/error.h" +#include "executable_semantics/common/error_builders.h" #include "executable_semantics/interpreter/value.h" #include "llvm/ADT/StringExtras.h" #include "llvm/Support/Casting.h" @@ -29,9 +29,8 @@ auto ImplScope::Resolve(Nonnull iface_type, ASSIGN_OR_RETURN(std::optional result, TryResolve(iface_type, type, source_loc)); if (!result.has_value()) { - return FATAL_COMPILATION_ERROR(source_loc) - << "could not find implementation of " << *iface_type << " for " - << *type; + return CompilationError(source_loc) << "could not find implementation of " + << *iface_type << " for " << *type; } return *result; } @@ -50,9 +49,8 @@ auto ImplScope::TryResolve(Nonnull iface_type, parent->TryResolve(iface_type, type, source_loc)); if (parent_result.has_value() && result.has_value() && *parent_result != *result) { - return FATAL_COMPILATION_ERROR(source_loc) - << "ambiguous implementations of " << *iface_type << " for " - << *type; + return CompilationError(source_loc) << "ambiguous implementations of " + << *iface_type << " for " << *type; } result = parent_result; } diff --git a/executable_semantics/interpreter/interpreter.cpp b/executable_semantics/interpreter/interpreter.cpp index cd110caec9cd..face14463c22 100644 --- a/executable_semantics/interpreter/interpreter.cpp +++ b/executable_semantics/interpreter/interpreter.cpp @@ -15,7 +15,7 @@ #include "executable_semantics/ast/declaration.h" #include "executable_semantics/ast/expression.h" #include "executable_semantics/common/arena.h" -#include "executable_semantics/common/error.h" +#include "executable_semantics/common/error_builders.h" #include "executable_semantics/interpreter/action.h" #include "executable_semantics/interpreter/action_stack.h" #include "executable_semantics/interpreter/stack.h" @@ -560,7 +560,7 @@ auto Interpreter::StepExp() -> ErrorOr { const auto& tuple = cast(*act.results()[0]); int i = cast(*act.results()[1]).value(); if (i < 0 || i >= static_cast(tuple.elements().size())) { - return FATAL_RUNTIME_ERROR_NO_LINE() + return RuntimeError(exp.source_loc()) << "index " << i << " out of range in " << tuple; } return todo_.FinishAction(tuple.elements()[i]); @@ -813,7 +813,7 @@ auto Interpreter::StepExp() -> ErrorOr { } } default: - return FATAL_RUNTIME_ERROR(exp.source_loc()) + return RuntimeError(exp.source_loc()) << "in call, expected a function, not " << *act.results()[0]; } } else if (act.pos() == 3) { diff --git a/executable_semantics/interpreter/resolve_control_flow.cpp b/executable_semantics/interpreter/resolve_control_flow.cpp index 293a0f23013e..13c7002338b7 100644 --- a/executable_semantics/interpreter/resolve_control_flow.cpp +++ b/executable_semantics/interpreter/resolve_control_flow.cpp @@ -7,7 +7,7 @@ #include "executable_semantics/ast/declaration.h" #include "executable_semantics/ast/return_term.h" #include "executable_semantics/ast/statement.h" -#include "executable_semantics/common/error.h" +#include "executable_semantics/common/error_builders.h" #include "llvm/Support/Casting.h" #include "llvm/Support/Error.h" @@ -39,14 +39,14 @@ static auto ResolveControlFlow(Nonnull statement, switch (statement->kind()) { case StatementKind::Return: { if (!function.has_value()) { - return FATAL_COMPILATION_ERROR(statement->source_loc()) + return CompilationError(statement->source_loc()) << "return is not within a function body"; } const ReturnTerm& function_return = (*function)->declaration->return_term(); if (function_return.is_auto()) { if ((*function)->saw_return_in_auto) { - return FATAL_COMPILATION_ERROR(statement->source_loc()) + return CompilationError(statement->source_loc()) << "Only one return is allowed in a function with an `auto` " "return type."; } @@ -55,7 +55,7 @@ static auto ResolveControlFlow(Nonnull statement, auto& ret = cast(*statement); ret.set_function((*function)->declaration); if (ret.is_omitted_expression() != function_return.is_omitted()) { - return FATAL_COMPILATION_ERROR(ret.source_loc()) + return CompilationError(ret.source_loc()) << ret << " should" << (function_return.is_omitted() ? " not" : "") << " provide a return value, to match the function's signature."; @@ -64,14 +64,14 @@ static auto ResolveControlFlow(Nonnull statement, } case StatementKind::Break: if (!loop.has_value()) { - return FATAL_COMPILATION_ERROR(statement->source_loc()) + return CompilationError(statement->source_loc()) << "break is not within a loop body"; } cast(*statement).set_loop(*loop); return Success(); case StatementKind::Continue: if (!loop.has_value()) { - return FATAL_COMPILATION_ERROR(statement->source_loc()) + return CompilationError(statement->source_loc()) << "continue is not within a loop body"; } cast(*statement).set_loop(*loop); diff --git a/executable_semantics/interpreter/resolve_names.cpp b/executable_semantics/interpreter/resolve_names.cpp index d113fb9922b5..39aa4888a21c 100644 --- a/executable_semantics/interpreter/resolve_names.cpp +++ b/executable_semantics/interpreter/resolve_names.cpp @@ -167,8 +167,7 @@ static auto ResolveNames(Expression& expression, case ExpressionKind::TypeTypeLiteral: break; case ExpressionKind::UnimplementedExpression: - return FATAL_COMPILATION_ERROR(expression.source_loc()) - << "Unimplemented"; + return CompilationError(expression.source_loc()) << "Unimplemented"; } return Success(); } @@ -376,7 +375,7 @@ static auto ResolveNames(Declaration& declaration, StaticScope& enclosing_scope) RETURN_IF_ERROR( ResolveNames(alternative->signature(), enclosing_scope)); if (!alternative_names.insert(alternative->name()).second) { - return FATAL_COMPILATION_ERROR(alternative->source_loc()) + return CompilationError(alternative->source_loc()) << "Duplicate name `" << alternative->name() << "` in choice type"; } diff --git a/executable_semantics/interpreter/type_checker.cpp b/executable_semantics/interpreter/type_checker.cpp index 2b8192068abe..4044001268d6 100644 --- a/executable_semantics/interpreter/type_checker.cpp +++ b/executable_semantics/interpreter/type_checker.cpp @@ -13,7 +13,7 @@ #include "common/ostream.h" #include "executable_semantics/ast/declaration.h" #include "executable_semantics/common/arena.h" -#include "executable_semantics/common/error.h" +#include "executable_semantics/common/error_builders.h" #include "executable_semantics/interpreter/impl_scope.h" #include "executable_semantics/interpreter/interpreter.h" #include "executable_semantics/interpreter/value.h" @@ -42,10 +42,9 @@ static auto ExpectExactType(SourceLocation source_loc, Nonnull expected, Nonnull actual) -> ErrorOr { if (!TypeEqual(expected, actual)) { - return FATAL_COMPILATION_ERROR(source_loc) - << "type error in " << context << "\n" - << "expected: " << *expected << "\n" - << "actual: " << *actual; + return CompilationError(source_loc) << "type error in " << context << "\n" + << "expected: " << *expected << "\n" + << "actual: " << *actual; } return Success(); } @@ -55,10 +54,9 @@ static auto ExpectPointerType(SourceLocation source_loc, Nonnull actual) -> ErrorOr { if (actual->kind() != Value::Kind::PointerType) { - return FATAL_COMPILATION_ERROR(source_loc) - << "type error in " << context << "\n" - << "expected a pointer type\n" - << "actual: " << *actual; + return CompilationError(source_loc) << "type error in " << context << "\n" + << "expected a pointer type\n" + << "actual: " << *actual; } return Success(); } @@ -116,7 +114,7 @@ auto TypeChecker::ExpectIsConcreteType(SourceLocation source_loc, Nonnull value) -> ErrorOr { if (!IsConcreteType(value)) { - return FATAL_COMPILATION_ERROR(source_loc) + return CompilationError(source_loc) << "Expected a type, but got " << *value; } else { return Success(); @@ -230,7 +228,7 @@ auto TypeChecker::ExpectType(SourceLocation source_loc, Nonnull expected, Nonnull actual) -> ErrorOr { if (!IsImplicitlyConvertible(actual, expected)) { - return FATAL_COMPILATION_ERROR(source_loc) + return CompilationError(source_loc) << "type error in " << context << ": " << "'" << *actual << "' is not implicitly convertible to '" << *expected << "'"; @@ -257,7 +255,7 @@ auto TypeChecker::ArgumentDeduction(SourceLocation source_loc, } case Value::Kind::TupleValue: { if (arg_type->kind() != Value::Kind::TupleValue) { - return FATAL_COMPILATION_ERROR(source_loc) + return CompilationError(source_loc) << "type error in argument deduction\n" << "expected: " << *param_type << "\n" << "actual: " << *arg_type; @@ -265,7 +263,7 @@ auto TypeChecker::ArgumentDeduction(SourceLocation source_loc, const auto& param_tup = cast(*param_type); const auto& arg_tup = cast(*arg_type); if (param_tup.elements().size() != arg_tup.elements().size()) { - return FATAL_COMPILATION_ERROR(source_loc) + return CompilationError(source_loc) << "mismatch in tuple sizes, expected " << param_tup.elements().size() << " but got " << arg_tup.elements().size(); @@ -279,7 +277,7 @@ auto TypeChecker::ArgumentDeduction(SourceLocation source_loc, } case Value::Kind::StructType: { if (arg_type->kind() != Value::Kind::StructType) { - return FATAL_COMPILATION_ERROR(source_loc) + return CompilationError(source_loc) << "type error in argument deduction\n" << "expected: " << *param_type << "\n" << "actual: " << *arg_type; @@ -287,14 +285,14 @@ auto TypeChecker::ArgumentDeduction(SourceLocation source_loc, const auto& param_struct = cast(*param_type); const auto& arg_struct = cast(*arg_type); if (param_struct.fields().size() != arg_struct.fields().size()) { - return FATAL_COMPILATION_ERROR(source_loc) + return CompilationError(source_loc) << "mismatch in struct field counts, expected " << param_struct.fields().size() << " but got " << arg_struct.fields().size(); } for (size_t i = 0; i < param_struct.fields().size(); ++i) { if (param_struct.fields()[i].name != arg_struct.fields()[i].name) { - return FATAL_COMPILATION_ERROR(source_loc) + return CompilationError(source_loc) << "mismatch in field names, " << param_struct.fields()[i].name << " != " << arg_struct.fields()[i].name; } @@ -306,7 +304,7 @@ auto TypeChecker::ArgumentDeduction(SourceLocation source_loc, } case Value::Kind::FunctionType: { if (arg_type->kind() != Value::Kind::FunctionType) { - return FATAL_COMPILATION_ERROR(source_loc) + return CompilationError(source_loc) << "type error in argument deduction\n" << "expected: " << *param_type << "\n" << "actual: " << *arg_type; @@ -322,7 +320,7 @@ auto TypeChecker::ArgumentDeduction(SourceLocation source_loc, } case Value::Kind::PointerType: { if (arg_type->kind() != Value::Kind::PointerType) { - return FATAL_COMPILATION_ERROR(source_loc) + return CompilationError(source_loc) << "type error in argument deduction\n" << "expected: " << *param_type << "\n" << "actual: " << *arg_type; @@ -349,7 +347,7 @@ auto TypeChecker::ArgumentDeduction(SourceLocation source_loc, return Success(); } } - return FATAL_COMPILATION_ERROR(source_loc) + return CompilationError(source_loc) << "type error in argument deduction\n" << "expected: " << *param_type << "\n" << "actual: " << *arg_type; @@ -497,7 +495,7 @@ auto TypeChecker::TypeCheckExp(Nonnull e, InterpExp(&index.offset(), arena_, trace_)); int i = cast(*offset_value).value(); if (i < 0 || i >= static_cast(tuple_type.elements().size())) { - return FATAL_COMPILATION_ERROR(e->source_loc()) + return CompilationError(e->source_loc()) << "index " << i << " is out of range for type " << tuple_type; } @@ -515,7 +513,7 @@ auto TypeChecker::TypeCheckExp(Nonnull e, return Success(); } default: - return FATAL_COMPILATION_ERROR(e->source_loc()) << "expected a tuple"; + return CompilationError(e->source_loc()) << "expected a tuple"; } } case ExpressionKind::TupleLiteral: { @@ -573,7 +571,7 @@ auto TypeChecker::TypeCheckExp(Nonnull e, return Success(); } } - return FATAL_COMPILATION_ERROR(access.source_loc()) + return CompilationError(access.source_loc()) << "struct " << struct_type << " does not have a field named " << access.field(); } @@ -599,7 +597,7 @@ auto TypeChecker::TypeCheckExp(Nonnull e, } return Success(); } else { - return FATAL_COMPILATION_ERROR(e->source_loc()) + return CompilationError(e->source_loc()) << "class " << t_class.declaration().name() << " does not have a field named " << access.field(); } @@ -610,7 +608,7 @@ auto TypeChecker::TypeCheckExp(Nonnull e, std::optional> parameter_types = choice.FindAlternative(access.field()); if (!parameter_types.has_value()) { - return FATAL_COMPILATION_ERROR(e->source_loc()) + return CompilationError(e->source_loc()) << "choice " << choice.name() << " does not have a field named " << access.field(); } @@ -641,10 +639,10 @@ auto TypeChecker::TypeCheckExp(Nonnull e, default: break; } - return FATAL_COMPILATION_ERROR(access.source_loc()) + return CompilationError(access.source_loc()) << access.field() << " is not a class function"; } else { - return FATAL_COMPILATION_ERROR(access.source_loc()) + return CompilationError(access.source_loc()) << class_type << " does not have a class function named " << access.field(); } @@ -674,14 +672,14 @@ auto TypeChecker::TypeCheckExp(Nonnull e, access.set_impl(*var_type.binding().impl_binding()); return Success(); } else { - return FATAL_COMPILATION_ERROR(e->source_loc()) + return CompilationError(e->source_loc()) << "field access, " << access.field() << " not in " << iface_decl.name(); } break; } default: - return FATAL_COMPILATION_ERROR(e->source_loc()) + return CompilationError(e->source_loc()) << "field access, unexpected " << aggregate_type << " of non-interface type " << typeof_var << " in " << *e; } @@ -708,14 +706,14 @@ auto TypeChecker::TypeCheckExp(Nonnull e, access.set_impl(*var_type.binding().impl_binding()); return Success(); } else { - return FATAL_COMPILATION_ERROR(e->source_loc()) + return CompilationError(e->source_loc()) << "field access, " << access.field() << " not in " << iface_decl.name(); } break; } default: - return FATAL_COMPILATION_ERROR(e->source_loc()) + return CompilationError(e->source_loc()) << "field access, unexpected " << aggregate_type << " in " << *e; } @@ -728,7 +726,7 @@ auto TypeChecker::TypeCheckExp(Nonnull e, cast(ident.value_node().base()); if (!function.has_static_type()) { CHECK(function.return_term().is_auto()); - return FATAL_COMPILATION_ERROR(ident.source_loc()) + return CompilationError(ident.source_loc()) << "Function calls itself, but has a deduced return type"; } } @@ -822,7 +820,7 @@ auto TypeChecker::TypeCheckExp(Nonnull e, return Success(); case Operator::AddressOf: if (op.arguments()[0]->value_category() != ValueCategory::Var) { - return FATAL_COMPILATION_ERROR(op.arguments()[0]->source_loc()) + return CompilationError(op.arguments()[0]->source_loc()) << "Argument to " << ToString(op.op()) << " should be an lvalue."; } @@ -853,7 +851,7 @@ auto TypeChecker::TypeCheckExp(Nonnull e, // has been added to the type checking of function signatures. if (auto it = deduced_type_args.find(deduced_param); it == deduced_type_args.end()) { - return FATAL_COMPILATION_ERROR(e->source_loc()) + return CompilationError(e->source_loc()) << "could not deduce type argument for type parameter " << deduced_param->name() << "\n" << "in " << call; @@ -880,7 +878,7 @@ auto TypeChecker::TypeCheckExp(Nonnull e, case Value::Kind::TypeType: break; default: - return FATAL_COMPILATION_ERROR(e->source_loc()) + return CompilationError(e->source_loc()) << "unexpected type of deduced parameter " << *impl_binding->interface(); } @@ -913,7 +911,7 @@ auto TypeChecker::TypeCheckExp(Nonnull e, CHECK(PatternMatch(&(*class_decl.type_params())->value(), arg, call.source_loc(), std::nullopt, generic_args)); } else { - return FATAL_COMPILATION_ERROR(call.source_loc()) + return CompilationError(call.source_loc()) << "attempt to instantiate a non-generic class: " << *e; } // Find impls for all the impl bindings of the class. @@ -934,7 +932,7 @@ auto TypeChecker::TypeCheckExp(Nonnull e, case Value::Kind::TypeType: break; default: - return FATAL_COMPILATION_ERROR(e->source_loc()) + return CompilationError(e->source_loc()) << "unexpected type of deduced parameter " << *impl_binding->interface(); } @@ -948,7 +946,7 @@ auto TypeChecker::TypeCheckExp(Nonnull e, return Success(); } default: { - return FATAL_COMPILATION_ERROR(e->source_loc()) + return CompilationError(e->source_loc()) << "in call, expected a function\n" << *e << "\nnot an operator of type " << call.function().static_type() << "\n"; @@ -980,7 +978,7 @@ auto TypeChecker::TypeCheckExp(Nonnull e, switch (cast(*e).intrinsic()) { case IntrinsicExpression::Intrinsic::Print: if (intrinsic_exp.args().fields().size() != 1) { - return FATAL_COMPILATION_ERROR(e->source_loc()) + return CompilationError(e->source_loc()) << "__intrinsic_print takes 1 argument"; } RETURN_IF_ERROR( @@ -1040,8 +1038,7 @@ auto TypeChecker::TypeCheckExp(Nonnull e, Nonnull size_value, InterpExp(&array_literal.size_expression(), arena_, trace_)); if (cast(size_value)->value() < 0) { - return FATAL_COMPILATION_ERROR( - array_literal.size_expression().source_loc()) + return CompilationError(array_literal.size_expression().source_loc()) << "Array size cannot be negative"; } array_literal.set_static_type(arena_->New()); @@ -1107,7 +1104,7 @@ auto TypeChecker::TypeCheckPattern( case PatternKind::BindingPattern: { auto& binding = cast(*p); if (!GetBindings(binding.type()).empty()) { - return FATAL_COMPILATION_ERROR(binding.type().source_loc()) + return CompilationError(binding.type().source_loc()) << "The type of a binding pattern cannot contain bindings."; } RETURN_IF_ERROR(TypeCheckPattern(&binding.type(), std::nullopt, @@ -1122,7 +1119,7 @@ auto TypeChecker::TypeCheckPattern( BindingMap generic_args; if (!PatternMatch(type, *expected, binding.type().source_loc(), std::nullopt, generic_args)) { - return FATAL_COMPILATION_ERROR(binding.type().source_loc()) + return CompilationError(binding.type().source_loc()) << "Type pattern '" << *type << "' does not match actual type '" << **expected << "'"; } @@ -1146,7 +1143,7 @@ auto TypeChecker::TypeCheckPattern( ASSIGN_OR_RETURN(Nonnull type, InterpExp(&binding.type(), arena_, trace_)); if (expected) { - return FATAL_COMPILATION_ERROR(binding.type().source_loc()) + return CompilationError(binding.type().source_loc()) << "Generic binding may not occur in pattern with expected " "type: " << binding; @@ -1165,12 +1162,11 @@ auto TypeChecker::TypeCheckPattern( auto& tuple = cast(*p); std::vector> field_types; if (expected && (*expected)->kind() != Value::Kind::TupleValue) { - return FATAL_COMPILATION_ERROR(p->source_loc()) - << "didn't expect a tuple"; + return CompilationError(p->source_loc()) << "didn't expect a tuple"; } if (expected && tuple.fields().size() != cast(**expected).elements().size()) { - return FATAL_COMPILATION_ERROR(tuple.source_loc()) + return CompilationError(tuple.source_loc()) << "tuples of different length"; } for (size_t i = 0; i < tuple.fields().size(); ++i) { @@ -1197,7 +1193,7 @@ auto TypeChecker::TypeCheckPattern( RETURN_IF_ERROR(TypeCheckExp(&alternative.choice_type(), impl_scope)); if (alternative.choice_type().static_type().kind() != Value::Kind::TypeOfChoiceType) { - return FATAL_COMPILATION_ERROR(alternative.source_loc()) + return CompilationError(alternative.source_loc()) << "alternative pattern does not name a choice type."; } if (expected) { @@ -1212,7 +1208,7 @@ auto TypeChecker::TypeCheckPattern( cast(choice_type) .FindAlternative(alternative.alternative_name()); if (parameter_types == std::nullopt) { - return FATAL_COMPILATION_ERROR(alternative.source_loc()) + return CompilationError(alternative.source_loc()) << "'" << alternative.alternative_name() << "' is not an alternative of " << choice_type; } @@ -1302,7 +1298,7 @@ auto TypeChecker::TypeCheckStmt(Nonnull s, &assign.lhs().static_type(), &assign.rhs().static_type())); if (assign.lhs().value_category() != ValueCategory::Var) { - return FATAL_COMPILATION_ERROR(assign.source_loc()) + return CompilationError(assign.source_loc()) << "Cannot assign to rvalue '" << assign.lhs() << "'"; } return Success(); @@ -1377,7 +1373,7 @@ auto TypeChecker::ExpectReturnOnAllPaths( std::optional> opt_stmt, SourceLocation source_loc) -> ErrorOr { if (!opt_stmt) { - return FATAL_COMPILATION_ERROR(source_loc) + return CompilationError(source_loc) << "control-flow reaches end of function that provides a `->` " "return " "type without reaching a return statement"; @@ -1387,7 +1383,7 @@ auto TypeChecker::ExpectReturnOnAllPaths( case StatementKind::Match: { auto& match = cast(*stmt); if (!IsExhaustive(match)) { - return FATAL_COMPILATION_ERROR(source_loc) + return CompilationError(source_loc) << "non-exhaustive match may allow control-flow to reach the " "end " "of a function that provides a `->` return type"; @@ -1402,7 +1398,7 @@ auto TypeChecker::ExpectReturnOnAllPaths( case StatementKind::Block: { auto& block = cast(*stmt); if (block.statements().empty()) { - return FATAL_COMPILATION_ERROR(stmt->source_loc()) + return CompilationError(stmt->source_loc()) << "control-flow reaches end of function that provides a `->` " "return type without reaching a return statement"; } @@ -1431,7 +1427,7 @@ auto TypeChecker::ExpectReturnOnAllPaths( case StatementKind::Break: case StatementKind::Continue: case StatementKind::VariableDefinition: - return FATAL_COMPILATION_ERROR(stmt->source_loc()) + return CompilationError(stmt->source_loc()) << "control-flow reaches end of function that provides a `->` " "return type without reaching a return statement"; } @@ -1497,7 +1493,7 @@ auto TypeChecker::DeclareFunctionDeclaration(Nonnull f, } else { // We have to type-check the body in order to determine the return type. if (!f->body().has_value()) { - return FATAL_COMPILATION_ERROR(f->return_term().source_loc()) + return CompilationError(f->return_term().source_loc()) << "Function declaration has deduced return type but no body"; } RETURN_IF_ERROR(TypeCheckStmt(*f->body(), function_scope)); @@ -1515,7 +1511,7 @@ auto TypeChecker::DeclareFunctionDeclaration(Nonnull f, if (f->name() == "Main") { if (!f->return_term().type_expression().has_value()) { - return FATAL_COMPILATION_ERROR(f->return_term().source_loc()) + return CompilationError(f->return_term().source_loc()) << "`Main` must have an explicit return type"; } RETURN_IF_ERROR(ExpectExactType( @@ -1696,7 +1692,7 @@ auto TypeChecker::DeclareImplDeclaration(Nonnull impl_decl, "member of implementation", iface_mem_type, &(*mem)->static_type())); } else { - return FATAL_COMPILATION_ERROR(impl_decl->source_loc()) + return CompilationError(impl_decl->source_loc()) << "implementation missing " << *mem_name; } } @@ -1793,7 +1789,7 @@ auto TypeChecker::TypeCheckDeclaration(Nonnull d, dyn_cast(&var.binding().type()); if (binding_type == nullptr) { // TODO: consider adding support for `auto` - return FATAL_COMPILATION_ERROR(var.source_loc()) + return CompilationError(var.source_loc()) << "Type of a top-level variable must be an expression."; } if (var.has_initializer()) { @@ -1845,7 +1841,7 @@ auto TypeChecker::DeclareDeclaration(Nonnull d, // Associate the variable name with it's declared type in the // compile-time symbol table. if (!llvm::isa(var.binding().type())) { - return FATAL_COMPILATION_ERROR(var.binding().type().source_loc()) + return CompilationError(var.binding().type().source_loc()) << "Expected expression for variable type"; } Expression& type = diff --git a/executable_semantics/interpreter/value.cpp b/executable_semantics/interpreter/value.cpp index 3cd3cff96c78..0aab3766049b 100644 --- a/executable_semantics/interpreter/value.cpp +++ b/executable_semantics/interpreter/value.cpp @@ -8,7 +8,7 @@ #include "common/check.h" #include "executable_semantics/common/arena.h" -#include "executable_semantics/common/error.h" +#include "executable_semantics/common/error_builders.h" #include "executable_semantics/interpreter/action.h" #include "llvm/ADT/StringExtras.h" #include "llvm/Support/Casting.h" @@ -49,7 +49,7 @@ static auto GetMember(Nonnull arena, Nonnull v, return *fun_decl.constant_value(); } } else { - return FATAL_COMPILATION_ERROR(source_loc) + return CompilationError(source_loc) << "member " << f << " not in " << *witness; } } @@ -62,8 +62,7 @@ static auto GetMember(Nonnull arena, Nonnull v, std::optional> field = cast(*v).FindField(f); if (field == std::nullopt) { - return FATAL_RUNTIME_ERROR(source_loc) - << "member " << f << " not in " << *v; + return RuntimeError(source_loc) << "member " << f << " not in " << *v; } return *field; } @@ -80,9 +79,8 @@ static auto GetMember(Nonnull arena, Nonnull v, std::optional> func = class_type.FindFunction(f); if (func == std::nullopt) { - return FATAL_RUNTIME_ERROR(source_loc) - << "member " << f << " not in " << *v << " or its " - << class_type; + return RuntimeError(source_loc) << "member " << f << " not in " << *v + << " or its " << class_type; } else if ((*func)->declaration().is_method()) { // Found a method. Turn it into a bound method. const FunctionValue& m = cast(**func); @@ -101,7 +99,7 @@ static auto GetMember(Nonnull arena, Nonnull v, case Value::Kind::ChoiceType: { const auto& choice = cast(*v); if (!choice.FindAlternative(f)) { - return FATAL_RUNTIME_ERROR(source_loc) + return RuntimeError(source_loc) << "alternative " << f << " not in " << *v; } return arena->New(f, choice.name()); @@ -112,7 +110,7 @@ static auto GetMember(Nonnull arena, Nonnull v, std::optional> fun = class_type.FindFunction(f); if (fun == std::nullopt) { - return FATAL_RUNTIME_ERROR(source_loc) + return RuntimeError(source_loc) << "class function " << f << " not in " << *v; } return arena->New(&(*fun)->declaration(), @@ -151,7 +149,7 @@ static auto SetFieldImpl( return element.name == (*path_begin).name(); }); if (it == elements.end()) { - return FATAL_RUNTIME_ERROR(source_loc) + return RuntimeError(source_loc) << "field " << (*path_begin).name() << " not in " << *value; } ASSIGN_OR_RETURN(it->value, @@ -169,9 +167,8 @@ static auto SetFieldImpl( // TODO(geoffromer): update FieldPath to hold integers as well as strings. int index = std::stoi((*path_begin).name()); if (index < 0 || static_cast(index) >= elements.size()) { - return FATAL_RUNTIME_ERROR(source_loc) - << "index " << (*path_begin).name() << " out of range in " - << *value; + return RuntimeError(source_loc) << "index " << (*path_begin).name() + << " out of range in " << *value; } ASSIGN_OR_RETURN(elements[index], SetFieldImpl(arena, elements[index], path_begin + 1, diff --git a/executable_semantics/syntax/BUILD b/executable_semantics/syntax/BUILD index f22fa09e23fb..41b31891ef10 100644 --- a/executable_semantics/syntax/BUILD +++ b/executable_semantics/syntax/BUILD @@ -85,7 +85,7 @@ cc_library( "//executable_semantics/ast:pattern", "//executable_semantics/ast:value_category", "//executable_semantics/common:arena", - "//executable_semantics/common:error", + "//executable_semantics/common:error_builders", "//executable_semantics/common:nonnull", "@llvm-project//llvm:Support", ], diff --git a/executable_semantics/syntax/parse.cpp b/executable_semantics/syntax/parse.cpp index dcd40fb4401e..1c4c8eec3d29 100644 --- a/executable_semantics/syntax/parse.cpp +++ b/executable_semantics/syntax/parse.cpp @@ -6,7 +6,7 @@ #include "common/check.h" #include "common/error.h" -#include "executable_semantics/common/error.h" +#include "executable_semantics/common/error_builders.h" #include "executable_semantics/syntax/lexer.h" #include "executable_semantics/syntax/parse_and_lex_context.h" #include "executable_semantics/syntax/parser.h" @@ -41,10 +41,11 @@ auto ParseImpl(yyscan_t scanner, Nonnull arena, auto Parse(Nonnull arena, std::string_view input_file_name, bool trace) -> ErrorOr { - FILE* input_file = fopen(std::string(input_file_name).c_str(), "r"); + std::string name_str(input_file_name); + FILE* input_file = fopen(name_str.c_str(), "r"); if (input_file == nullptr) { - return FATAL_PROGRAM_ERROR_NO_LINE() << "Error opening '" << input_file_name - << "': " << std::strerror(errno); + return ProgramError(SourceLocation(name_str.c_str(), 0)) + << "Error opening file: " << std::strerror(errno); } // Prepare the lexer. diff --git a/executable_semantics/testdata/array/fail_index.carbon b/executable_semantics/testdata/array/fail_index.carbon index 3baf5f62769a..622612203bf7 100644 --- a/executable_semantics/testdata/array/fail_index.carbon +++ b/executable_semantics/testdata/array/fail_index.carbon @@ -7,7 +7,7 @@ // RUN: %{not} %{executable_semantics} --trace %s 2>&1 | \ // RUN: %{FileCheck} --match-full-lines --allow-unused-prefixes %s // AUTOUPDATE: %{executable_semantics} %s -// CHECK: RUNTIME ERROR: index 2 out of range in (0, 1) +// CHECK: RUNTIME ERROR: {{.*}}/executable_semantics/testdata/array/fail_index.carbon:16: index 2 out of range in (0, 1) package ExecutableSemanticsTest api;