Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replaced std::exit() with return Carbon::ErrorOr for expected errors like invalid syntax #1120

Merged
merged 37 commits into from
Mar 22, 2022
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
1b4a940
Replaced std::exit() with return llvm::Expected/llvm::Error<T> for ex…
pk19604014 Mar 8, 2022
7f0370e
Use llvm::formatv() for formatting lexer error messages.
pk19604014 Mar 8, 2022
7fd9b3a
Addresed merge errors.
pk19604014 Mar 8, 2022
9784c4c
Fixed impl scope.
pk19604014 Mar 8, 2022
556e333
Made ErrorBuilder::operator<< nodiscard, to catch code forgetting 're…
pk19604014 Mar 8, 2022
ee8f6d1
Merge branch 'trunk' of https://github.com/carbon-language/carbon-lan…
pk19604014 Mar 8, 2022
24fbafb
FatalComplationError() -> ParseAndLexContext::RecordLexerError().
pk19604014 Mar 9, 2022
49ffab8
Update executable_semantics/syntax/parse_and_lex_context.h
pk19604014 Mar 9, 2022
1409cea
Code review fixes.
pk19604014 Mar 9, 2022
cf19372
Update executable_semantics/syntax/parser.ypp
pk19604014 Mar 9, 2022
8603e6e
More code review fixes.
pk19604014 Mar 9, 2022
a5f9e70
Merge branch 'exit_to_return' of https://github.com/pk19604014/carbon…
pk19604014 Mar 9, 2022
6b74b52
Update executable_semantics/interpreter/type_checker.h
pk19604014 Mar 9, 2022
8ce0d50
Yet more code review fixes...
pk19604014 Mar 9, 2022
ec34ac2
Merge branch 'exit_to_return' of https://github.com/pk19604014/carbon…
pk19604014 Mar 9, 2022
98a2fb5
Update executable_semantics/syntax/lexer.lpp
pk19604014 Mar 9, 2022
26979cb
code review comments
pk19604014 Mar 10, 2022
1768b43
Update executable_semantics/interpreter/interpreter.cpp
pk19604014 Mar 10, 2022
b60d244
Merge branch 'exit_to_return' of https://github.com/pk19604014/carbon…
pk19604014 Mar 10, 2022
3f2269f
Apply suggestions from code review
pk19604014 Mar 10, 2022
be40834
Merge branch 'exit_to_return' of https://github.com/pk19604014/carbon…
pk19604014 Mar 10, 2022
9ea0221
Update executable_semantics/syntax/lexer.lpp
pk19604014 Mar 10, 2022
3ff6a35
Merge branch 'exit_to_return' of https://github.com/pk19604014/carbon…
pk19604014 Mar 10, 2022
9e41fee
code review
pk19604014 Mar 10, 2022
310d6ab
code review
pk19604014 Mar 10, 2022
d555650
Apply suggestions from code review
pk19604014 Mar 10, 2022
536b654
Merge branch 'exit_to_return' of https://github.com/pk19604014/carbon…
pk19604014 Mar 10, 2022
de73ea4
formatted code
pk19604014 Mar 10, 2022
8fe0742
Merged
pk19604014 Mar 11, 2022
53e6acb
review comments
pk19604014 Mar 11, 2022
0c1df90
merged
pk19604014 Mar 22, 2022
68e7e83
Switched to the new ErrorOr<V> error implementation
pk19604014 Mar 22, 2022
7ff2410
code review comments
pk19604014 Mar 22, 2022
70bcd99
fixed comment
pk19604014 Mar 22, 2022
fff7f8e
restored ostream.h as #976 makes the change unnecesary
pk19604014 Mar 22, 2022
e478542
review comments
pk19604014 Mar 22, 2022
18f7d7c
merged
pk19604014 Mar 22, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions common/ostream.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,20 @@ void PrintTo(T* p, std::ostream* out) {
}
}

