Skip to content

Commit

Permalink
More error handling refactoring (FuelLabs#4895)
Browse files Browse the repository at this point in the history
## Description
This changes addresses the issues introduced in FuelLabs#4878

The `error_emitted` pattern is replaced by a scoping method on `Handler`
that collects errors within a closure and will error out if new erros
are introduced.

All instances of introducing `ErrorEmitted` as a literal value have now
been replaced by passing arround the receipt of the error emission
through the error representing values. Some types from the AST had to be
moved to `sway-types` to avoid circular dependencies introduced by this
change.

This also repairs some of the discrepancies to emitted warnings
introduced in FuelLabs#4878, as well as rename some of the `Handler` methods.

Fixes FuelLabs#4885
Fixes FuelLabs#4886

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.
  • Loading branch information
IGI-111 authored Aug 1, 2023
1 parent 0341a7e commit f48268e
Show file tree
Hide file tree
Showing 103 changed files with 1,566 additions and 1,752 deletions.
4 changes: 1 addition & 3 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 6 additions & 6 deletions forc-pkg/src/pkg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1800,8 +1800,8 @@ pub fn compile(
Ok(programs) => programs,
};
let typed_program = match programs.typed.as_ref() {
None => return fail(handler),
Some(typed_program) => typed_program,
Err(_) => return fail(handler),
Ok(typed_program) => typed_program,
};

if profile.print_ast {
Expand All @@ -1813,7 +1813,7 @@ pub fn compile(

let namespace = typed_program.root.namespace.clone().into();

if handler.has_error() {
if handler.has_errors() {
return fail(handler);
}

Expand Down Expand Up @@ -1893,7 +1893,7 @@ pub fn compile(
metrics
);

let errored = handler.has_error() || (handler.has_warning() && profile.error_on_warnings);
let errored = handler.has_errors() || (handler.has_warnings() && profile.error_on_warnings);

let compiled = match bc_res {
Ok(compiled) if !errored => compiled,
Expand Down Expand Up @@ -2663,7 +2663,7 @@ pub fn check(
};

match programs.typed.as_ref() {
Some(typed_program) => {
Ok(typed_program) => {
if let TreeType::Library = typed_program.kind.tree_type() {
let mut namespace = typed_program.root.namespace.clone();
namespace.name = Some(Ident::new_no_span(pkg.name.clone()));
Expand All @@ -2681,7 +2681,7 @@ pub fn check(

source_map.insert_dependency(manifest.dir());
}
None => {
Err(_) => {
results.push((programs_res.ok(), handler));
return Ok(results);
}
Expand Down
4 changes: 2 additions & 2 deletions forc-plugins/forc-doc/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ pub fn main() -> Result<()> {

if let Some(pkg_manifest_file) = manifest_map.get(id) {
let manifest_file = ManifestFile::from_dir(pkg_manifest_file.path())?;
let ty_program = match compile_result.and_then(|programs| programs.typed) {
let ty_program = match compile_result.and_then(|programs| programs.typed.ok()) {
Some(ty_program) => ty_program,
_ => bail!(
"documentation could not be built from manifest located at '{}'",
Expand All @@ -130,7 +130,7 @@ pub fn main() -> Result<()> {
let ty_program = match compile_results
.pop()
.and_then(|(programs, _handler)| programs)
.and_then(|p| p.typed)
.and_then(|p| p.typed.ok())
{
Some(ty_program) => ty_program,
_ => bail!(
Expand Down
2 changes: 1 addition & 1 deletion forc/src/ops/forc_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,6 @@ pub fn check(command: CheckCommand, engines: &Engines) -> Result<(Option<ty::TyP
let (res, handler) = v
.pop()
.expect("there is guaranteed to be at least one elem in the vector");
let res = res.and_then(|programs| programs.typed);
let res = res.and_then(|programs| programs.typed.ok());
Ok((res, handler))
}
1 change: 1 addition & 0 deletions sway-ast/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,5 @@ extension-trait = "1.0.1"
num-bigint = { version = "0.4.3", features = ["serde"] }
num-traits = "0.2.14"
serde = { version = "1.0", features = ["derive"] }
sway-error = { version = "0.42.1", path = "../sway-error" }
sway-types = { version = "0.42.1", path = "../sway-types" }
6 changes: 4 additions & 2 deletions sway-ast/src/expr/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use sway_error::handler::ErrorEmitted;

use crate::{priv_prelude::*, PathExprSegment};

pub mod asm;
Expand All @@ -8,7 +10,7 @@ pub enum Expr {
/// A malformed expression.
///
/// Used for parser recovery when we cannot form a more specific node.
Error(Box<[Span]>),
Error(Box<[Span]>, #[serde(skip_serializing)] ErrorEmitted),
Path(PathExpr),
Literal(Literal),
AbiCast {
Expand Down Expand Up @@ -188,7 +190,7 @@ pub enum Expr {
impl Spanned for Expr {
fn span(&self) -> Span {
match self {
Expr::Error(spans) => spans.iter().cloned().reduce(Span::join).unwrap(),
Expr::Error(spans, _) => spans.iter().cloned().reduce(Span::join).unwrap(),
Expr::Path(path_expr) => path_expr.span(),
Expr::Literal(literal) => literal.span(),
Expr::AbiCast { abi_token, args } => Span::join(abi_token.span(), args.span()),
Expand Down
6 changes: 4 additions & 2 deletions sway-ast/src/pattern.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use sway_error::handler::ErrorEmitted;

use crate::priv_prelude::*;

#[derive(Clone, Debug, Serialize)]
Expand Down Expand Up @@ -29,7 +31,7 @@ pub enum Pattern {
},
Tuple(Parens<Punctuated<Pattern, CommaToken>>),
// to handle parser recovery: Error represents an incomplete Constructor
Error(Box<[Span]>),
Error(Box<[Span]>, #[serde(skip_serializing)] ErrorEmitted),
}

impl Spanned for Pattern {
Expand Down Expand Up @@ -59,7 +61,7 @@ impl Spanned for Pattern {
Pattern::Constructor { path, args } => Span::join(path.span(), args.span()),
Pattern::Struct { path, fields } => Span::join(path.span(), fields.span()),
Pattern::Tuple(pat_tuple) => pat_tuple.span(),
Pattern::Error(spans) => spans.iter().cloned().reduce(Span::join).unwrap(),
Pattern::Error(spans, _) => spans.iter().cloned().reduce(Span::join).unwrap(),
}
}
}
Expand Down
7 changes: 5 additions & 2 deletions sway-ast/src/priv_prelude.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ pub use {
punctuated::Punctuated,
statement::{Statement, StatementLet},
submodule::Submodule,
token::{Delimiter, Group, Punct, PunctKind, Spacing, TokenStream, TokenTree},
token::{Group, Punct, Spacing, TokenStream, TokenTree},
ty::Ty,
where_clause::{WhereBound, WhereClause},
},
Expand All @@ -41,5 +41,8 @@ pub use {
std::{
fmt, marker::PhantomData, mem, ops::ControlFlow, path::PathBuf, str::FromStr, sync::Arc,
},
sway_types::{Ident, Span, Spanned},
sway_types::{
ast::{Delimiter, PunctKind},
Ident, Span, Spanned,
},
};
71 changes: 0 additions & 71 deletions sway-ast/src/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,28 +6,6 @@ pub enum Spacing {
Alone,
}

#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub enum PunctKind {
Semicolon,
Colon,
ForwardSlash,
Comma,
Star,
Add,
Sub,
LessThan,
GreaterThan,
Equals,
Dot,
Bang,
Percent,
Ampersand,
Caret,
Pipe,
Underscore,
Sharp,
}

#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Hash)]
pub struct Punct {
pub span: Span,
Expand All @@ -41,31 +19,6 @@ impl Spanned for Punct {
}
}

impl PunctKind {
pub fn as_char(&self) -> char {
match self {
PunctKind::Semicolon => ';',
PunctKind::Colon => ':',
PunctKind::ForwardSlash => '/',
PunctKind::Comma => ',',
PunctKind::Star => '*',
PunctKind::Add => '+',
PunctKind::Sub => '-',
PunctKind::LessThan => '<',
PunctKind::GreaterThan => '>',
PunctKind::Equals => '=',
PunctKind::Dot => '.',
PunctKind::Bang => '!',
PunctKind::Percent => '%',
PunctKind::Ampersand => '&',
PunctKind::Caret => '^',
PunctKind::Pipe => '|',
PunctKind::Underscore => '_',
PunctKind::Sharp => '#',
}
}
}

#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Hash)]
pub struct GenericGroup<T> {
pub delimiter: Delimiter,
Expand All @@ -82,30 +35,6 @@ impl<T> Spanned for GenericGroup<T> {
}
}

#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub enum Delimiter {
Parenthesis,
Brace,
Bracket,
}

impl Delimiter {
pub fn as_open_char(self) -> char {
match self {
Delimiter::Parenthesis => '(',
Delimiter::Brace => '{',
Delimiter::Bracket => '[',
}
}
pub fn as_close_char(self) -> char {
match self {
Delimiter::Parenthesis => ')',
Delimiter::Brace => '}',
Delimiter::Bracket => ']',
}
}
}

#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Hash)]
pub enum CommentKind {
/// A newlined comment is a comment with a preceding newline before another token.
Expand Down
2 changes: 1 addition & 1 deletion sway-core/src/abi_generation/evm_abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ pub fn abi_str(type_info: &TypeInfo, type_engine: &TypeEngine, decl_engine: &Dec
B256 => "uint256".into(),
Numeric => "u64".into(), // u64 is the default
Contract => "contract".into(),
ErrorRecovery => "unknown due to error".into(),
ErrorRecovery(_) => "unknown due to error".into(),
Enum(decl_ref) => {
let decl = decl_engine.get_enum(decl_ref);
format!("enum {}", decl.call_path.suffix)
Expand Down
2 changes: 1 addition & 1 deletion sway-core/src/abi_generation/fuel_abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -806,7 +806,7 @@ impl TypeInfo {
B256 => "b256".into(),
Numeric => "u64".into(), // u64 is the default
Contract => "contract".into(),
ErrorRecovery => "unknown due to error".into(),
ErrorRecovery(_) => "unknown due to error".into(),
Enum(decl_ref) => {
let decl = decl_engine.get_enum(decl_ref);
format!("enum {}", call_path_display(ctx, &decl.call_path))
Expand Down
Loading

0 comments on commit f48268e

Please sign in to comment.