From a87bf0a302719cd6f48bc806fa75bc2990db37b3 Mon Sep 17 00:00:00 2001 From: Folkert Date: Sat, 20 Feb 2021 22:16:42 +0100 Subject: [PATCH 01/17] clippy --- compiler/load/src/file.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/compiler/load/src/file.rs b/compiler/load/src/file.rs index ea760f8c28..b7378285e6 100644 --- a/compiler/load/src/file.rs +++ b/compiler/load/src/file.rs @@ -2212,7 +2212,7 @@ fn load_pkg_config<'a>( &header, pkg_module_timing, ) - .map(|x| x.1)?; + .1; let effects_module_msg = fabricate_effects_module( arena, @@ -2223,7 +2223,7 @@ fn load_pkg_config<'a>( header, effect_module_timing, ) - .map(|x| x.1)?; + .1; Ok(Msg::Many(vec![effects_module_msg, pkg_config_module_msg])) } @@ -3093,11 +3093,11 @@ fn fabricate_pkg_config_module<'a>( ident_ids_by_module: Arc>>, header: &PlatformHeader<'a>, module_timing: ModuleTiming, -) -> Result<(ModuleId, Msg<'a>), LoadingProblem<'a>> { +) -> (ModuleId, Msg<'a>) { let provides: &'a [Located>] = header.provides.clone().into_bump_slice(); - Ok(send_header_two( + send_header_two( arena, filename, shorthand, @@ -3110,7 +3110,7 @@ fn fabricate_pkg_config_module<'a>( module_ids, ident_ids_by_module, module_timing, - )) + ) } #[allow(clippy::too_many_arguments)] @@ -3122,7 +3122,7 @@ fn fabricate_effects_module<'a>( mode: Mode, header: PlatformHeader<'a>, module_timing: ModuleTiming, -) -> Result<(ModuleId, Msg<'a>), LoadingProblem<'a>> { +) -> (ModuleId, Msg<'a>) { let num_exposes = header.provides.len() + 1; let mut exposed: Vec = Vec::with_capacity(num_exposes); From df206edd8857182e972f7ecd8cd5595f220ef15a Mon Sep 17 00:00:00 2001 From: Folkert Date: Sat, 20 Feb 2021 22:48:44 +0100 Subject: [PATCH 02/17] clippy --- compiler/load/src/file.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/load/src/file.rs b/compiler/load/src/file.rs index b7378285e6..9cfe7c1f80 100644 --- a/compiler/load/src/file.rs +++ b/compiler/load/src/file.rs @@ -2465,7 +2465,7 @@ fn parse_header<'a>( }, } } - Ok((_, ast::Module::Platform { header }, _parse_state)) => fabricate_effects_module( + Ok((_, ast::Module::Platform { header }, _parse_state)) => Ok(fabricate_effects_module( arena, &"", module_ids, @@ -2473,7 +2473,7 @@ fn parse_header<'a>( mode, header, module_timing, - ), + )), Err((_, fail, _)) => Err(LoadingProblem::ParsingFailed( fail.into_parse_problem(filename, src_bytes), )), @@ -3350,7 +3350,7 @@ fn fabricate_effects_module<'a>( module_timing, }; - Ok(( + ( module_id, Msg::MadeEffectModule { type_shortname: effects.effect_shortname, @@ -3358,7 +3358,7 @@ fn fabricate_effects_module<'a>( canonicalization_problems: module_output.problems, module_docs, }, - )) + ) } fn unpack_exposes_entries<'a>( From daf6de950e8278c96b7060c2b843ef62c44b4c5a Mon Sep 17 00:00:00 2001 From: Folkert Date: Sat, 20 Feb 2021 22:56:01 +0100 Subject: [PATCH 03/17] crude prototype --- cli/src/lib.rs | 3 +++ compiler/load/src/file.rs | 12 +++++++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/cli/src/lib.rs b/cli/src/lib.rs index 6e69b361ad..2cedcf4ea8 100644 --- a/cli/src/lib.rs +++ b/cli/src/lib.rs @@ -135,6 +135,9 @@ pub fn build(target: &Triple, matches: &ArgMatches, run_after_build: bool) -> io Err(LoadingProblem::ParsingFailedReport(report)) => { print!("{}", report); } + Err(LoadingProblem::NoPlatform) => { + print!("The file you ran did not provide a platform\nMaybe you are trying to run an Interface module?"); + } Err(other) => { panic!("build_file failed with error:\n{:?}", other); } diff --git a/compiler/load/src/file.rs b/compiler/load/src/file.rs index 9cfe7c1f80..721fa41509 100644 --- a/compiler/load/src/file.rs +++ b/compiler/load/src/file.rs @@ -980,6 +980,8 @@ pub enum LoadingProblem<'a> { }, ParsingFailed(ParseProblem<'a, SyntaxError<'a>>), UnexpectedHeader(String), + /// there is no platform (likely running an Interface module) + NoPlatform, MsgChannelDied, ErrJoiningWorkerThreads, @@ -1479,7 +1481,7 @@ where state, subs, exposed_to_host, - ))); + )?)); } Msg::FailedToParse(problem) => { // Shut down all the worker threads. @@ -2044,7 +2046,7 @@ fn finish_specialization( state: State, subs: Subs, exposed_to_host: MutMap, -) -> MonomorphizedModule { +) -> Result { let module_ids = Arc::try_unwrap(state.arc_modules) .unwrap_or_else(|_| panic!("There were still outstanding Arc references to module_ids")) .into_inner() @@ -2085,7 +2087,7 @@ fn finish_specialization( } } Some(To::NewPackage(p_or_p)) => p_or_p, - None => panic!("no platform!"), + None => return Err(LoadingProblem::NoPlatform), }; match package_or_path { @@ -2097,7 +2099,7 @@ fn finish_specialization( let platform_path = path_to_platform.into(); - MonomorphizedModule { + Ok(MonomorphizedModule { can_problems, mono_problems, type_problems, @@ -2110,7 +2112,7 @@ fn finish_specialization( procedures, sources, timings: state.timings, - } + }) } fn finish( From 7f14f27ee40643af724bbe0d021e992f6f89171a Mon Sep 17 00:00:00 2001 From: Folkert Date: Sun, 21 Feb 2021 00:14:15 +0100 Subject: [PATCH 04/17] better formatting --- Cargo.lock | 2 +- cli/src/lib.rs | 4 +- compiler/load/Cargo.toml | 2 +- compiler/load/src/file.rs | 121 ++++++++++++++++++++++++++++++++++---- 4 files changed, 115 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8ac7115718..7e99da5cc3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2928,7 +2928,6 @@ version = "0.1.0" dependencies = [ "bumpalo", "crossbeam", - "indoc", "inlinable_string", "maplit", "num_cpus", @@ -2950,6 +2949,7 @@ dependencies = [ "roc_types", "roc_unify", "tempfile", + "ven_pretty", ] [[package]] diff --git a/cli/src/lib.rs b/cli/src/lib.rs index 2cedcf4ea8..dfec81975e 100644 --- a/cli/src/lib.rs +++ b/cli/src/lib.rs @@ -135,8 +135,8 @@ pub fn build(target: &Triple, matches: &ArgMatches, run_after_build: bool) -> io Err(LoadingProblem::ParsingFailedReport(report)) => { print!("{}", report); } - Err(LoadingProblem::NoPlatform) => { - print!("The file you ran did not provide a platform\nMaybe you are trying to run an Interface module?"); + Err(LoadingProblem::NoPlatform(report)) => { + print!("{}", report); } Err(other) => { panic!("build_file failed with error:\n{:?}", other); diff --git a/compiler/load/Cargo.toml b/compiler/load/Cargo.toml index 09caec0dec..701b7dd6da 100644 --- a/compiler/load/Cargo.toml +++ b/compiler/load/Cargo.toml @@ -19,6 +19,7 @@ roc_parse = { path = "../parse" } roc_solve = { path = "../solve" } roc_mono = { path = "../mono" } roc_reporting = { path = "../reporting" } +ven_pretty = { path = "../../vendor/pretty" } bumpalo = { version = "3.2", features = ["collections"] } inlinable_string = "0.1" parking_lot = { version = "0.11", features = ["deadlock_detection"] } @@ -29,6 +30,5 @@ num_cpus = "1" tempfile = "3.1.0" pretty_assertions = "0.5.1" maplit = "1.0.1" -indoc = "0.3.3" quickcheck = "0.8" quickcheck_macros = "0.8" diff --git a/compiler/load/src/file.rs b/compiler/load/src/file.rs index 721fa41509..7b0ccdc924 100644 --- a/compiler/load/src/file.rs +++ b/compiler/load/src/file.rs @@ -616,6 +616,7 @@ struct ModuleHeader<'a> { module_id: ModuleId, module_name: ModuleNameEnum<'a>, module_path: PathBuf, + is_root_module: bool, exposed_ident_ids: IdentIds, deps_by_name: MutMap, ModuleId>, packages: MutMap<&'a str, PackageOrPath<'a>>, @@ -767,6 +768,14 @@ enum Msg<'a> { FailedToParse(ParseProblem<'a, SyntaxError<'a>>), } +#[derive(Debug)] +enum PlatformPath<'a> { + NotSpecified, + Valid(To<'a>), + RootIsInterface, + RootIsPkgConfig, +} + #[derive(Debug)] struct State<'a> { pub root_id: ModuleId, @@ -775,7 +784,7 @@ struct State<'a> { pub stdlib: &'a StdLib, pub exposed_types: SubsByModule, pub output_path: Option<&'a str>, - pub platform_path: Option>, + pub platform_path: PlatformPath<'a>, pub headers_parsed: MutSet, @@ -981,7 +990,7 @@ pub enum LoadingProblem<'a> { ParsingFailed(ParseProblem<'a, SyntaxError<'a>>), UnexpectedHeader(String), /// there is no platform (likely running an Interface module) - NoPlatform, + NoPlatform(String), MsgChannelDied, ErrJoiningWorkerThreads, @@ -1135,6 +1144,7 @@ impl<'a> LoadStart<'a> { load_filename( arena, filename, + true, None, Arc::clone(&arc_modules), Arc::clone(&ident_ids_by_module), @@ -1403,7 +1413,7 @@ where goal_phase, stdlib, output_path: None, - platform_path: None, + platform_path: PlatformPath::NotSpecified, module_cache: ModuleCache::default(), dependencies: Dependencies::default(), procedures: MutMap::default(), @@ -1612,16 +1622,27 @@ fn update<'a>( match header_extra { App { to_platform } => { - debug_assert_eq!(state.platform_path, None); + debug_assert!(matches!(state.platform_path, PlatformPath::NotSpecified)); + debug_assert!(header.is_root_module); - state.platform_path = Some(to_platform.clone()); + state.platform_path = PlatformPath::Valid(to_platform.clone()); } PkgConfig { .. } => { debug_assert_eq!(state.platform_id, None); state.platform_id = Some(header.module_id); + + if header.is_root_module { + debug_assert!(matches!(state.platform_path, PlatformPath::NotSpecified)); + state.platform_path = PlatformPath::RootIsPkgConfig; + } + } + Interface => { + if header.is_root_module { + debug_assert!(matches!(state.platform_path, PlatformPath::NotSpecified)); + state.platform_path = PlatformPath::RootIsInterface; + } } - Interface => {} } // store an ID to name mapping, so we know the file to read when fetching dependencies' headers @@ -2073,21 +2094,89 @@ fn finish_specialization( .. } = module_cache; - let sources = sources + let sources: MutMap)> = sources .into_iter() .map(|(id, (path, src))| (id, (path, src.into()))) .collect(); let path_to_platform = { + use PlatformPath::*; let package_or_path = match platform_path { - Some(To::ExistingPackage(shorthand)) => { + Valid(To::ExistingPackage(shorthand)) => { match (*state.arc_shorthands).lock().get(shorthand) { Some(p_or_p) => p_or_p.clone(), None => unreachable!(), } } - Some(To::NewPackage(p_or_p)) => p_or_p, - None => return Err(LoadingProblem::NoPlatform), + Valid(To::NewPackage(p_or_p)) => p_or_p, + other => { + use roc_reporting::report::{Report, RocDocAllocator, DEFAULT_PALETTE}; + use ven_pretty::DocAllocator; + + let module_id = state.root_id; + + let palette = DEFAULT_PALETTE; + + // Report parsing and canonicalization problems + let alloc = RocDocAllocator::new(&[], module_id, &interns); + + let report = { + match other { + Valid(_) => unreachable!(), + NotSpecified => { + let doc = alloc.stack(vec![ + alloc.reflow("I could not find a platform based on your input file."), + alloc.reflow(r"Does the module header contain an entry that looks like this:"), + alloc + .parser_suggestion(" packages { base: \"platform\" }") + .indent(4), + alloc.reflow("See also TODO."), + ]); + + Report { + filename: "UNKNOWN.roc".into(), + doc, + title: "NO PLATFORM".to_string(), + } + } + RootIsInterface => { + let doc = alloc.stack(vec![ + alloc.reflow(r"The input file is a interface file, but only app modules can be ran."), + alloc.concat(vec![ + alloc.reflow(r"I will still parse and typecheck the input file and its dependencies,"), + alloc.reflow(r"but won't output any executable."), + ]) + ]); + + Report { + filename: "UNKNOWN.roc".into(), + doc, + title: "NO PLATFORM".to_string(), + } + } + RootIsPkgConfig => { + let doc = alloc.stack(vec![ + alloc.reflow(r"The input file is a package config file, but only app modules can be ran."), + alloc.concat(vec![ + alloc.reflow(r"I will still parse and typecheck the input file and its dependencies,"), + alloc.reflow(r"but won't output any executable."), + ]) + ]); + + Report { + filename: "UNKNOWN.roc".into(), + doc, + title: "NO PLATFORM".to_string(), + } + } + } + }; + let mut buf = String::new(); + + report.render_color_terminal(&mut buf, &alloc, &palette); + + return Err(LoadingProblem::NoPlatform(buf)); + } }; match package_or_path { @@ -2295,6 +2384,7 @@ fn load_module<'a>( load_filename( arena, filename, + false, opt_shorthand, module_ids, ident_ids_by_module, @@ -2334,6 +2424,7 @@ fn parse_header<'a>( arena: &'a Bump, read_file_duration: Duration, filename: PathBuf, + is_root_module: bool, opt_shorthand: Option<&'a str>, module_ids: Arc>>, ident_ids_by_module: Arc>>, @@ -2359,6 +2450,7 @@ fn parse_header<'a>( value: ModuleNameEnum::Interface(header.name.value), }, filename, + is_root_module, opt_shorthand, &[], header.exposes.into_bump_slice(), @@ -2381,6 +2473,7 @@ fn parse_header<'a>( value: ModuleNameEnum::App(header.name.value), }, filename, + is_root_module, opt_shorthand, packages, header.provides.into_bump_slice(), @@ -2486,6 +2579,7 @@ fn parse_header<'a>( fn load_filename<'a>( arena: &'a Bump, filename: PathBuf, + is_root_module: bool, opt_shorthand: Option<&'a str>, module_ids: Arc>>, ident_ids_by_module: Arc>>, @@ -2501,6 +2595,7 @@ fn load_filename<'a>( arena, file_io_duration, filename, + is_root_module, opt_shorthand, module_ids, ident_ids_by_module, @@ -2535,6 +2630,7 @@ fn load_from_str<'a>( arena, file_io_duration, filename, + false, None, module_ids, ident_ids_by_module, @@ -2556,6 +2652,7 @@ enum ModuleNameEnum<'a> { fn send_header<'a>( loc_name: Located>, filename: PathBuf, + is_root_module: bool, opt_shorthand: Option<&'a str>, packages: &'a [Located>], exposes: &'a [Located>], @@ -2742,6 +2839,7 @@ fn send_header<'a>( ModuleHeader { module_id: home, module_path: filename, + is_root_module, exposed_ident_ids: ident_ids, module_name: loc_name.value, packages: package_entries, @@ -2763,6 +2861,7 @@ fn send_header<'a>( fn send_header_two<'a>( arena: &'a Bump, filename: PathBuf, + is_root_module: bool, shorthand: &'a str, app_module_id: ModuleId, packages: &'a [Located>], @@ -2959,6 +3058,7 @@ fn send_header_two<'a>( ModuleHeader { module_id: home, module_path: filename, + is_root_module, exposed_ident_ids: ident_ids, module_name, packages: package_entries, @@ -3102,6 +3202,7 @@ fn fabricate_pkg_config_module<'a>( send_header_two( arena, filename, + false, shorthand, app_module_id, &[], From 362a1fa018aeb6d4e9836c9e55ce5a03ba24bebf Mon Sep 17 00:00:00 2001 From: Folkert Date: Sun, 21 Feb 2021 00:16:38 +0100 Subject: [PATCH 05/17] clippy --- compiler/load/src/file.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler/load/src/file.rs b/compiler/load/src/file.rs index d2adabdf3b..d84aff1709 100644 --- a/compiler/load/src/file.rs +++ b/compiler/load/src/file.rs @@ -2576,6 +2576,7 @@ fn parse_header<'a>( } /// Load a module by its filename +#[allow(clippy::too_many_arguments)] fn load_filename<'a>( arena: &'a Bump, filename: PathBuf, From 1ef33c42cb31e94ae0f9457a40ed8d421bcce1ac Mon Sep 17 00:00:00 2001 From: Anton-4 <17049058+Anton-4@users.noreply.github.com> Date: Mon, 22 Feb 2021 17:41:35 +0100 Subject: [PATCH 06/17] Use strings for expr2 positioning --- editor/src/lib.rs | 10 +- editor/src/render.rs | 310 +++++++++++-------------------------------- 2 files changed, 83 insertions(+), 237 deletions(-) diff --git a/editor/src/lib.rs b/editor/src/lib.rs index d917ba125a..24140fb43c 100644 --- a/editor/src/lib.rs +++ b/editor/src/lib.rs @@ -170,6 +170,7 @@ fn run_event_loop(file_path_opt: Option<&Path>) -> Result<(), Box> { let arena = Bump::new(); let mut rects_arena = Bump::new(); + let mut ast_arena = Bump::new(); // Render loop window.request_redraw(); @@ -278,7 +279,7 @@ fn run_event_loop(file_path_opt: Option<&Path>) -> Result<(), Box> { ); } else { // queue_no_file_text(&size, NOTHING_OPENED, CODE_TXT_XY.into(), &mut glyph_brush); - + ast_arena.reset(); let mut pool = Pool::with_capacity(1024); let mut var_store = VarStore::default(); let dep_idents = IdentIds::exposed_builtins(8); @@ -289,7 +290,7 @@ fn run_event_loop(file_path_opt: Option<&Path>) -> Result<(), Box> { let mut env = crate::lang::expr::Env::new( home, - &arena, + &ast_arena, &mut pool, &mut var_store, dep_idents, @@ -302,8 +303,8 @@ fn run_event_loop(file_path_opt: Option<&Path>) -> Result<(), Box> { let region = Region::new(0, 0, 0, 0); let (expr2, _) = crate::lang::expr::str_to_expr2( - &arena, - "{ x: 2, y: 5 }", + &ast_arena, + "{ a: 1, b: 2, c: {x: 3, y: 4} }", &mut env, &mut scope, region, @@ -311,6 +312,7 @@ fn run_event_loop(file_path_opt: Option<&Path>) -> Result<(), Box> { .unwrap(); render::render_expr2( + &ast_arena, &mut env, &expr2, &size, diff --git a/editor/src/render.rs b/editor/src/render.rs index ff39fc6a3e..a089eae377 100644 --- a/editor/src/render.rs +++ b/editor/src/render.rs @@ -1,3 +1,4 @@ +use bumpalo::Bump; use cgmath::Vector2; use wgpu_glyph::GlyphBrush; use winit::dpi::PhysicalSize; @@ -11,7 +12,72 @@ use crate::{ lang::{ast::Expr2, expr::Env}, }; +// TODO use arena allocation +pub fn expr2_to_str<'a>(arena: &'a Bump, env: &Env<'a>, expr2: &Expr2) -> String { + match expr2 { + Expr2::SmallInt { text, .. } => env.pool.get_str(text).to_owned(), + Expr2::I128 { text, .. } => env.pool.get_str(text).to_owned(), + Expr2::U128 { text, .. } => env.pool.get_str(text).to_owned(), + Expr2::Float { text, .. } => env.pool.get_str(text).to_owned(), + Expr2::Str(text) => env.pool.get_str(text).to_owned(), + Expr2::GlobalTag { name, .. } => env.pool.get_str(name).to_owned(), + Expr2::Call { expr: expr_id, .. } => { + let expr = env.pool.get(*expr_id); + + expr2_to_str(arena, env, expr) + } + Expr2::Var(symbol) => { + let text = format!("{:?}", symbol); + + text + } + Expr2::List { elems, .. } => { + let mut list_str = "[".to_owned(); + + for (idx, node_id) in elems.iter_node_ids().enumerate() { + let sub_expr2 = env.pool.get(node_id); + + list_str.push_str(&expr2_to_str(arena, env, sub_expr2)); + + if idx + 1 < elems.len() { + list_str.push_str(", ") + } + } + + list_str.push(']'); + + list_str + } + Expr2::Record { fields, .. } => { + let mut record_str = "{".to_owned(); + + for (idx, node_id) in fields.iter_node_ids().enumerate() { + let (pool_field_name, _, sub_expr2_node_id) = env.pool.get(node_id); + + let field_name = env.pool.get_str(pool_field_name); + let sub_expr2 = env.pool.get(*sub_expr2_node_id); + + record_str.push_str(&format!( + "{}:{}", + field_name, + &expr2_to_str(arena, env, sub_expr2) + )); + + if idx + 1 < fields.len() { + record_str.push_str(", ") + } + } + + record_str.push('}'); + + record_str + } + rest => todo!("implement expr2_to_str for {:?}", rest), + } +} + pub fn render_expr2<'a>( + arena: &'a Bump, env: &mut Env<'a>, expr2: &Expr2, size: &PhysicalSize, @@ -20,240 +86,18 @@ pub fn render_expr2<'a>( ) { let area_bounds = (size.width as f32, size.height as f32).into(); - match expr2 { - Expr2::SmallInt { text, .. } => { - let code_text = Text { - position, - area_bounds, - color: CODE_COLOR.into(), - text: env.pool.get_str(text), - size: CODE_FONT_SIZE, - ..Default::default() - }; + let expr_str = expr2_to_str(arena, env, expr2); - queue_code_text_draw(&code_text, glyph_brush); - } - Expr2::I128 { text, .. } => { - let code_text = Text { - position, - area_bounds, - color: CODE_COLOR.into(), - text: env.pool.get_str(text), - size: CODE_FONT_SIZE, - ..Default::default() - }; + // TODO format expr_str - queue_code_text_draw(&code_text, glyph_brush); - } - Expr2::U128 { text, .. } => { - let code_text = Text { - position, - area_bounds, - color: CODE_COLOR.into(), - text: env.pool.get_str(text), - size: CODE_FONT_SIZE, - ..Default::default() - }; - - queue_code_text_draw(&code_text, glyph_brush); - } - Expr2::Float { text, .. } => { - let code_text = Text { - position, - area_bounds, - color: CODE_COLOR.into(), - text: env.pool.get_str(text), - size: CODE_FONT_SIZE, - ..Default::default() - }; - - queue_code_text_draw(&code_text, glyph_brush); - } - Expr2::Str(text) => { - let code_text = Text { - position, - area_bounds, - color: CODE_COLOR.into(), - text: env.pool.get_str(text), - size: CODE_FONT_SIZE, - ..Default::default() - }; - - queue_code_text_draw(&code_text, glyph_brush); - } - Expr2::GlobalTag { name, .. } => { - let code_text = Text { - position, - area_bounds, - color: CODE_COLOR.into(), - text: env.pool.get_str(name), - size: CODE_FONT_SIZE, - ..Default::default() - }; - - queue_code_text_draw(&code_text, glyph_brush); - } - Expr2::Call { expr: expr_id, .. } => { - let expr = env.pool.get(*expr_id); - - render_expr2(env, expr, size, position, glyph_brush); - } - Expr2::Var(symbol) => { - let text = format!("{:?}", symbol); - - let code_text = Text { - position, - area_bounds, - color: CODE_COLOR.into(), - text: text.as_str(), - size: CODE_FONT_SIZE, - ..Default::default() - }; - - queue_code_text_draw(&code_text, glyph_brush); - } - Expr2::List { elems, .. } => { - let code_text = Text { - position, - area_bounds, - color: CODE_COLOR.into(), - text: "[", - size: CODE_FONT_SIZE, - ..Default::default() - }; - - queue_code_text_draw(&code_text, glyph_brush); - - let mut x_pos = position.x; - - for (idx, node_id) in elems.iter_node_ids().enumerate() { - let sub_expr2 = env.pool.get(node_id); - - x_pos += 20.0; - - render_expr2( - env, - sub_expr2, - size, - Vector2::new(x_pos, position.y), - glyph_brush, - ); - - if idx + 1 < elems.len() { - x_pos += 10.0; - - let code_text = Text { - position: Vector2::new(x_pos, position.y), - area_bounds, - color: CODE_COLOR.into(), - text: ",", - size: CODE_FONT_SIZE, - ..Default::default() - }; - - queue_code_text_draw(&code_text, glyph_brush); - } - } - - x_pos += 20.0; - - let code_text = Text { - position: Vector2::new(x_pos, position.y), - area_bounds, - color: CODE_COLOR.into(), - text: "]", - size: CODE_FONT_SIZE, - ..Default::default() - }; - - queue_code_text_draw(&code_text, glyph_brush); - } - Expr2::Record { fields, .. } => { - let code_text = Text { - position, - area_bounds, - color: CODE_COLOR.into(), - text: "{", - size: CODE_FONT_SIZE, - ..Default::default() - }; - - queue_code_text_draw(&code_text, glyph_brush); - - let mut x_pos = position.x; - - for (idx, node_id) in fields.iter_node_ids().enumerate() { - let (pool_field_name, _, sub_expr2_node_id) = env.pool.get(node_id); - - let field_name = env.pool.get_str(pool_field_name); - - x_pos += 20.0; - - let code_text = Text { - position: Vector2::new(x_pos, position.y), - area_bounds, - color: CODE_COLOR.into(), - text: field_name, - size: CODE_FONT_SIZE, - ..Default::default() - }; - - queue_code_text_draw(&code_text, glyph_brush); - - x_pos += 10.0; - - let code_text = Text { - position: Vector2::new(x_pos, position.y), - area_bounds, - color: CODE_COLOR.into(), - text: ":", - size: CODE_FONT_SIZE, - ..Default::default() - }; - - queue_code_text_draw(&code_text, glyph_brush); - - let sub_expr2 = env.pool.get(*sub_expr2_node_id); - - x_pos += 20.0; - - render_expr2( - env, - sub_expr2, - size, - Vector2::new(x_pos, position.y), - glyph_brush, - ); - - if idx + 1 < fields.len() { - x_pos += 10.0; - - let code_text = Text { - position: Vector2::new(x_pos, position.y), - area_bounds, - color: CODE_COLOR.into(), - text: ",", - size: CODE_FONT_SIZE, - ..Default::default() - }; - - queue_code_text_draw(&code_text, glyph_brush); - } - } - - x_pos += 20.0; - - let code_text = Text { - position: Vector2::new(x_pos, position.y), - area_bounds, - color: CODE_COLOR.into(), - text: "}", - size: CODE_FONT_SIZE, - ..Default::default() - }; - - queue_code_text_draw(&code_text, glyph_brush); - } - rest => todo!("implement {:?} render", rest), + let code_text = Text { + position, + area_bounds, + color: CODE_COLOR.into(), + text: &expr_str, + size: CODE_FONT_SIZE, + ..Default::default() }; + + queue_code_text_draw(&code_text, glyph_brush); } From 361ba3ae6c7084ce49d4e20abf96e78b262e8220 Mon Sep 17 00:00:00 2001 From: Anton-4 <17049058+Anton-4@users.noreply.github.com> Date: Mon, 22 Feb 2021 19:55:04 +0100 Subject: [PATCH 07/17] expr2_to_str arena conversion progress --- editor/src/render.rs | 119 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 96 insertions(+), 23 deletions(-) diff --git a/editor/src/render.rs b/editor/src/render.rs index a089eae377..611ecc4284 100644 --- a/editor/src/render.rs +++ b/editor/src/render.rs @@ -2,6 +2,8 @@ use bumpalo::Bump; use cgmath::Vector2; use wgpu_glyph::GlyphBrush; use winit::dpi::PhysicalSize; +use bumpalo::collections::String as BumpString; +use crate::lang::pool::{PoolStr}; use crate::{ graphics::{ @@ -12,44 +14,117 @@ use crate::{ lang::{ast::Expr2, expr::Env}, }; -// TODO use arena allocation -pub fn expr2_to_str<'a>(arena: &'a Bump, env: &Env<'a>, expr2: &Expr2) -> String { +// fn to_bump_str<'a>(arena: &'a Bump, str_ref: &str) -> BumpString { + +// } + +fn pool_str_len<'a>(env: &Env<'a>, pool_str: &PoolStr) -> usize { + env.pool.get_str(pool_str).len() +} + +// calculate the str len necessary for BumpString +fn expr2_to_len<'a>(env: &Env<'a>, expr2: &Expr2) -> usize { match expr2 { - Expr2::SmallInt { text, .. } => env.pool.get_str(text).to_owned(), - Expr2::I128 { text, .. } => env.pool.get_str(text).to_owned(), - Expr2::U128 { text, .. } => env.pool.get_str(text).to_owned(), - Expr2::Float { text, .. } => env.pool.get_str(text).to_owned(), - Expr2::Str(text) => env.pool.get_str(text).to_owned(), - Expr2::GlobalTag { name, .. } => env.pool.get_str(name).to_owned(), + Expr2::SmallInt { text, .. } => pool_str_len(env, text), + Expr2::I128 { text, .. } => pool_str_len(env, text), + Expr2::U128 { text, .. } => pool_str_len(env, text), + Expr2::Float { text, .. } => pool_str_len(env, text), + Expr2::Str(text) => pool_str_len(env, text), + Expr2::GlobalTag { name, .. } => pool_str_len(env, name), + Expr2::Call { expr: expr_id, .. } => { + let expr = env.pool.get(*expr_id); + + expr2_to_len(env, expr) + } + Expr2::Var(symbol) => { + //TODO make bump_format to use arena + let text = format!("{:?}", symbol); + + text.len() + } + Expr2::List { elems, .. } => { + let mut len_ctr = 2; // for '[' and ']' + + for (idx, node_id) in elems.iter_node_ids().enumerate() { + let sub_expr2 = env.pool.get(node_id); + + len_ctr += expr2_to_len(env, sub_expr2); + + if idx + 1 < elems.len() { + len_ctr += 2; // for ", " + } + } + + len_ctr + } + Expr2::Record { fields, .. } => { + let mut len_ctr = 2; // for '{' and '}' + + for (idx, node_id) in fields.iter_node_ids().enumerate() { + let (pool_field_name, _, sub_expr2_node_id) = env.pool.get(node_id); + + len_ctr += pool_str_len(env, &pool_field_name); + + let sub_expr2 = env.pool.get(*sub_expr2_node_id); + let sub_expr2_len = expr2_to_len(env, sub_expr2); + len_ctr += sub_expr2_len; + + if idx + 1 < fields.len() { + len_ctr += 2; // for ", " + } + } + + len_ctr + } + rest => todo!("implement expr2_to_str for {:?}", rest), + } +} + +pub fn expr2_to_str<'a>(arena: &'a Bump, env: &'a Env<'a>, expr2: &Expr2) -> &'a str { + + match expr2 { + Expr2::SmallInt { text, .. } => env.pool.get_str(text), + Expr2::I128 { text, .. } => env.pool.get_str(text), + Expr2::U128 { text, .. } => env.pool.get_str(text), + Expr2::Float { text, .. } => env.pool.get_str(text), + Expr2::Str(text) => env.pool.get_str(text), + Expr2::GlobalTag { name, .. } => env.pool.get_str(name), Expr2::Call { expr: expr_id, .. } => { let expr = env.pool.get(*expr_id); expr2_to_str(arena, env, expr) } Expr2::Var(symbol) => { + //TODO make bump_format to use arena let text = format!("{:?}", symbol); - text + &text } Expr2::List { elems, .. } => { - let mut list_str = "[".to_owned(); + let mut bump_str = + BumpString::with_capacity_in(expr2_to_len(env, expr2), arena); + + bump_str.push('['); for (idx, node_id) in elems.iter_node_ids().enumerate() { let sub_expr2 = env.pool.get(node_id); - list_str.push_str(&expr2_to_str(arena, env, sub_expr2)); + bump_str.push_str(&expr2_to_str(arena, env, sub_expr2)); if idx + 1 < elems.len() { - list_str.push_str(", ") + bump_str.push_str(", ") } } - list_str.push(']'); + bump_str.push(']'); - list_str + &bump_str } Expr2::Record { fields, .. } => { - let mut record_str = "{".to_owned(); + let mut bump_str = + BumpString::with_capacity_in(expr2_to_len(env, expr2), arena); + + bump_str.push('{'); for (idx, node_id) in fields.iter_node_ids().enumerate() { let (pool_field_name, _, sub_expr2_node_id) = env.pool.get(node_id); @@ -57,20 +132,18 @@ pub fn expr2_to_str<'a>(arena: &'a Bump, env: &Env<'a>, expr2: &Expr2) -> String let field_name = env.pool.get_str(pool_field_name); let sub_expr2 = env.pool.get(*sub_expr2_node_id); - record_str.push_str(&format!( - "{}:{}", - field_name, - &expr2_to_str(arena, env, sub_expr2) - )); + bump_str.push_str(field_name); + bump_str.push(':'); + bump_str.push_str(expr2_to_str(arena, env, sub_expr2)); if idx + 1 < fields.len() { - record_str.push_str(", ") + bump_str.push_str(", ") } } - record_str.push('}'); + bump_str.push('}'); - record_str + &bump_str } rest => todo!("implement expr2_to_str for {:?}", rest), } From 004474f3fc3fbf66d1ad6e7dd82553d577ba06e9 Mon Sep 17 00:00:00 2001 From: Anton-4 <17049058+Anton-4@users.noreply.github.com> Date: Tue, 23 Feb 2021 10:07:00 +0100 Subject: [PATCH 08/17] finished conversion to use arena --- editor/src/render.rs | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/editor/src/render.rs b/editor/src/render.rs index 611ecc4284..e14dc0368d 100644 --- a/editor/src/render.rs +++ b/editor/src/render.rs @@ -80,25 +80,31 @@ fn expr2_to_len<'a>(env: &Env<'a>, expr2: &Expr2) -> usize { } } -pub fn expr2_to_str<'a>(arena: &'a Bump, env: &'a Env<'a>, expr2: &Expr2) -> &'a str { +fn get_bump_str<'a, 'b>(arena: &'a Bump, env: &Env<'b>, pool_str: &PoolStr) -> BumpString<'a> { + let env_str = env.pool.get_str(pool_str); + + BumpString::from_str_in(env_str, arena) +} + +pub fn expr2_to_str<'a, 'b>(arena: &'a Bump, env: &Env<'b>, expr2: &Expr2) -> BumpString<'a> { match expr2 { - Expr2::SmallInt { text, .. } => env.pool.get_str(text), - Expr2::I128 { text, .. } => env.pool.get_str(text), - Expr2::U128 { text, .. } => env.pool.get_str(text), - Expr2::Float { text, .. } => env.pool.get_str(text), - Expr2::Str(text) => env.pool.get_str(text), - Expr2::GlobalTag { name, .. } => env.pool.get_str(name), + Expr2::SmallInt { text, .. } => get_bump_str(arena, env, text), + Expr2::I128 { text, .. } => get_bump_str(arena, env, text), + Expr2::U128 { text, .. } => get_bump_str(arena, env, text), + Expr2::Float { text, .. } => get_bump_str(arena, env, text), + Expr2::Str(text) => get_bump_str(arena, env, text), + Expr2::GlobalTag { name, .. } => get_bump_str(arena, env, name), Expr2::Call { expr: expr_id, .. } => { let expr = env.pool.get(*expr_id); expr2_to_str(arena, env, expr) } Expr2::Var(symbol) => { - //TODO make bump_format to use arena + //TODO make bump_format with arena let text = format!("{:?}", symbol); - &text + BumpString::from_str_in(&text, arena) } Expr2::List { elems, .. } => { let mut bump_str = @@ -118,7 +124,7 @@ pub fn expr2_to_str<'a>(arena: &'a Bump, env: &'a Env<'a>, expr2: &Expr2) -> &'a bump_str.push(']'); - &bump_str + bump_str } Expr2::Record { fields, .. } => { let mut bump_str = @@ -134,7 +140,7 @@ pub fn expr2_to_str<'a>(arena: &'a Bump, env: &'a Env<'a>, expr2: &Expr2) -> &'a bump_str.push_str(field_name); bump_str.push(':'); - bump_str.push_str(expr2_to_str(arena, env, sub_expr2)); + bump_str.push_str(&expr2_to_str(arena, env, sub_expr2)); if idx + 1 < fields.len() { bump_str.push_str(", ") @@ -143,7 +149,7 @@ pub fn expr2_to_str<'a>(arena: &'a Bump, env: &'a Env<'a>, expr2: &Expr2) -> &'a bump_str.push('}'); - &bump_str + bump_str } rest => todo!("implement expr2_to_str for {:?}", rest), } From b505a479ef36b124ece44191f5340c5f78fd627e Mon Sep 17 00:00:00 2001 From: Anton-4 <17049058+Anton-4@users.noreply.github.com> Date: Tue, 23 Feb 2021 10:21:15 +0100 Subject: [PATCH 09/17] fmt --- editor/src/render.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/editor/src/render.rs b/editor/src/render.rs index e14dc0368d..b97f976673 100644 --- a/editor/src/render.rs +++ b/editor/src/render.rs @@ -1,9 +1,9 @@ +use crate::lang::pool::PoolStr; +use bumpalo::collections::String as BumpString; use bumpalo::Bump; use cgmath::Vector2; use wgpu_glyph::GlyphBrush; use winit::dpi::PhysicalSize; -use bumpalo::collections::String as BumpString; -use crate::lang::pool::{PoolStr}; use crate::{ graphics::{ @@ -87,7 +87,6 @@ fn get_bump_str<'a, 'b>(arena: &'a Bump, env: &Env<'b>, pool_str: &PoolStr) -> B } pub fn expr2_to_str<'a, 'b>(arena: &'a Bump, env: &Env<'b>, expr2: &Expr2) -> BumpString<'a> { - match expr2 { Expr2::SmallInt { text, .. } => get_bump_str(arena, env, text), Expr2::I128 { text, .. } => get_bump_str(arena, env, text), @@ -107,8 +106,7 @@ pub fn expr2_to_str<'a, 'b>(arena: &'a Bump, env: &Env<'b>, expr2: &Expr2) -> Bu BumpString::from_str_in(&text, arena) } Expr2::List { elems, .. } => { - let mut bump_str = - BumpString::with_capacity_in(expr2_to_len(env, expr2), arena); + let mut bump_str = BumpString::with_capacity_in(expr2_to_len(env, expr2), arena); bump_str.push('['); @@ -127,8 +125,7 @@ pub fn expr2_to_str<'a, 'b>(arena: &'a Bump, env: &Env<'b>, expr2: &Expr2) -> Bu bump_str } Expr2::Record { fields, .. } => { - let mut bump_str = - BumpString::with_capacity_in(expr2_to_len(env, expr2), arena); + let mut bump_str = BumpString::with_capacity_in(expr2_to_len(env, expr2), arena); bump_str.push('{'); From 30ecd378a07e63ffa52a0a5efe47e54f446a7057 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 23 Feb 2021 14:31:48 +0100 Subject: [PATCH 10/17] refactor parse AST to allow multiple if branches --- compiler/can/src/expr.rs | 39 ++++++--- compiler/can/src/operator.rs | 21 +++-- compiler/fmt/src/expr.rs | 164 +++++++++++++++++++---------------- compiler/parse/src/ast.rs | 2 +- compiler/parse/src/expr.rs | 5 +- editor/src/lang/expr.rs | 23 +++-- 6 files changed, 145 insertions(+), 109 deletions(-) diff --git a/compiler/can/src/expr.rs b/compiler/can/src/expr.rs index a2248b7391..d1718703e6 100644 --- a/compiler/can/src/expr.rs +++ b/compiler/can/src/expr.rs @@ -674,32 +674,43 @@ pub fn canonicalize_expr<'a>( Output::default(), ) } - ast::Expr::If(cond, then_branch, else_branch) => { - let (loc_cond, mut output) = - canonicalize_expr(env, var_store, scope, cond.region, &cond.value); - let (loc_then, then_output) = canonicalize_expr( - env, - var_store, - scope, - then_branch.region, - &then_branch.value, - ); + ast::Expr::If(if_thens, final_else_branch) => { + let mut branches = Vec::with_capacity(1); + let mut output = Output::default(); + + for (condition, then_branch) in if_thens.iter() { + let (loc_cond, cond_output) = + canonicalize_expr(env, var_store, scope, condition.region, &condition.value); + + let (loc_then, then_output) = canonicalize_expr( + env, + var_store, + scope, + then_branch.region, + &then_branch.value, + ); + + branches.push((loc_cond, loc_then)); + + output.references = output.references.union(cond_output.references); + output.references = output.references.union(then_output.references); + } + let (loc_else, else_output) = canonicalize_expr( env, var_store, scope, - else_branch.region, - &else_branch.value, + final_else_branch.region, + &final_else_branch.value, ); - output.references = output.references.union(then_output.references); output.references = output.references.union(else_output.references); ( If { cond_var: var_store.fresh(), branch_var: var_store.fresh(), - branches: vec![(loc_cond, loc_then)], + branches, final_else: Box::new(loc_else), }, output, diff --git a/compiler/can/src/operator.rs b/compiler/can/src/operator.rs index 5f9d73bb70..048c2b9d83 100644 --- a/compiler/can/src/operator.rs +++ b/compiler/can/src/operator.rs @@ -290,16 +290,21 @@ pub fn desugar_expr<'a>(arena: &'a Bump, loc_expr: &'a Located>) -> &'a }), ) } - If(condition, then_branch, else_branch) - | Nested(If(condition, then_branch, else_branch)) => { - // If does not get desugared yet so we can give more targetted error messages during - // type checking. - let desugared_cond = &*arena.alloc(desugar_expr(arena, &condition)); - let desugared_then = &*arena.alloc(desugar_expr(arena, &then_branch)); - let desugared_else = &*arena.alloc(desugar_expr(arena, &else_branch)); + If(if_thens, final_else_branch) | Nested(If(if_thens, final_else_branch)) => { + // If does not get desugared into `when` so we can give more targetted error messages during type checking. + let desugared_final_else = &*arena.alloc(desugar_expr(arena, &final_else_branch)); + + let mut desugared_if_thens = Vec::with_capacity_in(if_thens.len(), arena); + + for (condition, then_branch) in if_thens.iter() { + desugared_if_thens.push(( + desugar_expr(arena, condition).clone(), + desugar_expr(arena, then_branch).clone(), + )); + } arena.alloc(Located { - value: If(desugared_cond, desugared_then, desugared_else), + value: If(desugared_if_thens.into_bump_slice(), desugared_final_else), region: loc_expr.region, }) } diff --git a/compiler/fmt/src/expr.rs b/compiler/fmt/src/expr.rs index 1be56ee514..80056cce7f 100644 --- a/compiler/fmt/src/expr.rs +++ b/compiler/fmt/src/expr.rs @@ -58,8 +58,11 @@ impl<'a> Formattable<'a> for Expr<'a> { loc_expr.is_multiline() || args.iter().any(|loc_arg| loc_arg.is_multiline()) } - If(loc_cond, loc_if_true, loc_if_false) => { - loc_cond.is_multiline() || loc_if_true.is_multiline() || loc_if_false.is_multiline() + If(branches, final_else) => { + final_else.is_multiline() + || branches + .iter() + .any(|(c, t)| c.is_multiline() || t.is_multiline()) } BinOp((loc_left, _, loc_right)) => { @@ -257,8 +260,8 @@ impl<'a> Formattable<'a> for Expr<'a> { // still print the return value. ret.format_with_options(buf, Parens::NotNeeded, Newlines::Yes, indent); } - If(loc_condition, loc_then, loc_else) => { - fmt_if(buf, loc_condition, loc_then, loc_else, indent); + If(branches, final_else) => { + fmt_if(buf, branches, final_else, self.is_multiline(), indent); } When(loc_condition, branches) => fmt_when(buf, loc_condition, branches, indent), List { @@ -629,15 +632,15 @@ fn fmt_when<'a>( fn fmt_if<'a>( buf: &mut String<'a>, - loc_condition: &'a Located>, - loc_then: &'a Located>, - loc_else: &'a Located>, + branches: &'a [(Located>, Located>)], + final_else: &'a Located>, + is_multiline: bool, indent: u16, ) { - let is_multiline_then = loc_then.is_multiline(); - let is_multiline_else = loc_else.is_multiline(); - let is_multiline_condition = loc_condition.is_multiline(); - let is_multiline = is_multiline_then || is_multiline_else || is_multiline_condition; + // let is_multiline_then = loc_then.is_multiline(); + // let is_multiline_else = final_else.is_multiline(); + // let is_multiline_condition = loc_condition.is_multiline(); + // let is_multiline = is_multiline_then || is_multiline_else || is_multiline_condition; let return_indent = if is_multiline { indent + INDENT @@ -645,80 +648,89 @@ fn fmt_if<'a>( indent }; - buf.push_str("if"); + for (loc_condition, loc_then) in branches.iter() { + let is_multiline_condition = loc_condition.is_multiline(); - if is_multiline_condition { - match &loc_condition.value { - Expr::SpaceBefore(expr_below, spaces_above_expr) => { - fmt_comments_only(buf, spaces_above_expr.iter(), NewlineAt::Top, return_indent); - newline(buf, return_indent); + buf.push_str("if"); - match &expr_below { - Expr::SpaceAfter(expr_above, spaces_below_expr) => { - expr_above.format(buf, return_indent); - fmt_comments_only( - buf, - spaces_below_expr.iter(), - NewlineAt::Top, - return_indent, - ); - newline(buf, indent); - } + if is_multiline_condition { + match &loc_condition.value { + Expr::SpaceBefore(expr_below, spaces_above_expr) => { + fmt_comments_only(buf, spaces_above_expr.iter(), NewlineAt::Top, return_indent); + newline(buf, return_indent); - _ => { - expr_below.format(buf, return_indent); + match &expr_below { + Expr::SpaceAfter(expr_above, spaces_below_expr) => { + expr_above.format(buf, return_indent); + fmt_comments_only( + buf, + spaces_below_expr.iter(), + NewlineAt::Top, + return_indent, + ); + newline(buf, indent); + } + + _ => { + expr_below.format(buf, return_indent); + } } } - } - Expr::SpaceAfter(expr_above, spaces_below_expr) => { - newline(buf, return_indent); - expr_above.format(buf, return_indent); - fmt_comments_only(buf, spaces_below_expr.iter(), NewlineAt::Top, return_indent); - newline(buf, indent); - } + Expr::SpaceAfter(expr_above, spaces_below_expr) => { + newline(buf, return_indent); + expr_above.format(buf, return_indent); + fmt_comments_only(buf, spaces_below_expr.iter(), NewlineAt::Top, return_indent); + newline(buf, indent); + } - _ => { - newline(buf, return_indent); - loc_condition.format(buf, return_indent); - newline(buf, indent); - } - } - } else { - buf.push(' '); - loc_condition.format_with_options(buf, Parens::NotNeeded, Newlines::Yes, indent); - buf.push(' '); - } - - buf.push_str("then"); - - if is_multiline { - match &loc_then.value { - Expr::SpaceBefore(expr_below, spaces_below) => { - // we want exactly one newline, user-inserted extra newlines are ignored. - newline(buf, return_indent); - fmt_comments_only(buf, spaces_below.iter(), NewlineAt::Bottom, return_indent); - - match &expr_below { - Expr::SpaceAfter(expr_above, spaces_above) => { - expr_above.format(buf, return_indent); - - fmt_comments_only(buf, spaces_above.iter(), NewlineAt::Top, return_indent); - newline(buf, indent); - } - - _ => { - expr_below.format(buf, return_indent); - } + _ => { + newline(buf, return_indent); + loc_condition.format(buf, return_indent); + newline(buf, indent); } } - _ => { - loc_condition.format(buf, return_indent); - } + } else { + buf.push(' '); + loc_condition.format_with_options(buf, Parens::NotNeeded, Newlines::Yes, indent); + buf.push(' '); + } + + buf.push_str("then"); + + if is_multiline { + match &loc_then.value { + Expr::SpaceBefore(expr_below, spaces_below) => { + // we want exactly one newline, user-inserted extra newlines are ignored. + newline(buf, return_indent); + fmt_comments_only(buf, spaces_below.iter(), NewlineAt::Bottom, return_indent); + + match &expr_below { + Expr::SpaceAfter(expr_above, spaces_above) => { + expr_above.format(buf, return_indent); + + fmt_comments_only( + buf, + spaces_above.iter(), + NewlineAt::Top, + return_indent, + ); + newline(buf, indent); + } + + _ => { + expr_below.format(buf, return_indent); + } + } + } + _ => { + loc_condition.format(buf, return_indent); + } + } + } else { + buf.push_str(" "); + loc_then.format(buf, return_indent); } - } else { - buf.push_str(" "); - loc_then.format(buf, return_indent); } if is_multiline { @@ -728,7 +740,7 @@ fn fmt_if<'a>( buf.push_str(" else "); } - loc_else.format(buf, return_indent); + final_else.format(buf, return_indent); } pub fn fmt_closure<'a>( diff --git a/compiler/parse/src/ast.rs b/compiler/parse/src/ast.rs index 70964e246e..f33ba8149b 100644 --- a/compiler/parse/src/ast.rs +++ b/compiler/parse/src/ast.rs @@ -127,7 +127,7 @@ pub enum Expr<'a> { UnaryOp(&'a Loc>, Loc), // Conditionals - If(&'a Loc>, &'a Loc>, &'a Loc>), + If(&'a [(Loc>, Loc>)], &'a Loc>), When( /// The condition &'a Loc>, diff --git a/compiler/parse/src/expr.rs b/compiler/parse/src/expr.rs index 8d187f10fb..5f05d89b17 100644 --- a/compiler/parse/src/expr.rs +++ b/compiler/parse/src/expr.rs @@ -324,7 +324,7 @@ pub fn expr_to_pattern<'a>( | Expr::Closure(_, _) | Expr::BinOp(_) | Expr::Defs(_, _) - | Expr::If(_, _, _) + | Expr::If(_, _) | Expr::When(_, _) | Expr::MalformedClosure | Expr::PrecedenceConflict(_, _, _, _) @@ -1264,8 +1264,7 @@ pub fn if_expr<'a>(min_indent: u16) -> impl Parser<'a, Expr<'a>, SyntaxError<'a> ), |arena: &'a Bump, (condition, (then_branch, else_branch))| { Expr::If( - &*arena.alloc(condition), - &*arena.alloc(then_branch), + arena.alloc([(condition, then_branch)]), &*arena.alloc(else_branch), ) } diff --git a/editor/src/lang/expr.rs b/editor/src/lang/expr.rs index 83791facb1..3cc1a4f912 100644 --- a/editor/src/lang/expr.rs +++ b/editor/src/lang/expr.rs @@ -508,22 +508,31 @@ pub fn to_expr2<'a>( Output::default(), ), - If(cond, then_branch, else_branch) => { - let (cond, mut output) = to_expr2(env, scope, &cond.value, cond.region); + If(branches, final_else) => { + let mut new_branches = Vec::with_capacity(branches.len()); + let mut output = Output::default(); - let (then_expr, then_output) = - to_expr2(env, scope, &then_branch.value, then_branch.region); + for (condition, then_branch) in branches.iter() { + let (cond, cond_output) = to_expr2(env, scope, &condition.value, condition.region); + + let (then_expr, then_output) = + to_expr2(env, scope, &then_branch.value, then_branch.region); + + output.references.union_mut(cond_output.references); + output.references.union_mut(then_output.references); + + new_branches.push((cond, then_expr)); + } let (else_expr, else_output) = - to_expr2(env, scope, &else_branch.value, else_branch.region); + to_expr2(env, scope, &final_else.value, final_else.region); - output.references.union_mut(then_output.references); output.references.union_mut(else_output.references); let expr = Expr2::If { cond_var: env.var_store.fresh(), expr_var: env.var_store.fresh(), - branches: PoolVec::new(vec![(cond, then_expr)].into_iter(), env.pool), + branches: PoolVec::new(new_branches.into_iter(), env.pool), final_else: env.pool.add(else_expr), }; From 5d8944fc6a4a9ca6910219bf66d6e46369fe334b Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 23 Feb 2021 15:05:25 +0100 Subject: [PATCH 11/17] use new parser for If --- compiler/parse/src/expr.rs | 90 ++++++++++++++++++++++-------------- compiler/parse/src/parser.rs | 20 ++++++++ 2 files changed, 76 insertions(+), 34 deletions(-) diff --git a/compiler/parse/src/expr.rs b/compiler/parse/src/expr.rs index 5f05d89b17..7fe80a1b02 100644 --- a/compiler/parse/src/expr.rs +++ b/compiler/parse/src/expr.rs @@ -11,7 +11,7 @@ use crate::number_literal::number_literal; use crate::parser::{ self, allocated, and_then_with_indent_level, ascii_char, ascii_string, attempt, backtrackable, fail, map, newline_char, not, not_followed_by, optional, sep_by1, specialize, specialize_ref, - then, unexpected, unexpected_eof, word1, word2, EExpr, Either, ParseResult, Parser, State, + then, unexpected, unexpected_eof, word1, word2, EExpr, Either, If, ParseResult, Parser, State, SyntaxError, When, }; use crate::pattern::loc_closure_param; @@ -1234,40 +1234,62 @@ mod when { } } -pub fn if_expr<'a>(min_indent: u16) -> impl Parser<'a, Expr<'a>, SyntaxError<'a>> { - map_with_arena!( - and!( - skip_first!( - parser::keyword(keyword::IF, min_indent), - space1_around( - loc!(move |arena, state| parse_expr(min_indent, arena, state)), - min_indent, - ) +pub fn if_expr_help<'a>(min_indent: u16) -> impl Parser<'a, Expr<'a>, If<'a>> { + move |arena: &'a Bump, state| { + let (_, _, state) = parser::keyword_e(keyword::IF, If::If).parse(arena, state)?; + + let mut branches = Vec::with_capacity_in(1, arena); + + let (_, cond, state) = space0_around_e( + specialize_ref( + If::Syntax, + loc!(move |arena, state| parse_expr(min_indent, arena, state)), ), - and!( - skip_first!( - parser::keyword(keyword::THEN, min_indent), - space1_around( - loc!(move |arena, state| parse_expr(min_indent, arena, state)), - min_indent, - ) - ), - skip_first!( - parser::keyword(keyword::ELSE, min_indent), - // NOTE changed this from space1_around to space1_before - space1_before( - loc!(move |arena, state| parse_expr(min_indent, arena, state)), - min_indent, - ) - ) - ) - ), - |arena: &'a Bump, (condition, (then_branch, else_branch))| { - Expr::If( - arena.alloc([(condition, then_branch)]), - &*arena.alloc(else_branch), - ) - } + min_indent, + If::Space, + If::IndentCondition, + ) + .parse(arena, state)?; + + let (_, _, state) = parser::keyword_e(keyword::THEN, If::Then).parse(arena, state)?; + + let (_, then_branch, state) = space0_around_e( + specialize_ref( + If::Syntax, + loc!(move |arena, state| parse_expr(min_indent, arena, state)), + ), + min_indent, + If::Space, + If::IndentThen, + ) + .parse(arena, state)?; + + let (_, _, state) = parser::keyword_e(keyword::ELSE, If::Else).parse(arena, state)?; + + branches.push((cond, then_branch)); + + let (_, else_branch, state) = space0_before_e( + specialize_ref( + If::Syntax, + loc!(move |arena, state| parse_expr(min_indent, arena, state)), + ), + min_indent, + If::Space, + If::IndentElse, + ) + .parse(arena, state)?; + + // parse the final else + let expr = Expr::If(branches.into_bump_slice(), arena.alloc(else_branch)); + + Ok((MadeProgress, expr, state)) + } +} + +pub fn if_expr<'a>(min_indent: u16) -> impl Parser<'a, Expr<'a>, SyntaxError<'a>> { + specialize( + |e, r, c| SyntaxError::Expr(EExpr::If(e, r, c)), + if_expr_help(min_indent), ) } diff --git a/compiler/parse/src/parser.rs b/compiler/parse/src/parser.rs index 71d2259bc1..a9da276780 100644 --- a/compiler/parse/src/parser.rs +++ b/compiler/parse/src/parser.rs @@ -378,6 +378,7 @@ pub enum EExpr<'a> { Space(BadInputError, Row, Col), When(When<'a>, Row, Col), + If(If<'a>, Row, Col), // EInParens(PInParens<'a>, Row, Col), IndentStart(Row, Col), @@ -408,6 +409,25 @@ pub enum When<'a> { PatternAlignment(u16, Row, Col), } +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum If<'a> { + Space(BadInputError, Row, Col), + If(Row, Col), + Then(Row, Col), + Else(Row, Col), + // TODO make EEXpr + Condition(&'a EExpr<'a>, Row, Col), + ThenBranch(&'a EExpr<'a>, Row, Col), + ElseBranch(&'a EExpr<'a>, Row, Col), + Syntax(&'a SyntaxError<'a>, Row, Col), + + IndentCondition(Row, Col), + IndentThen(Row, Col), + IndentElse(Row, Col), + + PatternAlignment(u16, Row, Col), +} + #[derive(Debug, Clone, PartialEq, Eq)] pub enum EPattern<'a> { Record(PRecord<'a>, Row, Col), From 3907680536dd608505395a3b82a3d842932cf3ad Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 23 Feb 2021 15:21:19 +0100 Subject: [PATCH 12/17] parse multiple if-then-else pairs into one AST node --- compiler/parse/src/expr.rs | 82 ++++++++++++++-------- compiler/parse/src/parser.rs | 1 + compiler/reporting/tests/test_reporting.rs | 59 ++++++++-------- 3 files changed, 85 insertions(+), 57 deletions(-) diff --git a/compiler/parse/src/expr.rs b/compiler/parse/src/expr.rs index 7fe80a1b02..84964e164b 100644 --- a/compiler/parse/src/expr.rs +++ b/compiler/parse/src/expr.rs @@ -3,7 +3,7 @@ use crate::ast::{ }; use crate::blankspace::{ line_comment, space0, space0_after, space0_after_e, space0_around, space0_around_e, - space0_before, space0_before_e, space0_e, space1, space1_around, space1_before, spaces_exactly, + space0_before, space0_before_e, space0_e, space1, space1_before, spaces_exactly, }; use crate::ident::{global_tag_or_ident, ident, lowercase_ident, Ident}; use crate::keyword; @@ -1240,33 +1240,59 @@ pub fn if_expr_help<'a>(min_indent: u16) -> impl Parser<'a, Expr<'a>, If<'a>> { let mut branches = Vec::with_capacity_in(1, arena); - let (_, cond, state) = space0_around_e( - specialize_ref( - If::Syntax, - loc!(move |arena, state| parse_expr(min_indent, arena, state)), - ), - min_indent, - If::Space, - If::IndentCondition, - ) - .parse(arena, state)?; + let mut loop_state = state; - let (_, _, state) = parser::keyword_e(keyword::THEN, If::Then).parse(arena, state)?; + let state_final_else = loop { + let state = loop_state; + let (_, cond, state) = space0_around_e( + specialize_ref( + If::Syntax, + loc!(move |arena, state| parse_expr(min_indent, arena, state)), + ), + min_indent, + If::Space, + If::IndentCondition, + ) + .parse(arena, state) + .map_err(|(_, f, s)| (MadeProgress, f, s))?; - let (_, then_branch, state) = space0_around_e( - specialize_ref( - If::Syntax, - loc!(move |arena, state| parse_expr(min_indent, arena, state)), - ), - min_indent, - If::Space, - If::IndentThen, - ) - .parse(arena, state)?; + let (_, _, state) = parser::keyword_e(keyword::THEN, If::Then) + .parse(arena, state) + .map_err(|(_, f, s)| (MadeProgress, f, s))?; - let (_, _, state) = parser::keyword_e(keyword::ELSE, If::Else).parse(arena, state)?; + let (_, then_branch, state) = space0_around_e( + specialize_ref( + If::Syntax, + loc!(move |arena, state| parse_expr(min_indent, arena, state)), + ), + min_indent, + If::Space, + If::IndentThen, + ) + .parse(arena, state) + .map_err(|(_, f, s)| (MadeProgress, f, s))?; - branches.push((cond, then_branch)); + let (_, _, state) = parser::keyword_e(keyword::ELSE, If::Else) + .parse(arena, state) + .map_err(|(_, f, s)| (MadeProgress, f, s))?; + + branches.push((cond, then_branch)); + + // try to parse another `if` + // NOTE this drops spaces between the `else` and the `if` + let optional_if = and!( + backtrackable(space0_e(min_indent, If::Space, If::IndentIf)), + parser::keyword_e(keyword::IF, If::If) + ); + + match optional_if.parse(arena, state) { + Err((_, _, state)) => break state, + Ok((_, _, state)) => { + loop_state = state; + continue; + } + } + }; let (_, else_branch, state) = space0_before_e( specialize_ref( @@ -1277,9 +1303,9 @@ pub fn if_expr_help<'a>(min_indent: u16) -> impl Parser<'a, Expr<'a>, If<'a>> { If::Space, If::IndentElse, ) - .parse(arena, state)?; + .parse(arena, state_final_else) + .map_err(|(_, f, s)| (MadeProgress, f, s))?; - // parse the final else let expr = Expr::If(branches.into_bump_slice(), arena.alloc(else_branch)); Ok((MadeProgress, expr, state)) @@ -1287,10 +1313,10 @@ pub fn if_expr_help<'a>(min_indent: u16) -> impl Parser<'a, Expr<'a>, If<'a>> { } pub fn if_expr<'a>(min_indent: u16) -> impl Parser<'a, Expr<'a>, SyntaxError<'a>> { - specialize( + debug!(specialize( |e, r, c| SyntaxError::Expr(EExpr::If(e, r, c)), if_expr_help(min_indent), - ) + )) } /// This is a helper function for parsing function args. diff --git a/compiler/parse/src/parser.rs b/compiler/parse/src/parser.rs index a9da276780..9f6eba16bd 100644 --- a/compiler/parse/src/parser.rs +++ b/compiler/parse/src/parser.rs @@ -422,6 +422,7 @@ pub enum If<'a> { Syntax(&'a SyntaxError<'a>, Row, Col), IndentCondition(Row, Col), + IndentIf(Row, Col), IndentThen(Row, Col), IndentElse(Row, Col), diff --git a/compiler/reporting/tests/test_reporting.rs b/compiler/reporting/tests/test_reporting.rs index 435f092b1d..66be20f5ca 100644 --- a/compiler/reporting/tests/test_reporting.rs +++ b/compiler/reporting/tests/test_reporting.rs @@ -801,35 +801,36 @@ mod test_reporting { ) } - // #[test] - // fn if_3_branch_mismatch() { - // report_problem_as( - // indoc!( - // r#" - // if True then 2 else if False then 2 else "foo" - // "# - // ), - // indoc!( - // r#" - // ── TYPE MISMATCH ─────────────────────────────────────────────────────────────── - - // The 2nd branch of this `if` does not match all the previous branches: - - // 1│ if True then 2 else "foo" - // ^^^^^ - - // The 2nd branch is a string of type - - // Str - - // But all the previous branches have the type - - // Num a - - // "# - // ), - // ) - // } + #[test] + fn if_3_branch_mismatch() { + report_problem_as( + indoc!( + r#" + if True then 2 else if False then 2 else "foo" + "# + ), + indoc!( + r#" + ── TYPE MISMATCH ─────────────────────────────────────────────────────────────── + + The 3rd branch of this `if` does not match all the previous branches: + + 1│ if True then 2 else if False then 2 else "foo" + ^^^^^ + + The 3rd branch is a string of type: + + Str + + But all the previous branches have type: + + Num a + + I need all branches in an `if` to have the same type! + "# + ), + ) + } #[test] fn when_branch_mismatch() { From 6eab8abe9e4b75e2fd5289ca64bbdc4546ede774 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 23 Feb 2021 18:34:08 +0100 Subject: [PATCH 13/17] improve message for outdented then --- compiler/parse/src/blankspace.rs | 12 +- compiler/parse/src/expr.rs | 88 +++++++------ compiler/parse/src/parser.rs | 11 +- compiler/parse/src/pattern.rs | 5 +- compiler/parse/src/type_annotation.rs | 10 +- compiler/reporting/src/error/parse.rs | 141 ++++++++++++++++++++- compiler/reporting/tests/test_reporting.rs | 60 ++++++++- 7 files changed, 270 insertions(+), 57 deletions(-) diff --git a/compiler/parse/src/blankspace.rs b/compiler/parse/src/blankspace.rs index 10b256b9b8..f2234ebed0 100644 --- a/compiler/parse/src/blankspace.rs +++ b/compiler/parse/src/blankspace.rs @@ -60,11 +60,12 @@ where ) } -pub fn space0_around_e<'a, P, S, E>( +pub fn space0_around_ee<'a, P, S, E>( parser: P, min_indent: u16, space_problem: fn(BadInputError, Row, Col) -> E, - indent_problem: fn(Row, Col) -> E, + indent_before_problem: fn(Row, Col) -> E, + indent_after_problem: fn(Row, Col) -> E, ) -> impl Parser<'a, Located, E> where S: Spaceable<'a>, @@ -75,8 +76,11 @@ where { parser::map_with_arena( and( - space0_e(min_indent, space_problem, indent_problem), - and(parser, space0_e(min_indent, space_problem, indent_problem)), + space0_e(min_indent, space_problem, indent_before_problem), + and( + parser, + space0_e(min_indent, space_problem, indent_after_problem), + ), ), move |arena: &'a Bump, tuples: ( diff --git a/compiler/parse/src/expr.rs b/compiler/parse/src/expr.rs index 84964e164b..1b01a08edb 100644 --- a/compiler/parse/src/expr.rs +++ b/compiler/parse/src/expr.rs @@ -2,7 +2,7 @@ use crate::ast::{ AssignedField, Attempting, CommentOrNewline, Def, Expr, Pattern, Spaceable, TypeAnnotation, }; use crate::blankspace::{ - line_comment, space0, space0_after, space0_after_e, space0_around, space0_around_e, + line_comment, space0, space0_after, space0_after_e, space0_around, space0_around_ee, space0_before, space0_before_e, space0_e, space1, space1_before, spaces_exactly, }; use crate::ident::{global_tag_or_ident, ident, lowercase_ident, Ident}; @@ -1029,14 +1029,15 @@ mod when { and!( when_with_indent(), skip_second!( - space0_around_e( + space0_around_ee( loc!(specialize_ref( When::Syntax, move |arena, state| parse_expr(min_indent, arena, state) )), min_indent, When::Space, - When::IndentCondition + When::IndentCondition, + When::IndentIs, ), parser::keyword_e(keyword::IS, When::Is) ) @@ -1182,13 +1183,14 @@ mod when { skip_first!( parser::keyword_e(keyword::IF, When::IfToken), // TODO we should require space before the expression but not after - space0_around_e( + space0_around_ee( loc!(specialize_ref(When::IfGuard, move |arena, state| { parse_expr(min_indent, arena, state) })), min_indent, When::Space, When::IndentIfGuard, + When::IndentArrow, ) ), Some @@ -1234,6 +1236,49 @@ mod when { } } +fn if_branch<'a>( + min_indent: u16, +) -> impl Parser<'a, (Located>, Located>), If<'a>> { + move |arena, state| { + // NOTE: only parse spaces before the expression + let (_, cond, state) = space0_around_ee( + specialize_ref( + If::Syntax, + loc!(move |arena, state| parse_expr(min_indent, arena, state)), + ), + min_indent, + If::Space, + If::IndentCondition, + If::IndentThenToken, + ) + .parse(arena, state) + .map_err(|(_, f, s)| (MadeProgress, f, s))?; + + let (_, _, state) = parser::keyword_e(keyword::THEN, If::Then) + .parse(arena, state) + .map_err(|(_, f, s)| (MadeProgress, f, s))?; + + let (_, then_branch, state) = space0_around_ee( + specialize_ref( + If::Syntax, + loc!(move |arena, state| parse_expr(min_indent, arena, state)), + ), + min_indent, + If::Space, + If::IndentThenBranch, + If::IndentElseToken, + ) + .parse(arena, state) + .map_err(|(_, f, s)| (MadeProgress, f, s))?; + + let (_, _, state) = parser::keyword_e(keyword::ELSE, If::Else) + .parse(arena, state) + .map_err(|(_, f, s)| (MadeProgress, f, s))?; + + Ok((MadeProgress, (cond, then_branch), state)) + } +} + pub fn if_expr_help<'a>(min_indent: u16) -> impl Parser<'a, Expr<'a>, If<'a>> { move |arena: &'a Bump, state| { let (_, _, state) = parser::keyword_e(keyword::IF, If::If).parse(arena, state)?; @@ -1243,38 +1288,7 @@ pub fn if_expr_help<'a>(min_indent: u16) -> impl Parser<'a, Expr<'a>, If<'a>> { let mut loop_state = state; let state_final_else = loop { - let state = loop_state; - let (_, cond, state) = space0_around_e( - specialize_ref( - If::Syntax, - loc!(move |arena, state| parse_expr(min_indent, arena, state)), - ), - min_indent, - If::Space, - If::IndentCondition, - ) - .parse(arena, state) - .map_err(|(_, f, s)| (MadeProgress, f, s))?; - - let (_, _, state) = parser::keyword_e(keyword::THEN, If::Then) - .parse(arena, state) - .map_err(|(_, f, s)| (MadeProgress, f, s))?; - - let (_, then_branch, state) = space0_around_e( - specialize_ref( - If::Syntax, - loc!(move |arena, state| parse_expr(min_indent, arena, state)), - ), - min_indent, - If::Space, - If::IndentThen, - ) - .parse(arena, state) - .map_err(|(_, f, s)| (MadeProgress, f, s))?; - - let (_, _, state) = parser::keyword_e(keyword::ELSE, If::Else) - .parse(arena, state) - .map_err(|(_, f, s)| (MadeProgress, f, s))?; + let (_, (cond, then_branch), state) = if_branch(min_indent).parse(arena, loop_state)?; branches.push((cond, then_branch)); @@ -1301,7 +1315,7 @@ pub fn if_expr_help<'a>(min_indent: u16) -> impl Parser<'a, Expr<'a>, If<'a>> { ), min_indent, If::Space, - If::IndentElse, + If::IndentElseBranch, ) .parse(arena, state_final_else) .map_err(|(_, f, s)| (MadeProgress, f, s))?; diff --git a/compiler/parse/src/parser.rs b/compiler/parse/src/parser.rs index 9f6eba16bd..b1d3d3545a 100644 --- a/compiler/parse/src/parser.rs +++ b/compiler/parse/src/parser.rs @@ -423,10 +423,10 @@ pub enum If<'a> { IndentCondition(Row, Col), IndentIf(Row, Col), - IndentThen(Row, Col), - IndentElse(Row, Col), - - PatternAlignment(u16, Row, Col), + IndentThenToken(Row, Col), + IndentElseToken(Row, Col), + IndentThenBranch(Row, Col), + IndentElseBranch(Row, Col), } #[derive(Debug, Clone, PartialEq, Eq)] @@ -1452,10 +1452,11 @@ macro_rules! collection_trailing_sep_e { and!( $crate::parser::trailing_sep_by0( $delimiter, - $crate::blankspace::space0_around_e( + $crate::blankspace::space0_around_ee( $elem, $min_indent, $space_problem, + $indent_problem, $indent_problem ) ), diff --git a/compiler/parse/src/pattern.rs b/compiler/parse/src/pattern.rs index b9b3f576e6..02daa8c5a9 100644 --- a/compiler/parse/src/pattern.rs +++ b/compiler/parse/src/pattern.rs @@ -1,5 +1,5 @@ use crate::ast::Pattern; -use crate::blankspace::{space0_around_e, space0_before_e, space0_e}; +use crate::blankspace::{space0_around_ee, space0_before_e, space0_e}; use crate::ident::{ident, lowercase_ident, Ident}; use crate::number_literal::number_literal; use crate::parser::Progress::{self, *}; @@ -133,11 +133,12 @@ fn loc_pattern_in_parens_help<'a>( ) -> impl Parser<'a, Located>, PInParens<'a>> { between!( word1(b'(', PInParens::Open), - space0_around_e( + space0_around_ee( move |arena, state| specialize_ref(PInParens::Syntax, loc_pattern(min_indent)) .parse(arena, state), min_indent, PInParens::Space, + PInParens::IndentOpen, PInParens::IndentEnd, ), word1(b')', PInParens::End) diff --git a/compiler/parse/src/type_annotation.rs b/compiler/parse/src/type_annotation.rs index 974ec7f94b..88181f0908 100644 --- a/compiler/parse/src/type_annotation.rs +++ b/compiler/parse/src/type_annotation.rs @@ -1,5 +1,5 @@ use crate::ast::{AssignedField, Tag, TypeAnnotation}; -use crate::blankspace::{space0_around_e, space0_before_e, space0_e}; +use crate::blankspace::{space0_around_ee, space0_before_e, space0_e}; use crate::ident::join_module_parts; use crate::keyword; use crate::parser::{ @@ -146,11 +146,12 @@ fn loc_type_in_parens<'a>( ) -> impl Parser<'a, Located>, TInParens<'a>> { between!( word1(b'(', TInParens::Open), - space0_around_e( + space0_around_ee( move |arena, state| specialize_ref(TInParens::Type, expression(min_indent)) .parse(arena, state), min_indent, TInParens::Space, + TInParens::IndentOpen, TInParens::IndentEnd, ), word1(b')', TInParens::End) @@ -436,11 +437,12 @@ fn expression<'a>(min_indent: u16) -> impl Parser<'a, Located let (p2, rest, state) = zero_or_more!(skip_first!( word1(b',', Type::TFunctionArgument), one_of![ - space0_around_e( + space0_around_ee( term(min_indent), min_indent, Type::TSpace, - Type::TIndentStart + Type::TIndentStart, + Type::TIndentEnd ), |_, state: State<'a>| Err(( NoProgress, diff --git a/compiler/reporting/src/error/parse.rs b/compiler/reporting/src/error/parse.rs index 3431c87379..33f09653c1 100644 --- a/compiler/reporting/src/error/parse.rs +++ b/compiler/reporting/src/error/parse.rs @@ -158,7 +158,9 @@ enum Context { enum Node { WhenCondition, WhenBranch, - // WhenIfGuard, + IfCondition, + IfThenBranch, + IfElseBranch, } fn to_expr_report<'a>( @@ -173,10 +175,130 @@ fn to_expr_report<'a>( match parse_problem { EExpr::When(when, row, col) => to_when_report(alloc, filename, context, &when, *row, *col), + EExpr::If(when, row, col) => to_if_report(alloc, filename, context, &when, *row, *col), _ => todo!("unhandled parse error: {:?}", parse_problem), } } +fn to_if_report<'a>( + alloc: &'a RocDocAllocator<'a>, + filename: PathBuf, + context: Context, + parse_problem: &roc_parse::parser::If<'a>, + start_row: Row, + start_col: Col, +) -> Report<'a> { + use roc_parse::parser::If; + + match *parse_problem { + If::Syntax(syntax, row, col) => to_syntax_report(alloc, filename, syntax, row, col), + If::Space(error, row, col) => to_space_report(alloc, filename, &error, row, col), + + If::Condition(expr, row, col) => to_expr_report( + alloc, + filename, + Context::InNode(Node::IfCondition, start_row, start_col, Box::new(context)), + expr, + row, + col, + ), + + If::ThenBranch(expr, row, col) => to_expr_report( + alloc, + filename, + Context::InNode(Node::IfThenBranch, start_row, start_col, Box::new(context)), + expr, + row, + col, + ), + + If::ElseBranch(expr, row, col) => to_expr_report( + alloc, + filename, + Context::InNode(Node::IfElseBranch, start_row, start_col, Box::new(context)), + expr, + row, + col, + ), + + If::If(_row, _col) => unreachable!("another branch would be taken"), + If::IndentIf(_row, _col) => unreachable!("another branch would be taken"), + + If::Then(row, col) | If::IndentThenBranch(row, col) | If::IndentThenToken(row, col) => { + to_unfinished_if_report( + alloc, + filename, + row, + col, + start_row, + start_col, + alloc.concat(vec![ + alloc.reflow(r"I was expecting to see the "), + alloc.keyword("then"), + alloc.reflow(r" keyword next."), + ]), + ) + } + + If::Else(row, col) | If::IndentElseBranch(row, col) | If::IndentElseToken(row, col) => { + to_unfinished_if_report( + alloc, + filename, + row, + col, + start_row, + start_col, + alloc.concat(vec![ + alloc.reflow(r"I was expecting to see the "), + alloc.keyword("else"), + alloc.reflow(r" keyword next."), + ]), + ) + } + + If::IndentCondition(row, col) => to_unfinished_if_report( + alloc, + filename, + row, + col, + start_row, + start_col, + alloc.concat(vec![ + alloc.reflow(r"I was expecting to see a expression next") + ]), + ), + } +} + +fn to_unfinished_if_report<'a>( + alloc: &'a RocDocAllocator<'a>, + filename: PathBuf, + row: Row, + col: Col, + start_row: Row, + start_col: Col, + message: RocDocBuilder<'a>, +) -> Report<'a> { + let surroundings = Region::from_rows_cols(start_row, start_col, row, col); + let region = Region::from_row_col(row, col); + + let doc = alloc.stack(vec![ + alloc.concat(vec![ + alloc.reflow(r"I was partway through parsing an "), + alloc.keyword("if"), + alloc.reflow(r" expression, but I got stuck here:"), + ]), + alloc.region_with_subregion(surroundings, region), + message, + ]); + + Report { + filename, + doc, + title: "UNFINISHED IF".to_string(), + } +} + fn to_when_report<'a>( alloc: &'a RocDocAllocator<'a>, filename: PathBuf, @@ -792,6 +914,23 @@ fn to_type_report<'a>( } } + Type::TIndentEnd(row, col) => { + let surroundings = Region::from_rows_cols(start_row, start_col, *row, *col); + let region = Region::from_row_col(*row, *col); + + let doc = alloc.stack(vec![ + alloc.reflow(r"I am partway through parsing a type, but I got stuck here:"), + alloc.region_with_subregion(surroundings, region), + alloc.note("I may be confused by indentation"), + ]); + + Report { + filename, + doc, + title: "UNFINISHED TYPE".to_string(), + } + } + Type::TAsIndentStart(row, col) => { let surroundings = Region::from_rows_cols(start_row, start_col, *row, *col); let region = Region::from_row_col(*row, *col); diff --git a/compiler/reporting/tests/test_reporting.rs b/compiler/reporting/tests/test_reporting.rs index 66be20f5ca..aa54cccaae 100644 --- a/compiler/reporting/tests/test_reporting.rs +++ b/compiler/reporting/tests/test_reporting.rs @@ -4636,12 +4636,12 @@ mod test_reporting { indoc!( r#" ── UNFINISHED TYPE ───────────────────────────────────────────────────────────── - - I just started parsing a type, but I got stuck here: - + + I am partway through parsing a type, but I got stuck here: + 1│ f : I64, I64 ^ - + Note: I may be confused by indentation "# ), @@ -4950,4 +4950,56 @@ mod test_reporting { ), ) } + + #[test] + fn if_outdented_then() { + // TODO I think we can do better here + report_problem_as( + indoc!( + r#" + x = + if 5 == 5 + then 2 else 3 + + x + "# + ), + indoc!( + r#" + ── UNFINISHED IF ─────────────────────────────────────────────────────────────── + + I was partway through parsing an `if` expression, but I got stuck here: + + 2│ if 5 == 5 + ^ + + I was expecting to see the `then` keyword next. + "# + ), + ) + } + + #[test] + fn if_missing_else() { + // this should get better with time + report_problem_as( + indoc!( + r#" + if 5 == 5 then 2 + "# + ), + indoc!( + r#" + ── UNFINISHED IF ─────────────────────────────────────────────────────────────── + + I was partway through parsing an `if` expression, but I got stuck here: + + 1│ if 5 == 5 then 2 + ^ + + I was expecting to see the `else` keyword next. + "# + ), + ) + } } From f3234e002ab8ee6c74be55c148dda5aca711ef28 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 23 Feb 2021 20:05:58 +0100 Subject: [PATCH 14/17] change list over --- compiler/parse/src/expr.rs | 65 +++++++++++++++++++----------------- compiler/parse/src/parser.rs | 14 ++++++++ 2 files changed, 49 insertions(+), 30 deletions(-) diff --git a/compiler/parse/src/expr.rs b/compiler/parse/src/expr.rs index 1b01a08edb..aebe5044b7 100644 --- a/compiler/parse/src/expr.rs +++ b/compiler/parse/src/expr.rs @@ -11,8 +11,8 @@ use crate::number_literal::number_literal; use crate::parser::{ self, allocated, and_then_with_indent_level, ascii_char, ascii_string, attempt, backtrackable, fail, map, newline_char, not, not_followed_by, optional, sep_by1, specialize, specialize_ref, - then, unexpected, unexpected_eof, word1, word2, EExpr, Either, If, ParseResult, Parser, State, - SyntaxError, When, + then, unexpected, unexpected_eof, word1, word2, BadInputError, EExpr, Either, If, List, + ParseResult, Parser, State, SyntaxError, When, }; use crate::pattern::loc_closure_param; use crate::type_annotation; @@ -1693,37 +1693,42 @@ fn binop<'a>() -> impl Parser<'a, BinOp, SyntaxError<'a>> { map!(ascii_char(b'%'), |_| BinOp::Percent) ) } - -pub fn list_literal<'a>(min_indent: u16) -> impl Parser<'a, Expr<'a>, SyntaxError<'a>> { - let elems = collection_trailing_sep!( - ascii_char(b'['), - loc!(expr(min_indent)), - ascii_char(b','), - ascii_char(b']'), - min_indent - ); - - parser::attempt( - Attempting::List, - map_with_arena!(elems, |arena, - (parsed_elems, final_comments): ( - Vec<'a, Located>>, - &'a [CommentOrNewline<'a>] - )| { - let mut allocated = Vec::with_capacity_in(parsed_elems.len(), arena); - - for parsed_elem in parsed_elems { - allocated.push(&*arena.alloc(parsed_elem)); - } - - Expr::List { - items: allocated.into_bump_slice(), - final_comments, - } - }), +fn list_literal<'a>(min_indent: u16) -> impl Parser<'a, Expr<'a>, SyntaxError<'a>> { + specialize( + |e, r, c| SyntaxError::Expr(EExpr::List(e, r, c)), + list_literal_help(min_indent), ) } +fn list_literal_help<'a>(min_indent: u16) -> impl Parser<'a, Expr<'a>, List<'a>> { + move |arena, state| { + let (_, (parsed_elems, final_comments), state) = collection_trailing_sep_e!( + word1(b'[', List::Open), + specialize_ref(List::Syntax, loc!(expr(min_indent))), + word1(b',', List::End), + word1(b']', List::End), + min_indent, + List::Open, + List::Space, + List::IndentEnd + ) + .parse(arena, state)?; + + let mut allocated = Vec::with_capacity_in(parsed_elems.len(), arena); + + for parsed_elem in parsed_elems { + allocated.push(&*arena.alloc(parsed_elem)); + } + + let expr = Expr::List { + items: allocated.into_bump_slice(), + final_comments, + }; + + Ok((MadeProgress, expr, state)) + } +} + // Parser<'a, Vec<'a, Located>>> fn record_literal<'a>(min_indent: u16) -> impl Parser<'a, Expr<'a>, SyntaxError<'a>> { then( diff --git a/compiler/parse/src/parser.rs b/compiler/parse/src/parser.rs index b1d3d3545a..1b2ea71cce 100644 --- a/compiler/parse/src/parser.rs +++ b/compiler/parse/src/parser.rs @@ -380,11 +380,25 @@ pub enum EExpr<'a> { When(When<'a>, Row, Col), If(If<'a>, Row, Col), + List(List<'a>, Row, Col), + // EInParens(PInParens<'a>, Row, Col), IndentStart(Row, Col), IndentEnd(Row, Col), } +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum List<'a> { + Open(Row, Col), + End(Row, Col), + Space(BadInputError, Row, Col), + + Syntax(&'a SyntaxError<'a>, Row, Col), + + IndentStart(Row, Col), + IndentEnd(Row, Col), +} + #[derive(Debug, Clone, PartialEq, Eq)] pub enum When<'a> { Space(BadInputError, Row, Col), From 80b64b42ff348ec22d9aec5950a65a76dce5e4aa Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 23 Feb 2021 23:57:17 +0100 Subject: [PATCH 15/17] tests and list error messages --- compiler/parse/src/parser.rs | 3 +- compiler/reporting/src/error/parse.rs | 114 ++++++++++++++++++++- compiler/reporting/tests/test_reporting.rs | 82 +++++++++++++++ 3 files changed, 197 insertions(+), 2 deletions(-) diff --git a/compiler/parse/src/parser.rs b/compiler/parse/src/parser.rs index 1b2ea71cce..ab980a5b38 100644 --- a/compiler/parse/src/parser.rs +++ b/compiler/parse/src/parser.rs @@ -394,8 +394,9 @@ pub enum List<'a> { Space(BadInputError, Row, Col), Syntax(&'a SyntaxError<'a>, Row, Col), + Expr(&'a EExpr<'a>, Row, Col), - IndentStart(Row, Col), + IndentOpen(Row, Col), IndentEnd(Row, Col), } diff --git a/compiler/reporting/src/error/parse.rs b/compiler/reporting/src/error/parse.rs index 33f09653c1..37c155e2cd 100644 --- a/compiler/reporting/src/error/parse.rs +++ b/compiler/reporting/src/error/parse.rs @@ -161,6 +161,7 @@ enum Node { IfCondition, IfThenBranch, IfElseBranch, + ListElement, } fn to_expr_report<'a>( @@ -175,11 +176,121 @@ fn to_expr_report<'a>( match parse_problem { EExpr::When(when, row, col) => to_when_report(alloc, filename, context, &when, *row, *col), - EExpr::If(when, row, col) => to_if_report(alloc, filename, context, &when, *row, *col), + EExpr::If(if_, row, col) => to_if_report(alloc, filename, context, &if_, *row, *col), + EExpr::List(list, row, col) => to_list_report(alloc, filename, context, &list, *row, *col), _ => todo!("unhandled parse error: {:?}", parse_problem), } } +fn to_list_report<'a>( + alloc: &'a RocDocAllocator<'a>, + filename: PathBuf, + context: Context, + parse_problem: &roc_parse::parser::List<'a>, + start_row: Row, + start_col: Col, +) -> Report<'a> { + use roc_parse::parser::List; + + match *parse_problem { + List::Syntax(syntax, row, col) => to_syntax_report(alloc, filename, syntax, row, col), + List::Space(error, row, col) => to_space_report(alloc, filename, &error, row, col), + + List::Expr(expr, row, col) => to_expr_report( + alloc, + filename, + Context::InNode(Node::ListElement, start_row, start_col, Box::new(context)), + expr, + row, + col, + ), + + List::Open(row, col) | List::End(row, col) => { + match dbg!(what_is_next(alloc.src_lines, row, col)) { + Next::Other(Some(',')) => { + let surroundings = Region::from_rows_cols(start_row, start_col, row, col); + let region = Region::from_row_col(row, col); + + let doc = alloc.stack(vec![ + alloc.reflow( + r"I am partway through started parsing a list, but I got stuck here:", + ), + alloc.region_with_subregion(surroundings, region), + alloc.concat(vec![ + alloc + .reflow(r"I was expecting to see a list entry before this comma, "), + alloc.reflow(r"so try adding a list entry"), + alloc.reflow(r" and see if that helps?"), + ]), + ]); + Report { + filename, + doc, + title: "UNFINISHED LIST".to_string(), + } + } + _ => { + let surroundings = Region::from_rows_cols(start_row, start_col, row, col); + let region = Region::from_row_col(row, col); + + let doc = alloc.stack(vec![ + alloc.reflow( + r"I am partway through started parsing a list, but I got stuck here:", + ), + alloc.region_with_subregion(surroundings, region), + alloc.concat(vec![ + alloc.reflow( + r"I was expecting to see a closing square bracket before this, ", + ), + alloc.reflow(r"so try adding a "), + alloc.parser_suggestion("]"), + alloc.reflow(r" and see if that helps?"), + ]), + alloc.concat(vec![ + alloc.note("When "), + alloc.reflow(r"I get stuck like this, "), + alloc.reflow(r"it usually means that there is a missing parenthesis "), + alloc.reflow(r"or bracket somewhere earlier. "), + alloc.reflow(r"It could also be a stray keyword or operator."), + ]), + ]); + + Report { + filename, + doc, + title: "UNFINISHED LIST".to_string(), + } + } + } + } + + List::IndentOpen(row, col) | List::IndentEnd(row, col) => { + let surroundings = Region::from_rows_cols(start_row, start_col, row, col); + let region = Region::from_row_col(row, col); + + let doc = alloc.stack(vec![ + alloc.reflow(r"I cannot find the end of this list:"), + alloc.region_with_subregion(surroundings, region), + alloc.concat(vec![ + alloc.reflow(r"You could change it to something like "), + alloc.parser_suggestion("[ 1, 2, 3 ]"), + alloc.reflow(" or even just "), + alloc.parser_suggestion("[]"), + alloc.reflow(". Anything where there is an open and a close square bracket, "), + alloc.reflow("and where the elements of the list are separated by commas."), + ]), + note_for_tag_union_type_indent(alloc), + ]); + + Report { + filename, + doc, + title: "UNFINISHED LIST".to_string(), + } + } + } +} + fn to_if_report<'a>( alloc: &'a RocDocAllocator<'a>, filename: PathBuf, @@ -1745,6 +1856,7 @@ fn to_space_report<'a>( } } +#[derive(Debug)] enum Next<'a> { Keyword(&'a str), // Operator(&'a str), diff --git a/compiler/reporting/tests/test_reporting.rs b/compiler/reporting/tests/test_reporting.rs index aa54cccaae..7ab6eefa14 100644 --- a/compiler/reporting/tests/test_reporting.rs +++ b/compiler/reporting/tests/test_reporting.rs @@ -5002,4 +5002,86 @@ mod test_reporting { ), ) } + + #[test] + fn list_double_comma() { + report_problem_as( + indoc!( + r#" + [ 1, 2, , 3 ] + "# + ), + indoc!( + r#" + ── UNFINISHED LIST ───────────────────────────────────────────────────────────── + + I am partway through started parsing a list, but I got stuck here: + + 1│ [ 1, 2, , 3 ] + ^ + + I was expecting to see a list entry before this comma, so try adding a + list entry and see if that helps? + "# + ), + ) + } + + #[test] + fn list_without_end() { + report_problem_as( + indoc!( + r#" + [ 1, 2, + "# + ), + indoc!( + r#" + ── UNFINISHED LIST ───────────────────────────────────────────────────────────── + + I am partway through started parsing a list, but I got stuck here: + + 1│ [ 1, 2, + ^ + + I was expecting to see a closing square bracket before this, so try + adding a ] and see if that helps? + + Note: When I get stuck like this, it usually means that there is a + missing parenthesis or bracket somewhere earlier. It could also be a + stray keyword or operator. + "# + ), + ) + } + + #[test] + fn list_bad_indent() { + report_problem_as( + indoc!( + r#" + x = [ 1, 2, + ] + + x + "# + ), + indoc!( + r#" + ── UNFINISHED LIST ───────────────────────────────────────────────────────────── + + I cannot find the end of this list: + + 1│ x = [ 1, 2, + ^ + + You could change it to something like [ 1, 2, 3 ] or even just []. + Anything where there is an open and a close square bracket, and where + the elements of the list are separated by commas. + + Note: I may be confused by indentation + "# + ), + ) + } } From 1c98bca071b71bc7db4985ccd1974f526dbb3ee3 Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 24 Feb 2021 00:56:27 +0100 Subject: [PATCH 16/17] astar test does not use stdin --- cli/tests/cli_run.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cli/tests/cli_run.rs b/cli/tests/cli_run.rs index 6ad19aed58..49388b27b0 100644 --- a/cli/tests/cli_run.rs +++ b/cli/tests/cli_run.rs @@ -230,9 +230,8 @@ mod cli_run { #[test] #[serial(astar)] fn run_astar_optimized_1() { - check_output_with_stdin( + check_output( &example_file("benchmarks", "AStarTests.roc"), - "1", "astar-tests", &[], "True\n", From e16f6f8c49da25ee450a261fbdbb002ee7bd24a5 Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 24 Feb 2021 23:36:42 +0100 Subject: [PATCH 17/17] fix compile errors --- Cargo.lock | 1 + compiler/load/Cargo.toml | 1 + compiler/load/src/file.rs | 2 -- 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7e99da5cc3..aa6ca32dbe 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2928,6 +2928,7 @@ version = "0.1.0" dependencies = [ "bumpalo", "crossbeam", + "indoc", "inlinable_string", "maplit", "num_cpus", diff --git a/compiler/load/Cargo.toml b/compiler/load/Cargo.toml index 701b7dd6da..1afe0974bd 100644 --- a/compiler/load/Cargo.toml +++ b/compiler/load/Cargo.toml @@ -30,5 +30,6 @@ num_cpus = "1" tempfile = "3.1.0" pretty_assertions = "0.5.1" maplit = "1.0.1" +indoc = "0.3.3" quickcheck = "0.8" quickcheck_macros = "0.8" diff --git a/compiler/load/src/file.rs b/compiler/load/src/file.rs index bfb82bfc92..cee4691104 100644 --- a/compiler/load/src/file.rs +++ b/compiler/load/src/file.rs @@ -1620,8 +1620,6 @@ fn update<'a>( match header_extra { App { to_platform } => { debug_assert!(matches!(state.platform_path, PlatformPath::NotSpecified)); - debug_assert!(header.is_root_module); - state.platform_path = PlatformPath::Valid(to_platform.clone()); } PkgConfig { .. } => {