namespace Internal {

// A trait for determining the existence of operator<<(llvm::raw_ostream&, const
// T&).
template <typename T, typename = void>
struct HasLlvmRawOstreamOp : std::false_type {};
pk19604014 marked this conversation as resolved.
Show resolved Hide resolved

template <typename T>
struct HasLlvmRawOstreamOp<
T, std::void_t<decltype(std::declval<llvm::raw_ostream&>()
<< std::declval<const T&>())>> : std::true_type {};
pk19604014 marked this conversation as resolved.
Show resolved Hide resolved

} // namespace Internal

} // namespace Carbon

namespace llvm {
Expand All @@ -86,10 +100,12 @@ namespace llvm {
// supporting `std::ostream` isn't a priority for LLVM so we handle it locally
// instead.
template <typename S, typename T,
// S is-a ostream.
typename = std::enable_if_t<std::is_base_of_v<
std::ostream, std::remove_reference_t<std::remove_cv_t<S>>>>,
typename = std::enable_if_t<!std::is_same_v<
std::remove_reference_t<std::remove_cv_t<T>>, raw_ostream>>>
// T can be streamed to an llvm::raw_ostream.
typename =
std::enable_if_t<Carbon::Internal::HasLlvmRawOstreamOp<T>::value>>
jonmeow marked this conversation as resolved.
Show resolved Hide resolved
auto operator<<(S& standard_out, const T& value) -> S& {
raw_os_ostream(standard_out) << value;
return standard_out;
Expand Down
1 change: 1 addition & 0 deletions executable_semantics/ast/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ cc_library(
"//common:check",
"//executable_semantics/common:error",
"//executable_semantics/common:nonnull",
"@llvm-project//llvm:Support",
],
)

Expand Down
31 changes: 20 additions & 11 deletions executable_semantics/ast/declaration.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,29 +112,38 @@ void ReturnTerm::Print(llvm::raw_ostream& out) const {
}
}

// Look for the `me` parameter in the `deduced_parameters_`
// and put it in the `me_pattern_`.
void FunctionDeclaration::ResolveDeducedAndReceiver(
const std::vector<Nonnull<AstNode*>>& deduced_params) {
auto FunctionDeclaration::Create(
Nonnull<Arena*> arena, SourceLocation source_loc, std::string name,
std::vector<Nonnull<AstNode*>> deduced_params,
std::optional<Nonnull<BindingPattern*>> me_pattern,
Nonnull<TuplePattern*> param_pattern, ReturnTerm return_term,
std::optional<Nonnull<Block*>> body)
-> llvm::Expected<Nonnull<FunctionDeclaration*>> {
std::vector<Nonnull<GenericBinding*>> resolved_params;
// Look for the `me` parameter in the `deduced_parameters`
// and put it in the `me_pattern`.
for (Nonnull<AstNode*> param : deduced_params) {
switch (param->kind()) {
case AstNodeKind::GenericBinding:
deduced_parameters_.push_back(&cast<GenericBinding>(*param));
resolved_params.push_back(&cast<GenericBinding>(*param));
break;
case AstNodeKind::BindingPattern: {
Nonnull<BindingPattern*> bp = &cast<BindingPattern>(*param);
if (me_pattern_.has_value() || bp->name() != "me") {
FATAL_COMPILATION_ERROR(source_loc())
<< "illegal binding pattern in implicit parameter list";
if (me_pattern.has_value() || bp->name() != "me") {
return FATAL_COMPILATION_ERROR(source_loc)
<< "illegal binding pattern in implicit parameter list";
}
me_pattern_ = bp;
me_pattern = bp;
break;
}
default:
FATAL_COMPILATION_ERROR(source_loc())
<< "illegal AST node in implicit parameter list";
return FATAL_COMPILATION_ERROR(source_loc)
<< "illegal AST node in implicit parameter list";
}
}
return arena->New<FunctionDeclaration>(source_loc, name, resolved_params,
me_pattern, param_pattern, return_term,
body);
}

void FunctionDeclaration::PrintDepth(int depth, llvm::raw_ostream& out) const {
Expand Down
19 changes: 13 additions & 6 deletions executable_semantics/ast/declaration.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,20 +82,28 @@ class FunctionDeclaration : public Declaration {
public:
using ImplementsCarbonValueNode = void;

static auto Create(Nonnull<Arena*> arena, SourceLocation source_loc,
std::string name,
std::vector<Nonnull<AstNode*>> deduced_params,
std::optional<Nonnull<BindingPattern*>> me_pattern,
Nonnull<TuplePattern*> param_pattern,
ReturnTerm return_term,
std::optional<Nonnull<Block*>> body)
-> llvm::Expected<Nonnull<FunctionDeclaration*>>;
pk19604014 marked this conversation as resolved.
Show resolved Hide resolved

FunctionDeclaration(SourceLocation source_loc, std::string name,
pk19604014 marked this conversation as resolved.
Show resolved Hide resolved
std::vector<Nonnull<AstNode*>> deduced_params,
std::vector<Nonnull<GenericBinding*>> deduced_params,
std::optional<Nonnull<BindingPattern*>> me_pattern,
Nonnull<TuplePattern*> param_pattern,
ReturnTerm return_term,
std::optional<Nonnull<Block*>> body)
: Declaration(AstNodeKind::FunctionDeclaration, source_loc),
name_(std::move(name)),
deduced_parameters_(std::move(deduced_params)),
me_pattern_(me_pattern),
param_pattern_(param_pattern),
return_term_(return_term),
body_(body) {
ResolveDeducedAndReceiver(deduced_params);
}
body_(body) {}

static auto classof(const AstNode* node) -> bool {
return InheritsFromFunctionDeclaration(node->kind());
Expand Down Expand Up @@ -132,10 +140,9 @@ class FunctionDeclaration : public Declaration {
constant_value_ = value;
}

bool is_method() const { return me_pattern_.has_value(); }
auto is_method() const -> bool { return me_pattern_.has_value(); }

private:
void ResolveDeducedAndReceiver(const std::vector<Nonnull<AstNode*>>&);
std::string name_;
std::vector<Nonnull<GenericBinding*>> deduced_parameters_;
std::optional<Nonnull<BindingPattern*>> me_pattern_;
Expand Down
5 changes: 3 additions & 2 deletions executable_semantics/ast/expression.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,14 @@ using llvm::isa;

auto IntrinsicExpression::FindIntrinsic(std::string_view name,
SourceLocation source_loc)
-> Intrinsic {
-> llvm::Expected<Intrinsic> {
static const auto& intrinsic_map =
*new std::map<std::string_view, Intrinsic>({{"print", Intrinsic::Print}});
name.remove_prefix(std::strlen("__intrinsic_"));
auto it = intrinsic_map.find(name);
if (it == intrinsic_map.end()) {
FATAL_COMPILATION_ERROR(source_loc) << "Unknown intrinsic '" << name << "'";
return FATAL_COMPILATION_ERROR(source_loc)
<< "Unknown intrinsic '" << name << "'";
}
return it->second;
}
Expand Down
15 changes: 7 additions & 8 deletions executable_semantics/ast/expression.h
Original file line number Diff line number Diff line change
Expand Up @@ -465,11 +465,15 @@ class IntrinsicExpression : public Expression {
Print,
};

explicit IntrinsicExpression(std::string_view intrinsic_name,
Nonnull<TupleLiteral*> args,
// Returns the enumerator corresponding to the intrinsic named `name`,
// or raises a fatal compile error if there is no such enumerator.
static auto FindIntrinsic(std::string_view name, SourceLocation source_loc)
-> llvm::Expected<Intrinsic>;

explicit IntrinsicExpression(Intrinsic intrinsic, Nonnull<TupleLiteral*> args,
SourceLocation source_loc)
: Expression(AstNodeKind::IntrinsicExpression, source_loc),
intrinsic_(FindIntrinsic(intrinsic_name, source_loc)),
intrinsic_(intrinsic),
args_(args) {}

static auto classof(const AstNode* node) -> bool {
Expand All @@ -481,11 +485,6 @@ class IntrinsicExpression : public Expression {
auto args() -> TupleLiteral& { return *args_; }

private:
// Returns the enumerator corresponding to the intrinsic named `name`,
// or raises a fatal compile error if there is no such enumerator.
static auto FindIntrinsic(std::string_view name, SourceLocation source_loc)
-> Intrinsic;

Intrinsic intrinsic_;
Nonnull<TupleLiteral*> args_;
};
Expand Down
18 changes: 5 additions & 13 deletions executable_semantics/ast/pattern.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,23 +72,15 @@ auto TuplePatternFromParenContents(Nonnull<Arena*> arena,
// Used by AlternativePattern for constructor initialization. Produces a helpful
// error for incorrect expressions, rather than letting a default cast error
// apply.
static auto RequireFieldAccess(Nonnull<Expression*> alternative)
-> FieldAccessExpression& {
auto AlternativePattern::RequireFieldAccess(Nonnull<Expression*> alternative)
-> llvm::Expected<Nonnull<FieldAccessExpression*>> {
if (alternative->kind() != ExpressionKind::FieldAccessExpression) {
FATAL_PROGRAM_ERROR(alternative->source_loc())
<< "Alternative pattern must have the form of a field access.";
return FATAL_PROGRAM_ERROR(alternative->source_loc())
<< "Alternative pattern must have the form of a field access.";
}
return cast<FieldAccessExpression>(*alternative);
return &cast<FieldAccessExpression>(*alternative);
}

AlternativePattern::AlternativePattern(SourceLocation source_loc,
Nonnull<Expression*> alternative,
Nonnull<TuplePattern*> arguments)
: Pattern(AstNodeKind::AlternativePattern, source_loc),
choice_type_(&RequireFieldAccess(alternative).aggregate()),
alternative_name_(RequireFieldAccess(alternative).field()),
arguments_(arguments) {}

auto ParenExpressionToParenPattern(Nonnull<Arena*> arena,
const ParenContents<Expression>& contents)
-> ParenContents<Pattern> {
Expand Down
22 changes: 16 additions & 6 deletions executable_semantics/ast/pattern.h
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,19 @@ auto ParenExpressionToParenPattern(Nonnull<Arena*> arena,
// A pattern that matches an alternative of a choice type.
class AlternativePattern : public Pattern {
public:
// Constructs an AlternativePattern that matches the alternative specified
// by `alternative`, if its arguments match `arguments`.
static auto Create(Nonnull<Arena*> arena, SourceLocation source_loc,
Nonnull<Expression*> alternative,
Nonnull<TuplePattern*> arguments)
-> llvm::Expected<Nonnull<AlternativePattern*>> {
ASSIGN_OR_RETURN(FieldAccessExpression * field_access,
pk19604014 marked this conversation as resolved.
Show resolved Hide resolved
RequireFieldAccess(alternative));
return arena->New<AlternativePattern>(source_loc,
&field_access->aggregate(),
field_access->field(), arguments);
}

// Constructs an AlternativePattern that matches a value of the type
// specified by choice_type if it represents an alternative named
// alternative_name, and its arguments match `arguments`.
Expand All @@ -186,12 +199,6 @@ class AlternativePattern : public Pattern {
alternative_name_(std::move(alternative_name)),
arguments_(arguments) {}

// Constructs an AlternativePattern that matches the alternative specified
// by `alternative`, if its arguments match `arguments`.
AlternativePattern(SourceLocation source_loc,
Nonnull<Expression*> alternative,
Nonnull<TuplePattern*> arguments);

static auto classof(const AstNode* node) -> bool {
return InheritsFromAlternativePattern(node->kind());
}
Expand All @@ -205,6 +212,9 @@ class AlternativePattern : public Pattern {
auto arguments() -> TuplePattern& { return *arguments_; }

private:
static auto RequireFieldAccess(Nonnull<Expression*> alternative)
-> llvm::Expected<Nonnull<FieldAccessExpression*>>;

Nonnull<Expression*> choice_type_;
std::string alternative_name_;
Nonnull<TuplePattern*> arguments_;
Expand Down
34 changes: 20 additions & 14 deletions executable_semantics/ast/static_scope.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,43 +5,49 @@
#include "executable_semantics/ast/static_scope.h"

#include "executable_semantics/common/error.h"
#include "llvm/Support/Error.h"

namespace Carbon {

void StaticScope::Add(std::string name, ValueNodeView entity) {
auto StaticScope::Add(std::string name, ValueNodeView entity) -> llvm::Error {
auto [it, success] = declared_names_.insert({name, entity});
if (!success && it->second != entity) {
FATAL_COMPILATION_ERROR(entity.base().source_loc())
<< "Duplicate name `" << name << "` also found at "
<< it->second.base().source_loc();
return FATAL_COMPILATION_ERROR(entity.base().source_loc())
<< "Duplicate name `" << name << "` also found at "
<< it->second.base().source_loc();
}
return llvm::Error::success();
}

auto StaticScope::Resolve(const std::string& name,
SourceLocation source_loc) const -> ValueNodeView {
std::optional<ValueNodeView> result = TryResolve(name, source_loc);
if (!result.has_value()) {
FATAL_COMPILATION_ERROR(source_loc) << "could not resolve '" << name << "'";
SourceLocation source_loc) const
-> llvm::Expected<ValueNodeView> {
ASSIGN_OR_RETURN(std::optional<ValueNodeView> result,
TryResolve(name, source_loc));
if (!result) {
return FATAL_COMPILATION_ERROR(source_loc)
<< "could not resolve '" << name << "'";
}
return *result;
}

auto StaticScope::TryResolve(const std::string& name,
SourceLocation source_loc) const
-> std::optional<ValueNodeView> {
-> llvm::Expected<std::optional<ValueNodeView>> {
auto it = declared_names_.find(name);
if (it != declared_names_.end()) {
return it->second;
}
std::optional<ValueNodeView> result;
for (Nonnull<const StaticScope*> parent : parent_scopes_) {
auto parent_result = parent->TryResolve(name, source_loc);
ASSIGN_OR_RETURN(std::optional<ValueNodeView> parent_result,
parent->TryResolve(name, source_loc));
if (parent_result.has_value() && result.has_value() &&
*parent_result != *result) {
FATAL_COMPILATION_ERROR(source_loc)
<< "'" << name << "' is ambiguous between "
<< result->base().source_loc() << " and "
<< parent_result->base().source_loc();
return FATAL_COMPILATION_ERROR(source_loc)
<< "'" << name << "' is ambiguous between "
<< result->base().source_loc() << " and "
<< parent_result->base().source_loc();
}
result = parent_result;
}
Expand Down
7 changes: 4 additions & 3 deletions executable_semantics/ast/static_scope.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "executable_semantics/ast/source_location.h"
#include "executable_semantics/ast/value_category.h"
#include "executable_semantics/common/nonnull.h"
#include "llvm/Support/Error.h"

namespace Carbon {

Expand Down Expand Up @@ -133,7 +134,7 @@ class StaticScope {
public:
// Defines `name` to be `entity` in this scope, or reports a compilation error
// if `name` is already defined to be a different entity in this scope.
void Add(std::string name, ValueNodeView entity);
auto Add(std::string name, ValueNodeView entity) -> llvm::Error;

// Make `parent` a parent of this scope.
// REQUIRES: `parent` is not already a parent of this scope.
Expand All @@ -145,14 +146,14 @@ class StaticScope {
// scope, or reports a compilation error at `source_loc` there isn't exactly
// one such definition.
auto Resolve(const std::string& name, SourceLocation source_loc) const
-> ValueNodeView;
-> llvm::Expected<ValueNodeView>;

private:
// Equivalent to Resolve, but returns `nullopt` instead of raising an error
// if no definition can be found. Still raises a compilation error if more
// than one definition is found.
auto TryResolve(const std::string& name, SourceLocation source_loc) const
-> std::optional<ValueNodeView>;
-> llvm::Expected<std::optional<ValueNodeView>>;

// Maps locally declared names to their entities.
std::unordered_map<std::string, ValueNodeView> declared_names_;
Expand Down
3 changes: 3 additions & 0 deletions executable_semantics/common/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ cc_library(
hdrs = ["error.h"],
deps = [
"//common:check",
"//executable_semantics/ast:source_location",
"@llvm-project//llvm:Support",
],
)

Expand All @@ -26,6 +28,7 @@ cc_test(
deps = [
":error",
"@com_google_googletest//:gtest_main",
"@llvm-project//llvm:Support",
],
)

Expand Down
Loading