From 15a8f528aeef31736c5d2e085ddf45e4a2ac3ef0 Mon Sep 17 00:00:00 2001 From: J Henry Waugh Date: Fri, 21 Aug 2020 22:29:04 -0500 Subject: [PATCH 1/5] Avoid export_fn+cfg attributes in Rhai packages --- src/packages/string_basic.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/packages/string_basic.rs b/src/packages/string_basic.rs index 020f9292..827ba183 100644 --- a/src/packages/string_basic.rs +++ b/src/packages/string_basic.rs @@ -88,9 +88,9 @@ def_package!(crate:BasicStringPackage:"Basic string utilities, including printin #[cfg(not(feature = "no_object"))] { - set_exported_fn!(lib, KEYWORD_PRINT, format_map); - set_exported_fn!(lib, FN_TO_STRING, format_map); - set_exported_fn!(lib, KEYWORD_DEBUG, format_map); + set_exported_fn!(lib, KEYWORD_PRINT, format_map::format_map); + set_exported_fn!(lib, FN_TO_STRING, format_map::format_map); + set_exported_fn!(lib, KEYWORD_DEBUG, format_map::format_map); } }); @@ -154,8 +154,11 @@ fn to_debug(x: &mut T) -> ImmutableString { format!("{:?}", x).into() } #[cfg(not(feature = "no_object"))] -#[export_fn] -#[inline] -fn format_map(x: &mut Map) -> ImmutableString { - format!("#{:?}", x).into() +mod format_map { + use super::*; + #[inline] + #[export_fn] + pub fn format_map(x: &mut Map) -> ImmutableString { + format!("#{:?}", x).into() + } } From 708c0f385e825cb14d66846a804d8c5d007ea416 Mon Sep 17 00:00:00 2001 From: J Henry Waugh Date: Fri, 21 Aug 2020 22:30:10 -0500 Subject: [PATCH 2/5] Block #[cfg] attributes on exported or inner functions --- codegen/src/function.rs | 10 ++++++++++ codegen/src/module.rs | 11 +++++++++++ codegen/ui_tests/export_fn_cfg.rs | 25 +++++++++++++++++++++++++ codegen/ui_tests/export_fn_cfg.stderr | 11 +++++++++++ codegen/ui_tests/module_cfg_fn.rs | 27 +++++++++++++++++++++++++++ codegen/ui_tests/module_cfg_fn.stderr | 11 +++++++++++ 6 files changed, 95 insertions(+) create mode 100644 codegen/ui_tests/export_fn_cfg.rs create mode 100644 codegen/ui_tests/export_fn_cfg.stderr create mode 100644 codegen/ui_tests/module_cfg_fn.rs create mode 100644 codegen/ui_tests/module_cfg_fn.stderr diff --git a/codegen/src/function.rs b/codegen/src/function.rs index 0dba5afa..011ff650 100644 --- a/codegen/src/function.rs +++ b/codegen/src/function.rs @@ -148,6 +148,16 @@ impl Parse for ExportedFn { let entire_span = fn_all.span(); let str_type_path = syn::parse2::(quote! { str }).unwrap(); + // #[cfg] attributes are not allowed on functions due to what is generated for them + if let Some(cfg_attr) = fn_all.attrs.iter().find(|a| { + a.path + .get_ident() + .map(|i| i.to_string() == "cfg") + .unwrap_or(false) + }) { + return Err(syn::Error::new(cfg_attr.span(), "cfg attributes not allowed on this item")); + } + // Determine if the function is public. let is_public = match fn_all.vis { syn::Visibility::Public(_) => true, diff --git a/codegen/src/module.rs b/codegen/src/module.rs index c6bd6469..d3e09eca 100644 --- a/codegen/src/module.rs +++ b/codegen/src/module.rs @@ -16,6 +16,17 @@ use std::borrow::Cow; use std::collections::HashMap; fn inner_fn_attributes(f: &mut syn::ItemFn) -> syn::Result { + // #[cfg] attributes are not allowed on objects + if let Some(cfg_attr) = f.attrs.iter().find(|a| { + a.path + .get_ident() + .map(|i| i.to_string() == "cfg") + .unwrap_or(false) + }) { + return Err(syn::Error::new(cfg_attr.span(), "cfg attributes not allowed on this item")); + } + + // Find the #[rhai_fn] attribute which will turn be read for the function parameters. if let Some(rhai_fn_idx) = f.attrs.iter().position(|a| { a.path .get_ident() diff --git a/codegen/ui_tests/export_fn_cfg.rs b/codegen/ui_tests/export_fn_cfg.rs new file mode 100644 index 00000000..52d4c963 --- /dev/null +++ b/codegen/ui_tests/export_fn_cfg.rs @@ -0,0 +1,25 @@ +use rhai::plugin::*; + +#[derive(Clone)] +pub struct Point { + x: f32, + y: f32, +} + +#[cfg(not(feature = "foo"))] +#[export_fn] +pub fn test_fn(input: Point) -> bool { + input.x > input.y +} + +fn main() { + let n = Point { + x: 0.0, + y: 10.0, + }; + if test_fn(n) { + println!("yes"); + } else { + println!("no"); + } +} diff --git a/codegen/ui_tests/export_fn_cfg.stderr b/codegen/ui_tests/export_fn_cfg.stderr new file mode 100644 index 00000000..ac068be8 --- /dev/null +++ b/codegen/ui_tests/export_fn_cfg.stderr @@ -0,0 +1,11 @@ +error: cfg attributes not allowed on this item + --> $DIR/export_fn_cfg.rs:9:1 + | +9 | #[cfg(not(feature = "foo"))] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error[E0425]: cannot find function `test_fn` in this scope + --> $DIR/export_fn_cfg.rs:20:8 + | +20 | if test_fn(n) { + | ^^^^^^^ not found in this scope diff --git a/codegen/ui_tests/module_cfg_fn.rs b/codegen/ui_tests/module_cfg_fn.rs new file mode 100644 index 00000000..3613b9a5 --- /dev/null +++ b/codegen/ui_tests/module_cfg_fn.rs @@ -0,0 +1,27 @@ +use rhai::plugin::*; + +#[derive(Clone)] +pub struct Point { + x: f32, + y: f32, +} + +#[export_module] +pub mod test_module { + #[cfg(not(feature = "foo"))] + pub fn test_fn(input: Point) -> bool { + input.x > input.y + } +} + +fn main() { + let n = Point { + x: 0.0, + y: 10.0, + }; + if test_module::test_fn(n) { + println!("yes"); + } else { + println!("no"); + } +} diff --git a/codegen/ui_tests/module_cfg_fn.stderr b/codegen/ui_tests/module_cfg_fn.stderr new file mode 100644 index 00000000..80ac12f8 --- /dev/null +++ b/codegen/ui_tests/module_cfg_fn.stderr @@ -0,0 +1,11 @@ +error: cfg attributes not allowed on this item + --> $DIR/module_cfg_fn.rs:11:5 + | +11 | #[cfg(not(feature = "foo"))] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error[E0433]: failed to resolve: use of undeclared type or module `test_module` + --> $DIR/module_cfg_fn.rs:22:8 + | +22 | if test_module::test_fn(n) { + | ^^^^^^^^^^^ use of undeclared type or module `test_module` From 382e60e91a21094baf836ff3aefa35c510a836af Mon Sep 17 00:00:00 2001 From: J Henry Waugh Date: Fri, 21 Aug 2020 22:49:15 -0500 Subject: [PATCH 3/5] Move UI tests to separate tests file --- codegen/src/function.rs | 9 --------- codegen/tests/ui_tests.rs | 8 ++++++++ 2 files changed, 8 insertions(+), 9 deletions(-) create mode 100644 codegen/tests/ui_tests.rs diff --git a/codegen/src/function.rs b/codegen/src/function.rs index 011ff650..89358274 100644 --- a/codegen/src/function.rs +++ b/codegen/src/function.rs @@ -1067,12 +1067,3 @@ mod generate_tests { assert_streams_eq(item_fn.generate(), expected_tokens); } } - -#[cfg(test)] -mod ui_tests { - #[test] - fn all() { - let t = trybuild::TestCases::new(); - t.compile_fail("ui_tests/*.rs"); - } -} diff --git a/codegen/tests/ui_tests.rs b/codegen/tests/ui_tests.rs new file mode 100644 index 00000000..ef7bf58f --- /dev/null +++ b/codegen/tests/ui_tests.rs @@ -0,0 +1,8 @@ +#![cfg(test)] +mod ui_tests { + #[test] + fn all() { + let t = trybuild::TestCases::new(); + t.compile_fail("ui_tests/*.rs"); + } +} From 55870e7b37c40d26f9a5406f442b3a1ddbb6e7e7 Mon Sep 17 00:00:00 2001 From: J Henry Waugh Date: Fri, 21 Aug 2020 23:05:18 -0500 Subject: [PATCH 4/5] Add #[cfg] support in submodules --- codegen/src/module.rs | 107 ++++++++++++++++-- codegen/src/rhai_module.rs | 17 ++- codegen/ui_tests/rhai_mod_inner_cfg_false.rs | 29 +++++ .../ui_tests/rhai_mod_inner_cfg_false.stderr | 5 + 4 files changed, 141 insertions(+), 17 deletions(-) create mode 100644 codegen/ui_tests/rhai_mod_inner_cfg_false.rs create mode 100644 codegen/ui_tests/rhai_mod_inner_cfg_false.stderr diff --git a/codegen/src/module.rs b/codegen/src/module.rs index d3e09eca..7aa98409 100644 --- a/codegen/src/module.rs +++ b/codegen/src/module.rs @@ -11,6 +11,8 @@ use std::vec as new_vec; #[cfg(no_std)] use core::mem; +#[cfg(not(no_std))] +use std::mem; use std::borrow::Cow; use std::collections::HashMap; @@ -282,6 +284,10 @@ impl Parse for Module { } impl Module { + pub fn attrs(&self) -> Option<&Vec> { + self.mod_all.as_ref().map(|m| &m.attrs) + } + pub fn module_name(&self) -> Option<&syn::Ident> { self.mod_all.as_ref().map(|m| &m.ident) } @@ -324,8 +330,10 @@ impl Module { let mut mod_all = mod_all.unwrap(); let mod_name = mod_all.ident.clone(); let (_, orig_content) = mod_all.content.take().unwrap(); + let mod_attrs = mem::replace(&mut mod_all.attrs, Vec::with_capacity(0)); Ok(quote! { + #(#mod_attrs)* pub mod #mod_name { #(#orig_content)* #(#inner_modules)* @@ -1112,7 +1120,82 @@ mod generate_tests { #[allow(unused_mut)] pub fn rhai_module_generate() -> Module { let mut m = Module::new(); - m.set_sub_module("it_is", self::it_is::rhai_module_generate()); + { m.set_sub_module("it_is", self::it_is::rhai_module_generate()); } + m + } + } + }; + + let item_mod = syn::parse2::(input_tokens).unwrap(); + assert_streams_eq(item_mod.generate(), expected_tokens); + } + + #[test] + fn one_fn_with_cfg_module() { + let input_tokens: TokenStream = quote! { + pub mod one_fn { + #[cfg(not(feature = "no_float"))] + pub mod it_is { + pub fn increment(x: &mut FLOAT) { + *x += 1.0 as FLOAT; + } + } + } + }; + + let expected_tokens = quote! { + pub mod one_fn { + #[cfg(not(feature = "no_float"))] + pub mod it_is { + pub fn increment(x: &mut FLOAT) { + *x += 1.0 as FLOAT; + } + #[allow(unused_imports)] + use super::*; + #[allow(unused_mut)] + pub fn rhai_module_generate() -> Module { + let mut m = Module::new(); + m.set_fn("increment", FnAccess::Public, + &[core::any::TypeId::of::()], + CallableFunction::from_plugin(increment_token())); + m + } + #[allow(non_camel_case_types)] + struct increment_token(); + impl PluginFunction for increment_token { + fn call(&self, + args: &mut [&mut Dynamic], pos: Position + ) -> Result> { + debug_assert_eq!(args.len(), 1usize, + "wrong arg count: {} != {}", args.len(), 1usize); + let arg0: &mut _ = &mut args[0usize].write_lock::().unwrap(); + Ok(Dynamic::from(increment(arg0))) + } + + fn is_method_call(&self) -> bool { true } + fn is_varadic(&self) -> bool { false } + fn clone_boxed(&self) -> Box { + Box::new(increment_token()) + } + fn input_types(&self) -> Box<[TypeId]> { + new_vec![TypeId::of::()].into_boxed_slice() + } + } + pub fn increment_token_callable() -> CallableFunction { + CallableFunction::from_plugin(increment_token()) + } + pub fn increment_token_input_types() -> Box<[TypeId]> { + increment_token().input_types() + } + } + #[allow(unused_imports)] + use super::*; + #[allow(unused_mut)] + pub fn rhai_module_generate() -> Module { + let mut m = Module::new(); + #[cfg(not(feature = "no_float"))] { + m.set_sub_module("it_is", self::it_is::rhai_module_generate()); + } m } } @@ -1150,7 +1233,7 @@ mod generate_tests { #[allow(unused_mut)] pub fn rhai_module_generate() -> Module { let mut m = Module::new(); - m.set_sub_module("it_is", self::it_is::rhai_module_generate()); + { m.set_sub_module("it_is", self::it_is::rhai_module_generate()); } m } } @@ -1202,8 +1285,8 @@ mod generate_tests { #[allow(unused_mut)] pub fn rhai_module_generate() -> Module { let mut m = Module::new(); - m.set_sub_module("first_is", self::first_is::rhai_module_generate()); - m.set_sub_module("second_is", self::second_is::rhai_module_generate()); + { m.set_sub_module("first_is", self::first_is::rhai_module_generate()); } + { m.set_sub_module("second_is", self::second_is::rhai_module_generate()); } m } } @@ -1280,8 +1363,8 @@ mod generate_tests { pub fn rhai_module_generate() -> Module { let mut m = Module::new(); m.set_var("VALUE", 17); - m.set_sub_module("left", self::left::rhai_module_generate()); - m.set_sub_module("right", self::right::rhai_module_generate()); + { m.set_sub_module("left", self::left::rhai_module_generate()); } + { m.set_sub_module("right", self::right::rhai_module_generate()); } m } } @@ -1302,8 +1385,8 @@ mod generate_tests { pub fn rhai_module_generate() -> Module { let mut m = Module::new(); m.set_var("VALUE", 19); - m.set_sub_module("left", self::left::rhai_module_generate()); - m.set_sub_module("right", self::right::rhai_module_generate()); + { m.set_sub_module("left", self::left::rhai_module_generate()); } + { m.set_sub_module("right", self::right::rhai_module_generate()); } m } } @@ -1337,8 +1420,8 @@ mod generate_tests { pub fn rhai_module_generate() -> Module { let mut m = Module::new(); m.set_var("VALUE", 36); - m.set_sub_module("left", self::left::rhai_module_generate()); - m.set_sub_module("right", self::right::rhai_module_generate()); + { m.set_sub_module("left", self::left::rhai_module_generate()); } + { m.set_sub_module("right", self::right::rhai_module_generate()); } m } } @@ -1348,8 +1431,8 @@ mod generate_tests { pub fn rhai_module_generate() -> Module { let mut m = Module::new(); m.set_var("VALUE", 100); - m.set_sub_module("left", self::left::rhai_module_generate()); - m.set_sub_module("right", self::right::rhai_module_generate()); + { m.set_sub_module("left", self::left::rhai_module_generate()); } + { m.set_sub_module("right", self::right::rhai_module_generate()); } m } } diff --git a/codegen/src/rhai_module.rs b/codegen/src/rhai_module.rs index f4f7369e..c327c71f 100644 --- a/codegen/src/rhai_module.rs +++ b/codegen/src/rhai_module.rs @@ -12,7 +12,7 @@ pub(crate) fn generate_body( ) -> proc_macro2::TokenStream { let mut set_fn_stmts: Vec = Vec::new(); let mut set_const_stmts: Vec = Vec::new(); - let mut add_mod_stmts: Vec = Vec::new(); + let mut add_mod_blocks: Vec = Vec::new(); let str_type_path = syn::parse2::(quote! { str }).unwrap(); for (const_name, const_expr) in consts { @@ -32,9 +32,16 @@ pub(crate) fn generate_body( } else { syn::LitStr::new(&module_name.to_string(), proc_macro2::Span::call_site()) }; - add_mod_stmts.push( - syn::parse2::(quote! { - m.set_sub_module(#exported_name, self::#module_name::rhai_module_generate()); + let cfg_attrs: Vec<&syn::Attribute> = itemmod.attrs().unwrap().iter().filter(|&a| { + a.path.get_ident() + .map(|i| i.to_string() == "cfg") + .unwrap_or(false) + }).collect(); + add_mod_blocks.push( + syn::parse2::(quote! { + #(#cfg_attrs)* { + m.set_sub_module(#exported_name, self::#module_name::rhai_module_generate()); + } }) .unwrap(), ); @@ -117,7 +124,7 @@ pub(crate) fn generate_body( let mut m = Module::new(); #(#set_fn_stmts)* #(#set_const_stmts)* - #(#add_mod_stmts)* + #(#add_mod_blocks)* m } } diff --git a/codegen/ui_tests/rhai_mod_inner_cfg_false.rs b/codegen/ui_tests/rhai_mod_inner_cfg_false.rs new file mode 100644 index 00000000..cfb3e1b6 --- /dev/null +++ b/codegen/ui_tests/rhai_mod_inner_cfg_false.rs @@ -0,0 +1,29 @@ +use rhai::plugin::*; + +#[derive(Clone)] +pub struct Point { + x: f32, + y: f32, +} + +#[export_module] +pub mod test_module { + #[cfg(feature = "unset_feature")] + pub mod test_mod { + pub fn test_fn(input: Point) -> bool { + input.x > input.y + } + } +} + +fn main() { + let n = Point { + x: 0.0, + y: 10.0, + }; + if test_module::test_mod::test_fn(n) { + println!("yes"); + } else { + println!("no"); + } +} diff --git a/codegen/ui_tests/rhai_mod_inner_cfg_false.stderr b/codegen/ui_tests/rhai_mod_inner_cfg_false.stderr new file mode 100644 index 00000000..04068220 --- /dev/null +++ b/codegen/ui_tests/rhai_mod_inner_cfg_false.stderr @@ -0,0 +1,5 @@ +error[E0433]: failed to resolve: could not find `test_mod` in `test_module` + --> $DIR/rhai_mod_inner_cfg_false.rs:24:21 + | +24 | if test_module::test_mod::test_fn(n) { + | ^^^^^^^^ could not find `test_mod` in `test_module` From 65009dd19301a81c873daeca299d0bb5a083cf96 Mon Sep 17 00:00:00 2001 From: J Henry Waugh Date: Fri, 21 Aug 2020 23:21:56 -0500 Subject: [PATCH 5/5] Block #[cfg] attributes on inner constants --- codegen/src/module.rs | 34 +++++++++++++++--------- codegen/ui_tests/module_cfg_const.rs | 31 +++++++++++++++++++++ codegen/ui_tests/module_cfg_const.stderr | 11 ++++++++ 3 files changed, 63 insertions(+), 13 deletions(-) create mode 100644 codegen/ui_tests/module_cfg_const.rs create mode 100644 codegen/ui_tests/module_cfg_const.stderr diff --git a/codegen/src/module.rs b/codegen/src/module.rs index 7aa98409..34b67876 100644 --- a/codegen/src/module.rs +++ b/codegen/src/module.rs @@ -199,7 +199,7 @@ impl Parse for Module { fn parse(input: ParseStream) -> syn::Result { let mut mod_all: syn::ItemMod = input.parse()?; let fns: Vec<_>; - let consts: Vec<_>; + let mut consts: Vec<_> = new_vec![]; let mut submodules: Vec<_> = Vec::new(); if let Some((_, ref mut content)) = mod_all.content { // Gather and parse functions. @@ -223,24 +223,33 @@ impl Parse for Module { .map(|_| vec) })?; // Gather and parse constants definitions. - consts = content - .iter() - .filter_map(|item| match item { + for item in content.iter() { + match item { syn::Item::Const(syn::ItemConst { vis, ref expr, ident, + attrs, .. }) => { - if let syn::Visibility::Public(_) = vis { - Some((ident.to_string(), expr.as_ref().clone())) - } else { - None + // #[cfg] attributes are not allowed on const declarations + if let Some(cfg_attr) = attrs.iter().find(|a| { + a.path + .get_ident() + .map(|i| i.to_string() == "cfg") + .unwrap_or(false) + }) { + return Err(syn::Error::new( + cfg_attr.span(), + "cfg attributes not allowed on this item")); } - } - _ => None, - }) - .collect(); + if let syn::Visibility::Public(_) = vis { + consts.push((ident.to_string(), expr.as_ref().clone())); + } + }, + _ => {}, + } + }; // Gather and parse submodule definitions. // // They are actually removed from the module's body, because they will need @@ -270,7 +279,6 @@ impl Parse for Module { } } } else { - consts = new_vec![]; fns = new_vec![]; } Ok(Module { diff --git a/codegen/ui_tests/module_cfg_const.rs b/codegen/ui_tests/module_cfg_const.rs new file mode 100644 index 00000000..599f52da --- /dev/null +++ b/codegen/ui_tests/module_cfg_const.rs @@ -0,0 +1,31 @@ +use rhai::plugin::*; + +#[derive(Clone)] +pub struct Point { + x: f32, + y: f32, +} + +#[export_module] +pub mod test_module { + use rhai::FLOAT; + + #[cfg(feature = "foo")] + pub const MAGIC: FLOAT = 42.0 as FLOAT; + + pub fn test_fn(input: Point) -> bool { + input.x > input.y + } +} + +fn main() { + let n = Point { + x: 0.0, + y: 10.0, + }; + if test_module::test_fn(n) { + println!("yes"); + } else { + println!("no"); + } +} diff --git a/codegen/ui_tests/module_cfg_const.stderr b/codegen/ui_tests/module_cfg_const.stderr new file mode 100644 index 00000000..4eaf1b06 --- /dev/null +++ b/codegen/ui_tests/module_cfg_const.stderr @@ -0,0 +1,11 @@ +error: cfg attributes not allowed on this item + --> $DIR/module_cfg_const.rs:13:5 + | +13 | #[cfg(feature = "foo")] + | ^^^^^^^^^^^^^^^^^^^^^^^ + +error[E0433]: failed to resolve: use of undeclared type or module `test_module` + --> $DIR/module_cfg_const.rs:26:8 + | +26 | if test_module::test_fn(n) { + | ^^^^^^^^^^^ use of undeclared type or module `test_module`