From 7952f83f1a76a8160e810d287867f6c0ce2f0765 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 11 May 2024 20:21:06 +0800 Subject: [PATCH 1/3] Allow closures in custom syntax. --- src/parser.rs | 94 ++++++++++++++++++------------------------ tests/custom_syntax.rs | 14 ++++++- 2 files changed, 52 insertions(+), 56 deletions(-) diff --git a/src/parser.rs b/src/parser.rs index 38e21e895..a88476b02 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1424,40 +1424,7 @@ impl Engine { } #[cfg(not(feature = "no_function"))] Token::Pipe | Token::Or if settings.has_option(LangOptions::ANON_FN) => { - let (expr, fn_def, _externals) = self.parse_anon_fn( - state, - settings, - false, - #[cfg(not(feature = "no_closure"))] - true, - )?; - - #[cfg(not(feature = "no_closure"))] - for Ident { name, pos } in &_externals { - let (index, is_func) = self.access_var(state, name, *pos); - - if !is_func - && index.is_none() - && !settings.has_flag(ParseSettingFlags::CLOSURE_SCOPE) - && settings.has_option(LangOptions::STRICT_VAR) - && !state - .external_constants - .map_or(false, |scope| scope.contains(name)) - { - // If the parent scope is not inside another capturing closure - // then we can conclude that the captured variable doesn't exist. - // Under Strict Variables mode, this is not allowed. - return Err(PERR::VariableUndefined(name.to_string()).into_err(*pos)); - } - } - - let hash_script = calc_fn_hash(None, &fn_def.name, fn_def.params.len()); - state.lib.insert(hash_script, fn_def); - - #[cfg(not(feature = "no_closure"))] - let expr = self.make_curry_from_externals(state, expr, _externals, settings.pos); - - expr + self.parse_anon_fn(state, settings, false)? } // Interpolated string @@ -2532,19 +2499,7 @@ impl Engine { .into_err(*fwd_pos)) } }; - - let (expr, fn_def, _) = self.parse_anon_fn( - state, - settings, - skip, - #[cfg(not(feature = "no_closure"))] - false, - )?; - - let hash_script = calc_fn_hash(None, &fn_def.name, fn_def.params.len()); - state.lib.insert(hash_script, fn_def); - - inputs.push(expr); + inputs.push(self.parse_anon_fn(state, settings, skip)?); let keyword = self.get_interned_string(CUSTOM_SYNTAX_MARKER_FUNC); segments.push(keyword.clone()); tokens.push(keyword); @@ -2631,6 +2586,9 @@ impl Engine { // If the last symbol is `;` or `}`, it is self-terminating KEYWORD_SEMICOLON | KEYWORD_CLOSE_BRACE ); + // It is self-terminating if the last symbol is a block + #[cfg(not(feature = "no_function"))] + let self_terminated = required_token == CUSTOM_SYNTAX_MARKER_FUNC || self_terminated; Ok(Expr::Custom( crate::ast::CustomExpr { @@ -3711,8 +3669,7 @@ impl Engine { state: &mut ParseState, settings: ParseSettings, skip_parameters: bool, - #[cfg(not(feature = "no_closure"))] allow_capture: bool, - ) -> ParseResult<(Expr, Shared, ThinVec)> { + ) -> ParseResult { // Build new parse state let new_state = &mut ParseState::new( state.external_constants, @@ -3803,15 +3760,13 @@ impl Engine { // External variables may need to be processed in a consistent order, // so extract them into a list. #[cfg(not(feature = "no_closure"))] - let (mut params, externals) = if allow_capture { + let (mut params, externals) = { let externals = std::mem::take(&mut new_state.external_vars); let mut params = FnArgsVec::with_capacity(params_list.len() + externals.len()); params.extend(externals.iter().map(|Ident { name, .. }| name.clone())); (params, externals) - } else { - (FnArgsVec::with_capacity(params_list.len()), ThinVec::new()) }; #[cfg(feature = "no_closure")] let (mut params, externals) = (FnArgsVec::with_capacity(params_list.len()), ThinVec::new()); @@ -3826,7 +3781,7 @@ impl Engine { let fn_name = self.get_interned_string(make_anonymous_fn(hash)); // Define the function - let script = Shared::new(ScriptFuncDef { + let fn_def = Shared::new(ScriptFuncDef { name: fn_name.clone(), access: crate::FnAccess::Public, #[cfg(not(feature = "no_object"))] @@ -3838,16 +3793,45 @@ impl Engine { comments: <_>::default(), }); + // Define the function pointer let fn_ptr = crate::FnPtr { name: fn_name, curry: ThinVec::new(), environ: None, #[cfg(not(feature = "no_function"))] - fn_def: Some(script.clone()), + fn_def: Some(fn_def.clone()), }; + let expr = Expr::DynamicConstant(Box::new(fn_ptr.into()), new_settings.pos); - Ok((expr, script, externals)) + // Finished with `new_state` here. Revert back to using `state`. + + #[cfg(not(feature = "no_closure"))] + for Ident { name, pos } in &externals { + let (index, is_func) = self.access_var(state, name, *pos); + + if !is_func + && index.is_none() + && !settings.has_flag(ParseSettingFlags::CLOSURE_SCOPE) + && settings.has_option(LangOptions::STRICT_VAR) + && !state + .external_constants + .map_or(false, |scope| scope.contains(name)) + { + // If the parent scope is not inside another capturing closure + // then we can conclude that the captured variable doesn't exist. + // Under Strict Variables mode, this is not allowed. + return Err(PERR::VariableUndefined(name.to_string()).into_err(*pos)); + } + } + + let hash_script = calc_fn_hash(None, &fn_def.name, fn_def.params.len()); + state.lib.insert(hash_script, fn_def); + + #[cfg(not(feature = "no_closure"))] + let expr = self.make_curry_from_externals(state, expr, externals, settings.pos); + + Ok(expr) } /// Parse a global level expression. diff --git a/tests/custom_syntax.rs b/tests/custom_syntax.rs index 8beb869a5..47eee83d4 100644 --- a/tests/custom_syntax.rs +++ b/tests/custom_syntax.rs @@ -278,11 +278,23 @@ fn test_custom_syntax_func() { let mut engine = Engine::new(); engine - .register_custom_syntax(["hello", "$func$"], false, |_, inputs| Ok(inputs[0].get_literal_value::().unwrap().into())) + .register_custom_syntax(["hello", "$func$"], false, |context, inputs| context.eval_expression_tree(&inputs[0])) .unwrap(); assert_eq!(engine.eval::("call(hello |x| { x + 1 }, 41)").unwrap(), 42); assert_eq!(engine.eval::("call(hello { 42 })").unwrap(), 42); + assert_eq!( + engine + .eval::( + " + let a = 1; + let f = hello |x| { x + a }; + call(f, 41) + " + ) + .unwrap(), + 42 + ); } #[test] From 7b3a8b1236c3d874240e689c70eb4af8837d405b Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 11 May 2024 22:10:33 +0800 Subject: [PATCH 2/3] Fix build. --- src/parser.rs | 53 +++++++++++++++++++++++++++++---------------------- 1 file changed, 30 insertions(+), 23 deletions(-) diff --git a/src/parser.rs b/src/parser.rs index a88476b02..048d44aa6 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -3689,25 +3689,6 @@ impl Engine { new_state.global_imports.extend(state.imports.clone()); } - // Brand new options - #[cfg(not(feature = "no_closure"))] - let options = self.options & !LangOptions::STRICT_VAR; // a capturing closure can access variables not defined locally, so turn off Strict Variables mode - #[cfg(feature = "no_closure")] - let options = self.options | (settings.options & LangOptions::STRICT_VAR); - - // Brand new flags, turn on function scope and closure scope - let flags = ParseSettingFlags::FN_SCOPE - | ParseSettingFlags::CLOSURE_SCOPE - | (settings.flags - & (ParseSettingFlags::DISALLOW_UNQUOTED_MAP_PROPERTIES - | ParseSettingFlags::DISALLOW_STATEMENTS_IN_BLOCKS)); - - let new_settings = ParseSettings { - flags, - options, - ..settings - }; - let mut params_list = StaticVec::::new_const(); // Parse parameters @@ -3754,13 +3735,34 @@ impl Engine { } } + // Brand new options + #[cfg(not(feature = "no_closure"))] + let options = self.options & !LangOptions::STRICT_VAR; // a capturing closure can access variables not defined locally, so turn off Strict Variables mode + #[cfg(feature = "no_closure")] + let options = self.options | (settings.options & LangOptions::STRICT_VAR); + + // Brand new flags, turn on function scope and closure scope + let flags = ParseSettingFlags::FN_SCOPE + | ParseSettingFlags::CLOSURE_SCOPE + | (settings.flags + & (ParseSettingFlags::DISALLOW_UNQUOTED_MAP_PROPERTIES + | ParseSettingFlags::DISALLOW_STATEMENTS_IN_BLOCKS)); + + let new_settings = ParseSettings { + flags, + options, + ..settings + }; + // Parse function body let body = self.parse_stmt(new_state, new_settings.level_up()?)?; + let _ = new_settings; // Make sure it doesn't leak into code below + // External variables may need to be processed in a consistent order, // so extract them into a list. #[cfg(not(feature = "no_closure"))] - let (mut params, externals) = { + let (mut params, _externals) = { let externals = std::mem::take(&mut new_state.external_vars); let mut params = FnArgsVec::with_capacity(params_list.len() + externals.len()); @@ -3769,7 +3771,12 @@ impl Engine { (params, externals) }; #[cfg(feature = "no_closure")] - let (mut params, externals) = (FnArgsVec::with_capacity(params_list.len()), ThinVec::new()); + let (mut params, _externals) = ( + FnArgsVec::with_capacity(params_list.len()), + ThinVec::::new(), + ); + + let _ = new_state; // Make sure it doesn't leak into code below params.append(&mut params_list); @@ -3807,7 +3814,7 @@ impl Engine { // Finished with `new_state` here. Revert back to using `state`. #[cfg(not(feature = "no_closure"))] - for Ident { name, pos } in &externals { + for Ident { name, pos } in &_externals { let (index, is_func) = self.access_var(state, name, *pos); if !is_func @@ -3829,7 +3836,7 @@ impl Engine { state.lib.insert(hash_script, fn_def); #[cfg(not(feature = "no_closure"))] - let expr = self.make_curry_from_externals(state, expr, externals, settings.pos); + let expr = self.make_curry_from_externals(state, expr, _externals, settings.pos); Ok(expr) } From cc25d831b6ad1764cc947d95ae0e88034acb4026 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 11 May 2024 22:14:09 +0800 Subject: [PATCH 3/3] Fix test. --- tests/custom_syntax.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/custom_syntax.rs b/tests/custom_syntax.rs index 47eee83d4..ac504dfe7 100644 --- a/tests/custom_syntax.rs +++ b/tests/custom_syntax.rs @@ -283,6 +283,8 @@ fn test_custom_syntax_func() { assert_eq!(engine.eval::("call(hello |x| { x + 1 }, 41)").unwrap(), 42); assert_eq!(engine.eval::("call(hello { 42 })").unwrap(), 42); + + #[cfg(not(feature = "no_closure"))] assert_eq!( engine .eval::(