diff --git a/crates/cli/tests/cli_run.rs b/crates/cli/tests/cli_run.rs index 1544a888873..a27aa457ab0 100644 --- a/crates/cli/tests/cli_run.rs +++ b/crates/cli/tests/cli_run.rs @@ -922,6 +922,25 @@ mod cli_run { ); } + #[test] + #[cfg_attr(windows, ignore)] + fn module_params_multiline_pattern() { + test_roc_app( + "crates/cli/tests/module_params", + "multiline_params.roc", + &[], + &[], + &[], + indoc!( + r#" + hi + "# + ), + UseValgrind::No, + TestCliCommands::Dev, + ); + } + #[test] #[cfg_attr(windows, ignore)] fn transitive_expects() { diff --git a/crates/cli/tests/module_params/MultilineParams.roc b/crates/cli/tests/module_params/MultilineParams.roc new file mode 100644 index 00000000000..631b88473be --- /dev/null +++ b/crates/cli/tests/module_params/MultilineParams.roc @@ -0,0 +1,8 @@ +module { + sendHttpReq, + getEnvVar +} -> [hi] + +hi : Str +hi = + "hi" diff --git a/crates/cli/tests/module_params/multiline_params.roc b/crates/cli/tests/module_params/multiline_params.roc new file mode 100644 index 00000000000..531721c9258 --- /dev/null +++ b/crates/cli/tests/module_params/multiline_params.roc @@ -0,0 +1,11 @@ +app [main] { + pf: platform "../fixtures/multi-dep-str/platform/main.roc", +} + +import MultilineParams { + sendHttpReq: \_ -> crash "todo", + getEnvVar: \_ -> crash "todo", +} + +main = + MultilineParams.hi \ No newline at end of file diff --git a/crates/compiler/can/src/desugar.rs b/crates/compiler/can/src/desugar.rs index 3f4872366e4..1e253c61346 100644 --- a/crates/compiler/can/src/desugar.rs +++ b/crates/compiler/can/src/desugar.rs @@ -11,8 +11,8 @@ use roc_module::called_via::{BinOp, CalledVia}; use roc_module::ident::ModuleName; use roc_parse::ast::Expr::{self, *}; use roc_parse::ast::{ - AssignedField, Collection, Defs, ModuleImportParams, OldRecordBuilderField, Pattern, - StrLiteral, StrSegment, TypeAnnotation, ValueDef, WhenBranch, + AssignedField, Collection, Defs, ModuleImportParams, Pattern, StrLiteral, StrSegment, + TypeAnnotation, ValueDef, WhenBranch, }; use roc_problem::can::Problem; use roc_region::all::{Loc, Region}; @@ -321,8 +321,6 @@ pub fn desugar_expr<'a>( | MalformedClosure | MalformedSuffixed(..) | PrecedenceConflict { .. } - | MultipleOldRecordBuilders(_) - | UnappliedOldRecordBuilder(_) | EmptyRecordBuilder(_) | SingleFieldRecordBuilder(_) | OptionalFieldInRecordBuilder { .. } @@ -555,10 +553,6 @@ pub fn desugar_expr<'a>( } } } - OldRecordBuilder(_) => env.arena.alloc(Loc { - value: UnappliedOldRecordBuilder(loc_expr), - region: loc_expr.region, - }), RecordBuilder { mapper, fields } => { // NOTE the `mapper` is always a `Var { .. }`, we only desugar it to get rid of // any spaces before/after @@ -853,25 +847,11 @@ pub fn desugar_expr<'a>( } Apply(loc_fn, loc_args, called_via) => { let mut desugared_args = Vec::with_capacity_in(loc_args.len(), env.arena); - let mut builder_apply_exprs = None; for loc_arg in loc_args.iter() { let mut current = loc_arg.value; let arg = loop { match current { - OldRecordBuilder(fields) => { - if builder_apply_exprs.is_some() { - return env.arena.alloc(Loc { - value: MultipleOldRecordBuilders(loc_expr), - region: loc_expr.region, - }); - } - - let builder_arg = old_record_builder_arg(env, loc_arg.region, fields); - builder_apply_exprs = Some(builder_arg.apply_exprs); - - break builder_arg.closure; - } SpaceBefore(expr, _) | SpaceAfter(expr, _) => { current = *expr; } @@ -884,33 +864,14 @@ pub fn desugar_expr<'a>( let desugared_args = desugared_args.into_bump_slice(); - let mut apply: &Loc = env.arena.alloc(Loc { + env.arena.alloc(Loc { value: Apply( desugar_expr(env, scope, loc_fn), desugared_args, *called_via, ), region: loc_expr.region, - }); - - match builder_apply_exprs { - None => {} - - Some(apply_exprs) => { - for expr in apply_exprs { - let desugared_expr = desugar_expr(env, scope, expr); - - let args = std::slice::from_ref(env.arena.alloc(apply)); - - apply = env.arena.alloc(Loc { - value: Apply(desugared_expr, args, CalledVia::OldRecordBuilder), - region: loc_expr.region, - }); - } - } - } - - apply + }) } When(loc_cond_expr, branches) => { let loc_desugared_cond = &*env.arena.alloc(desugar_expr(env, scope, loc_cond_expr)); @@ -1226,17 +1187,7 @@ fn desugar_pattern<'a>(env: &mut Env<'a>, scope: &mut Scope, pattern: Pattern<'a Apply(tag, desugared_arg_patterns.into_bump_slice()) } RecordDestructure(field_patterns) => { - let mut allocated = Vec::with_capacity_in(field_patterns.len(), env.arena); - for field_pattern in field_patterns.iter() { - let value = desugar_pattern(env, scope, field_pattern.value); - allocated.push(Loc { - value, - region: field_pattern.region, - }); - } - let field_patterns = field_patterns.replace_items(allocated.into_bump_slice()); - - RecordDestructure(field_patterns) + RecordDestructure(desugar_record_destructures(env, scope, field_patterns)) } RequiredField(name, field_pattern) => { RequiredField(name, desugar_loc_pattern(env, scope, field_pattern)) @@ -1274,6 +1225,23 @@ fn desugar_pattern<'a>(env: &mut Env<'a>, scope: &mut Scope, pattern: Pattern<'a } } +pub fn desugar_record_destructures<'a>( + env: &mut Env<'a>, + scope: &mut Scope, + field_patterns: Collection<'a, Loc>>, +) -> Collection<'a, Loc>> { + let mut allocated = Vec::with_capacity_in(field_patterns.len(), env.arena); + for field_pattern in field_patterns.iter() { + let value = desugar_pattern(env, scope, field_pattern.value); + allocated.push(Loc { + value, + region: field_pattern.region, + }); + } + + field_patterns.replace_items(allocated.into_bump_slice()) +} + /// Desugars a `dbg expr` expression into a statement block that prints and returns the /// value produced by `expr`. Essentially: /// ( @@ -1383,97 +1351,6 @@ fn desugar_dbg_stmt<'a>( )) } -struct OldRecordBuilderArg<'a> { - closure: &'a Loc>, - apply_exprs: Vec<'a, &'a Loc>>, -} - -fn old_record_builder_arg<'a>( - env: &mut Env<'a>, - region: Region, - fields: Collection<'a, Loc>>, -) -> OldRecordBuilderArg<'a> { - let mut record_fields = Vec::with_capacity_in(fields.len(), env.arena); - let mut apply_exprs = Vec::with_capacity_in(fields.len(), env.arena); - let mut apply_field_names = Vec::with_capacity_in(fields.len(), env.arena); - - // Build the record that the closure will return and gather apply expressions - - for field in fields.iter() { - let mut current = field.value; - - let new_field = loop { - match current { - OldRecordBuilderField::Value(label, spaces, expr) => { - break AssignedField::RequiredValue(label, spaces, expr) - } - OldRecordBuilderField::ApplyValue(label, _, _, expr) => { - apply_field_names.push(label); - apply_exprs.push(expr); - - let var = env.arena.alloc(Loc { - region: label.region, - value: Expr::Var { - module_name: "", - ident: env.arena.alloc("#".to_owned() + label.value), - }, - }); - - break AssignedField::RequiredValue(label, &[], var); - } - OldRecordBuilderField::LabelOnly(label) => break AssignedField::LabelOnly(label), - OldRecordBuilderField::SpaceBefore(sub_field, _) => { - current = *sub_field; - } - OldRecordBuilderField::SpaceAfter(sub_field, _) => { - current = *sub_field; - } - OldRecordBuilderField::Malformed(malformed) => { - break AssignedField::Malformed(malformed) - } - } - }; - - record_fields.push(Loc { - value: new_field, - region: field.region, - }); - } - - let record_fields = fields.replace_items(record_fields.into_bump_slice()); - - let mut body = env.arena.alloc(Loc { - value: Record(record_fields), - region, - }); - - // Construct the builder's closure - // - // { x: #x, y: #y, z: 3 } - // \#y -> { x: #x, y: #y, z: 3 } - // \#x -> \#y -> { x: #x, y: #y, z: 3 } - - for label in apply_field_names.iter().rev() { - let name = env.arena.alloc("#".to_owned() + label.value); - let ident = roc_parse::ast::Pattern::Identifier { ident: name }; - - let arg_pattern = env.arena.alloc(Loc { - value: ident, - region: label.region, - }); - - body = env.arena.alloc(Loc { - value: Closure(std::slice::from_ref(arg_pattern), body), - region, - }); - } - - OldRecordBuilderArg { - closure: body, - apply_exprs, - } -} - // TODO move this desugaring to canonicalization, so we can use Symbols instead of strings #[inline(always)] fn binop_to_function(binop: BinOp) -> (&'static str, &'static str) { diff --git a/crates/compiler/can/src/expr.rs b/crates/compiler/can/src/expr.rs index 9955a5a833a..18f295bb0fe 100644 --- a/crates/compiler/can/src/expr.rs +++ b/crates/compiler/can/src/expr.rs @@ -1013,11 +1013,8 @@ pub fn canonicalize_expr<'a>( can_defs_with_return(env, var_store, inner_scope, env.arena.alloc(defs), loc_ret) }) } - ast::Expr::OldRecordBuilder(_) => { - internal_error!("Old record builder should have been desugared by now") - } ast::Expr::RecordBuilder { .. } => { - internal_error!("New record builder should have been desugared by now") + internal_error!("Record builder should have been desugared by now") } ast::Expr::Backpassing(_, _, _) => { internal_error!("Backpassing should have been desugared by now") @@ -1356,22 +1353,6 @@ pub fn canonicalize_expr<'a>( use roc_problem::can::RuntimeError::*; (RuntimeError(MalformedSuffixed(region)), Output::default()) } - ast::Expr::MultipleOldRecordBuilders(sub_expr) => { - use roc_problem::can::RuntimeError::*; - - let problem = MultipleOldRecordBuilders(sub_expr.region); - env.problem(Problem::RuntimeError(problem.clone())); - - (RuntimeError(problem), Output::default()) - } - ast::Expr::UnappliedOldRecordBuilder(sub_expr) => { - use roc_problem::can::RuntimeError::*; - - let problem = UnappliedOldRecordBuilder(sub_expr.region); - env.problem(Problem::RuntimeError(problem.clone())); - - (RuntimeError(problem), Output::default()) - } ast::Expr::EmptyRecordBuilder(sub_expr) => { use roc_problem::can::RuntimeError::*; @@ -2552,8 +2533,6 @@ pub fn is_valid_interpolation(expr: &ast::Expr<'_>) -> bool { .iter() .all(|loc_field| is_valid_interpolation(&loc_field.value)), ast::Expr::MalformedSuffixed(loc_expr) - | ast::Expr::MultipleOldRecordBuilders(loc_expr) - | ast::Expr::UnappliedOldRecordBuilder(loc_expr) | ast::Expr::EmptyRecordBuilder(loc_expr) | ast::Expr::SingleFieldRecordBuilder(loc_expr) | ast::Expr::OptionalFieldInRecordBuilder(_, loc_expr) @@ -2603,27 +2582,6 @@ pub fn is_valid_interpolation(expr: &ast::Expr<'_>) -> bool { | ast::AssignedField::SpaceAfter(_, _) => false, }) } - ast::Expr::OldRecordBuilder(fields) => { - fields.iter().all(|loc_field| match loc_field.value { - ast::OldRecordBuilderField::Value(_label, comments, loc_expr) => { - comments.is_empty() && is_valid_interpolation(&loc_expr.value) - } - ast::OldRecordBuilderField::ApplyValue( - _label, - comments_before, - comments_after, - loc_expr, - ) => { - comments_before.is_empty() - && comments_after.is_empty() - && is_valid_interpolation(&loc_expr.value) - } - ast::OldRecordBuilderField::Malformed(_) - | ast::OldRecordBuilderField::LabelOnly(_) => true, - ast::OldRecordBuilderField::SpaceBefore(_, _) - | ast::OldRecordBuilderField::SpaceAfter(_, _) => false, - }) - } ast::Expr::RecordBuilder { mapper, fields } => { is_valid_interpolation(&mapper.value) && fields.iter().all(|loc_field| match loc_field.value { diff --git a/crates/compiler/can/src/module.rs b/crates/compiler/can/src/module.rs index 14dc2f827b8..3a45b6ba912 100644 --- a/crates/compiler/can/src/module.rs +++ b/crates/compiler/can/src/module.rs @@ -3,6 +3,7 @@ use std::path::Path; use crate::abilities::{AbilitiesStore, ImplKey, PendingAbilitiesStore, ResolvedImpl}; use crate::annotation::{canonicalize_annotation, AnnotationFor}; use crate::def::{canonicalize_defs, report_unused_imports, Def}; +use crate::desugar::desugar_record_destructures; use crate::env::Env; use crate::expr::{ ClosureData, DbgLookup, Declarations, ExpectLookup, Expr, Output, PendingDerives, @@ -326,13 +327,16 @@ pub fn canonicalize_module_defs<'a>( before_arrow: _, after_arrow: _, }| { + let desugared_patterns = + desugar_record_destructures(&mut env, &mut scope, pattern.value); + let (destructs, _) = canonicalize_record_destructs( &mut env, var_store, &mut scope, &mut output, PatternType::ModuleParams, - &pattern.value, + &desugared_patterns, pattern.region, PermitShadows(false), ); diff --git a/crates/compiler/can/src/pattern.rs b/crates/compiler/can/src/pattern.rs index ddd6efda308..ffd097e3ae8 100644 --- a/crates/compiler/can/src/pattern.rs +++ b/crates/compiler/can/src/pattern.rs @@ -914,7 +914,10 @@ pub fn canonicalize_record_destructs<'a>( } }; } - _ => unreachable!("Any other pattern should have given a parse error"), + _ => unreachable!( + "Any other pattern should have given a parse error: {:?}", + loc_pattern.value + ), } } diff --git a/crates/compiler/can/tests/test_can.rs b/crates/compiler/can/tests/test_can.rs index 724d1f24b1a..8ecc016a426 100644 --- a/crates/compiler/can/tests/test_can.rs +++ b/crates/compiler/can/tests/test_can.rs @@ -19,7 +19,6 @@ mod test_can { use roc_can::expr::{ClosureData, IntValue, Recursive}; use roc_can::pattern::Pattern; use roc_module::called_via::CalledVia; - use roc_module::symbol::Symbol; use roc_problem::can::{CycleEntry, FloatErrorKind, IntErrorKind, Problem, RuntimeError}; use roc_region::all::{Loc, Position, Region}; use roc_types::subs::Variable; @@ -653,101 +652,6 @@ mod test_can { } // RECORD BUILDERS - #[test] - fn old_record_builder_desugar() { - let src = indoc!( - r#" - succeed = \_ -> crash "succeed" - apply = \_ -> crash "get" - - d = 3 - - succeed { - a: 1, - b: <- apply "b", - c: <- apply "c", - d - } - "# - ); - let arena = Bump::new(); - let out = can_expr_with(&arena, test_home(), src); - - assert_eq!(out.problems.len(), 0); - - // Assert that we desugar to: - // - // (apply "c") ((apply "b") (succeed \b -> \c -> { a: 1, b, c, d })) - - // (apply "c") .. - let (apply_c, c_to_b) = simplify_curried_call(&out.loc_expr.value); - assert_apply_call(apply_c, "c", &out.interns); - - // (apply "b") .. - let (apply_b, b_to_succeed) = simplify_curried_call(c_to_b); - assert_apply_call(apply_b, "b", &out.interns); - - // (succeed ..) - let (succeed, b_closure) = simplify_curried_call(b_to_succeed); - - match succeed { - Var(sym, _) => assert_eq!(sym.as_str(&out.interns), "succeed"), - _ => panic!("Not calling succeed: {:?}", succeed), - } - - // \b -> .. - let (b_sym, c_closure) = simplify_builder_closure(b_closure); - - // \c -> .. - let (c_sym, c_body) = simplify_builder_closure(c_closure); - - // { a: 1, b, c, d } - match c_body { - Record { fields, .. } => { - match get_field_expr(fields, "a") { - Num(_, num_str, _, _) => { - assert_eq!(num_str.to_string(), "1"); - } - expr => panic!("a is not a Num: {:?}", expr), - } - - assert_eq!(get_field_var_sym(fields, "b"), b_sym); - assert_eq!(get_field_var_sym(fields, "c"), c_sym); - assert_eq!(get_field_var_sym(fields, "d").as_str(&out.interns), "d"); - } - _ => panic!("Closure body wasn't a Record: {:?}", c_body), - } - } - - fn simplify_curried_call(expr: &Expr) -> (&Expr, &Expr) { - match expr { - LetNonRec(_, loc_expr) => simplify_curried_call(&loc_expr.value), - Call(fun, args, _) => (&fun.1.value, &args[0].1.value), - _ => panic!("Final Expr is not a Call: {:?}", expr), - } - } - - fn assert_apply_call(expr: &Expr, expected: &str, interns: &roc_module::symbol::Interns) { - match simplify_curried_call(expr) { - (Var(sym, _), Str(val)) => { - assert_eq!(sym.as_str(interns), "apply"); - assert_eq!(val.to_string(), expected); - } - call => panic!("Not a valid (get {}) call: {:?}", expected, call), - }; - } - - fn simplify_builder_closure(expr: &Expr) -> (Symbol, &Expr) { - use roc_can::pattern::Pattern::*; - - match expr { - Closure(closure) => match &closure.arguments[0].2.value { - Identifier(sym) => (*sym, &closure.loc_body.value), - pattern => panic!("Not an identifier pattern: {:?}", pattern), - }, - _ => panic!("Not a closure: {:?}", expr), - } - } fn get_field_expr<'a>( fields: &'a roc_collections::SendMap, @@ -769,97 +673,7 @@ mod test_can { } #[test] - fn old_record_builder_field_names_do_not_shadow() { - let src = indoc!( - r#" - succeed = \_ -> crash "succeed" - parse = \_ -> crash "parse" - - number = "42" - - succeed { - number: <- parse number, - raw: number, - } - "# - ); - let arena = Bump::new(); - let out = can_expr_with(&arena, test_home(), src); - - assert_eq!(out.problems.len(), 0); - - let (_, number_to_succeed) = simplify_curried_call(&out.loc_expr.value); - let (_, number_closure) = simplify_curried_call(number_to_succeed); - let (apply_number_sym, record) = simplify_builder_closure(number_closure); - - match record { - Record { fields, .. } => { - assert_eq!(get_field_var_sym(fields, "number"), apply_number_sym); - - match get_field_expr(fields, "raw") { - Var(number_sym, _) => { - assert_ne!(number_sym.ident_id(), apply_number_sym.ident_id()); - assert_eq!(number_sym.as_str(&out.interns), "number") - } - expr => panic!("a is not a Num: {:?}", expr), - } - } - _ => panic!("Closure body wasn't a Record: {:?}", record), - } - } - - #[test] - fn multiple_old_record_builders_error() { - let src = indoc!( - r#" - succeed - { a: <- apply "a" } - { b: <- apply "b" } - "# - ); - let arena = Bump::new(); - let CanExprOut { - problems, loc_expr, .. - } = can_expr_with(&arena, test_home(), src); - - assert_eq!(problems.len(), 1); - assert!(problems.iter().all(|problem| matches!( - problem, - Problem::RuntimeError(roc_problem::can::RuntimeError::MultipleOldRecordBuilders { .. }) - ))); - - assert!(matches!( - loc_expr.value, - Expr::RuntimeError(roc_problem::can::RuntimeError::MultipleOldRecordBuilders { .. }) - )); - } - - #[test] - fn hanging_old_record_builder() { - let src = indoc!( - r#" - { a: <- apply "a" } - "# - ); - let arena = Bump::new(); - let CanExprOut { - problems, loc_expr, .. - } = can_expr_with(&arena, test_home(), src); - - assert_eq!(problems.len(), 1); - assert!(problems.iter().all(|problem| matches!( - problem, - Problem::RuntimeError(roc_problem::can::RuntimeError::UnappliedOldRecordBuilder { .. }) - ))); - - assert!(matches!( - loc_expr.value, - Expr::RuntimeError(roc_problem::can::RuntimeError::UnappliedOldRecordBuilder { .. }) - )); - } - - #[test] - fn new_record_builder_desugar() { + fn record_builder_desugar() { let src = indoc!( r#" map2 = \a, b, combine -> combine a b diff --git a/crates/compiler/fmt/src/annotation.rs b/crates/compiler/fmt/src/annotation.rs index 3e6b18a158c..16efa21795e 100644 --- a/crates/compiler/fmt/src/annotation.rs +++ b/crates/compiler/fmt/src/annotation.rs @@ -5,7 +5,7 @@ use crate::{ }; use roc_parse::ast::{ AbilityImpls, AssignedField, Collection, Expr, ExtractSpaces, ImplementsAbilities, - ImplementsAbility, ImplementsClause, OldRecordBuilderField, Tag, TypeAnnotation, TypeHeader, + ImplementsAbility, ImplementsClause, Tag, TypeAnnotation, TypeHeader, }; use roc_parse::ident::UppercaseIdent; use roc_region::all::Loc; @@ -524,101 +524,6 @@ fn format_assigned_field_help( } } -impl<'a> Formattable for OldRecordBuilderField<'a> { - fn is_multiline(&self) -> bool { - is_multiline_record_builder_field_help(self) - } - - fn format_with_options(&self, buf: &mut Buf, _parens: Parens, newlines: Newlines, indent: u16) { - // we abuse the `Newlines` type to decide between multiline or single-line layout - format_record_builder_field_help(self, buf, indent, newlines == Newlines::Yes); - } -} - -fn is_multiline_record_builder_field_help(afield: &OldRecordBuilderField<'_>) -> bool { - use self::OldRecordBuilderField::*; - - match afield { - Value(_, spaces, ann) => !spaces.is_empty() || ann.value.is_multiline(), - ApplyValue(_, colon_spaces, arrow_spaces, ann) => { - !colon_spaces.is_empty() || !arrow_spaces.is_empty() || ann.value.is_multiline() - } - LabelOnly(_) => false, - SpaceBefore(_, _) | SpaceAfter(_, _) => true, - Malformed(text) => text.chars().any(|c| c == '\n'), - } -} - -fn format_record_builder_field_help( - zelf: &OldRecordBuilderField, - buf: &mut Buf, - indent: u16, - is_multiline: bool, -) { - use self::OldRecordBuilderField::*; - - match zelf { - Value(name, spaces, ann) => { - if is_multiline { - buf.newline(); - } - - buf.indent(indent); - buf.push_str(name.value); - - if !spaces.is_empty() { - fmt_spaces(buf, spaces.iter(), indent); - } - - buf.push(':'); - buf.spaces(1); - ann.value.format(buf, indent); - } - ApplyValue(name, colon_spaces, arrow_spaces, ann) => { - if is_multiline { - buf.newline(); - buf.indent(indent); - } - - buf.push_str(name.value); - - if !colon_spaces.is_empty() { - fmt_spaces(buf, colon_spaces.iter(), indent); - } - - buf.push(':'); - buf.spaces(1); - - if !arrow_spaces.is_empty() { - fmt_spaces(buf, arrow_spaces.iter(), indent); - } - - buf.push_str("<-"); - buf.spaces(1); - ann.value.format(buf, indent); - } - LabelOnly(name) => { - if is_multiline { - buf.newline(); - buf.indent(indent); - } - - buf.push_str(name.value); - } - SpaceBefore(sub_field, spaces) => { - fmt_comments_only(buf, spaces.iter(), NewlineAt::Bottom, indent); - format_record_builder_field_help(sub_field, buf, indent, is_multiline); - } - SpaceAfter(sub_field, spaces) => { - format_record_builder_field_help(sub_field, buf, indent, is_multiline); - fmt_comments_only(buf, spaces.iter(), NewlineAt::Bottom, indent); - } - Malformed(raw) => { - buf.push_str(raw); - } - } -} - impl<'a> Formattable for Tag<'a> { fn is_multiline(&self) -> bool { use self::Tag::*; diff --git a/crates/compiler/fmt/src/expr.rs b/crates/compiler/fmt/src/expr.rs index 62c1e42e6f8..46e004d7f1e 100644 --- a/crates/compiler/fmt/src/expr.rs +++ b/crates/compiler/fmt/src/expr.rs @@ -10,7 +10,7 @@ use crate::Buf; use roc_module::called_via::{self, BinOp}; use roc_parse::ast::{ is_expr_suffixed, AssignedField, Base, Collection, CommentOrNewline, Expr, ExtractSpaces, - OldRecordBuilderField, Pattern, TryTarget, WhenBranch, + Pattern, TryTarget, WhenBranch, }; use roc_parse::ast::{StrLiteral, StrSegment}; use roc_parse::ident::Accessor; @@ -91,8 +91,6 @@ impl<'a> Formattable for Expr<'a> { | PrecedenceConflict(roc_parse::ast::PrecedenceConflict { expr: loc_subexpr, .. }) - | MultipleOldRecordBuilders(loc_subexpr) - | UnappliedOldRecordBuilder(loc_subexpr) | EmptyRecordBuilder(loc_subexpr) | SingleFieldRecordBuilder(loc_subexpr) | OptionalFieldInRecordBuilder(_, loc_subexpr) => loc_subexpr.is_multiline(), @@ -118,7 +116,6 @@ impl<'a> Formattable for Expr<'a> { Record(fields) => is_collection_multiline(fields), Tuple(fields) => is_collection_multiline(fields), RecordUpdate { fields, .. } => is_collection_multiline(fields), - OldRecordBuilder(fields) => is_collection_multiline(fields), RecordBuilder { fields, .. } => is_collection_multiline(fields), } } @@ -244,10 +241,7 @@ impl<'a> Formattable for Expr<'a> { a.extract_spaces().item.is_multiline() && matches!( a.value.extract_spaces().item, - Expr::Tuple(_) - | Expr::List(_) - | Expr::Record(_) - | Expr::OldRecordBuilder(_) + Expr::Tuple(_) | Expr::List(_) | Expr::Record(_) ) && a.extract_spaces().before == [CommentOrNewline::Newline] }) @@ -392,16 +386,6 @@ impl<'a> Formattable for Expr<'a> { assigned_field_to_space_before, ); } - OldRecordBuilder(fields) => { - fmt_record_like( - buf, - None, - *fields, - indent, - format_record_builder_field_multiline, - record_builder_field_to_space_before, - ); - } Closure(loc_patterns, loc_ret) => { fmt_closure(buf, loc_patterns, loc_ret, indent); } @@ -563,8 +547,6 @@ impl<'a> Formattable for Expr<'a> { } MalformedClosure => {} PrecedenceConflict { .. } => {} - MultipleOldRecordBuilders { .. } => {} - UnappliedOldRecordBuilder { .. } => {} EmptyRecordBuilder { .. } => {} SingleFieldRecordBuilder { .. } => {} OptionalFieldInRecordBuilder(_, _) => {} @@ -625,11 +607,7 @@ pub(crate) fn format_sq_literal(buf: &mut Buf, s: &str) { fn is_outdentable(expr: &Expr) -> bool { matches!( expr.extract_spaces().item, - Expr::Tuple(_) - | Expr::List(_) - | Expr::Record(_) - | Expr::OldRecordBuilder(_) - | Expr::Closure(..) + Expr::Tuple(_) | Expr::List(_) | Expr::Record(_) | Expr::Closure(..) ) } @@ -1644,113 +1622,6 @@ fn assigned_field_to_space_before<'a, T>( } } -fn format_record_builder_field_multiline( - buf: &mut Buf, - field: &OldRecordBuilderField, - indent: u16, - separator_prefix: &str, -) { - use self::OldRecordBuilderField::*; - match field { - Value(name, spaces, ann) => { - buf.newline(); - buf.indent(indent); - buf.push_str(name.value); - - if !spaces.is_empty() { - fmt_spaces(buf, spaces.iter(), indent); - buf.indent(indent); - } - - buf.push_str(separator_prefix); - buf.push_str(":"); - - if ann.value.is_multiline() { - buf.newline(); - ann.value.format(buf, indent + INDENT); - } else { - buf.spaces(1); - ann.value.format(buf, indent); - } - - buf.push(','); - } - ApplyValue(name, colon_spaces, arrow_spaces, ann) => { - buf.newline(); - buf.indent(indent); - buf.push_str(name.value); - - if !colon_spaces.is_empty() { - fmt_spaces(buf, colon_spaces.iter(), indent); - buf.indent(indent); - } - - buf.push_str(separator_prefix); - buf.push(':'); - buf.spaces(1); - - if !arrow_spaces.is_empty() { - fmt_spaces(buf, arrow_spaces.iter(), indent); - buf.indent(indent + INDENT); - } - - buf.push_str("<-"); - - if ann.value.is_multiline() { - buf.newline(); - ann.value.format(buf, indent + INDENT); - } else { - buf.spaces(1); - ann.value.format(buf, indent); - } - buf.push(','); - } - LabelOnly(name) => { - buf.newline(); - buf.indent(indent); - buf.push_str(name.value); - buf.push(','); - } - SpaceBefore(sub_field, _spaces) => { - // We have something like that: - // ``` - // # comment - // field, - // ``` - // we'd like to preserve this - - format_record_builder_field_multiline(buf, sub_field, indent, separator_prefix); - } - SpaceAfter(sub_field, spaces) => { - // We have something like that: - // ``` - // field # comment - // , otherfield - // ``` - // we'd like to transform it into: - // ``` - // field, - // # comment - // otherfield - // ``` - format_record_builder_field_multiline(buf, sub_field, indent, separator_prefix); - fmt_comments_only(buf, spaces.iter(), NewlineAt::Top, indent); - } - Malformed(raw) => { - buf.push_str(raw); - } - } -} - -fn record_builder_field_to_space_before<'a>( - field: &'a OldRecordBuilderField<'a>, -) -> Option<(&OldRecordBuilderField<'a>, &'a [CommentOrNewline<'a>])> { - match field { - OldRecordBuilderField::SpaceBefore(sub_field, spaces) => Some((sub_field, spaces)), - _ => None, - } -} - fn sub_expr_requests_parens(expr: &Expr<'_>) -> bool { match expr { Expr::BinOps(left_side, _) => { diff --git a/crates/compiler/gen_llvm/src/llvm/build.rs b/crates/compiler/gen_llvm/src/llvm/build.rs index c941fff5936..79cc09f19e4 100644 --- a/crates/compiler/gen_llvm/src/llvm/build.rs +++ b/crates/compiler/gen_llvm/src/llvm/build.rs @@ -2915,130 +2915,108 @@ fn list_literal<'a, 'ctx>( let list_length = elems.len(); let list_length_intval = env.ptr_int().const_int(list_length as _, false); - // TODO re-enable, currently causes morphic segfaults because it tries to update - // constants in-place... - // if element_type.is_int_type() { - if false { - let element_type = element_type.into_int_type(); + let is_refcounted = layout_interner.contains_refcounted(element_layout); + let is_all_constant = elems.iter().all(|e| e.is_literal()); + + if is_all_constant && !is_refcounted { + // Build a global literal in-place instead of GEPing and storing individual elements. + // Exceptions: + // - Anything that is refcounted has nested pointers, + // and nested pointers in globals will break the surgical linker. + // Ignore such cases for now. + let element_width = layout_interner.stack_size(element_layout); - let size = list_length * element_width as usize; + let alignment = layout_interner .alignment_bytes(element_layout) .max(env.target.ptr_width() as u32); - let mut is_all_constant = true; - let zero_elements = - (env.target.ptr_width() as u8 as f64 / element_width as f64).ceil() as usize; + let refcount_slot_bytes = alignment as usize; - // runtime-evaluated elements - let mut runtime_evaluated_elements = Vec::with_capacity_in(list_length, env.arena); + let refcount_slot_elements = + (refcount_slot_bytes as f64 / element_width as f64).ceil() as usize; - // set up a global that contains all the literal elements of the array - // any variables or expressions are represented as `undef` - let global = { - let mut global_elements = Vec::with_capacity_in(list_length, env.arena); + let data_bytes = list_length * element_width as usize; - // Add zero bytes that represent the refcount - // - // - if all elements are const, then we store the whole list as a constant. - // It then needs a refcount before the first element. - // - but if the list is not all constants, then we will just copy the constant values, - // and we do not need that refcount at the start - // - // In the latter case, we won't store the zeros in the globals - // (we slice them off again below) - for _ in 0..zero_elements { - global_elements.push(element_type.const_zero()); + assert!(refcount_slot_elements > 0); + + let global = if element_type.is_int_type() { + let element_type = element_type.into_int_type(); + let mut bytes = Vec::with_capacity_in(refcount_slot_elements + data_bytes, env.arena); + + // Fill the refcount slot with nulls + for _ in 0..(refcount_slot_elements) { + bytes.push(element_type.const_zero()); } // Copy the elements from the list literal into the array - for (index, element) in elems.iter().enumerate() { - match element { - ListLiteralElement::Literal(literal) => { - let val = build_exp_literal(env, layout_interner, element_layout, literal); - global_elements.push(val.into_int_value()); - } - ListLiteralElement::Symbol(symbol) => { - let val = scope.load_symbol(symbol); - - // here we'd like to furthermore check for intval.is_const(). - // if all elements are const for LLVM, we could make the array a constant. - // BUT morphic does not know about this, and could allow us to modify that - // array in-place. That would cause a segfault. So, we'll have to find - // constants ourselves and cannot lean on LLVM here. + for element in elems.iter() { + let literal = element.get_literal().expect("is_all_constant is true"); + let val = build_exp_literal(env, layout_interner, element_layout, &literal); + bytes.push(val.into_int_value()); + } - is_all_constant = false; + let typ = element_type.array_type(bytes.len() as u32); + let global = env.module.add_global(typ, None, "roc__list_literal"); - runtime_evaluated_elements.push((index, val)); + global.set_initializer(&element_type.const_array(bytes.into_bump_slice())); + global + } else if element_type.is_float_type() { + let element_type = element_type.into_float_type(); + let mut bytes = Vec::with_capacity_in(refcount_slot_elements + data_bytes, env.arena); - global_elements.push(element_type.get_undef()); - } - }; + // Fill the refcount slot with nulls + for _ in 0..(refcount_slot_elements) { + bytes.push(element_type.const_zero()); } - let const_elements = if is_all_constant { - global_elements.into_bump_slice() - } else { - &global_elements[zero_elements..] - }; + // Copy the elements from the list literal into the array + for element in elems.iter() { + let literal = element.get_literal().expect("is_all_constant is true"); + let val = build_exp_literal(env, layout_interner, element_layout, &literal); + bytes.push(val.into_float_value()); + } - // use None for the address space (e.g. Const does not work) - let typ = element_type.array_type(const_elements.len() as u32); + let typ = element_type.array_type(bytes.len() as u32); let global = env.module.add_global(typ, None, "roc__list_literal"); - global.set_constant(true); - global.set_alignment(alignment); - global.set_unnamed_addr(true); - global.set_linkage(inkwell::module::Linkage::Private); - - global.set_initializer(&element_type.const_array(const_elements)); - global.as_pointer_value() + global.set_initializer(&element_type.const_array(bytes.into_bump_slice())); + global + } else { + internal_error!("unexpected element type: {:?}", element_type); }; - if is_all_constant { - // all elements are constants, so we can use the memory in the constants section directly - // here we make a pointer to the first actual element (skipping the 0 bytes that - // represent the refcount) - let zero = env.ptr_int().const_zero(); - let offset = env.ptr_int().const_int(zero_elements as _, false); - - let ptr = unsafe { - env.builder.new_build_in_bounds_gep( - element_type, - global, - &[zero, offset], - "first_element_pointer", - ) - }; + global.set_constant(true); + global.set_alignment(alignment); + global.set_unnamed_addr(true); + global.set_linkage(inkwell::module::Linkage::Private); - super::build_list::store_list(env, ptr, list_length_intval).into() - } else { - // some of our elements are non-constant, so we must allocate space on the heap - let ptr = allocate_list(env, layout_interner, element_layout, list_length_intval); + // Refcount the global itself in a new allocation. + // Necessary because morphic MAY attempt to update the global in-place, which is + // illegal if the global is in the read-only section. + let with_rc_ptr = global.as_pointer_value(); - // then, copy the relevant segment from the constant section into the heap - env.builder - .build_memcpy( - ptr, - alignment, - global, - alignment, - env.ptr_int().const_int(size as _, false), - ) - .unwrap(); + let const_data_ptr = unsafe { + env.builder.new_build_in_bounds_gep( + element_type, + with_rc_ptr, + &[env + .ptr_int() + .const_int(refcount_slot_elements as u64, false)], + "get_data_ptr", + ) + }; - // then replace the `undef`s with the values that we evaluate at runtime - for (index, val) in runtime_evaluated_elements { - let index_val = ctx.i64_type().const_int(index as u64, false); - let elem_ptr = unsafe { - builder.new_build_in_bounds_gep(element_type, ptr, &[index_val], "index") - }; + let data_ptr = allocate_list(env, layout_interner, element_layout, list_length_intval); - builder.new_build_store(elem_ptr, val); - } + let byte_size = env + .ptr_int() + .const_int(list_length as u64 * element_width as u64, false); + builder + .build_memcpy(data_ptr, alignment, const_data_ptr, alignment, byte_size) + .unwrap(); - super::build_list::store_list(env, ptr, list_length_intval).into() - } + super::build_list::store_list(env, data_ptr, list_length_intval).into() } else { let ptr = allocate_list(env, layout_interner, element_layout, list_length_intval); diff --git a/crates/compiler/load/tests/test_reporting.rs b/crates/compiler/load/tests/test_reporting.rs index 99baf06e159..6d964dcd129 100644 --- a/crates/compiler/load/tests/test_reporting.rs +++ b/crates/compiler/load/tests/test_reporting.rs @@ -4972,30 +4972,6 @@ mod test_reporting { "### ); - test_report!( - record_builder_in_module_params, - indoc!( - r" - import Menu { - echo, - name: <- applyName - } - " - ),@r###" - ── OLD-STYLE RECORD BUILDER IN MODULE PARAMS in ...r_in_module_params/Test.roc ─ - - I was partway through parsing module params, but I got stuck here: - - 4│ import Menu { - 5│ echo, - 6│ name: <- applyName - ^^^^^^^^^^^^^^^^^^ - - This looks like an old-style record builder field, but those are not - allowed in module params. - "### - ); - test_report!( record_update_in_module_params, indoc!( @@ -10661,163 +10637,6 @@ All branches in an `if` must have the same type! // Record Builders - test_report!( - optional_field_in_old_record_builder, - indoc!( - r#" - { - a: <- apply "a", - b, - c ? "optional" - } - "# - ), - @r#" - ── BAD OLD-STYLE RECORD BUILDER in ...nal_field_in_old_record_builder/Test.roc ─ - - I am partway through parsing a record builder, and I found an optional - field: - - 1│ app "test" provides [main] to "./platform" - 2│ - 3│ main = - 4│ { - 5│ a: <- apply "a", - 6│ b, - 7│ c ? "optional" - ^^^^^^^^^^^^^^ - - Optional fields can only appear when you destructure a record. - "# - ); - - test_report!( - record_update_old_builder, - indoc!( - r#" - { rec & - a: <- apply "a", - b: 3 - } - "# - ), - @r#" - ── BAD RECORD UPDATE in tmp/record_update_old_builder/Test.roc ───────────────── - - I am partway through parsing a record update, and I found an old-style - record builder field: - - 1│ app "test" provides [main] to "./platform" - 2│ - 3│ main = - 4│ { rec & - 5│ a: <- apply "a", - ^^^^^^^^^^^^^^^ - - Old-style record builders cannot be updated like records. - "# - ); - - test_report!( - multiple_old_record_builders, - indoc!( - r#" - succeed - { a: <- apply "a" } - { b: <- apply "b" } - "# - ), - @r#" - ── MULTIPLE OLD-STYLE RECORD BUILDERS in /code/proj/Main.roc ─────────────────── - - This function is applied to multiple old-style record builders: - - 4│> succeed - 5│> { a: <- apply "a" } - 6│> { b: <- apply "b" } - - Note: Functions can only take at most one old-style record builder! - - Tip: You can combine them or apply them separately. - "# - ); - - test_report!( - unapplied_old_record_builder, - indoc!( - r#" - { a: <- apply "a" } - "# - ), - @r#" - ── UNAPPLIED OLD-STYLE RECORD BUILDER in /code/proj/Main.roc ─────────────────── - - This old-style record builder was not applied to a function: - - 4│ { a: <- apply "a" } - ^^^^^^^^^^^^^^^^^^^ - - However, we need a function to construct the record. - - Note: Functions must be applied directly. The pipe operator (|>) cannot be used. - "# - ); - - test_report!( - old_record_builder_apply_non_function, - indoc!( - r#" - succeed = \_ -> crash "" - - succeed { - a: <- "a", - } - "# - ), - @r#" - ── TOO MANY ARGS in /code/proj/Main.roc ──────────────────────────────────────── - - This value is not a function, but it was given 1 argument: - - 7│ a: <- "a", - ^^^ - - Tip: Remove <- to assign the field directly. - "# - ); - - // Skipping test because opaque types defined in the same module - // do not fail with the special opaque type error - // - // test_report!( - // record_builder_apply_opaque, - // indoc!( - // r#" - // succeed = \_ -> crash "" - - // Decode := {} - - // get : Str -> Decode - // get = \_ -> @Decode {} - - // succeed { - // a: <- get "a", - // # missing |> apply ^ - // } - // "# - // ), - // @r#" - // ── TOO MANY ARGS in /code/proj/Main.roc ──────────────────────────────────────── - - // This value is an opaque type, so it cannot be called with an argument: - - // 12│ a: <- get "a", - // ^^^^^^^ - - // Hint: Did you mean to apply it to a function first? - // "# - // ); - test_report!( empty_record_builder, indoc!( diff --git a/crates/compiler/module/src/called_via.rs b/crates/compiler/module/src/called_via.rs index e64cbbce385..b1422f67042 100644 --- a/crates/compiler/module/src/called_via.rs +++ b/crates/compiler/module/src/called_via.rs @@ -75,10 +75,6 @@ pub enum CalledVia { /// e.g. "$(first) $(last)" is transformed into Str.concat (Str.concat first " ") last. StringInterpolation, - /// This call is the result of desugaring an old style Record Builder field. - /// e.g. succeed { a <- get "a" } is transformed into (get "a") (succeed \a -> { a }) - OldRecordBuilder, - /// This call is the result of desugaring a map2-based Record Builder field. e.g. /// ```roc /// { Task.parallel <- diff --git a/crates/compiler/mono/src/ir/literal.rs b/crates/compiler/mono/src/ir/literal.rs index c7dcabf6825..36f68d87a1e 100644 --- a/crates/compiler/mono/src/ir/literal.rs +++ b/crates/compiler/mono/src/ir/literal.rs @@ -49,6 +49,17 @@ impl<'a> ListLiteralElement<'a> { _ => None, } } + + pub fn get_literal(&self) -> Option> { + match self { + Self::Literal(l) => Some(*l), + _ => None, + } + } + + pub fn is_literal(&self) -> bool { + matches!(self, Self::Literal(_)) + } } pub enum NumLiteral { diff --git a/crates/compiler/parse/src/ast.rs b/crates/compiler/parse/src/ast.rs index 051f4d0cca1..b59fdb591e3 100644 --- a/crates/compiler/parse/src/ast.rs +++ b/crates/compiler/parse/src/ast.rs @@ -456,13 +456,6 @@ pub enum Expr<'a> { Tuple(Collection<'a, &'a Loc>>), - // Record Builders - /// Applicative record builders, e.g. - /// build { - /// foo: <- getData Foo, - /// bar: <- getData Bar, - /// } - OldRecordBuilder(Collection<'a, Loc>>), /// Mapper-based record builders, e.g. /// { Task.parallel <- /// foo: Task.getData Foo, @@ -541,8 +534,6 @@ pub enum Expr<'a> { // Both operators were non-associative, e.g. (True == False == False). // We should tell the author to disambiguate by grouping them with parens. PrecedenceConflict(&'a PrecedenceConflict<'a>), - MultipleOldRecordBuilders(&'a Loc>), - UnappliedOldRecordBuilder(&'a Loc>), EmptyRecordBuilder(&'a Loc>), SingleFieldRecordBuilder(&'a Loc>), OptionalFieldInRecordBuilder(&'a Loc<&'a str>, &'a Loc>), @@ -663,9 +654,6 @@ pub fn is_expr_suffixed(expr: &Expr) -> bool { .iter() .any(|field| is_assigned_value_suffixed(&field.value)), Expr::Tuple(items) => items.iter().any(|x| is_expr_suffixed(&x.value)), - Expr::OldRecordBuilder(items) => items - .iter() - .any(|rbf| is_record_builder_field_suffixed(&rbf.value)), Expr::RecordBuilder { mapper: _, fields } => fields .iter() .any(|field| is_assigned_value_suffixed(&field.value)), @@ -688,8 +676,6 @@ pub fn is_expr_suffixed(expr: &Expr) -> bool { Expr::MalformedClosure => false, Expr::MalformedSuffixed(_) => false, Expr::PrecedenceConflict(_) => false, - Expr::MultipleOldRecordBuilders(_) => false, - Expr::UnappliedOldRecordBuilder(_) => false, Expr::EmptyRecordBuilder(_) => false, Expr::SingleFieldRecordBuilder(_) => false, Expr::OptionalFieldInRecordBuilder(_, _) => false, @@ -717,17 +703,6 @@ fn is_assigned_value_suffixed<'a>(value: &AssignedField<'a, Expr<'a>>) -> bool { } } -fn is_record_builder_field_suffixed(field: &OldRecordBuilderField<'_>) -> bool { - match field { - OldRecordBuilderField::Value(_, _, a) => is_expr_suffixed(&a.value), - OldRecordBuilderField::ApplyValue(_, _, _, a) => is_expr_suffixed(&a.value), - OldRecordBuilderField::LabelOnly(_) => false, - OldRecordBuilderField::SpaceBefore(a, _) => is_record_builder_field_suffixed(a), - OldRecordBuilderField::SpaceAfter(a, _) => is_record_builder_field_suffixed(a), - OldRecordBuilderField::Malformed(_) => false, - } -} - pub fn split_around(items: &[T], target: usize) -> (&[T], &[T]) { let (before, rest) = items.split_at(target); let after = &rest[1..]; @@ -935,26 +910,6 @@ impl<'a, 'b> RecursiveValueDefIter<'a, 'b> { expr_stack.push(&loc_expr.value); } } - OldRecordBuilder(fields) => { - expr_stack.reserve(fields.len()); - for loc_record_builder_field in fields.items { - let mut current_field = loc_record_builder_field.value; - - loop { - use OldRecordBuilderField::*; - - match current_field { - Value(_, _, loc_val) => break expr_stack.push(&loc_val.value), - ApplyValue(_, _, _, loc_val) => { - break expr_stack.push(&loc_val.value) - } - SpaceBefore(next_field, _) => current_field = *next_field, - SpaceAfter(next_field, _) => current_field = *next_field, - LabelOnly(_) | Malformed(_) => break, - } - } - } - } RecordBuilder { mapper: map2, fields, @@ -1039,9 +994,7 @@ impl<'a, 'b> RecursiveValueDefIter<'a, 'b> { | SpaceAfter(expr, _) | ParensAround(expr) => expr_stack.push(expr), - MultipleOldRecordBuilders(loc_expr) - | UnappliedOldRecordBuilder(loc_expr) - | EmptyRecordBuilder(loc_expr) + EmptyRecordBuilder(loc_expr) | SingleFieldRecordBuilder(loc_expr) | OptionalFieldInRecordBuilder(_, loc_expr) => expr_stack.push(&loc_expr.value), @@ -1667,30 +1620,6 @@ impl<'a, Val> AssignedField<'a, Val> { } } -#[derive(Debug, Clone, Copy, PartialEq)] -pub enum OldRecordBuilderField<'a> { - // A field with a value, e.g. `{ name: "blah" }` - Value(Loc<&'a str>, &'a [CommentOrNewline<'a>], &'a Loc>), - - // A field with a function we can apply to build part of the record, e.g. `{ name: <- apply getName }` - ApplyValue( - Loc<&'a str>, - &'a [CommentOrNewline<'a>], - &'a [CommentOrNewline<'a>], - &'a Loc>, - ), - - // A label with no value, e.g. `{ name }` (this is sugar for { name: name }) - LabelOnly(Loc<&'a str>), - - // We preserve this for the formatter; canonicalization ignores it. - SpaceBefore(&'a OldRecordBuilderField<'a>, &'a [CommentOrNewline<'a>]), - SpaceAfter(&'a OldRecordBuilderField<'a>, &'a [CommentOrNewline<'a>]), - - /// A malformed assigned field, which will code gen to a runtime error - Malformed(&'a str), -} - #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum CommentOrNewline<'a> { Newline, @@ -2225,15 +2154,6 @@ impl<'a, Val> Spaceable<'a> for AssignedField<'a, Val> { } } -impl<'a> Spaceable<'a> for OldRecordBuilderField<'a> { - fn before(&'a self, spaces: &'a [CommentOrNewline<'a>]) -> Self { - OldRecordBuilderField::SpaceBefore(self, spaces) - } - fn after(&'a self, spaces: &'a [CommentOrNewline<'a>]) -> Self { - OldRecordBuilderField::SpaceAfter(self, spaces) - } -} - impl<'a> Spaceable<'a> for Tag<'a> { fn before(&'a self, spaces: &'a [CommentOrNewline<'a>]) -> Self { Tag::SpaceBefore(self, spaces) @@ -2536,7 +2456,6 @@ impl<'a> Malformed for Expr<'a> { Record(items) => items.is_malformed(), Tuple(items) => items.is_malformed(), - OldRecordBuilder(items) => items.is_malformed(), RecordBuilder { mapper: map2, fields } => map2.is_malformed() || fields.is_malformed(), Closure(args, body) => args.iter().any(|arg| arg.is_malformed()) || body.is_malformed(), @@ -2560,8 +2479,6 @@ impl<'a> Malformed for Expr<'a> { MalformedClosure | MalformedSuffixed(..) | PrecedenceConflict(_) | - MultipleOldRecordBuilders(_) | - UnappliedOldRecordBuilder(_) | EmptyRecordBuilder(_) | SingleFieldRecordBuilder(_) | OptionalFieldInRecordBuilder(_, _) => true, @@ -2641,19 +2558,6 @@ impl<'a, T: Malformed> Malformed for AssignedField<'a, T> { } } -impl<'a> Malformed for OldRecordBuilderField<'a> { - fn is_malformed(&self) -> bool { - match self { - OldRecordBuilderField::Value(_, _, expr) - | OldRecordBuilderField::ApplyValue(_, _, _, expr) => expr.is_malformed(), - OldRecordBuilderField::LabelOnly(_) => false, - OldRecordBuilderField::SpaceBefore(field, _) - | OldRecordBuilderField::SpaceAfter(field, _) => field.is_malformed(), - OldRecordBuilderField::Malformed(_) => true, - } - } -} - impl<'a> Malformed for Pattern<'a> { fn is_malformed(&self) -> bool { use Pattern::*; diff --git a/crates/compiler/parse/src/expr.rs b/crates/compiler/parse/src/expr.rs index 723eeb14866..20a00f3daa0 100644 --- a/crates/compiler/parse/src/expr.rs +++ b/crates/compiler/parse/src/expr.rs @@ -2,8 +2,8 @@ use crate::ast::{ is_expr_suffixed, AssignedField, Collection, CommentOrNewline, Defs, Expr, ExtractSpaces, Implements, ImplementsAbilities, ImportAlias, ImportAsKeyword, ImportExposingKeyword, ImportedModuleName, IngestedFileAnnotation, IngestedFileImport, ModuleImport, - ModuleImportParams, OldRecordBuilderField, Pattern, Spaceable, Spaced, Spaces, SpacesBefore, - TryTarget, TypeAnnotation, TypeDef, TypeHeader, ValueDef, + ModuleImportParams, Pattern, Spaceable, Spaced, Spaces, SpacesBefore, TryTarget, + TypeAnnotation, TypeDef, TypeHeader, ValueDef, }; use crate::blankspace::{ loc_space0_e, require_newline_or_eof, space0_after_e, space0_around_ee, space0_before_e, @@ -925,15 +925,11 @@ fn import_params<'a>() -> impl Parser<'a, ModuleImportParams<'a>, EImportParams< .fields .map_items_result(arena, |loc_field| { match loc_field.value.to_assigned_field(arena) { - Ok(AssignedField::IgnoredValue(_, _, _)) => Err(( + AssignedField::IgnoredValue(_, _, _) => Err(( MadeProgress, EImportParams::RecordIgnoredFieldFound(loc_field.region), )), - Ok(field) => Ok(Loc::at(loc_field.region, field)), - Err(FoundApplyValue) => Err(( - MadeProgress, - EImportParams::RecordApplyFound(loc_field.region), - )), + field => Ok(Loc::at(loc_field.region, field)), } })?; @@ -2179,8 +2175,6 @@ fn expr_to_pattern_help<'a>(arena: &'a Bump, expr: &Expr<'a>) -> Result(arena: &'a Bump, expr: &Expr<'a>) -> Result return Err(()), Expr::Str(string) => Pattern::StrLiteral(string), @@ -3390,38 +3383,12 @@ pub enum RecordField<'a> { LabelOnly(Loc<&'a str>), SpaceBefore(&'a RecordField<'a>, &'a [CommentOrNewline<'a>]), SpaceAfter(&'a RecordField<'a>, &'a [CommentOrNewline<'a>]), - ApplyValue( - Loc<&'a str>, - &'a [CommentOrNewline<'a>], - &'a [CommentOrNewline<'a>], - &'a Loc>, - ), } #[derive(Debug)] pub struct FoundApplyValue; -#[derive(Debug)] -pub enum NotOldBuilderFieldValue { - FoundOptionalValue, - FoundIgnoredValue, -} - impl<'a> RecordField<'a> { - fn is_apply_value(&self) -> bool { - let mut current = self; - - loop { - match current { - RecordField::ApplyValue(_, _, _, _) => break true, - RecordField::SpaceBefore(field, _) | RecordField::SpaceAfter(field, _) => { - current = *field; - } - _ => break false, - } - } - } - fn is_ignored_value(&self) -> bool { let mut current = self; @@ -3436,74 +3403,34 @@ impl<'a> RecordField<'a> { } } - pub fn to_assigned_field( - self, - arena: &'a Bump, - ) -> Result>, FoundApplyValue> { + pub fn to_assigned_field(self, arena: &'a Bump) -> AssignedField<'a, Expr<'a>> { use AssignedField::*; match self { RecordField::RequiredValue(loc_label, spaces, loc_expr) => { - Ok(RequiredValue(loc_label, spaces, loc_expr)) + RequiredValue(loc_label, spaces, loc_expr) } RecordField::OptionalValue(loc_label, spaces, loc_expr) => { - Ok(OptionalValue(loc_label, spaces, loc_expr)) + OptionalValue(loc_label, spaces, loc_expr) } RecordField::IgnoredValue(loc_label, spaces, loc_expr) => { - Ok(IgnoredValue(loc_label, spaces, loc_expr)) + IgnoredValue(loc_label, spaces, loc_expr) } - RecordField::LabelOnly(loc_label) => Ok(LabelOnly(loc_label)), - - RecordField::ApplyValue(_, _, _, _) => Err(FoundApplyValue), + RecordField::LabelOnly(loc_label) => LabelOnly(loc_label), RecordField::SpaceBefore(field, spaces) => { - let assigned_field = field.to_assigned_field(arena)?; + let assigned_field = field.to_assigned_field(arena); - Ok(SpaceBefore(arena.alloc(assigned_field), spaces)) + SpaceBefore(arena.alloc(assigned_field), spaces) } RecordField::SpaceAfter(field, spaces) => { - let assigned_field = field.to_assigned_field(arena)?; + let assigned_field = field.to_assigned_field(arena); - Ok(SpaceAfter(arena.alloc(assigned_field), spaces)) - } - } - } - - fn to_builder_field( - self, - arena: &'a Bump, - ) -> Result, NotOldBuilderFieldValue> { - use OldRecordBuilderField::*; - - match self { - RecordField::RequiredValue(loc_label, spaces, loc_expr) => { - Ok(Value(loc_label, spaces, loc_expr)) - } - - RecordField::OptionalValue(_, _, _) => Err(NotOldBuilderFieldValue::FoundOptionalValue), - - RecordField::IgnoredValue(_, _, _) => Err(NotOldBuilderFieldValue::FoundIgnoredValue), - - RecordField::LabelOnly(loc_label) => Ok(LabelOnly(loc_label)), - - RecordField::ApplyValue(loc_label, colon_spaces, arrow_spaces, loc_expr) => { - Ok(ApplyValue(loc_label, colon_spaces, arrow_spaces, loc_expr)) - } - - RecordField::SpaceBefore(field, spaces) => { - let builder_field = field.to_builder_field(arena)?; - - Ok(SpaceBefore(arena.alloc(builder_field), spaces)) - } - - RecordField::SpaceAfter(field, spaces) => { - let builder_field = field.to_builder_field(arena)?; - - Ok(SpaceAfter(arena.alloc(builder_field), spaces)) + SpaceAfter(arena.alloc(assigned_field), spaces) } } } @@ -3557,14 +3484,10 @@ pub fn record_field<'a>() -> impl Parser<'a, RecordField<'a>, ERecord<'a>> { match field_data { Either::First((loc_label, (spaces, opt_loc_val))) => { match opt_loc_val { - Some(Either::First((_, RecordFieldExpr::Value(loc_val)))) => { + Some(Either::First((_, loc_val))) => { RequiredValue(loc_label, spaces, arena.alloc(loc_val)) } - Some(Either::First((_, RecordFieldExpr::Apply(arrow_spaces, loc_val)))) => { - ApplyValue(loc_label, spaces, arrow_spaces, arena.alloc(loc_val)) - } - Some(Either::Second((_, loc_val))) => { OptionalValue(loc_label, spaces, arena.alloc(loc_val)) } @@ -3591,34 +3514,17 @@ pub fn record_field<'a>() -> impl Parser<'a, RecordField<'a>, ERecord<'a>> { ) } -enum RecordFieldExpr<'a> { - Apply(&'a [CommentOrNewline<'a>], Loc>), - Value(Loc>), -} - -fn record_field_expr<'a>() -> impl Parser<'a, RecordFieldExpr<'a>, ERecord<'a>> { +fn record_field_expr<'a>() -> impl Parser<'a, Loc>, ERecord<'a>> { map_with_arena( - and( - spaces(), - either( - and( - two_bytes(b'<', b'-', ERecord::Arrow), - spaces_before(specialize_err_ref(ERecord::Expr, loc_expr(false))), - ), - specialize_err_ref(ERecord::Expr, loc_expr(false)), - ), - ), - |arena: &'a bumpalo::Bump, (spaces, either)| match either { - Either::First((_, loc_expr)) => RecordFieldExpr::Apply(spaces, loc_expr), - Either::Second(loc_expr) => RecordFieldExpr::Value({ - if spaces.is_empty() { - loc_expr - } else { - arena - .alloc(loc_expr.value) - .with_spaces_before(spaces, loc_expr.region) - } - }), + and(spaces(), specialize_err_ref(ERecord::Expr, loc_expr(false))), + |arena: &'a bumpalo::Bump, (spaces, loc_expr)| { + if spaces.is_empty() { + loc_expr + } else { + arena + .alloc(loc_expr.value) + .with_spaces_before(spaces, loc_expr.region) + } }, ) } @@ -3686,13 +3592,11 @@ fn record_literal_help<'a>() -> impl Parser<'a, Expr<'a>, EExpr<'a>> { record_update_help(arena, update, record.fields) } Some((mapper, RecordHelpPrefix::Mapper)) => { - new_record_builder_help(arena, mapper, record.fields) + record_builder_help(arena, mapper, record.fields) } None => { let special_field_found = record.fields.iter().find_map(|field| { - if field.value.is_apply_value() { - Some(old_record_builder_help(arena, record.fields)) - } else if field.value.is_ignored_value() { + if field.value.is_ignored_value() { Some(Err(EExpr::RecordUpdateIgnoredField(field.region))) } else { None @@ -3701,7 +3605,7 @@ fn record_literal_help<'a>() -> impl Parser<'a, Expr<'a>, EExpr<'a>> { special_field_found.unwrap_or_else(|| { let fields = record.fields.map_items(arena, |loc_field| { - loc_field.map(|field| field.to_assigned_field(arena).unwrap()) + loc_field.map(|field| field.to_assigned_field(arena)) }); Ok(Expr::Record(fields)) @@ -3728,14 +3632,13 @@ fn record_update_help<'a>( ) -> Result, EExpr<'a>> { let result = fields.map_items_result(arena, |loc_field| { match loc_field.value.to_assigned_field(arena) { - Ok(AssignedField::IgnoredValue(_, _, _)) => { + AssignedField::IgnoredValue(_, _, _) => { Err(EExpr::RecordUpdateIgnoredField(loc_field.region)) } - Ok(builder_field) => Ok(Loc { + builder_field => Ok(Loc { region: loc_field.region, value: builder_field, }), - Err(FoundApplyValue) => Err(EExpr::RecordUpdateOldBuilderField(loc_field.region)), } }); @@ -3745,19 +3648,18 @@ fn record_update_help<'a>( }) } -fn new_record_builder_help<'a>( +fn record_builder_help<'a>( arena: &'a Bump, mapper: Loc>, fields: Collection<'a, Loc>>, ) -> Result, EExpr<'a>> { let result = fields.map_items_result(arena, |loc_field| { - match loc_field.value.to_assigned_field(arena) { - Ok(builder_field) => Ok(Loc { - region: loc_field.region, - value: builder_field, - }), - Err(FoundApplyValue) => Err(EExpr::RecordBuilderOldBuilderField(loc_field.region)), - } + let builder_field = loc_field.value.to_assigned_field(arena); + + Ok(Loc { + region: loc_field.region, + value: builder_field, + }) }); result.map(|fields| Expr::RecordBuilder { @@ -3766,28 +3668,6 @@ fn new_record_builder_help<'a>( }) } -fn old_record_builder_help<'a>( - arena: &'a Bump, - fields: Collection<'a, Loc>>, -) -> Result, EExpr<'a>> { - let result = fields.map_items_result(arena, |loc_field| { - match loc_field.value.to_builder_field(arena) { - Ok(builder_field) => Ok(Loc { - region: loc_field.region, - value: builder_field, - }), - Err(NotOldBuilderFieldValue::FoundOptionalValue) => { - Err(EExpr::OptionalValueInOldRecordBuilder(loc_field.region)) - } - Err(NotOldBuilderFieldValue::FoundIgnoredValue) => { - Err(EExpr::IgnoredValueInOldRecordBuilder(loc_field.region)) - } - } - }); - - result.map(Expr::OldRecordBuilder) -} - fn apply_expr_access_chain<'a>( arena: &'a Bump, value: Expr<'a>, diff --git a/crates/compiler/parse/src/module.rs b/crates/compiler/parse/src/module.rs deleted file mode 100644 index 899ec154e2f..00000000000 --- a/crates/compiler/parse/src/module.rs +++ /dev/null @@ -1,905 +0,0 @@ -use crate::ast::{Collection, CommentOrNewline, Defs, Header, Module, Spaced, Spaces}; -use crate::blankspace::{space0_around_ee, space0_before_e, space0_e}; -use crate::expr::merge_spaces; -use crate::header::{ - package_entry, package_name, AppHeader, ExposedName, ExposesKeyword, HostedHeader, - ImportsCollection, ImportsEntry, ImportsKeyword, ImportsKeywordItem, Keyword, KeywordItem, - ModuleHeader, ModuleName, ModuleParams, PackageEntry, PackageHeader, PackagesKeyword, - PlatformHeader, PlatformRequires, ProvidesKeyword, ProvidesTo, RequiresKeyword, To, ToKeyword, - TypedIdent, -}; -use crate::ident::{self, lowercase_ident, unqualified_ident, UppercaseIdent}; -use crate::parser::Progress::{self, *}; -use crate::parser::{ - and, backtrackable, byte, collection_trailing_sep_e, increment_min_indent, loc, map, - map_with_arena, optional, reset_min_indent, skip_first, skip_second, specialize_err, succeed, - two_bytes, zero_or_more, EExposes, EHeader, EImports, EPackages, EParams, EProvides, ERequires, - ETypedIdent, Parser, SourceError, SpaceProblem, SyntaxError, -}; -use crate::pattern::record_pattern_fields; -use crate::state::State; -use crate::string_literal::{self, parse_str_literal}; -use crate::type_annotation; -use roc_region::all::{Loc, Position, Region}; - -fn end_of_file<'a>() -> impl Parser<'a, (), SyntaxError<'a>> { - |_arena, state: State<'a>, _min_indent: u32| { - if state.has_reached_end() { - Ok((NoProgress, (), state)) - } else { - Err((NoProgress, SyntaxError::NotEndOfFile(state.pos()))) - } - } -} - -pub fn parse_module_defs<'a>( - arena: &'a bumpalo::Bump, - state: State<'a>, - defs: Defs<'a>, -) -> Result, SyntaxError<'a>> { - let min_indent = 0; - match crate::expr::parse_top_level_defs(arena, state.clone(), defs) { - Ok((_, defs, state)) => match end_of_file().parse(arena, state, min_indent) { - Ok(_) => Ok(defs), - Err((_, fail)) => Err(fail), - }, - Err((_, fail)) => Err(SyntaxError::Expr(fail, state.pos())), - } -} - -pub fn parse_header<'a>( - arena: &'a bumpalo::Bump, - state: State<'a>, -) -> Result<(Module<'a>, State<'a>), SourceError<'a, EHeader<'a>>> { - let min_indent = 0; - match header().parse(arena, state.clone(), min_indent) { - Ok((_, module, state)) => Ok((module, state)), - Err((_, fail)) => Err(SourceError::new(fail, &state)), - } -} - -pub fn header<'a>() -> impl Parser<'a, Module<'a>, EHeader<'a>> { - use crate::parser::keyword; - - record!(Module { - comments: space0_e(EHeader::IndentStart), - header: one_of![ - map( - skip_first( - keyword("module", EHeader::Start), - increment_min_indent(module_header()) - ), - Header::Module - ), - map( - skip_first( - keyword("interface", EHeader::Start), - increment_min_indent(interface_header()) - ), - Header::Module - ), - map( - skip_first( - keyword("app", EHeader::Start), - increment_min_indent(one_of![app_header(), old_app_header()]) - ), - Header::App - ), - map( - skip_first( - keyword("package", EHeader::Start), - increment_min_indent(one_of![package_header(), old_package_header()]) - ), - Header::Package - ), - map( - skip_first( - keyword("platform", EHeader::Start), - increment_min_indent(platform_header()) - ), - Header::Platform - ), - map( - skip_first( - keyword("hosted", EHeader::Start), - increment_min_indent(hosted_header()) - ), - Header::Hosted - ), - ] - }) -} - -#[inline(always)] -fn module_header<'a>() -> impl Parser<'a, ModuleHeader<'a>, EHeader<'a>> { - record!(ModuleHeader { - after_keyword: space0_e(EHeader::IndentStart), - params: optional(specialize_err(EHeader::Params, module_params())), - exposes: specialize_err(EHeader::Exposes, exposes_list()), - interface_imports: succeed(None) - }) - .trace("module_header") -} - -fn module_params<'a>() -> impl Parser<'a, ModuleParams<'a>, EParams<'a>> { - record!(ModuleParams { - params: specialize_err(EParams::Pattern, record_pattern_fields()), - before_arrow: skip_second( - space0_e(EParams::BeforeArrow), - loc(two_bytes(b'-', b'>', EParams::Arrow)) - ), - after_arrow: space0_e(EParams::AfterArrow), - }) -} - -// TODO does this need to be a macro? -macro_rules! merge_n_spaces { - ($arena:expr, $($slice:expr),*) => { - { - let mut merged = bumpalo::collections::Vec::with_capacity_in(0 $(+ $slice.len())*, $arena); - $(merged.extend_from_slice($slice);)* - merged.into_bump_slice() - } - }; -} - -/// Parse old interface headers so we can format them into module headers -#[inline(always)] -fn interface_header<'a>() -> impl Parser<'a, ModuleHeader<'a>, EHeader<'a>> { - let after_keyword = map_with_arena( - and( - skip_second( - space0_e(EHeader::IndentStart), - loc(module_name_help(EHeader::ModuleName)), - ), - specialize_err(EHeader::Exposes, exposes_kw()), - ), - |arena: &'a bumpalo::Bump, - (before_name, kw): (&'a [CommentOrNewline<'a>], Spaces<'a, ExposesKeyword>)| { - merge_n_spaces!(arena, before_name, kw.before, kw.after) - }, - ); - - record!(ModuleHeader { - after_keyword: after_keyword, - params: succeed(None), - exposes: specialize_err(EHeader::Exposes, exposes_list()).trace("exposes_list"), - interface_imports: map( - specialize_err(EHeader::Imports, imports()), - imports_none_if_empty - ) - .trace("imports"), - }) - .trace("interface_header") -} - -fn imports_none_if_empty(value: ImportsKeywordItem<'_>) -> Option> { - if value.item.is_empty() { - None - } else { - Some(value) - } -} - -#[inline(always)] -fn hosted_header<'a>() -> impl Parser<'a, HostedHeader<'a>, EHeader<'a>> { - record!(HostedHeader { - before_name: space0_e(EHeader::IndentStart), - name: loc(module_name_help(EHeader::ModuleName)), - exposes: specialize_err(EHeader::Exposes, exposes_values_kw()), - imports: specialize_err(EHeader::Imports, imports()), - }) - .trace("hosted_header") -} - -fn chomp_module_name(buffer: &[u8]) -> Result<&str, Progress> { - use encode_unicode::CharExt; - - let mut chomped = 0; - - if let Ok((first_letter, width)) = char::from_utf8_slice_start(&buffer[chomped..]) { - if first_letter.is_uppercase() { - chomped += width; - } else { - return Err(Progress::NoProgress); - } - } - - while let Ok((ch, width)) = char::from_utf8_slice_start(&buffer[chomped..]) { - // After the first character, only these are allowed: - // - // * Unicode alphabetic chars - you might include `鹏` if that's clear to your readers - // * ASCII digits - e.g. `1` but not `¾`, both of which pass .is_numeric() - // * A '.' separating module parts - if ch.is_alphabetic() || ch.is_ascii_digit() { - chomped += width; - } else if ch == '.' { - chomped += width; - - if let Ok((first_letter, width)) = char::from_utf8_slice_start(&buffer[chomped..]) { - if first_letter.is_uppercase() { - chomped += width; - } else if first_letter == '{' { - // the .{ starting a `Foo.{ bar, baz }` importing clauses - chomped -= width; - break; - } else { - return Err(Progress::MadeProgress); - } - } - } else { - // we're done - break; - } - } - - let name = unsafe { std::str::from_utf8_unchecked(&buffer[..chomped]) }; - - Ok(name) -} - -#[inline(always)] -fn module_name<'a>() -> impl Parser<'a, ModuleName<'a>, ()> { - |_, mut state: State<'a>, _min_indent: u32| match chomp_module_name(state.bytes()) { - Ok(name) => { - let width = name.len(); - state = state.advance(width); - - Ok((MadeProgress, ModuleName::new(name), state)) - } - Err(progress) => Err((progress, ())), - } -} - -#[inline(always)] -fn app_header<'a>() -> impl Parser<'a, AppHeader<'a>, EHeader<'a>> { - record!(AppHeader { - before_provides: space0_e(EHeader::IndentStart), - provides: specialize_err(EHeader::Exposes, exposes_list()), - before_packages: space0_e(EHeader::IndentStart), - packages: specialize_err(EHeader::Packages, loc(packages_collection())), - old_imports: succeed(None), - old_provides_to_new_package: succeed(None), - }) - .trace("app_header") -} - -struct OldAppHeader<'a> { - pub before_name: &'a [CommentOrNewline<'a>], - pub packages: Option>>, - pub imports: Option>>, - pub provides: ProvidesTo<'a>, -} - -type OldAppPackages<'a> = - KeywordItem<'a, PackagesKeyword, Collection<'a, Loc>>>>; - -#[inline(always)] -fn old_app_header<'a>() -> impl Parser<'a, AppHeader<'a>, EHeader<'a>> { - let old = record!(OldAppHeader { - before_name: skip_second( - space0_e(EHeader::IndentStart), - loc(crate::parser::specialize_err( - EHeader::AppName, - string_literal::parse_str_literal() - )) - ), - packages: optional(specialize_err(EHeader::Packages, loc(packages()))), - imports: optional(specialize_err(EHeader::Imports, imports())), - provides: specialize_err(EHeader::Provides, provides_to()), - }); - - map_with_arena(old, |arena: &'a bumpalo::Bump, old: OldAppHeader<'a>| { - let mut before_packages: &'a [CommentOrNewline] = &[]; - - let packages = match old.packages { - Some(packages) => { - before_packages = merge_spaces( - arena, - packages.value.keyword.before, - packages.value.keyword.after, - ); - - if let To::ExistingPackage(platform_shorthand) = old.provides.to.value { - packages.map(|coll| { - coll.item.map_items(arena, |loc_spaced_pkg| { - if loc_spaced_pkg.value.item().shorthand == platform_shorthand { - loc_spaced_pkg.map(|spaced_pkg| { - spaced_pkg.map(arena, |pkg| { - let mut new_pkg = *pkg; - new_pkg.platform_marker = Some(merge_spaces( - arena, - old.provides.to_keyword.before, - old.provides.to_keyword.after, - )); - new_pkg - }) - }) - } else { - *loc_spaced_pkg - } - }) - }) - } else { - packages.map(|kw| kw.item) - } - } - None => Loc { - region: Region::zero(), - value: Collection::empty(), - }, - }; - - let provides = match old.provides.types { - Some(types) => { - let mut combined_items = bumpalo::collections::Vec::with_capacity_in( - old.provides.entries.items.len() + types.items.len(), - arena, - ); - - combined_items.extend_from_slice(old.provides.entries.items); - - for loc_spaced_type_ident in types.items { - combined_items.push(loc_spaced_type_ident.map(|spaced_type_ident| { - spaced_type_ident.map(arena, |type_ident| { - ExposedName::new(From::from(*type_ident)) - }) - })); - } - - let value_comments = old.provides.entries.final_comments(); - let type_comments = types.final_comments(); - - let mut combined_comments = bumpalo::collections::Vec::with_capacity_in( - value_comments.len() + type_comments.len(), - arena, - ); - combined_comments.extend_from_slice(value_comments); - combined_comments.extend_from_slice(type_comments); - - Collection::with_items_and_comments( - arena, - combined_items.into_bump_slice(), - combined_comments.into_bump_slice(), - ) - } - None => old.provides.entries, - }; - - AppHeader { - before_provides: merge_spaces( - arena, - old.before_name, - old.provides.provides_keyword.before, - ), - provides, - before_packages: merge_spaces( - arena, - before_packages, - old.provides.provides_keyword.after, - ), - packages, - old_imports: old.imports.and_then(imports_none_if_empty), - old_provides_to_new_package: match old.provides.to.value { - To::NewPackage(new_pkg) => Some(new_pkg), - To::ExistingPackage(_) => None, - }, - } - }) -} - -#[inline(always)] -fn package_header<'a>() -> impl Parser<'a, PackageHeader<'a>, EHeader<'a>> { - record!(PackageHeader { - before_exposes: space0_e(EHeader::IndentStart), - exposes: specialize_err(EHeader::Exposes, exposes_module_collection()), - before_packages: space0_e(EHeader::IndentStart), - packages: specialize_err(EHeader::Packages, loc(packages_collection())), - }) - .trace("package_header") -} - -#[derive(Debug, Clone, PartialEq)] -struct OldPackageHeader<'a> { - before_name: &'a [CommentOrNewline<'a>], - exposes: KeywordItem<'a, ExposesKeyword, Collection<'a, Loc>>>>, - packages: - Loc>>>>>, -} - -#[inline(always)] -fn old_package_header<'a>() -> impl Parser<'a, PackageHeader<'a>, EHeader<'a>> { - map_with_arena( - record!(OldPackageHeader { - before_name: skip_second( - space0_e(EHeader::IndentStart), - specialize_err(EHeader::PackageName, package_name()) - ), - exposes: specialize_err(EHeader::Exposes, exposes_modules()), - packages: specialize_err(EHeader::Packages, loc(packages())), - }), - |arena: &'a bumpalo::Bump, old: OldPackageHeader<'a>| { - let before_exposes = merge_n_spaces!( - arena, - old.before_name, - old.exposes.keyword.before, - old.exposes.keyword.after - ); - let before_packages = merge_spaces( - arena, - old.packages.value.keyword.before, - old.packages.value.keyword.after, - ); - - PackageHeader { - before_exposes, - exposes: old.exposes.item, - before_packages, - packages: old.packages.map(|kw| kw.item), - } - }, - ) - .trace("old_package_header") -} - -#[inline(always)] -fn platform_header<'a>() -> impl Parser<'a, PlatformHeader<'a>, EHeader<'a>> { - record!(PlatformHeader { - before_name: space0_e(EHeader::IndentStart), - name: loc(specialize_err(EHeader::PlatformName, package_name())), - requires: specialize_err(EHeader::Requires, requires()), - exposes: specialize_err(EHeader::Exposes, exposes_modules()), - packages: specialize_err(EHeader::Packages, packages()), - imports: specialize_err(EHeader::Imports, imports()), - provides: specialize_err(EHeader::Provides, provides_exposed()), - }) - .trace("platform_header") -} - -fn provides_to_package<'a>() -> impl Parser<'a, To<'a>, EProvides<'a>> { - one_of![ - specialize_err( - |_, pos| EProvides::Identifier(pos), - map(lowercase_ident(), To::ExistingPackage) - ), - specialize_err(EProvides::Package, map(package_name(), To::NewPackage)) - ] -} - -#[inline(always)] -fn provides_to<'a>() -> impl Parser<'a, ProvidesTo<'a>, EProvides<'a>> { - record!(ProvidesTo { - provides_keyword: spaces_around_keyword( - ProvidesKeyword, - EProvides::Provides, - EProvides::IndentProvides, - EProvides::IndentListStart - ), - entries: collection_trailing_sep_e( - byte(b'[', EProvides::ListStart), - exposes_entry(EProvides::Identifier), - byte(b',', EProvides::ListEnd), - byte(b']', EProvides::ListEnd), - Spaced::SpaceBefore - ), - types: optional(backtrackable(provides_types())), - to_keyword: spaces_around_keyword( - ToKeyword, - EProvides::To, - EProvides::IndentTo, - EProvides::IndentListStart - ), - to: loc(provides_to_package()), - }) - .trace("provides_to") -} - -fn provides_exposed<'a>() -> impl Parser< - 'a, - KeywordItem<'a, ProvidesKeyword, Collection<'a, Loc>>>>, - EProvides<'a>, -> { - record!(KeywordItem { - keyword: spaces_around_keyword( - ProvidesKeyword, - EProvides::Provides, - EProvides::IndentProvides, - EProvides::IndentListStart - ), - item: collection_trailing_sep_e( - byte(b'[', EProvides::ListStart), - exposes_entry(EProvides::Identifier), - byte(b',', EProvides::ListEnd), - byte(b']', EProvides::ListEnd), - Spaced::SpaceBefore - ), - }) -} - -#[inline(always)] -fn provides_types<'a>( -) -> impl Parser<'a, Collection<'a, Loc>>>, EProvides<'a>> { - skip_first( - // We only support spaces here, not newlines, because this is not intended - // to be the design forever. Someday it will hopefully work like Elm, - // where platform authors can provide functions like Browser.sandbox which - // present an API based on ordinary-looking type variables. - zero_or_more(byte( - b' ', - // HACK: If this errors, EProvides::Provides is not an accurate reflection - // of what went wrong. However, this is both skipped and zero_or_more, - // so this error should never be visible to anyone in practice! - EProvides::Provides, - )), - collection_trailing_sep_e( - byte(b'{', EProvides::ListStart), - provides_type_entry(EProvides::Identifier), - byte(b',', EProvides::ListEnd), - byte(b'}', EProvides::ListEnd), - Spaced::SpaceBefore, - ), - ) -} - -fn provides_type_entry<'a, F, E>( - to_expectation: F, -) -> impl Parser<'a, Loc>>, E> -where - F: Fn(Position) -> E, - F: Copy, - E: 'a, -{ - loc(map( - specialize_err(move |_, pos| to_expectation(pos), ident::uppercase()), - Spaced::Item, - )) -} - -fn exposes_entry<'a, F, E>( - to_expectation: F, -) -> impl Parser<'a, Loc>>, E> -where - F: Fn(Position) -> E, - F: Copy, - E: 'a, -{ - loc(map( - specialize_err(move |_, pos| to_expectation(pos), unqualified_ident()), - |n| Spaced::Item(ExposedName::new(n)), - )) -} - -#[inline(always)] -fn requires<'a>( -) -> impl Parser<'a, KeywordItem<'a, RequiresKeyword, PlatformRequires<'a>>, ERequires<'a>> { - record!(KeywordItem { - keyword: spaces_around_keyword( - RequiresKeyword, - ERequires::Requires, - ERequires::IndentRequires, - ERequires::IndentListStart - ), - item: platform_requires(), - }) -} - -#[inline(always)] -fn platform_requires<'a>() -> impl Parser<'a, PlatformRequires<'a>, ERequires<'a>> { - record!(PlatformRequires { - rigids: skip_second(requires_rigids(), space0_e(ERequires::ListStart)), - signature: requires_typed_ident() - }) -} - -#[inline(always)] -fn requires_rigids<'a>( -) -> impl Parser<'a, Collection<'a, Loc>>>, ERequires<'a>> { - collection_trailing_sep_e( - byte(b'{', ERequires::ListStart), - specialize_err( - |_, pos| ERequires::Rigid(pos), - loc(map(ident::uppercase(), Spaced::Item)), - ), - byte(b',', ERequires::ListEnd), - byte(b'}', ERequires::ListEnd), - Spaced::SpaceBefore, - ) -} - -#[inline(always)] -fn requires_typed_ident<'a>() -> impl Parser<'a, Loc>>, ERequires<'a>> { - skip_first( - byte(b'{', ERequires::ListStart), - skip_second( - reset_min_indent(space0_around_ee( - specialize_err(ERequires::TypedIdent, loc(typed_ident())), - ERequires::ListStart, - ERequires::ListEnd, - )), - byte(b'}', ERequires::ListStart), - ), - ) -} - -#[inline(always)] -fn exposes_values_kw<'a>() -> impl Parser< - 'a, - KeywordItem<'a, ExposesKeyword, Collection<'a, Loc>>>>, - EExposes, -> { - record!(KeywordItem { - keyword: exposes_kw(), - item: exposes_list() - }) -} - -#[inline(always)] -fn exposes_kw<'a>() -> impl Parser<'a, Spaces<'a, ExposesKeyword>, EExposes> { - spaces_around_keyword( - ExposesKeyword, - EExposes::Exposes, - EExposes::IndentExposes, - EExposes::IndentListStart, - ) -} - -#[inline(always)] -fn exposes_list<'a>() -> impl Parser<'a, Collection<'a, Loc>>>, EExposes> -{ - collection_trailing_sep_e( - byte(b'[', EExposes::ListStart), - exposes_entry(EExposes::Identifier), - byte(b',', EExposes::ListEnd), - byte(b']', EExposes::ListEnd), - Spaced::SpaceBefore, - ) -} - -pub fn spaces_around_keyword<'a, K: Keyword, E>( - keyword_item: K, - expectation: fn(Position) -> E, - indent_problem1: fn(Position) -> E, - indent_problem2: fn(Position) -> E, -) -> impl Parser<'a, Spaces<'a, K>, E> -where - E: 'a + SpaceProblem, -{ - map( - and( - skip_second( - // parse any leading space before the keyword - backtrackable(space0_e(indent_problem1)), - // parse the keyword - crate::parser::keyword(K::KEYWORD, expectation), - ), - // parse the trailing space - space0_e(indent_problem2), - ), - move |(before, after)| Spaces { - before, - item: keyword_item, - after, - }, - ) -} - -#[inline(always)] -fn exposes_modules<'a>() -> impl Parser< - 'a, - KeywordItem<'a, ExposesKeyword, Collection<'a, Loc>>>>, - EExposes, -> { - record!(KeywordItem { - keyword: spaces_around_keyword( - ExposesKeyword, - EExposes::Exposes, - EExposes::IndentExposes, - EExposes::IndentListStart - ), - item: exposes_module_collection(), - }) -} - -fn exposes_module_collection<'a>( -) -> impl Parser<'a, Collection<'a, Loc>>>, EExposes> { - collection_trailing_sep_e( - byte(b'[', EExposes::ListStart), - exposes_module(EExposes::Identifier), - byte(b',', EExposes::ListEnd), - byte(b']', EExposes::ListEnd), - Spaced::SpaceBefore, - ) -} - -fn exposes_module<'a, F, E>( - to_expectation: F, -) -> impl Parser<'a, Loc>>, E> -where - F: Fn(Position) -> E, - F: Copy, - E: 'a, -{ - loc(map( - specialize_err(move |_, pos| to_expectation(pos), module_name()), - Spaced::Item, - )) -} - -#[inline(always)] -fn packages<'a>() -> impl Parser< - 'a, - KeywordItem<'a, PackagesKeyword, Collection<'a, Loc>>>>, - EPackages<'a>, -> { - record!(KeywordItem { - keyword: packages_kw(), - item: packages_collection() - }) -} - -#[inline(always)] -fn packages_kw<'a>() -> impl Parser<'a, Spaces<'a, PackagesKeyword>, EPackages<'a>> { - spaces_around_keyword( - PackagesKeyword, - EPackages::Packages, - EPackages::IndentPackages, - EPackages::IndentListStart, - ) -} - -#[inline(always)] -fn packages_collection<'a>( -) -> impl Parser<'a, Collection<'a, Loc>>>, EPackages<'a>> { - collection_trailing_sep_e( - byte(b'{', EPackages::ListStart), - specialize_err(EPackages::PackageEntry, loc(package_entry())), - byte(b',', EPackages::ListEnd), - byte(b'}', EPackages::ListEnd), - Spaced::SpaceBefore, - ) -} - -fn imports<'a>() -> impl Parser< - 'a, - KeywordItem<'a, ImportsKeyword, Collection<'a, Loc>>>>, - EImports, -> { - record!(KeywordItem { - keyword: spaces_around_keyword( - ImportsKeyword, - EImports::Imports, - EImports::IndentImports, - EImports::IndentListStart - ), - item: collection_trailing_sep_e( - byte(b'[', EImports::ListStart), - loc(imports_entry()), - byte(b',', EImports::ListEnd), - byte(b']', EImports::ListEnd), - Spaced::SpaceBefore - ) - }) - .trace("imports") -} - -#[inline(always)] -pub fn typed_ident<'a>() -> impl Parser<'a, Spaced<'a, TypedIdent<'a>>, ETypedIdent<'a>> { - // e.g. - // - // printLine : Str -> Task {} * - map( - and( - and( - loc(specialize_err( - |_, pos| ETypedIdent::Identifier(pos), - lowercase_ident(), - )), - space0_e(ETypedIdent::IndentHasType), - ), - skip_first( - byte(b':', ETypedIdent::HasType), - space0_before_e( - specialize_err( - ETypedIdent::Type, - reset_min_indent(type_annotation::located(true)), - ), - ETypedIdent::IndentType, - ), - ), - ), - |((ident, spaces_before_colon), ann)| { - Spaced::Item(TypedIdent { - ident, - spaces_before_colon, - ann, - }) - }, - ) -} - -fn shortname<'a>() -> impl Parser<'a, &'a str, EImports> { - specialize_err(|_, pos| EImports::Shorthand(pos), lowercase_ident()) -} - -pub fn module_name_help<'a, F, E>(to_expectation: F) -> impl Parser<'a, ModuleName<'a>, E> -where - F: Fn(Position) -> E, - E: 'a, - F: 'a, -{ - specialize_err(move |_, pos| to_expectation(pos), module_name()) -} - -#[inline(always)] -fn imports_entry<'a>() -> impl Parser<'a, Spaced<'a, ImportsEntry<'a>>, EImports> { - type Temp<'a> = ( - (Option<&'a str>, ModuleName<'a>), - Option>>>>, - ); - - let spaced_import = |((opt_shortname, module_name), opt_values): Temp<'a>| { - let exposed_values = opt_values.unwrap_or_else(Collection::empty); - - let entry = match opt_shortname { - Some(shortname) => ImportsEntry::Package(shortname, module_name, exposed_values), - - None => ImportsEntry::Module(module_name, exposed_values), - }; - - Spaced::Item(entry) - }; - - one_of!( - map( - and( - and( - // e.g. `pf.` - optional(backtrackable(skip_second( - shortname(), - byte(b'.', EImports::ShorthandDot) - ))), - // e.g. `Task` - module_name_help(EImports::ModuleName) - ), - // e.g. `.{ Task, after}` - optional(skip_first( - byte(b'.', EImports::ExposingDot), - collection_trailing_sep_e( - byte(b'{', EImports::SetStart), - exposes_entry(EImports::Identifier), - byte(b',', EImports::SetEnd), - byte(b'}', EImports::SetEnd), - Spaced::SpaceBefore - ) - )) - ), - spaced_import - ) - .trace("normal_import"), - map( - and( - and( - // e.g. "filename" - // TODO: str literal allows for multiline strings. We probably don't want that for file names. - specialize_err(|_, pos| EImports::StrLiteral(pos), parse_str_literal()), - // e.g. as - and( - and( - space0_e(EImports::AsKeyword), - two_bytes(b'a', b's', EImports::AsKeyword) - ), - space0_e(EImports::AsKeyword) - ) - ), - // e.g. file : Str - specialize_err(|_, pos| EImports::TypedIdent(pos), typed_ident()) - ), - |((file_name, _), typed_ident)| { - // TODO: look at blacking block strings during parsing. - Spaced::Item(ImportsEntry::IngestedFile(file_name, typed_ident)) - } - ) - .trace("ingest_file_import") - ) - .trace("imports_entry") -} diff --git a/crates/compiler/parse/src/normalize.rs b/crates/compiler/parse/src/normalize.rs index 8561279b8f6..a1def6c6776 100644 --- a/crates/compiler/parse/src/normalize.rs +++ b/crates/compiler/parse/src/normalize.rs @@ -8,9 +8,9 @@ use crate::{ AbilityImpls, AbilityMember, AssignedField, Collection, Defs, Expr, FullAst, Header, Implements, ImplementsAbilities, ImplementsAbility, ImplementsClause, ImportAlias, ImportAsKeyword, ImportExposingKeyword, ImportedModuleName, IngestedFileAnnotation, - IngestedFileImport, ModuleImport, ModuleImportParams, OldRecordBuilderField, Pattern, - PatternAs, Spaced, Spaces, SpacesBefore, StrLiteral, StrSegment, Tag, TypeAnnotation, - TypeDef, TypeHeader, ValueDef, WhenBranch, + IngestedFileImport, ModuleImport, ModuleImportParams, Pattern, PatternAs, Spaced, Spaces, + SpacesBefore, StrLiteral, StrSegment, Tag, TypeAnnotation, TypeDef, TypeHeader, ValueDef, + WhenBranch, }, header::{ AppHeader, ExposedName, ExposesKeyword, HostedHeader, ImportsEntry, ImportsKeyword, @@ -562,30 +562,6 @@ impl<'a, T: Normalize<'a> + Copy + std::fmt::Debug> Normalize<'a> for AssignedFi } } -impl<'a> Normalize<'a> for OldRecordBuilderField<'a> { - fn normalize(&self, arena: &'a Bump) -> Self { - match *self { - OldRecordBuilderField::Value(a, _, c) => OldRecordBuilderField::Value( - a.normalize(arena), - &[], - arena.alloc(c.normalize(arena)), - ), - OldRecordBuilderField::ApplyValue(a, _, _, c) => OldRecordBuilderField::ApplyValue( - a.normalize(arena), - &[], - &[], - arena.alloc(c.normalize(arena)), - ), - OldRecordBuilderField::LabelOnly(a) => { - OldRecordBuilderField::LabelOnly(a.normalize(arena)) - } - OldRecordBuilderField::Malformed(a) => OldRecordBuilderField::Malformed(a), - OldRecordBuilderField::SpaceBefore(a, _) => a.normalize(arena), - OldRecordBuilderField::SpaceAfter(a, _) => a.normalize(arena), - } - } -} - impl<'a> Normalize<'a> for StrLiteral<'a> { fn normalize(&self, arena: &'a Bump) -> Self { match *self { @@ -738,7 +714,6 @@ impl<'a> Normalize<'a> for Expr<'a> { fields: fields.normalize(arena), }, Expr::Record(a) => Expr::Record(a.normalize(arena)), - Expr::OldRecordBuilder(a) => Expr::OldRecordBuilder(a.normalize(arena)), Expr::RecordBuilder { mapper, fields } => Expr::RecordBuilder { mapper: arena.alloc(mapper.normalize(arena)), fields: fields.normalize(arena), @@ -817,12 +792,6 @@ impl<'a> Normalize<'a> for Expr<'a> { Expr::SpaceBefore(a, _) => a.normalize(arena), Expr::SpaceAfter(a, _) => a.normalize(arena), Expr::SingleQuote(a) => Expr::Num(a), - Expr::MultipleOldRecordBuilders(a) => { - Expr::MultipleOldRecordBuilders(arena.alloc(a.normalize(arena))) - } - Expr::UnappliedOldRecordBuilder(a) => { - Expr::UnappliedOldRecordBuilder(arena.alloc(a.normalize(arena))) - } Expr::EmptyRecordBuilder(a) => { Expr::EmptyRecordBuilder(arena.alloc(a.normalize(arena))) } @@ -1092,12 +1061,6 @@ impl<'a> Normalize<'a> for EExpr<'a> { EExpr::Record(inner_err, _pos) => { EExpr::Record(inner_err.normalize(arena), Position::zero()) } - EExpr::OptionalValueInOldRecordBuilder(_pos) => { - EExpr::OptionalValueInOldRecordBuilder(Region::zero()) - } - EExpr::IgnoredValueInOldRecordBuilder(_pos) => { - EExpr::OptionalValueInOldRecordBuilder(Region::zero()) - } EExpr::Str(inner_err, _pos) => EExpr::Str(inner_err.normalize(arena), Position::zero()), EExpr::Number(inner_err, _pos) => EExpr::Number(inner_err.clone(), Position::zero()), EExpr::List(inner_err, _pos) => { @@ -1331,7 +1294,6 @@ impl<'a> Normalize<'a> for EImportParams<'a> { EImportParams::Record(inner_err.normalize(arena), Position::zero()) } EImportParams::RecordUpdateFound(_) => EImportParams::RecordUpdateFound(Region::zero()), - EImportParams::RecordApplyFound(_) => EImportParams::RecordApplyFound(Region::zero()), EImportParams::RecordIgnoredFieldFound(_) => { EImportParams::RecordIgnoredFieldFound(Region::zero()) } diff --git a/crates/compiler/parse/src/parser.rs b/crates/compiler/parse/src/parser.rs index a5e0900a66f..71ebcf647b5 100644 --- a/crates/compiler/parse/src/parser.rs +++ b/crates/compiler/parse/src/parser.rs @@ -344,8 +344,6 @@ pub enum EExpr<'a> { InParens(EInParens<'a>, Position), Record(ERecord<'a>, Position), - OptionalValueInOldRecordBuilder(Region), - IgnoredValueInOldRecordBuilder(Region), RecordUpdateOldBuilderField(Region), RecordUpdateIgnoredField(Region), RecordBuilderOldBuilderField(Region), @@ -551,7 +549,6 @@ pub enum EImportParams<'a> { Record(ERecord<'a>, Position), RecordUpdateFound(Region), RecordBuilderFound(Region), - RecordApplyFound(Region), RecordIgnoredFieldFound(Region), Space(BadInputError, Position), } diff --git a/crates/compiler/parse/src/type_annotation.rs b/crates/compiler/parse/src/type_annotation.rs index 2802a902e26..03714e8ea77 100644 --- a/crates/compiler/parse/src/type_annotation.rs +++ b/crates/compiler/parse/src/type_annotation.rs @@ -5,7 +5,7 @@ use crate::ast::{ use crate::blankspace::{ space0_around_ee, space0_before_e, space0_before_optional_after, space0_e, }; -use crate::expr::{record_field, FoundApplyValue}; +use crate::expr::record_field; use crate::ident::{lowercase_ident, lowercase_ident_keyword_e}; use crate::keyword; use crate::parser::{ @@ -565,11 +565,10 @@ fn parse_implements_ability<'a>() -> impl Parser<'a, ImplementsAbility<'a>, ETyp fn ability_impl_field<'a>() -> impl Parser<'a, AssignedField<'a, Expr<'a>>, ERecord<'a>> { then(record_field(), move |arena, state, _, field| { match field.to_assigned_field(arena) { - Ok(AssignedField::IgnoredValue(_, _, _)) => { + AssignedField::IgnoredValue(_, _, _) => { Err((MadeProgress, ERecord::Field(state.pos()))) } - Ok(assigned_field) => Ok((MadeProgress, assigned_field, state)), - Err(FoundApplyValue) => Err((MadeProgress, ERecord::Field(state.pos()))), + assigned_field => Ok((MadeProgress, assigned_field, state)), } }) } diff --git a/crates/compiler/problem/src/can.rs b/crates/compiler/problem/src/can.rs index 5c6383eb875..31af6a39aa9 100644 --- a/crates/compiler/problem/src/can.rs +++ b/crates/compiler/problem/src/can.rs @@ -427,8 +427,6 @@ impl Problem { | Problem::RuntimeError(RuntimeError::EmptySingleQuote(region)) | Problem::RuntimeError(RuntimeError::MultipleCharsInSingleQuote(region)) | Problem::RuntimeError(RuntimeError::DegenerateBranch(region)) - | Problem::RuntimeError(RuntimeError::MultipleOldRecordBuilders(region)) - | Problem::RuntimeError(RuntimeError::UnappliedOldRecordBuilder(region)) | Problem::RuntimeError(RuntimeError::EmptyRecordBuilder(region)) | Problem::RuntimeError(RuntimeError::SingleFieldRecordBuilder(region)) | Problem::RuntimeError(RuntimeError::OptionalFieldInRecordBuilder { @@ -686,9 +684,6 @@ pub enum RuntimeError { DegenerateBranch(Region), - MultipleOldRecordBuilders(Region), - UnappliedOldRecordBuilder(Region), - EmptyRecordBuilder(Region), SingleFieldRecordBuilder(Region), OptionalFieldInRecordBuilder { @@ -739,8 +734,6 @@ impl RuntimeError { | RuntimeError::DegenerateBranch(region) | RuntimeError::InvalidInterpolation(region) | RuntimeError::InvalidHexadecimal(region) - | RuntimeError::MultipleOldRecordBuilders(region) - | RuntimeError::UnappliedOldRecordBuilder(region) | RuntimeError::EmptyRecordBuilder(region) | RuntimeError::SingleFieldRecordBuilder(region) | RuntimeError::OptionalFieldInRecordBuilder { diff --git a/crates/compiler/test_syntax/tests/test_fmt.rs b/crates/compiler/test_syntax/tests/test_fmt.rs index ace412cf3ad..a6349f2c8b6 100644 --- a/crates/compiler/test_syntax/tests/test_fmt.rs +++ b/crates/compiler/test_syntax/tests/test_fmt.rs @@ -1986,7 +1986,7 @@ mod test_fmt { } #[test] - fn new_record_builder() { + fn record_builder() { expr_formats_same(indoc!( r" { shoes <- leftShoe: nothing } @@ -2077,170 +2077,6 @@ mod test_fmt { ); } - #[test] - fn old_record_builder() { - expr_formats_same(indoc!( - r#" - { a: 1, b: <- get "b" |> batch, c: <- get "c" |> batch, d } - "# - )); - - expr_formats_to( - indoc!( - r#" - { a: 1, b: <- get "b" |> batch, c:<- get "c" |> batch } - "# - ), - indoc!( - r#" - { a: 1, b: <- get "b" |> batch, c: <- get "c" |> batch } - "# - ), - ); - - expr_formats_same(indoc!( - r#" - { - a: 1, - b: <- get "b" |> batch, - c: <- get "c" |> batch, - d, - } - "# - )); - - expr_formats_to( - indoc!( - r#" - { a: 1, b: <- get "b" |> batch, - c: <- get "c" |> batch, d } - "# - ), - indoc!( - r#" - { - a: 1, - b: <- get "b" |> batch, - c: <- get "c" |> batch, - d, - } - "# - ), - ); - } - - #[test] - fn multiline_record_builder_field() { - expr_formats_to( - indoc!( - r#" - succeed { - a: <- get "a" |> map (\x -> x * 2) - |> batch, - b: <- get "b" |> batch, - c: items - |> List.map \x -> x * 2 - } - "# - ), - indoc!( - r#" - succeed { - a: <- - get "a" - |> map (\x -> x * 2) - |> batch, - b: <- get "b" |> batch, - c: - items - |> List.map \x -> x * 2, - } - "# - ), - ); - - expr_formats_same(indoc!( - r#" - succeed { - a: # I like to comment in weird places - <- get "a" |> batch, - b: <- get "b" |> batch, - } - "# - )); - - expr_formats_same(indoc!( - r#" - succeed { - a: - # I like to comment in weird places - <- get "a" |> batch, - b: <- get "b" |> batch, - } - "# - )); - } - - #[test] - fn outdentable_record_builders() { - expr_formats_to( - indoc!( - r#" - succeed { a: <- get "a" |> batch, - b: <- get "b" |> batch, - } - "# - ), - indoc!( - r#" - succeed { - a: <- get "a" |> batch, - b: <- get "b" |> batch, - } - "# - ), - ); - - expr_formats_to( - indoc!( - r#" - succeed - { - a: <- get "a" |> batch, - b: <- get "b" |> batch, - } - "# - ), - indoc!( - r#" - succeed { - a: <- get "a" |> batch, - b: <- get "b" |> batch, - } - "# - ), - ); - } - - #[test] - fn can_format_multiple_record_builders() { - expr_formats_to( - indoc!( - r#" - succeed { a: <- get "a" } - { b: <- get "b" } - "# - ), - indoc!( - r#" - succeed - { a: <- get "a" } - { b: <- get "b" } - "# - ), - ); - } - #[test] fn final_comments_in_records() { expr_formats_same(indoc!( diff --git a/crates/language_server/src/analysis/tokens.rs b/crates/language_server/src/analysis/tokens.rs index b923712f4f5..10918316ec7 100644 --- a/crates/language_server/src/analysis/tokens.rs +++ b/crates/language_server/src/analysis/tokens.rs @@ -6,9 +6,8 @@ use roc_module::called_via::{BinOp, UnaryOp}; use roc_parse::{ ast::{ AbilityImpls, AbilityMember, AssignedField, Collection, Defs, Expr, Header, Implements, - ImplementsAbilities, ImplementsAbility, ImplementsClause, OldRecordBuilderField, Pattern, - PatternAs, Spaced, StrLiteral, Tag, TypeAnnotation, TypeDef, TypeHeader, ValueDef, - WhenBranch, + ImplementsAbilities, ImplementsAbility, ImplementsClause, Pattern, PatternAs, Spaced, + StrLiteral, Tag, TypeAnnotation, TypeDef, TypeHeader, ValueDef, WhenBranch, }, header::{ AppHeader, ExposedName, HostedHeader, ImportsEntry, ModuleHeader, ModuleName, ModuleParams, @@ -672,7 +671,6 @@ impl IterTokens for Loc> { .collect_in(arena), Expr::Record(rcd) => rcd.iter_tokens(arena), Expr::Tuple(tup) => tup.iter_tokens(arena), - Expr::OldRecordBuilder(rb) => rb.iter_tokens(arena), Expr::RecordBuilder { mapper, fields } => (mapper.iter_tokens(arena).into_iter()) .chain(fields.iter().flat_map(|f| f.iter_tokens(arena))) .collect_in(arena), @@ -724,8 +722,6 @@ impl IterTokens for Loc> { Loc::at(region, *e).iter_tokens(arena) } Expr::ParensAround(e) => Loc::at(region, *e).iter_tokens(arena), - Expr::MultipleOldRecordBuilders(e) => e.iter_tokens(arena), - Expr::UnappliedOldRecordBuilder(e) => e.iter_tokens(arena), Expr::EmptyRecordBuilder(e) => e.iter_tokens(arena), Expr::SingleFieldRecordBuilder(e) => e.iter_tokens(arena), Expr::OptionalFieldInRecordBuilder(_name, e) => e.iter_tokens(arena), @@ -748,24 +744,6 @@ impl IterTokens for Loc> { } } -impl IterTokens for Loc> { - fn iter_tokens<'a>(&self, arena: &'a Bump) -> BumpVec<'a, Loc> { - match self.value { - OldRecordBuilderField::Value(field, _, e) - | OldRecordBuilderField::ApplyValue(field, _, _, e) => field_token(field.region, arena) - .into_iter() - .chain(e.iter_tokens(arena)) - .collect_in(arena), - OldRecordBuilderField::LabelOnly(field) => field_token(field.region, arena), - OldRecordBuilderField::SpaceBefore(rbf, _) - | OldRecordBuilderField::SpaceAfter(rbf, _) => { - Loc::at(self.region, *rbf).iter_tokens(arena) - } - OldRecordBuilderField::Malformed(_) => bumpvec![in arena;], - } - } -} - impl IterTokens for &WhenBranch<'_> { fn iter_tokens<'a>(&self, arena: &'a Bump) -> BumpVec<'a, Loc> { let WhenBranch { diff --git a/crates/reporting/src/error/canonicalize.rs b/crates/reporting/src/error/canonicalize.rs index efbd05994f7..7e60ea07294 100644 --- a/crates/reporting/src/error/canonicalize.rs +++ b/crates/reporting/src/error/canonicalize.rs @@ -2488,32 +2488,6 @@ fn pretty_runtime_error<'b>( title = "DEGENERATE BRANCH"; } - RuntimeError::MultipleOldRecordBuilders(region) => { - let tip = alloc - .tip() - .append(alloc.reflow("You can combine them or apply them separately.")); - - doc = alloc.stack([ - alloc.reflow("This function is applied to multiple old-style record builders:"), - alloc.region(lines.convert_region(region), severity), - alloc.note("Functions can only take at most one old-style record builder!"), - tip, - ]); - - title = "MULTIPLE OLD-STYLE RECORD BUILDERS"; - } - RuntimeError::UnappliedOldRecordBuilder(region) => { - doc = alloc.stack([ - alloc.reflow("This old-style record builder was not applied to a function:"), - alloc.region(lines.convert_region(region), severity), - alloc.reflow("However, we need a function to construct the record."), - alloc.note( - "Functions must be applied directly. The pipe operator (|>) cannot be used.", - ), - ]); - - title = "UNAPPLIED OLD-STYLE RECORD BUILDER"; - } RuntimeError::EmptyRecordBuilder(region) => { doc = alloc.stack([ alloc.reflow("This record builder has no fields:"), diff --git a/crates/reporting/src/error/parse.rs b/crates/reporting/src/error/parse.rs index 79e83a7e189..20ec3cce560 100644 --- a/crates/reporting/src/error/parse.rs +++ b/crates/reporting/src/error/parse.rs @@ -545,46 +545,6 @@ fn to_expr_report<'a>( to_record_report(alloc, lines, filename, erecord, *pos, start) } - EExpr::OptionalValueInOldRecordBuilder(region) => { - let surroundings = Region::new(start, region.end()); - let region = lines.convert_region(*region); - - let doc = alloc.stack([ - alloc.reflow( - r"I am partway through parsing a record builder, and I found an optional field:", - ), - alloc.region_with_subregion(lines.convert_region(surroundings), region, severity), - alloc.reflow("Optional fields can only appear when you destructure a record."), - ]); - - Report { - filename, - doc, - title: "BAD OLD-STYLE RECORD BUILDER".to_string(), - severity, - } - } - - EExpr::RecordUpdateOldBuilderField(region) => { - let surroundings = Region::new(start, region.end()); - let region = lines.convert_region(*region); - - let doc = alloc.stack([ - alloc.reflow( - r"I am partway through parsing a record update, and I found an old-style record builder field:", - ), - alloc.region_with_subregion(lines.convert_region(surroundings), region, severity), - alloc.reflow("Old-style record builders cannot be updated like records."), - ]); - - Report { - filename, - doc, - title: "BAD RECORD UPDATE".to_string(), - severity, - } - } - EExpr::Space(error, pos) => to_space_report(alloc, lines, filename, error, *pos), &EExpr::Number(ENumber::End, pos) => { @@ -1550,23 +1510,6 @@ fn to_import_report<'a>( Params(EImportParams::Record(problem, pos), _) => { to_record_report(alloc, lines, filename, problem, *pos, start) } - Params(EImportParams::RecordApplyFound(region), _) => { - let surroundings = Region::new(start, region.end()); - let region = lines.convert_region(*region); - - let doc = alloc.stack([ - alloc.reflow("I was partway through parsing module params, but I got stuck here:"), - alloc.region_with_subregion(lines.convert_region(surroundings), region, severity), - alloc.reflow("This looks like an old-style record builder field, but those are not allowed in module params."), - ]); - - Report { - filename, - doc, - title: "OLD-STYLE RECORD BUILDER IN MODULE PARAMS".to_string(), - severity, - } - } Params(EImportParams::RecordIgnoredFieldFound(region), _) => { let surroundings = Region::new(start, region.end()); let region = lines.convert_region(*region); diff --git a/crates/reporting/src/error/type.rs b/crates/reporting/src/error/type.rs index 9256289ab98..23fed78a231 100644 --- a/crates/reporting/src/error/type.rs +++ b/crates/reporting/src/error/type.rs @@ -1238,14 +1238,7 @@ fn to_expr_report<'b>( ), ]), alloc.region(lines.convert_region(expr_region), severity), - match called_via { - CalledVia::OldRecordBuilder => { - alloc.hint("Did you mean to apply it to a function first?") - }, - _ => { - alloc.reflow("I can't call an opaque type because I don't know what it is! Maybe you meant to unwrap it first?") - } - } + alloc.reflow("I can't call an opaque type because I don't know what it is! Maybe you meant to unwrap it first?"), ]), Other => alloc.stack([ alloc.concat([ @@ -1261,14 +1254,6 @@ fn to_expr_report<'b>( ]), alloc.region(lines.convert_region(expr_region), severity), match called_via { - CalledVia::OldRecordBuilder => { - alloc.concat([ - alloc.tip(), - alloc.reflow("Remove "), - alloc.backpassing_arrow(), - alloc.reflow(" to assign the field directly.") - ]) - } CalledVia::RecordBuilder => { alloc.concat([ alloc.note(""), diff --git a/www/content/tutorial.md b/www/content/tutorial.md index a282b47e7c9..2524d006a30 100644 --- a/www/content/tutorial.md +++ b/www/content/tutorial.md @@ -1807,22 +1807,6 @@ This is a useful technique to use when we don't want to write out a bunch of err - If we're using an editor that supports it, hovering over the `_` might display the inferred type that goes there. - We can put an obviously wrong type in there (e.g. replace the `{}` with `Str`, which is totally wrong) and look at the compiler error to see what it inferred as the correct type. -We can also use `_` in type aliases, to express that two types are the same without annotating either of them. For example: - -```roc -RunErr : _ -``` - -```roc -run : Task {} RunErr -``` - -```roc -handleErr : RunErr -> [Exit I32 Str] -``` - -Of course, we could also choose not to use `_` at all and populate the `RunErr` type alias with the full list of errors that could happen in our `run` task. All of these are totally reasonable stylistic choices, depending on how you prefer the code to look. They all compile to exactly the same thing, and have the same runtime characteristics. - ### [The ! suffix](#the-!-suffix) {#the-!-suffix} The `!` suffix operator is syntax sugar for the `Task.await` function, which has this type: @@ -2311,8 +2295,6 @@ expect If you want to see other examples of using record builders, look at the [Record Builder Example](https://www.roc-lang.org/examples/RecordBuilder/README.html) for a moderately-sized example or the [Arg.Builder](https://github.com/roc-lang/basic-cli/blob/main/platform/Arg/Builder.roc) module in our `basic-cli` platform for a complex example. -_Note: This syntax replaces the old `field: <- value` record builder syntax using applicative functors because it is much simpler to understand and use. The old syntax will be removed soon._ - ### [Reserved Keywords](#reserved-keywords) {#reserved-keywords} These are reserved keywords in Roc. You can't choose any of them as names, except as record field names.