Skip to content

Commit

Permalink
Merge pull request #7452 from smores56/remove-backpassing
Browse files Browse the repository at this point in the history 
Remove backpassing
  • Loading branch information
@smores56
smores56 authored Jan 2, 2025
2 parents b6082f8 + cbcbfd3 commit 91ed6a5
Show file tree
Hide file tree
Showing 94 changed files with 231 additions and 2,246 deletions.
3 changes: 1 addition & 2 deletions crates/compiler/DESIGN.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,7 @@ program. Among other things, canonicalization
solving.
- Determines the order definitions are used in, if they are defined
out-of-order.
- Eliminates syntax sugar (for example, renaming `+` to the function call `add`
and converting backpassing to function calls).
- Eliminates syntax sugar (for example, renaming `+` to the function call `add`).
- Collects declared abilities, and ability implementations defined for opaque
types. Derived abilities for opaque types are elaborated during
canonicalization.
Expand Down
44 changes: 0 additions & 44 deletions crates/compiler/can/src/desugar.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -734,50 +734,6 @@ pub fn desugar_expr<'a>(
desugar_expr(env, scope, loc_ret),
),
}),
Backpassing(loc_patterns, loc_body, loc_ret) => {
// loc_patterns <- loc_body
//
// loc_ret

let problem_region = Region::span_across(
&Region::across_all(loc_patterns.iter().map(|loc_pattern| &loc_pattern.region)),
&loc_body.region,
);
env.problem(Problem::DeprecatedBackpassing(problem_region));

// first desugar the body, because it may contain |>
let desugared_body = desugar_expr(env, scope, loc_body);

let desugared_ret = desugar_expr(env, scope, loc_ret);
let desugared_loc_patterns = desugar_loc_patterns(env, scope, loc_patterns);
let closure = Expr::Closure(desugared_loc_patterns, desugared_ret);
let loc_closure = Loc::at(loc_expr.region, closure);

match &desugared_body.value {
Expr::Apply(function, arguments, called_via) => {
let mut new_arguments: Vec<'a, &'a Loc<Expr<'a>>> =
Vec::with_capacity_in(arguments.len() + 1, env.arena);
new_arguments.extend(arguments.iter());
new_arguments.push(env.arena.alloc(loc_closure));

let call = Expr::Apply(function, new_arguments.into_bump_slice(), *called_via);
let loc_call = Loc::at(loc_expr.region, call);

env.arena.alloc(loc_call)
}
_ => {
// e.g. `x <- (if b then (\a -> a) else (\c -> c))`
let call = Expr::Apply(
desugared_body,
env.arena.alloc([&*env.arena.alloc(loc_closure)]),
CalledVia::Space,
);
let loc_call = Loc::at(loc_expr.region, call);

env.arena.alloc(loc_call)
}
}
}
RecordBuilder { mapper, fields } => {
// NOTE the `mapper` is always a `Var { .. }`, we only desugar it to get rid of
// any spaces before/after
Expand Down
4 changes: 0 additions & 4 deletions crates/compiler/can/src/expr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1148,9 +1148,6 @@ pub fn canonicalize_expr<'a>(
ast::Expr::RecordBuilder { .. } => {
internal_error!("Record builder should have been desugared by now")
}
ast::Expr::Backpassing(_, _, _) => {
internal_error!("Backpassing should have been desugared by now")
}
ast::Expr::RecordUpdater(_) => {
internal_error!("Record updater should have been desugared by now")
}
Expand Down Expand Up @@ -2219,7 +2216,6 @@ pub fn is_valid_interpolation(expr: &ast::Expr<'_>) -> bool {
| ast::Expr::LowLevelDbg(_, _, _)
| ast::Expr::Return(_, _)
| ast::Expr::When(_, _)
| ast::Expr::Backpassing(_, _, _)
| ast::Expr::SpaceBefore(_, _)
| ast::Expr::Str(StrLiteral::Block(_))
| ast::Expr::SpaceAfter(_, _) => false,
Expand Down
3 changes: 1 addition & 2 deletions crates/compiler/fmt/src/def.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1039,7 +1039,6 @@ pub fn fmt_body<'a>(
&& pattern_extracted.after.iter().all(|s| s.is_newline())
&& !matches!(body.extract_spaces().item, Expr::Defs(..))
&& !matches!(body.extract_spaces().item, Expr::Return(..))
&& !matches!(body.extract_spaces().item, Expr::Backpassing(..))
&& !matches!(body.extract_spaces().item, Expr::DbgStmt { .. })
&& !starts_with_expect_ident(body)
} else {
Expand Down Expand Up @@ -1116,7 +1115,7 @@ pub fn fmt_body<'a>(
buf.ensure_ends_with_newline();
body.format_with_options(buf, Parens::NotNeeded, Newlines::Yes, indent + INDENT);
}
Expr::Defs(..) | Expr::BinOps(_, _) | Expr::Backpassing(..) => {
Expr::Defs(..) | Expr::BinOps(_, _) => {
// Binop chains always get a newline. Otherwise you can have things like:
//
// something = foo
Expand Down
167 changes: 3 additions & 164 deletions crates/compiler/fmt/src/expr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ use crate::annotation::{except_last, is_collection_multiline, Formattable, Newli
use crate::collection::{fmt_collection, Braces};
use crate::def::{fmt_defs, valdef_lift_spaces_before};
use crate::pattern::{
fmt_pattern, pattern_lift_spaces, pattern_lift_spaces_before, snakify_camel_ident,
starts_with_inline_comment,
fmt_pattern, pattern_lift_spaces, snakify_camel_ident, starts_with_inline_comment,
};
use crate::spaces::{
count_leading_newlines, fmt_comments_only, fmt_spaces, fmt_spaces_no_blank_lines,
Expand Down Expand Up @@ -166,9 +165,6 @@ fn format_expr_only(
Expr::Closure(loc_patterns, loc_ret) => {
fmt_closure(buf, loc_patterns, loc_ret, indent);
}
Expr::Backpassing(loc_patterns, loc_body, loc_ret) => {
fmt_backpassing(buf, loc_patterns, loc_body, loc_ret, indent);
}
Expr::Defs(defs, ret) => {
let defs_needs_parens = parens == Parens::InOperator || parens == Parens::InApply;

Expand Down Expand Up @@ -566,14 +562,6 @@ pub fn expr_is_multiline(me: &Expr<'_>, comments_only: bool) -> bool {
.iter()
.any(|loc_pattern| loc_pattern.value.is_multiline())
}
Expr::Backpassing(loc_patterns, loc_body, loc_ret) => {
// check the body first because it's more likely to be multiline
expr_is_multiline(&loc_body.value, comments_only)
|| expr_is_multiline(&loc_ret.value, comments_only)
|| loc_patterns
.iter()
.any(|loc_pattern| loc_pattern.value.is_multiline())
}

Expr::Record(fields) => is_collection_multiline(fields),
Expr::Tuple(fields) => is_collection_multiline(fields),
Expand Down Expand Up @@ -1225,37 +1213,6 @@ pub fn expr_lift_spaces<'a, 'b: 'a>(
}
}
}
Expr::Backpassing(pats, call, continuation) => {
let pats = arena.alloc_slice_copy(pats);
let before = if let Some(first) = pats.first_mut() {
let lifted = pattern_lift_spaces_before(arena, &first.value);
*first = Loc::at(first.region, lifted.item);
lifted.before
} else {
&[]
};
let continuation_lifted =
expr_lift_spaces_after(Parens::NotNeeded, arena, &continuation.value);

let mut res = Spaces {
before,
item: Expr::Backpassing(
pats,
call,
arena.alloc(Loc::at(continuation.region, continuation_lifted.item)),
),
after: continuation_lifted.after,
};

if parens == Parens::InApply || parens == Parens::InApplyLastArg {
res = Spaces {
before: &[],
item: Expr::ParensAround(arena.alloc(lower(arena, res))),
after: &[],
};
}
res
}
Expr::SpaceBefore(expr, spaces) => {
let mut inner = expr_lift_spaces(parens, arena, expr);
inner.before = merge_spaces_conservative(arena, spaces, inner.before);
Expand Down Expand Up @@ -1825,7 +1782,7 @@ fn fmt_return<'a>(
format_spaces(buf, value.before, newlines, return_indent);
}

if matches!(value.item, Expr::Defs(..) | Expr::Backpassing(..)) {
if matches!(value.item, Expr::Defs(..)) {
buf.ensure_ends_with_newline();
} else {
buf.spaces(1);
Expand Down Expand Up @@ -2025,124 +1982,6 @@ fn fmt_closure<'a>(
}
}

fn fmt_backpassing<'a>(
buf: &mut Buf,
loc_patterns: &'a [Loc<Pattern<'a>>],
loc_body: &'a Loc<Expr<'a>>,
loc_ret: &'a Loc<Expr<'a>>,
outer_indent: u16,
) {
let arguments_are_multiline = loc_patterns
.iter()
.any(|loc_pattern| loc_pattern.is_multiline());

// If the arguments are multiline, go down a line and indent.
let arg_indent = if arguments_are_multiline {
outer_indent + INDENT
} else {
outer_indent
};

let mut first = true;

for loc_pattern in loc_patterns.iter() {
let needs_parens = if pattern_needs_parens_when_backpassing(&loc_pattern.value) {
Parens::InApply
} else {
Parens::NotNeeded
};

let pat = pattern_lift_spaces(buf.text.bump(), &loc_pattern.value);

if !first {
buf.indent(arg_indent);
buf.push(',');
}

fmt_comments_only(buf, pat.before.iter(), NewlineAt::Bottom, arg_indent);

if !first {
if arguments_are_multiline {
buf.ensure_ends_with_newline();
} else {
buf.spaces(1);
}
}

pat.item.format_with_options(
buf,
needs_parens,
Newlines::No,
if first { outer_indent } else { arg_indent },
);
fmt_comments_only(buf, pat.after.iter(), NewlineAt::Bottom, arg_indent);

first = false;
}

if arguments_are_multiline {
buf.ensure_ends_with_newline();
buf.indent(arg_indent);
} else {
buf.spaces(1);
}

buf.push_str("<-");

let is_multiline = loc_ret.value.is_multiline();

// If the body is multiline, go down a line and indent.
let body_indent = if is_multiline {
arg_indent + INDENT
} else {
arg_indent
};

buf.spaces(1);
let body_lifted = expr_lift_spaces(Parens::NotNeeded, buf.text.bump(), &loc_body.value);
let ret_lifted = expr_lift_spaces(Parens::NotNeeded, buf.text.bump(), &loc_ret.value);

if !body_lifted.before.is_empty() {
format_spaces(buf, body_lifted.before, Newlines::Yes, body_indent);
}

format_expr_only(
&body_lifted.item,
buf,
Parens::NotNeeded,
Newlines::Yes,
body_indent,
);

let between = merge_spaces(buf.text.bump(), body_lifted.after, ret_lifted.before);

if !between.is_empty() {
format_spaces(buf, between, Newlines::Yes, outer_indent);
}

format_expr_only(
&ret_lifted.item,
buf,
Parens::NotNeeded,
Newlines::Yes,
outer_indent,
);

if !ret_lifted.after.is_empty() {
format_spaces(buf, ret_lifted.after, Newlines::Yes, outer_indent);
}
}

fn pattern_needs_parens_when_backpassing(pat: &Pattern) -> bool {
match pat {
Pattern::Apply(_, _) => true,
Pattern::SpaceBefore(a, _) | Pattern::SpaceAfter(a, _) => {
pattern_needs_parens_when_backpassing(a)
}
_ => false,
}
}

enum RecordPrefix<'a> {
Update(&'a Loc<Expr<'a>>),
Mapper(&'a Loc<Expr<'a>>),
Expand Down Expand Up @@ -2321,7 +2160,7 @@ pub fn sub_expr_requests_parens(expr: &Expr<'_>) -> bool {
}
Expr::If { .. } => true,
Expr::Defs(_, _) => true,
Expr::Return(..) | Expr::Backpassing(..) | Expr::DbgStmt { .. } => {
Expr::Return(..) | Expr::DbgStmt { .. } => {
// This is because e.g. (return x)\nfoo would be de-parenthesized and cause the `after_return` to be `foo`.
// That _is_ a semantic change technically right now, because that transform is done in the parser.
// When that's moved to `can`, we can remove this
Expand Down
25 changes: 0 additions & 25 deletions crates/compiler/load/tests/test_reporting.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11685,31 +11685,6 @@ All branches in an `if` must have the same type!
"
);

test_report!(
deprecated_backpassing,
indoc!(
r#"
foo = \bar ->
baz <- Result.try bar
Ok (baz * 3)
foo (Ok 123)
"#
),
@r###"
── BACKPASSING DEPRECATED in /code/proj/Main.roc ───────────────────────────────
Backpassing (<-) like this will soon be deprecated:
5│ baz <- Result.try bar
^^^^^^^^^^^^^^^^^^^^^
You should use a ! for awaiting tasks or a ? for trying results, and
functions everywhere else.
"###
);

test_report!(
unknown_shorthand_no_deps,
indoc!(
Expand Down
9 changes: 0 additions & 9 deletions crates/compiler/parse/src/ast.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -535,8 +535,6 @@ pub enum Expr<'a> {
/// Multiple defs in a row
Defs(&'a Defs<'a>, &'a Loc<Expr<'a>>),

Backpassing(&'a [Loc<Pattern<'a>>], &'a Loc<Expr<'a>>, &'a Loc<Expr<'a>>),

Dbg,
DbgStmt {
first: &'a Loc<Expr<'a>>,
Expand Down Expand Up @@ -722,7 +720,6 @@ pub fn is_expr_suffixed(expr: &Expr) -> bool {
Expr::Crash => false,
Expr::Tag(_) => false,
Expr::OpaqueRef(_) => false,
Expr::Backpassing(_, _, _) => false, // TODO: we might want to check this?
Expr::Dbg => false,
Expr::DbgStmt {
first,
Expand Down Expand Up @@ -987,11 +984,6 @@ impl<'a, 'b> RecursiveValueDefIter<'a, 'b> {
push_stack_from_record_fields!(fields);
}
Closure(_, body) => expr_stack.push(&body.value),
Backpassing(_, a, b) => {
expr_stack.reserve(2);
expr_stack.push(&a.value);
expr_stack.push(&b.value);
}
DbgStmt {
first,
extra_args,
Expand Down Expand Up @@ -2575,7 +2567,6 @@ impl<'a> Malformed for Expr<'a> {

Closure(args, body) => args.iter().any(|arg| arg.is_malformed()) || body.is_malformed(),
Defs(defs, body) => defs.is_malformed() || body.is_malformed(),
Backpassing(args, call, body) => args.iter().any(|arg| arg.is_malformed()) || call.is_malformed() || body.is_malformed(),
Dbg => false,
DbgStmt { first, extra_args, continuation } => first.is_malformed() || extra_args.iter().any(|a| a.is_malformed()) || continuation.is_malformed(),
LowLevelDbg(_, condition, continuation) => condition.is_malformed() || continuation.is_malformed(),
Expand Down
Loading

0 comments on commit 91ed6a5

Please sign in to comment.