From ef0db6b9349f5d451c5ff204598501de93b3089d Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Thu, 28 Jul 2022 17:05:15 +0800 Subject: [PATCH 01/14] Copied the core concepts across from fj_plugins --- crates/fj/src/context.rs | 226 ++++++++++++++++++++++++++++++++++++++ crates/fj/src/host.rs | 50 +++++++++ crates/fj/src/lib.rs | 16 ++- crates/fj/src/metadata.rs | 222 +++++++++++++++++++++++++++++++++++++ crates/fj/src/model.rs | 23 ++++ 5 files changed, 536 insertions(+), 1 deletion(-) create mode 100644 crates/fj/src/context.rs create mode 100644 crates/fj/src/host.rs create mode 100644 crates/fj/src/metadata.rs create mode 100644 crates/fj/src/model.rs diff --git a/crates/fj/src/context.rs b/crates/fj/src/context.rs new file mode 100644 index 000000000..14b05c625 --- /dev/null +++ b/crates/fj/src/context.rs @@ -0,0 +1,226 @@ +use std::{ + collections::HashMap, + fmt::{self, Display, Formatter}, + str::FromStr, +}; + +/// Contextual information passed to a [`Model`][crate::Model] when it is being +/// initialized. +/// +/// Check out the [`ContextExt`] trait for some helper methods. +pub trait Context { + /// The arguments dictionary associated with this [`Context`]. + fn arguments(&self) -> &HashMap; +} + +impl Context for &'_ C { + fn arguments(&self) -> &HashMap { + (**self).arguments() + } +} + +impl Context for Box { + fn arguments(&self) -> &HashMap { + (**self).arguments() + } +} + +impl Context for std::rc::Rc { + fn arguments(&self) -> &HashMap { + (**self).arguments() + } +} + +impl Context for std::sync::Arc { + fn arguments(&self) -> &HashMap { + (**self).arguments() + } +} + +impl Context for HashMap { + fn arguments(&self) -> &HashMap { + self + } +} + +/// Extension methods for the [`Context`] type. +/// +/// By splitting these methods out into a separate trait, [`Context`] can stay +/// object-safe while allowing convenience methods that use generics. +pub trait ContextExt { + /// Get an argument that was passed to this model. + fn get_argument(&self, name: &str) -> Option<&str>; + + /// Get an argument, returning a [`MissingArgument`] error if it doesn't + /// exist. + fn get_required_argument( + &self, + name: &str, + ) -> Result<&str, MissingArgument>; + + /// Parse an argument from its string representation using [`FromStr`]. + fn parse_argument(&self, name: &str) -> Result + where + T: FromStr, + T::Err: std::error::Error + Send + Sync + 'static; + + /// Try to parse an argument, if it is present. + fn parse_optional_argument( + &self, + name: &str, + ) -> Result, ParseFailed> + where + T: FromStr, + T::Err: std::error::Error + Send + Sync + 'static; +} + +impl ContextExt for C { + fn get_argument(&self, name: &str) -> Option<&str> { + self.arguments().get(name).map(|s| s.as_str()) + } + + fn get_required_argument( + &self, + name: &str, + ) -> Result<&str, MissingArgument> { + self.get_argument(name).ok_or_else(|| MissingArgument { + name: name.to_string(), + }) + } + + fn parse_argument(&self, name: &str) -> Result + where + T: FromStr, + T::Err: std::error::Error + Send + Sync + 'static, + { + let value = self.get_required_argument(name)?; + + value + .parse() + .map_err(|e| ParseFailed { + name: name.to_string(), + value: value.to_string(), + error: Box::new(e), + }) + .map_err(ContextError::from) + } + + fn parse_optional_argument( + &self, + name: &str, + ) -> Result, ParseFailed> + where + T: FromStr, + T::Err: std::error::Error + Send + Sync + 'static, + { + let value = match self.get_argument(name) { + Some(value) => value, + None => return Ok(None), + }; + + let parsed = value.parse().map_err(|e| ParseFailed { + name: name.to_string(), + value: value.to_string(), + error: Box::new(e), + })?; + + Ok(Some(parsed)) + } +} + +/// An error that may be returned from a [`Context`] method. +#[derive(Debug)] +pub enum ContextError { + /// An argument was missing. + MissingArgument(MissingArgument), + /// An argument was present, but we were unable to parse it into the final + /// type. + ParseFailed(ParseFailed), +} + +impl From for ContextError { + fn from(m: MissingArgument) -> Self { + ContextError::MissingArgument(m) + } +} + +impl From for ContextError { + fn from(p: ParseFailed) -> Self { + ContextError::ParseFailed(p) + } +} + +impl Display for ContextError { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + match self { + ContextError::MissingArgument(_) => { + write!(f, "An argument was missing") + } + ContextError::ParseFailed(_) => { + write!(f, "Unable to parse an argument") + } + } + } +} + +impl std::error::Error for ContextError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + ContextError::MissingArgument(m) => Some(m), + ContextError::ParseFailed(p) => Some(p), + } + } +} + +/// The error returned when a required argument wasn't provided. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct MissingArgument { + /// The argument's name. + pub name: String, +} + +impl Display for MissingArgument { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + let MissingArgument { name } = self; + + write!(f, "The \"{name}\" argument was missing") + } +} + +impl std::error::Error for MissingArgument {} + +/// The error returned when [`ContextExt::parse_argument()`] is unable to parse +/// the argument's value. +#[derive(Debug)] +pub struct ParseFailed { + /// The argument's name. + pub name: String, + /// The actual value. + pub value: String, + /// The error that occurred. + pub error: Box, +} + +impl Display for ParseFailed { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + let ParseFailed { name, value, .. } = self; + + write!(f, "Unable to parse the \"{name}\" argument (\"{value:?}\")") + } +} + +impl std::error::Error for ParseFailed { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + Some(&*self.error) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn context_is_object_safe() { + let _: &dyn Context; + } +} diff --git a/crates/fj/src/host.rs b/crates/fj/src/host.rs new file mode 100644 index 000000000..fbd8154bb --- /dev/null +++ b/crates/fj/src/host.rs @@ -0,0 +1,50 @@ +use crate::Model; + +/// An abstract interface to the Fornjot host. +pub trait Host { + /// Register a model. + /// + /// This is mainly for more advanced use cases (e.g. when you need to close + /// over extra state to load the model). For simpler models, you probably + /// want to use [`HostExt::register_model()`] instead. + fn register_boxed_model(&mut self, model: Box); +} + +impl Host for &'_ mut H { + fn register_boxed_model(&mut self, model: Box) { + (*self).register_boxed_model(model); + } +} + +impl Host for Box { + fn register_boxed_model(&mut self, model: Box) { + (**self).register_boxed_model(model); + } +} + +/// Extension methods to augment the [`Host`] API. +pub trait HostExt { + /// Register a model with the Fornjot runtime. + fn register_model(&mut self, model: M) + where + M: Model + 'static; +} + +impl HostExt for H { + fn register_model(&mut self, model: M) + where + M: Model + 'static, + { + self.register_boxed_model(Box::new(model)); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn host_is_object_safe() { + let _: &dyn Host; + } +} diff --git a/crates/fj/src/lib.rs b/crates/fj/src/lib.rs index 74a717750..b234c9a5e 100644 --- a/crates/fj/src/lib.rs +++ b/crates/fj/src/lib.rs @@ -21,13 +21,27 @@ pub mod syntax; mod angle; +mod context; mod group; +mod host; +mod metadata; +mod model; mod shape_2d; mod sweep; mod transform; pub use self::{ - angle::*, group::Group, shape_2d::*, sweep::Sweep, transform::Transform, + angle::*, + context::{ + Context, ContextError, ContextExt, MissingArgument, ParseFailed, + }, + group::Group, + host::{Host, HostExt}, + metadata::{ArgumentMetadata, ModelMetadata, PluginMetadata}, + model::Model, + shape_2d::*, + sweep::Sweep, + transform::Transform, }; pub use fj_proc::*; #[cfg(feature = "serde")] diff --git a/crates/fj/src/metadata.rs b/crates/fj/src/metadata.rs new file mode 100644 index 000000000..4ae59231f --- /dev/null +++ b/crates/fj/src/metadata.rs @@ -0,0 +1,222 @@ +/// Information about a particular plugin that can be used by the host for +/// things like introspection and search. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PluginMetadata { + /// A short, human-friendly name used to identify this plugin. + pub name: String, + /// A semver-compliant version number for the plugin. + pub version: String, + /// A short, one-line description of what the plugin does. + pub short_description: Option, + /// A more elaborate description of what the plugin does. + pub description: Option, + /// A link to the plugin's homepage. + pub homepage: Option, + /// A link to the plugin's source code. + pub repository: Option, + /// The name of the software license(s) this plugin is released under. + /// + /// This is interpreted as a SPDX license expression (e.g. `MIT OR + /// Apache-2.0`). See [the SPDX site][spdx] for more information. + /// + /// [spdx]: https://spdx.dev/spdx-specification-21-web-version/#h.jxpfx0ykyb60 + pub license: Option, +} + +impl PluginMetadata { + /// Create metadata for a plugin. + /// + /// # Panics + /// + /// The `name` and `version` fields must not be empty. + pub fn new(name: impl Into, version: impl Into) -> Self { + let name = name.into(); + assert!(!name.is_empty()); + let version = version.into(); + assert!(!version.is_empty()); + + PluginMetadata { + name, + version, + short_description: None, + description: None, + homepage: None, + repository: None, + license: None, + } + } + + /// Set the [`PluginMetadata::short_description`] field. + pub fn set_short_description( + self, + short_description: impl Into, + ) -> Self { + let short_description = short_description.into(); + if short_description.is_empty() { + return self; + } + + PluginMetadata { + short_description: Some(short_description), + ..self + } + } + + /// Set the [`PluginMetadata::description`] field. + pub fn set_description(self, description: impl Into) -> Self { + let description = description.into(); + if description.is_empty() { + return self; + } + + PluginMetadata { + description: Some(description), + ..self + } + } + + /// Set the [`PluginMetadata::homepage`] field. + pub fn set_homepage(self, homepage: impl Into) -> Self { + let homepage = homepage.into(); + if homepage.is_empty() { + return self; + } + + PluginMetadata { + homepage: Some(homepage), + ..self + } + } + + /// Set the [`PluginMetadata::repository`] field. + pub fn set_repository(self, repository: impl Into) -> Self { + let repository = repository.into(); + if repository.is_empty() { + return self; + } + + PluginMetadata { + repository: Some(repository), + ..self + } + } + + /// Set the [`PluginMetadata::license`] field. + pub fn set_license(self, license: impl Into) -> Self { + let license = license.into(); + if license.is_empty() { + return self; + } + + PluginMetadata { + license: Some(license), + ..self + } + } +} + +/// Metadata about a [`crate::Model`]. +#[derive(Debug, Clone, PartialEq)] +pub struct ModelMetadata { + /// A short, human-friendly name used to identify this model. + pub name: String, + /// A description of what this model does. + pub description: Option, + /// Arguments that the model uses when calculating its geometry. + pub arguments: Vec, +} + +impl ModelMetadata { + /// Create metadata for a model. + /// + /// # Panics + /// + /// The `name` must not be empty. + pub fn new(name: impl Into) -> Self { + let name = name.into(); + assert!(!name.is_empty()); + + ModelMetadata { + name, + description: None, + arguments: Vec::new(), + } + } + + /// Set the [`ModelMetadata::description`]. + pub fn with_description(self, description: impl Into) -> Self { + let description = description.into(); + if description.is_empty() { + return self; + } + + ModelMetadata { + description: Some(description), + ..self + } + } + + /// Add an argument to the [`ModelMetadata::arguments`] list. + /// + /// As a convenience, string literals can be automatically converted into + /// [`ArgumentMetadata`] with no description or default value. + pub fn with_argument(mut self, arg: impl Into) -> Self { + self.arguments.push(arg.into()); + self + } +} + +/// Metadata describing a model's argument. +#[derive(Debug, Clone, PartialEq)] +pub struct ArgumentMetadata { + /// The name used to refer to this argument. + pub name: String, + /// A short description of this argument that could be shown to the user + /// in something like a tooltip. + pub description: Option, + /// Something that could be used as a default if no value was provided. + pub default_value: Option, +} + +impl ArgumentMetadata { + /// Create a new [`ArgumentMetadata`]. + /// + /// # Panics + /// + /// The `name` must not be empty. + pub fn new(name: impl Into) -> Self { + let name = name.into(); + assert!(!name.is_empty()); + ArgumentMetadata { + name, + description: None, + default_value: None, + } + } + + /// Set the [`ArgumentMetadata::description`]. + pub fn with_description(mut self, description: impl Into) -> Self { + let description = description.into(); + if description.is_empty() { + return self; + } + + self.description = Some(description); + self + } + + /// Set the [`ArgumentMetadata::default_value`]. + pub fn with_default_value( + mut self, + default_value: impl Into, + ) -> Self { + self.default_value = Some(default_value.into()); + self + } +} + +impl From<&str> for ArgumentMetadata { + fn from(name: &str) -> Self { + ArgumentMetadata::new(name) + } +} diff --git a/crates/fj/src/model.rs b/crates/fj/src/model.rs new file mode 100644 index 000000000..da82c65d7 --- /dev/null +++ b/crates/fj/src/model.rs @@ -0,0 +1,23 @@ +use crate::{Context, ModelMetadata, Shape}; + +/// A model. +pub trait Model: Send + Sync { + /// Calculate this model's concrete geometry. + fn shape( + &self, + ctx: &dyn Context, + ) -> Result>; + + /// Get metadata for the model. + fn metadata(&self) -> ModelMetadata; +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn model_is_object_safe() { + let _: &dyn Model; + } +} From ed7a3cc1b61cd46facf3a629eb2f646e574e7461 Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Thu, 28 Jul 2022 21:05:29 +0800 Subject: [PATCH 02/14] Put all guest-host code inside its own "fj::abi" module --- crates/fj/src/abi/context.rs | 55 +++++++ crates/fj/src/abi/host.rs | 47 ++++++ crates/fj/src/abi/metadata.rs | 35 +++++ crates/fj/src/abi/mod.rs | 68 ++++++++ crates/fj/src/abi/model.rs | 98 ++++++++++++ crates/fj/src/abi/wrappers.rs | 283 ++++++++++++++++++++++++++++++++++ crates/fj/src/context.rs | 31 ++-- crates/fj/src/lib.rs | 2 + 8 files changed, 600 insertions(+), 19 deletions(-) create mode 100644 crates/fj/src/abi/context.rs create mode 100644 crates/fj/src/abi/host.rs create mode 100644 crates/fj/src/abi/metadata.rs create mode 100644 crates/fj/src/abi/mod.rs create mode 100644 crates/fj/src/abi/model.rs create mode 100644 crates/fj/src/abi/wrappers.rs diff --git a/crates/fj/src/abi/context.rs b/crates/fj/src/abi/context.rs new file mode 100644 index 000000000..a36a94c5a --- /dev/null +++ b/crates/fj/src/abi/context.rs @@ -0,0 +1,55 @@ +use std::{marker::PhantomData, os::raw::c_void, panic::AssertUnwindSafe}; + +use crate::abi::wrappers::StringSlice; + +#[repr(C)] +pub struct Context<'a> { + user_data: *const c_void, + get_argument: + unsafe extern "C" fn(*const c_void, StringSlice) -> StringSlice, + _lifetime: PhantomData<&'a ()>, +} + +impl<'a> From<&'a &dyn crate::Context> for Context<'a> { + fn from(ctx: &'a &dyn crate::Context) -> Self { + unsafe extern "C" fn get_argument( + user_data: *const c_void, + name: StringSlice, + ) -> StringSlice { + let ctx = &*(user_data as *const &dyn crate::Context); + + match std::panic::catch_unwind(AssertUnwindSafe(|| { + ctx.get_argument(&*name) + })) { + Ok(Some(arg)) => StringSlice::from_str(arg), + Ok(None) => StringSlice::from_str(""), + Err(payload) => crate::abi::on_panic(payload), + } + } + + Context { + user_data: ctx as *const &dyn crate::Context as *const c_void, + get_argument, + _lifetime: PhantomData, + } + } +} + +impl crate::Context for Context<'_> { + fn get_argument(&self, name: &str) -> Option<&str> { + unsafe { + let Context { + user_data, + get_argument, + _lifetime, + } = *self; + + let name = StringSlice::from_str(name); + + match get_argument(user_data, name).into_str() { + "" => None, + other => Some(other), + } + } + } +} diff --git a/crates/fj/src/abi/host.rs b/crates/fj/src/abi/host.rs new file mode 100644 index 000000000..ca3500d1c --- /dev/null +++ b/crates/fj/src/abi/host.rs @@ -0,0 +1,47 @@ +use std::{marker::PhantomData, os::raw::c_void, panic::AssertUnwindSafe}; + +use crate::abi::Model; + +/// A FFI-safe `&mut dyn Host`. +pub struct Host<'a> { + user_data: *mut c_void, + register_boxed_model: unsafe extern "C" fn(*mut c_void, model: Model), + _lifetime: PhantomData<&'a mut ()>, +} + +impl<'a, H: crate::Host + Sized> From<&'a mut H> for Host<'a> { + fn from(host: &'a mut H) -> Self { + extern "C" fn register_boxed_model( + user_data: *mut c_void, + model: Model, + ) { + let host = unsafe { &mut *(user_data as *mut H) }; + + if let Err(e) = std::panic::catch_unwind(AssertUnwindSafe(|| { + host.register_boxed_model(Box::new(model)) + })) { + crate::abi::on_panic(e); + } + } + + Host { + user_data: host as *mut H as *mut c_void, + register_boxed_model: register_boxed_model::, + _lifetime: PhantomData, + } + } +} + +impl<'a> crate::Host for Host<'a> { + fn register_boxed_model(&mut self, model: Box) { + let Host { + user_data, + register_boxed_model, + .. + } = *self; + + unsafe { + register_boxed_model(user_data, model.into()); + } + } +} diff --git a/crates/fj/src/abi/metadata.rs b/crates/fj/src/abi/metadata.rs new file mode 100644 index 000000000..76b54274b --- /dev/null +++ b/crates/fj/src/abi/metadata.rs @@ -0,0 +1,35 @@ +use crate::abi::FfiSafeString; + +#[derive(Debug)] +#[repr(C)] +pub struct ModelMetadata { + pub name: FfiSafeString, +} + +impl From for crate::ModelMetadata { + fn from(_m: ModelMetadata) -> Self { + todo!() + } +} + +impl From for ModelMetadata { + fn from(_m: crate::ModelMetadata) -> Self { + todo!() + } +} + +#[derive(Debug)] +#[repr(C)] +pub struct PluginMetadata {} + +impl From for crate::PluginMetadata { + fn from(_m: PluginMetadata) -> Self { + todo!() + } +} + +impl From for PluginMetadata { + fn from(_m: crate::PluginMetadata) -> Self { + todo!() + } +} diff --git a/crates/fj/src/abi/mod.rs b/crates/fj/src/abi/mod.rs new file mode 100644 index 000000000..3c355f05a --- /dev/null +++ b/crates/fj/src/abi/mod.rs @@ -0,0 +1,68 @@ +//! Internal implementation details for the host-guest interface. +//! +//! Note that the vast majority of this module is just providing FFI-safe +//! versions of common `std` types (e.g. `Vec`, `String`, and `Box`), +//! or FFI-safe trait objects. + +mod context; +mod host; +mod metadata; +mod model; +mod wrappers; + +use std::any::Any; + +pub use self::{ + context::Context, + host::Host, + metadata::{ModelMetadata, PluginMetadata}, + model::Model, + wrappers::{BoxedError, FfiSafeResult, FfiSafeString, FfiSafeVec}, +}; + +#[macro_export] +macro_rules! register_model { + ($init:expr) => { + #[no_mangle] + unsafe extern "C" fn fj_model_init( + mut host: $crate::abi::Host<'_>, + ) -> $crate::abi::InitResult { + let host: &mut dyn $crate::Host = &mut host; + + let result: Result< + $crate::PluginMetadata, + Box, + > = $init(host); + + match result { + Ok(meta) => $crate::abi::InitResult::Ok(meta.into()), + Err(e) => $crate::abi::InitResult::Err(e.into()), + } + } + }; +} + +/// The signature of the function generated by [`register_model`]. +/// +/// ```rust +/// fj::register_model!(|_| { todo!() }); +/// +/// const _: fj::abi::InitFunction = fj_model_init; +/// ``` +pub type InitFunction = unsafe extern "C" fn(Host<'_>) -> InitResult; +pub type InitResult = FfiSafeResult; +pub type ShapeResult = FfiSafeResult; + +fn on_panic(payload: Box) -> ! { + let msg: &str = if let Some(s) = payload.downcast_ref::() { + s.as_str() + } else if let Some(s) = payload.downcast_ref::<&str>() { + *s + } else { + "A panic occurred" + }; + + eprintln!("{msg}"); + // It's not ideal, but panicking across the FFI boundary is UB. + std::process::abort(); +} diff --git a/crates/fj/src/abi/model.rs b/crates/fj/src/abi/model.rs new file mode 100644 index 000000000..f432d1d70 --- /dev/null +++ b/crates/fj/src/abi/model.rs @@ -0,0 +1,98 @@ +use std::{os::raw::c_void, panic::AssertUnwindSafe}; + +use crate::abi::{Context, ModelMetadata, ShapeResult}; + +#[repr(C)] +pub struct Model { + ptr: *mut c_void, + metadata: unsafe extern "C" fn(*mut c_void) -> ModelMetadata, + shape: unsafe extern "C" fn(*mut c_void, Context<'_>) -> ShapeResult, + free: unsafe extern "C" fn(*mut c_void), +} + +impl crate::Model for Model { + fn shape( + &self, + ctx: &dyn crate::Context, + ) -> Result> { + let ctx = Context::from(&ctx); + + let Model { ptr, shape, .. } = *self; + + let result = unsafe { shape(ptr, ctx) }; + + match result { + super::FfiSafeResult::Ok(shape) => Ok(shape), + super::FfiSafeResult::Err(err) => Err(err.into()), + } + } + + fn metadata(&self) -> crate::ModelMetadata { + let Model { ptr, metadata, .. } = *self; + + unsafe { metadata(ptr).into() } + } +} + +impl From> for Model { + fn from(m: Box) -> Self { + unsafe extern "C" fn metadata(user_data: *mut c_void) -> ModelMetadata { + let model = &*(user_data as *mut Box); + + match std::panic::catch_unwind(AssertUnwindSafe(|| { + model.metadata() + })) { + Ok(meta) => meta.into(), + Err(payload) => crate::abi::on_panic(payload), + } + } + + unsafe extern "C" fn shape( + user_data: *mut c_void, + ctx: Context<'_>, + ) -> ShapeResult { + let model = &*(user_data as *mut Box); + + match std::panic::catch_unwind(AssertUnwindSafe(|| { + model.shape(&ctx) + })) { + Ok(Ok(shape)) => ShapeResult::Ok(shape), + Ok(Err(err)) => ShapeResult::Err(err.into()), + Err(payload) => crate::abi::on_panic(payload), + } + } + + unsafe extern "C" fn free(user_data: *mut c_void) { + let model = user_data as *mut Box; + + if let Err(e) = std::panic::catch_unwind(AssertUnwindSafe(|| { + let model = Box::from_raw(model); + drop(model); + })) { + crate::abi::on_panic(e); + }; + } + + Model { + ptr: Box::into_raw(Box::new(m)).cast(), + metadata, + shape, + free, + } + } +} + +impl Drop for Model { + fn drop(&mut self) { + let Model { ptr, free, .. } = *self; + + unsafe { + free(ptr); + } + } +} + +// Safety: Our Model type is a FFI-safe version of Box, and +// Box: Send+Sync. +unsafe impl Send for Model {} +unsafe impl Sync for Model {} diff --git a/crates/fj/src/abi/wrappers.rs b/crates/fj/src/abi/wrappers.rs new file mode 100644 index 000000000..376fe9b90 --- /dev/null +++ b/crates/fj/src/abi/wrappers.rs @@ -0,0 +1,283 @@ +//! FFI-safe versions of common `std` types. + +use std::{ + alloc::{GlobalAlloc, Layout, System}, + fmt::{self, Debug, Display, Formatter}, + ops::Deref, + ptr::NonNull, +}; + +/// A FFI-safe version of `Vec`. +#[repr(C)] +pub struct FfiSafeVec { + ptr: NonNull, + len: usize, +} + +impl Debug for FfiSafeVec { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + write!(f, "{:?}", &**self) + } +} + +impl PartialEq for FfiSafeVec { + fn eq(&self, other: &Self) -> bool { + **self == **other + } +} + +impl From> for FfiSafeVec { + fn from(mut items: Vec) -> Self { + // Safety: To avoid accidental double-frees and other memory issues, we + // need to go through a specific dance. + unsafe { + // first, get a pointer to the first element and its length + let first_item = items.as_mut_ptr(); + let len = items.len(); + + // next, tell Vec to forget about these items so it won't try to + // run their destructors if we return early (e.g. via a panic). + // We've now taken over ownership of the items, but *not* the Vec's + // backing array. + items.set_len(0); + + // Use the system allocator to create some space for our + // FfiSafeVec's buffer. + let layout = Layout::array::(len).unwrap(); + let ptr: *mut T = System::default().alloc(layout).cast(); + let ptr = NonNull::new(ptr).expect("Allocation failed"); + + // Now, we can copy the items across + std::ptr::copy_nonoverlapping(first_item, ptr.as_ptr(), len); + + // the items are gone, time to free the original vec + drop(items); + + FfiSafeVec { ptr, len } + } + } +} + +impl From> for Box<[T]> { + fn from(v: FfiSafeVec) -> Self { + Box::from(&*v) + } +} + +impl FromIterator for FfiSafeVec { + fn from_iter>(iter: I) -> Self { + let vec: Vec = iter.into_iter().collect(); + vec.into() + } +} + +impl Deref for FfiSafeVec { + type Target = [T]; + + fn deref(&self) -> &Self::Target { + // Safety: We control "ptr" and "len", so we know they are always + // initialized and within bounds. + unsafe { + let FfiSafeVec { ptr, len } = *self; + std::slice::from_raw_parts(ptr.as_ptr(), len) + } + } +} + +impl Drop for FfiSafeVec { + fn drop(&mut self) { + let FfiSafeVec { ptr, len } = *self; + let ptr = ptr.as_ptr(); + + for i in 0..self.len { + // Safety: We control the "len" field, so the item we're accessing + // is always within bounds. We also don't touch values after their + // destructors are called. + unsafe { + let item = ptr.add(i); + std::ptr::drop_in_place(item); + } + } + + // Safety: This vec is immutable, so we're using the same layout as the + // original allocation. It's also not possible to touch the allocation + // after Drop completes. + unsafe { + let layout = Layout::array::(len).unwrap(); + System::default().dealloc(ptr.cast(), layout); + } + } +} + +// Safety: We're Send+Sync as long as the underlying type is. +unsafe impl Send for FfiSafeVec {} +unsafe impl Sync for FfiSafeVec {} + +/// A FFI-safe version of `Box`. +#[repr(transparent)] +#[derive(Debug, PartialEq)] +pub struct FfiSafeString(FfiSafeVec); + +impl From for FfiSafeString { + fn from(s: String) -> Self { + FfiSafeString(s.into_bytes().into()) + } +} + +impl From for Box { + fn from(s: FfiSafeString) -> Self { + Box::from(&*s) + } +} + +impl Display for FfiSafeString { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + Display::fmt(&**self, f) + } +} + +impl Deref for FfiSafeString { + type Target = str; + + fn deref(&self) -> &Self::Target { + // Safety: The only way to create a FfiSafeString is from a valid Rust + // string, so we can skip the UTF-8 checks. + unsafe { std::str::from_utf8_unchecked(&*self.0) } + } +} + +/// A version of `Result` that is `#[repr(C)]`. +#[repr(C)] +pub enum FfiSafeResult { + Ok(T), + Err(E), +} + +impl From> for FfiSafeResult { + fn from(result: Result) -> Self { + match result { + Ok(ok) => FfiSafeResult::Ok(ok), + Err(err) => FfiSafeResult::Err(err), + } + } +} + +impl From> for Result { + fn from(result: FfiSafeResult) -> Self { + match result { + FfiSafeResult::Ok(ok) => Result::Ok(ok), + FfiSafeResult::Err(err) => Result::Err(err), + } + } +} + +#[repr(C)] +pub(crate) struct Slice { + ptr: NonNull, + len: usize, +} + +impl Slice { + /// Create a new [`Slice`] from a slice. + /// + /// # Safety + /// + /// It is the caller's responsibility to make sure this [`Slice`] doesn't + /// outlive the slice that was passed in. + pub unsafe fn from_slice(items: &[T]) -> Self { + let ptr = items.as_ptr(); + let len = items.len(); + Slice { + // Safety: It's okay to cast away the const because you can't mutate + // a slice. + ptr: NonNull::new(ptr as *mut T).unwrap(), + len, + } + } + + pub unsafe fn into_slice<'a>(self) -> &'a [T] { + let Slice { ptr, len } = self; + std::slice::from_raw_parts(ptr.as_ptr(), len) + } +} + +impl Debug for Slice { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + Debug::fmt(&**self, f) + } +} + +impl PartialEq for Slice { + fn eq(&self, other: &Self) -> bool { + **self == **other + } +} + +impl Deref for Slice { + type Target = [T]; + + fn deref(&self) -> &Self::Target { + // Safety: We control both "ptr" and "len", so the array is always + // initialized and within bounds. + // + // The lifetime of the &[T] is also bound to the lifetime of &self, so + // this should be safe as long as people can never get a Slice that + // outlives the data it points to. + unsafe { + let Slice { ptr, len, .. } = *self; + std::slice::from_raw_parts(ptr.as_ptr(), len) + } + } +} + +#[repr(transparent)] +pub(crate) struct StringSlice(Slice); + +impl StringSlice { + /// Create a new [`StringSlice`]. + /// + /// # Safety + /// + /// It is the caller's responsibility to make sure this [`Slice`] doesn't + /// outlive the slice that was passed in. + pub unsafe fn from_str(s: &str) -> StringSlice { + StringSlice(Slice::from_slice(s.as_bytes())) + } + + pub unsafe fn into_str<'a>(self) -> &'a str { + let bytes = self.0.into_slice(); + std::str::from_utf8_unchecked(bytes) + } +} + +impl Deref for StringSlice { + type Target = str; + + fn deref(&self) -> &Self::Target { + // Safety: the only way you can construct a StringSlice is via a string. + unsafe { std::str::from_utf8_unchecked(&*self.0) } + } +} + +#[derive(Debug)] +#[repr(C)] +pub struct BoxedError { + msg: FfiSafeString, +} + +impl Display for BoxedError { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + Display::fmt(&self.msg, f) + } +} + +impl std::error::Error for BoxedError {} + +impl From> for BoxedError { + fn from(err: Box) -> Self { + // TODO: is it worth capturing the message from each source error, too? + BoxedError { + msg: err.to_string().into(), + } + } +} diff --git a/crates/fj/src/context.rs b/crates/fj/src/context.rs index 14b05c625..e893a128e 100644 --- a/crates/fj/src/context.rs +++ b/crates/fj/src/context.rs @@ -9,37 +9,37 @@ use std::{ /// /// Check out the [`ContextExt`] trait for some helper methods. pub trait Context { - /// The arguments dictionary associated with this [`Context`]. - fn arguments(&self) -> &HashMap; + /// Get an argument that was passed to this model. + fn get_argument(&self, name: &str) -> Option<&str>; } impl Context for &'_ C { - fn arguments(&self) -> &HashMap { - (**self).arguments() + fn get_argument(&self, name: &str) -> Option<&str> { + (**self).get_argument(name) } } impl Context for Box { - fn arguments(&self) -> &HashMap { - (**self).arguments() + fn get_argument(&self, name: &str) -> Option<&str> { + (**self).get_argument(name) } } impl Context for std::rc::Rc { - fn arguments(&self) -> &HashMap { - (**self).arguments() + fn get_argument(&self, name: &str) -> Option<&str> { + (**self).get_argument(name) } } impl Context for std::sync::Arc { - fn arguments(&self) -> &HashMap { - (**self).arguments() + fn get_argument(&self, name: &str) -> Option<&str> { + (**self).get_argument(name) } } impl Context for HashMap { - fn arguments(&self) -> &HashMap { - self + fn get_argument(&self, name: &str) -> Option<&str> { + self.get(name).map(|s| s.as_str()) } } @@ -48,9 +48,6 @@ impl Context for HashMap { /// By splitting these methods out into a separate trait, [`Context`] can stay /// object-safe while allowing convenience methods that use generics. pub trait ContextExt { - /// Get an argument that was passed to this model. - fn get_argument(&self, name: &str) -> Option<&str>; - /// Get an argument, returning a [`MissingArgument`] error if it doesn't /// exist. fn get_required_argument( @@ -75,10 +72,6 @@ pub trait ContextExt { } impl ContextExt for C { - fn get_argument(&self, name: &str) -> Option<&str> { - self.arguments().get(name).map(|s| s.as_str()) - } - fn get_required_argument( &self, name: &str, diff --git a/crates/fj/src/lib.rs b/crates/fj/src/lib.rs index b234c9a5e..4f5d060ca 100644 --- a/crates/fj/src/lib.rs +++ b/crates/fj/src/lib.rs @@ -20,6 +20,8 @@ pub mod syntax; +#[doc(hidden)] +pub mod abi; mod angle; mod context; mod group; From 95b3bfee1df757adf10e5b6989cd205b87573ab2 Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Thu, 28 Jul 2022 22:16:03 +0800 Subject: [PATCH 03/14] Convert metadata and rename the wrappers module to ffi_safe --- crates/fj/src/abi/context.rs | 2 +- .../fj/src/abi/{wrappers.rs => ffi_safe.rs} | 144 +++++++++++++----- crates/fj/src/abi/host.rs | 1 + crates/fj/src/abi/metadata.rs | 116 ++++++++++++-- crates/fj/src/abi/mod.rs | 67 ++++++-- crates/fj/src/abi/model.rs | 4 +- crates/fj/src/lib.rs | 1 + 7 files changed, 273 insertions(+), 62 deletions(-) rename crates/fj/src/abi/{wrappers.rs => ffi_safe.rs} (68%) diff --git a/crates/fj/src/abi/context.rs b/crates/fj/src/abi/context.rs index a36a94c5a..03e641b0d 100644 --- a/crates/fj/src/abi/context.rs +++ b/crates/fj/src/abi/context.rs @@ -1,6 +1,6 @@ use std::{marker::PhantomData, os::raw::c_void, panic::AssertUnwindSafe}; -use crate::abi::wrappers::StringSlice; +use crate::abi::ffi_safe::StringSlice; #[repr(C)] pub struct Context<'a> { diff --git a/crates/fj/src/abi/wrappers.rs b/crates/fj/src/abi/ffi_safe.rs similarity index 68% rename from crates/fj/src/abi/wrappers.rs rename to crates/fj/src/abi/ffi_safe.rs index 376fe9b90..957005e06 100644 --- a/crates/fj/src/abi/wrappers.rs +++ b/crates/fj/src/abi/ffi_safe.rs @@ -9,25 +9,25 @@ use std::{ /// A FFI-safe version of `Vec`. #[repr(C)] -pub struct FfiSafeVec { +pub(crate) struct Vec { ptr: NonNull, len: usize, } -impl Debug for FfiSafeVec { +impl Debug for Vec { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { write!(f, "{:?}", &**self) } } -impl PartialEq for FfiSafeVec { +impl PartialEq for Vec { fn eq(&self, other: &Self) -> bool { **self == **other } } -impl From> for FfiSafeVec { - fn from(mut items: Vec) -> Self { +impl From> for Vec { + fn from(mut items: std::vec::Vec) -> Self { // Safety: To avoid accidental double-frees and other memory issues, we // need to go through a specific dance. unsafe { @@ -53,40 +53,52 @@ impl From> for FfiSafeVec { // the items are gone, time to free the original vec drop(items); - FfiSafeVec { ptr, len } + Vec { ptr, len } } } } -impl From> for Box<[T]> { - fn from(v: FfiSafeVec) -> Self { +impl From> for std::vec::Vec { + fn from(v: Vec) -> Self { + v.iter().map(Clone::clone).collect() + } +} + +impl Clone for Vec { + fn clone(&self) -> Self { + self.iter().cloned().collect() + } +} + +impl From> for Box<[T]> { + fn from(v: Vec) -> Self { Box::from(&*v) } } -impl FromIterator for FfiSafeVec { +impl FromIterator for Vec { fn from_iter>(iter: I) -> Self { - let vec: Vec = iter.into_iter().collect(); + let vec: std::vec::Vec = iter.into_iter().collect(); vec.into() } } -impl Deref for FfiSafeVec { +impl Deref for Vec { type Target = [T]; fn deref(&self) -> &Self::Target { // Safety: We control "ptr" and "len", so we know they are always // initialized and within bounds. unsafe { - let FfiSafeVec { ptr, len } = *self; + let Vec { ptr, len } = *self; std::slice::from_raw_parts(ptr.as_ptr(), len) } } } -impl Drop for FfiSafeVec { +impl Drop for Vec { fn drop(&mut self) { - let FfiSafeVec { ptr, len } = *self; + let Vec { ptr, len } = *self; let ptr = ptr.as_ptr(); for i in 0..self.len { @@ -110,33 +122,50 @@ impl Drop for FfiSafeVec { } // Safety: We're Send+Sync as long as the underlying type is. -unsafe impl Send for FfiSafeVec {} -unsafe impl Sync for FfiSafeVec {} +unsafe impl Send for Vec {} +unsafe impl Sync for Vec {} /// A FFI-safe version of `Box`. #[repr(transparent)] -#[derive(Debug, PartialEq)] -pub struct FfiSafeString(FfiSafeVec); +#[derive(Debug, PartialEq, Clone)] +pub struct String(Vec); + +impl From for String { + fn from(s: std::string::String) -> Self { + String(s.into_bytes().into()) + } +} -impl From for FfiSafeString { +impl From for std::string::String { fn from(s: String) -> Self { - FfiSafeString(s.into_bytes().into()) + s.to_string() } } -impl From for Box { - fn from(s: FfiSafeString) -> Self { +impl From for Box { + fn from(s: String) -> Self { Box::from(&*s) } } +impl PartialEq for String { + fn eq(&self, other: &str) -> bool { + **self == *other + } +} -impl Display for FfiSafeString { +impl PartialEq<&str> for String { + fn eq(&self, other: &&str) -> bool { + *self == **other + } +} + +impl Display for String { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { Display::fmt(&**self, f) } } -impl Deref for FfiSafeString { +impl Deref for String { type Target = str; fn deref(&self) -> &Self::Target { @@ -147,26 +176,36 @@ impl Deref for FfiSafeString { } /// A version of `Result` that is `#[repr(C)]`. +#[must_use] #[repr(C)] -pub enum FfiSafeResult { +pub enum Result { Ok(T), Err(E), } -impl From> for FfiSafeResult { - fn from(result: Result) -> Self { +impl Result { + pub fn unwrap(self) -> T { + match self { + Result::Ok(value) => value, + Result::Err(e) => panic!("Unwrapped an Err({e:?})"), + } + } +} + +impl From> for Result { + fn from(result: std::result::Result) -> Self { match result { - Ok(ok) => FfiSafeResult::Ok(ok), - Err(err) => FfiSafeResult::Err(err), + Ok(ok) => Result::Ok(ok), + Err(err) => Result::Err(err), } } } -impl From> for Result { - fn from(result: FfiSafeResult) -> Self { +impl From> for std::result::Result { + fn from(result: Result) -> Self { match result { - FfiSafeResult::Ok(ok) => Result::Ok(ok), - FfiSafeResult::Err(err) => Result::Err(err), + Result::Ok(ok) => std::result::Result::Ok(ok), + Result::Err(err) => std::result::Result::Err(err), } } } @@ -262,7 +301,7 @@ impl Deref for StringSlice { #[derive(Debug)] #[repr(C)] pub struct BoxedError { - msg: FfiSafeString, + msg: String, } impl Display for BoxedError { @@ -281,3 +320,40 @@ impl From> for BoxedError { } } } + +#[derive(Debug, Clone)] +#[repr(C)] +pub enum Option { + Some(T), + None, +} + +impl Option { + pub fn map(self, func: impl FnOnce(T) -> T2) -> Option { + match self { + Option::Some(value) => Option::Some(func(value)), + Option::None => Option::None, + } + } +} + +impl From> for Option +where + T1: Into, +{ + fn from(opt: std::option::Option) -> Self { + match opt { + Some(value) => Option::Some(value.into()), + None => Option::None, + } + } +} + +impl From> for std::option::Option { + fn from(opt: Option) -> Self { + match opt { + Option::Some(value) => Some(value), + Option::None => None, + } + } +} diff --git a/crates/fj/src/abi/host.rs b/crates/fj/src/abi/host.rs index ca3500d1c..e129a0d66 100644 --- a/crates/fj/src/abi/host.rs +++ b/crates/fj/src/abi/host.rs @@ -3,6 +3,7 @@ use std::{marker::PhantomData, os::raw::c_void, panic::AssertUnwindSafe}; use crate::abi::Model; /// A FFI-safe `&mut dyn Host`. +#[repr(C)] pub struct Host<'a> { user_data: *mut c_void, register_boxed_model: unsafe extern "C" fn(*mut c_void, model: Model), diff --git a/crates/fj/src/abi/metadata.rs b/crates/fj/src/abi/metadata.rs index 76b54274b..48c081104 100644 --- a/crates/fj/src/abi/metadata.rs +++ b/crates/fj/src/abi/metadata.rs @@ -1,14 +1,26 @@ -use crate::abi::FfiSafeString; +use crate::abi::ffi_safe; #[derive(Debug)] #[repr(C)] pub struct ModelMetadata { - pub name: FfiSafeString, + name: ffi_safe::String, + description: ffi_safe::Option, + arguments: ffi_safe::Vec, } impl From for crate::ModelMetadata { - fn from(_m: ModelMetadata) -> Self { - todo!() + fn from(m: ModelMetadata) -> Self { + let ModelMetadata { + name, + description, + arguments, + } = m; + + crate::ModelMetadata { + name: name.into(), + description: description.map(Into::into).into(), + arguments: arguments.iter().cloned().map(|a| a.into()).collect(), + } } } @@ -18,18 +30,102 @@ impl From for ModelMetadata { } } -#[derive(Debug)] +#[derive(Debug, Clone)] #[repr(C)] -pub struct PluginMetadata {} +pub struct PluginMetadata { + name: ffi_safe::String, + version: ffi_safe::String, + short_description: ffi_safe::Option, + description: ffi_safe::Option, + homepage: ffi_safe::Option, + repository: ffi_safe::Option, + license: ffi_safe::Option, +} impl From for crate::PluginMetadata { - fn from(_m: PluginMetadata) -> Self { - todo!() + fn from(m: PluginMetadata) -> Self { + let PluginMetadata { + name, + version, + short_description, + description, + homepage, + repository, + license, + } = m; + + crate::PluginMetadata { + name: name.into(), + version: version.into(), + short_description: short_description.map(Into::into).into(), + description: description.map(Into::into).into(), + homepage: homepage.map(Into::into).into(), + repository: repository.map(Into::into).into(), + license: license.map(Into::into).into(), + } } } impl From for PluginMetadata { - fn from(_m: crate::PluginMetadata) -> Self { - todo!() + fn from(m: crate::PluginMetadata) -> Self { + let crate::PluginMetadata { + name, + version, + short_description, + description, + homepage, + repository, + license, + } = m; + + PluginMetadata { + name: name.into(), + version: version.into(), + short_description: short_description.into(), + description: description.into(), + homepage: homepage.into(), + repository: repository.into(), + license: license.into(), + } + } +} + +#[derive(Debug, Clone)] +#[repr(C)] +pub struct ArgumentMetadata { + name: ffi_safe::String, + description: ffi_safe::Option, + default_value: ffi_safe::Option, +} + +impl From for ArgumentMetadata { + fn from(meta: crate::ArgumentMetadata) -> Self { + let crate::ArgumentMetadata { + name, + description, + default_value, + } = meta; + + ArgumentMetadata { + name: name.into(), + description: description.into(), + default_value: default_value.into(), + } + } +} + +impl From for crate::ArgumentMetadata { + fn from(meta: ArgumentMetadata) -> Self { + let ArgumentMetadata { + name, + description, + default_value, + } = meta; + + crate::ArgumentMetadata { + name: name.into(), + description: description.map(Into::into).into(), + default_value: default_value.map(Into::into).into(), + } } } diff --git a/crates/fj/src/abi/mod.rs b/crates/fj/src/abi/mod.rs index 3c355f05a..644d26023 100644 --- a/crates/fj/src/abi/mod.rs +++ b/crates/fj/src/abi/mod.rs @@ -3,12 +3,45 @@ //! Note that the vast majority of this module is just providing FFI-safe //! versions of common `std` types (e.g. `Vec`, `String`, and `Box`), //! or FFI-safe trait objects. - +//! +/// If the macro generated the wrong code, this doctest would fail. +/// +/// ```rust +/// use fj::{abi::INIT_FUNCTION_NAME, PluginMetadata}; +/// +/// fj::register_model!(|_| { +/// Ok(PluginMetadata::new("Plugin", "1.2.3")) +/// }); +/// +/// mod x { +/// extern "C" { +/// pub fn fj_model_init(_: *mut fj::abi::Host<'_>) -> fj::abi::InitResult; +/// } +/// } +/// +/// // make sure our function has the right signature +/// let func: fj::abi::InitFunction = fj_model_init; +/// +/// // We can also make sure the unmangled name is correct by calling it. +/// +/// let metadata: fj::PluginMetadata = unsafe { +/// let mut host = Host; +/// let mut host = fj::abi::Host::from(&mut host); +/// x::fj_model_init(&mut host).unwrap().into() +/// }; +/// +/// assert_eq!(metadata.name, "Plugin"); +/// +/// struct Host; +/// impl fj::Host for Host { +/// fn register_boxed_model(&mut self, model: Box) { todo!() } +/// } +/// ``` mod context; +pub mod ffi_safe; mod host; mod metadata; mod model; -mod wrappers; use std::any::Any; @@ -17,7 +50,6 @@ pub use self::{ host::Host, metadata::{ModelMetadata, PluginMetadata}, model::Model, - wrappers::{BoxedError, FfiSafeResult, FfiSafeString, FfiSafeVec}, }; #[macro_export] @@ -25,9 +57,9 @@ macro_rules! register_model { ($init:expr) => { #[no_mangle] unsafe extern "C" fn fj_model_init( - mut host: $crate::abi::Host<'_>, + mut host: *mut $crate::abi::Host<'_>, ) -> $crate::abi::InitResult { - let host: &mut dyn $crate::Host = &mut host; + let host: &mut dyn $crate::Host = &mut *host; let result: Result< $crate::PluginMetadata, @@ -49,18 +81,23 @@ macro_rules! register_model { /// /// const _: fj::abi::InitFunction = fj_model_init; /// ``` -pub type InitFunction = unsafe extern "C" fn(Host<'_>) -> InitResult; -pub type InitResult = FfiSafeResult; -pub type ShapeResult = FfiSafeResult; +pub type InitFunction = unsafe extern "C" fn(*mut Host<'_>) -> InitResult; +pub type InitResult = ffi_safe::Result; +pub type ShapeResult = ffi_safe::Result; + +/// The name of the function generated by [`register_model`]. +/// +pub const INIT_FUNCTION_NAME: &str = "fj_model"; fn on_panic(payload: Box) -> ! { - let msg: &str = if let Some(s) = payload.downcast_ref::() { - s.as_str() - } else if let Some(s) = payload.downcast_ref::<&str>() { - *s - } else { - "A panic occurred" - }; + let msg: &str = + if let Some(s) = payload.downcast_ref::() { + s.as_str() + } else if let Some(s) = payload.downcast_ref::<&str>() { + *s + } else { + "A panic occurred" + }; eprintln!("{msg}"); // It's not ideal, but panicking across the FFI boundary is UB. diff --git a/crates/fj/src/abi/model.rs b/crates/fj/src/abi/model.rs index f432d1d70..4dab289e9 100644 --- a/crates/fj/src/abi/model.rs +++ b/crates/fj/src/abi/model.rs @@ -22,8 +22,8 @@ impl crate::Model for Model { let result = unsafe { shape(ptr, ctx) }; match result { - super::FfiSafeResult::Ok(shape) => Ok(shape), - super::FfiSafeResult::Err(err) => Err(err.into()), + super::ffi_safe::Result::Ok(shape) => Ok(shape), + super::ffi_safe::Result::Err(err) => Err(err.into()), } } diff --git a/crates/fj/src/lib.rs b/crates/fj/src/lib.rs index 4f5d060ca..3b0b2d54a 100644 --- a/crates/fj/src/lib.rs +++ b/crates/fj/src/lib.rs @@ -53,6 +53,7 @@ use serde::{Deserialize, Serialize}; #[derive(Clone, Debug, PartialEq)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] #[repr(C)] +#[allow(improper_ctypes)] // Box isn't FFI-safe pub enum Shape { /// A group of two 3-dimensional shapes Group(Box), From 5598a54b53beef76baaf94061890937093056356 Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Thu, 28 Jul 2022 22:44:23 +0800 Subject: [PATCH 04/14] Wire the new fj_plugin_init function up to fj-host --- crates/fj-host/src/lib.rs | 51 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 48 insertions(+), 3 deletions(-) diff --git a/crates/fj-host/src/lib.rs b/crates/fj-host/src/lib.rs index dbcd4aae4..e831326dc 100644 --- a/crates/fj-host/src/lib.rs +++ b/crates/fj-host/src/lib.rs @@ -29,6 +29,7 @@ use std::{ thread, }; +use fj::abi; use notify::Watcher as _; use thiserror::Error; @@ -131,8 +132,24 @@ impl Model { // https://github.com/hannobraun/Fornjot/issues/71 let shape = unsafe { let lib = libloading::Library::new(&self.lib_path)?; - let model: libloading::Symbol = lib.get(b"model")?; - model(arguments) + let init: libloading::Symbol = + lib.get(abi::INIT_FUNCTION_NAME.as_bytes())?; + + let mut host = Host { + args: arguments, + model: None, + }; + + match init(&mut abi::Host::from(&mut host)) { + abi::ffi_safe::Result::Ok(_metadata) => {} + abi::ffi_safe::Result::Err(e) => { + return Err(Error::InitializeModel(e.into())); + } + } + + let model = host.model.take().ok_or(Error::NoModelRegistered)?; + + model.shape(&host).map_err(Error::Shape)? }; Ok(shape) @@ -363,6 +380,19 @@ pub enum Error { #[error("Error loading model from dynamic library")] LibLoading(#[from] libloading::Error), + /// Initializing a model failed. + #[error("Unable to initialize the model")] + InitializeModel(#[source] Box), + + /// The user forgot to register a model when calling + /// [`fj::register_model!()`]. + #[error("No model was registered")] + NoModelRegistered, + + /// An error was returned from [`fj::Model::shape()`]. + #[error("Unable to determine the model's geometry")] + Shape(#[source] Box), + /// Error while watching the model code for changes #[error("Error watching model for changes")] Notify(#[from] notify::Error), @@ -390,4 +420,19 @@ pub enum Error { }, } -type ModelFn = unsafe extern "C" fn(args: &Parameters) -> fj::Shape; +struct Host<'a> { + args: &'a Parameters, + model: Option>, +} + +impl<'a> fj::Host for Host<'a> { + fn register_boxed_model(&mut self, model: Box) { + self.model = Some(model); + } +} + +impl<'a> fj::Context for Host<'a> { + fn get_argument(&self, name: &str) -> Option<&str> { + self.args.get(name).map(|s| s.as_str()) + } +} From d447bc199e72462359e0519b29a070bfab79890c Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Thu, 28 Jul 2022 22:56:25 +0800 Subject: [PATCH 05/14] Added an integration test that loads a model --- Cargo.lock | 1 + crates/fj-app/Cargo.toml | 3 +++ crates/fj-app/tests/smoke_test.rs | 44 +++++++++++++++++++++++++++++++ crates/fj/src/abi/mod.rs | 2 +- 4 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 crates/fj-app/tests/smoke_test.rs diff --git a/Cargo.lock b/Cargo.lock index 7b3f1896f..e9d9d8b4b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -853,6 +853,7 @@ dependencies = [ "fj-viewer", "fj-window", "serde", + "tempfile", "tracing-subscriber", ] diff --git a/crates/fj-app/Cargo.toml b/crates/fj-app/Cargo.toml index 40bb21a0e..00cf7b202 100644 --- a/crates/fj-app/Cargo.toml +++ b/crates/fj-app/Cargo.toml @@ -69,3 +69,6 @@ features = ["derive"] [dependencies.tracing-subscriber] version = "0.3.15" features = ["env-filter", "fmt"] + +[dev-dependencies] +tempfile = "3.3.0" diff --git a/crates/fj-app/tests/smoke_test.rs b/crates/fj-app/tests/smoke_test.rs new file mode 100644 index 000000000..79d78744f --- /dev/null +++ b/crates/fj-app/tests/smoke_test.rs @@ -0,0 +1,44 @@ +use std::{ + path::Path, + process::{Command, Output, Stdio}, +}; + +use tempfile::NamedTempFile; + +const FJ_APP: &str = env!("CARGO_BIN_EXE_fj-app"); + +#[test] +fn spacer_model() { + let project_root = Path::new(env!("CARGO_MANIFEST_DIR")) + .ancestors() + .nth(2) + .unwrap(); + let dest = NamedTempFile::new().unwrap(); + + let mut cmd = Command::new(FJ_APP); + cmd.arg("--model") + .arg("cuboid") + .arg("--export") + .arg(dest.path()) + .env("RUST_BACKTRACE", "1") + .env("RUST_LOG", "debug") + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .current_dir(&project_root); + let Output { + status, + stdout, + stderr, + } = cmd.output().unwrap(); + + let stdout = String::from_utf8(stdout).unwrap(); + let stderr = String::from_utf8(stderr).unwrap(); + + if !status.success() { + println!("---- Stdout ----"); + println!("{stdout}"); + println!("---- Stderr ----"); + println!("{stderr}"); + panic!("`{cmd:?}` failed with exit code {:?}", status.code()); + } +} diff --git a/crates/fj/src/abi/mod.rs b/crates/fj/src/abi/mod.rs index 644d26023..d9144a845 100644 --- a/crates/fj/src/abi/mod.rs +++ b/crates/fj/src/abi/mod.rs @@ -87,7 +87,7 @@ pub type ShapeResult = ffi_safe::Result; /// The name of the function generated by [`register_model`]. /// -pub const INIT_FUNCTION_NAME: &str = "fj_model"; +pub const INIT_FUNCTION_NAME: &str = "fj_model_init"; fn on_panic(payload: Box) -> ! { let msg: &str = From 0877cebdbd0f1e57aa534f5b68700ae4d10f4b63 Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Thu, 28 Jul 2022 23:06:58 +0800 Subject: [PATCH 06/14] Run all of our demo models during the smoke tests --- Cargo.lock | 1 + crates/fj-app/Cargo.toml | 1 + crates/fj-app/tests/smoke_test.rs | 43 +++++++++++++++++++++++++++---- 3 files changed, 40 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e9d9d8b4b..deaef3136 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -853,6 +853,7 @@ dependencies = [ "fj-viewer", "fj-window", "serde", + "stl", "tempfile", "tracing-subscriber", ] diff --git a/crates/fj-app/Cargo.toml b/crates/fj-app/Cargo.toml index 00cf7b202..e26d19d68 100644 --- a/crates/fj-app/Cargo.toml +++ b/crates/fj-app/Cargo.toml @@ -71,4 +71,5 @@ version = "0.3.15" features = ["env-filter", "fmt"] [dev-dependencies] +stl = "0.2.1" tempfile = "3.3.0" diff --git a/crates/fj-app/tests/smoke_test.rs b/crates/fj-app/tests/smoke_test.rs index 79d78744f..d838725a9 100644 --- a/crates/fj-app/tests/smoke_test.rs +++ b/crates/fj-app/tests/smoke_test.rs @@ -1,25 +1,55 @@ use std::{ + fs::File, path::Path, process::{Command, Output, Stdio}, }; -use tempfile::NamedTempFile; +use stl::BinaryStlFile; +use tempfile::TempDir; const FJ_APP: &str = env!("CARGO_BIN_EXE_fj-app"); #[test] -fn spacer_model() { +fn spacer() { + let BinaryStlFile { header, triangles } = execute_model("spacer"); + assert_eq!(header.num_triangles, 42); + assert_eq!(header.num_triangles as usize, triangles.len()); +} + +#[test] +fn test() { + let BinaryStlFile { header, triangles } = execute_model("test"); + assert_eq!(header.num_triangles, 42); + assert_eq!(header.num_triangles as usize, triangles.len()); +} + +#[test] +fn cuboid() { + let BinaryStlFile { header, triangles } = execute_model("cuboid"); + assert_eq!(header.num_triangles, 42); + assert_eq!(header.num_triangles as usize, triangles.len()); +} + +#[test] +fn star() { + let BinaryStlFile { header, triangles } = execute_model("star"); + assert_eq!(header.num_triangles, 42); + assert_eq!(header.num_triangles as usize, triangles.len()); +} + +fn execute_model(name: &str) -> BinaryStlFile { let project_root = Path::new(env!("CARGO_MANIFEST_DIR")) .ancestors() .nth(2) .unwrap(); - let dest = NamedTempFile::new().unwrap(); + let temp = TempDir::new().unwrap(); + let dest = temp.path().join("output.stl"); let mut cmd = Command::new(FJ_APP); cmd.arg("--model") - .arg("cuboid") + .arg(name) .arg("--export") - .arg(dest.path()) + .arg(&dest) .env("RUST_BACKTRACE", "1") .env("RUST_LOG", "debug") .stdout(Stdio::piped()) @@ -41,4 +71,7 @@ fn spacer_model() { println!("{stderr}"); panic!("`{cmd:?}` failed with exit code {:?}", status.code()); } + + let mut f = File::open(&dest).unwrap(); + stl::read_stl(&mut f).unwrap() } From 8f2f2b22f6eaa3c791b233085968b26a9467890c Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Thu, 28 Jul 2022 23:29:52 +0800 Subject: [PATCH 07/14] Exporting existing models is already handled by the "export-validator" --- Cargo.lock | 2 - crates/fj-app/Cargo.toml | 4 -- crates/fj-app/tests/smoke_test.rs | 77 ------------------------------- crates/fj/src/abi/mod.rs | 10 ++-- 4 files changed, 5 insertions(+), 88 deletions(-) delete mode 100644 crates/fj-app/tests/smoke_test.rs diff --git a/Cargo.lock b/Cargo.lock index deaef3136..7b3f1896f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -853,8 +853,6 @@ dependencies = [ "fj-viewer", "fj-window", "serde", - "stl", - "tempfile", "tracing-subscriber", ] diff --git a/crates/fj-app/Cargo.toml b/crates/fj-app/Cargo.toml index e26d19d68..40bb21a0e 100644 --- a/crates/fj-app/Cargo.toml +++ b/crates/fj-app/Cargo.toml @@ -69,7 +69,3 @@ features = ["derive"] [dependencies.tracing-subscriber] version = "0.3.15" features = ["env-filter", "fmt"] - -[dev-dependencies] -stl = "0.2.1" -tempfile = "3.3.0" diff --git a/crates/fj-app/tests/smoke_test.rs b/crates/fj-app/tests/smoke_test.rs deleted file mode 100644 index d838725a9..000000000 --- a/crates/fj-app/tests/smoke_test.rs +++ /dev/null @@ -1,77 +0,0 @@ -use std::{ - fs::File, - path::Path, - process::{Command, Output, Stdio}, -}; - -use stl::BinaryStlFile; -use tempfile::TempDir; - -const FJ_APP: &str = env!("CARGO_BIN_EXE_fj-app"); - -#[test] -fn spacer() { - let BinaryStlFile { header, triangles } = execute_model("spacer"); - assert_eq!(header.num_triangles, 42); - assert_eq!(header.num_triangles as usize, triangles.len()); -} - -#[test] -fn test() { - let BinaryStlFile { header, triangles } = execute_model("test"); - assert_eq!(header.num_triangles, 42); - assert_eq!(header.num_triangles as usize, triangles.len()); -} - -#[test] -fn cuboid() { - let BinaryStlFile { header, triangles } = execute_model("cuboid"); - assert_eq!(header.num_triangles, 42); - assert_eq!(header.num_triangles as usize, triangles.len()); -} - -#[test] -fn star() { - let BinaryStlFile { header, triangles } = execute_model("star"); - assert_eq!(header.num_triangles, 42); - assert_eq!(header.num_triangles as usize, triangles.len()); -} - -fn execute_model(name: &str) -> BinaryStlFile { - let project_root = Path::new(env!("CARGO_MANIFEST_DIR")) - .ancestors() - .nth(2) - .unwrap(); - let temp = TempDir::new().unwrap(); - let dest = temp.path().join("output.stl"); - - let mut cmd = Command::new(FJ_APP); - cmd.arg("--model") - .arg(name) - .arg("--export") - .arg(&dest) - .env("RUST_BACKTRACE", "1") - .env("RUST_LOG", "debug") - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .current_dir(&project_root); - let Output { - status, - stdout, - stderr, - } = cmd.output().unwrap(); - - let stdout = String::from_utf8(stdout).unwrap(); - let stderr = String::from_utf8(stderr).unwrap(); - - if !status.success() { - println!("---- Stdout ----"); - println!("{stdout}"); - println!("---- Stderr ----"); - println!("{stderr}"); - panic!("`{cmd:?}` failed with exit code {:?}", status.code()); - } - - let mut f = File::open(&dest).unwrap(); - stl::read_stl(&mut f).unwrap() -} diff --git a/crates/fj/src/abi/mod.rs b/crates/fj/src/abi/mod.rs index d9144a845..6f9c2d49e 100644 --- a/crates/fj/src/abi/mod.rs +++ b/crates/fj/src/abi/mod.rs @@ -59,14 +59,14 @@ macro_rules! register_model { unsafe extern "C" fn fj_model_init( mut host: *mut $crate::abi::Host<'_>, ) -> $crate::abi::InitResult { - let host: &mut dyn $crate::Host = &mut *host; - - let result: Result< + let init: fn( + &mut dyn $crate::Host, + ) -> Result< $crate::PluginMetadata, Box, - > = $init(host); + > = $init; - match result { + match init(&mut *host) { Ok(meta) => $crate::abi::InitResult::Ok(meta.into()), Err(e) => $crate::abi::InitResult::Err(e.into()), } From 29277766ffdd7ca3d62652799cae52b3492b2ed6 Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Thu, 28 Jul 2022 23:39:33 +0800 Subject: [PATCH 08/14] Migrated all models over to the new API --- crates/fj/src/host.rs | 1 + crates/fj/src/metadata.rs | 10 ++-- models/cuboid/src/lib.rs | 60 +++++++++++++++------ models/spacer/src/lib.rs | 64 +++++++++++++++++----- models/star/src/lib.rs | 111 ++++++++++++++++++++++++++------------ models/test/Cargo.toml | 4 ++ models/test/src/lib.rs | 47 +++++++++++----- 7 files changed, 218 insertions(+), 79 deletions(-) diff --git a/crates/fj/src/host.rs b/crates/fj/src/host.rs index fbd8154bb..ba712fb84 100644 --- a/crates/fj/src/host.rs +++ b/crates/fj/src/host.rs @@ -7,6 +7,7 @@ pub trait Host { /// This is mainly for more advanced use cases (e.g. when you need to close /// over extra state to load the model). For simpler models, you probably /// want to use [`HostExt::register_model()`] instead. + #[doc(hidden)] fn register_boxed_model(&mut self, model: Box); } diff --git a/crates/fj/src/metadata.rs b/crates/fj/src/metadata.rs index 4ae59231f..b4fff0730 100644 --- a/crates/fj/src/metadata.rs +++ b/crates/fj/src/metadata.rs @@ -47,7 +47,7 @@ impl PluginMetadata { } /// Set the [`PluginMetadata::short_description`] field. - pub fn set_short_description( + pub fn with_short_description( self, short_description: impl Into, ) -> Self { @@ -63,7 +63,7 @@ impl PluginMetadata { } /// Set the [`PluginMetadata::description`] field. - pub fn set_description(self, description: impl Into) -> Self { + pub fn with_description(self, description: impl Into) -> Self { let description = description.into(); if description.is_empty() { return self; @@ -76,7 +76,7 @@ impl PluginMetadata { } /// Set the [`PluginMetadata::homepage`] field. - pub fn set_homepage(self, homepage: impl Into) -> Self { + pub fn with_homepage(self, homepage: impl Into) -> Self { let homepage = homepage.into(); if homepage.is_empty() { return self; @@ -89,7 +89,7 @@ impl PluginMetadata { } /// Set the [`PluginMetadata::repository`] field. - pub fn set_repository(self, repository: impl Into) -> Self { + pub fn with_repository(self, repository: impl Into) -> Self { let repository = repository.into(); if repository.is_empty() { return self; @@ -102,7 +102,7 @@ impl PluginMetadata { } /// Set the [`PluginMetadata::license`] field. - pub fn set_license(self, license: impl Into) -> Self { + pub fn with_license(self, license: impl Into) -> Self { let license = license.into(); if license.is_empty() { return self; diff --git a/models/cuboid/src/lib.rs b/models/cuboid/src/lib.rs index 6505e3612..2c2b3cd45 100644 --- a/models/cuboid/src/lib.rs +++ b/models/cuboid/src/lib.rs @@ -1,18 +1,44 @@ -#[fj::model] -pub fn model( - #[param(default = 3.0)] x: f64, - #[param(default = 2.0)] y: f64, - #[param(default = 1.0)] z: f64, -) -> fj::Shape { - #[rustfmt::skip] - let rectangle = fj::Sketch::from_points(vec![ - [-x / 2., -y / 2.], - [ x / 2., -y / 2.], - [ x / 2., y / 2.], - [-x / 2., y / 2.], - ]).with_color([100,255,0,200]); - - let cuboid = fj::Sweep::from_path(rectangle.into(), [0., 0., z]); - - cuboid.into() +use fj::{ + ArgumentMetadata, ContextExt, HostExt, Model, ModelMetadata, PluginMetadata, +}; + +fj::register_model!(|host| { + host.register_model(Cuboid); + + Ok(PluginMetadata::new( + env!("CARGO_PKG_NAME"), + env!("CARGO_PKG_VERSION"), + )) +}); + +pub struct Cuboid; + +impl Model for Cuboid { + fn shape( + &self, + ctx: &dyn fj::Context, + ) -> Result> { + let x: f64 = ctx.parse_optional_argument("x")?.unwrap_or(3.0); + let y: f64 = ctx.parse_optional_argument("y")?.unwrap_or(2.0); + let z: f64 = ctx.parse_optional_argument("z")?.unwrap_or(1.0); + + #[rustfmt::skip] + let rectangle = fj::Sketch::from_points(vec![ + [-x / 2., -y / 2.], + [ x / 2., -y / 2.], + [ x / 2., y / 2.], + [-x / 2., y / 2.], + ]).with_color([100,255,0,200]); + + let cuboid = fj::Sweep::from_path(rectangle.into(), [0., 0., z]); + + Ok(cuboid.into()) + } + + fn metadata(&self) -> ModelMetadata { + ModelMetadata::new("Cuboid") + .with_argument(ArgumentMetadata::new("x").with_default_value("3.0")) + .with_argument(ArgumentMetadata::new("y").with_default_value("2.0")) + .with_argument(ArgumentMetadata::new("z").with_default_value("1.0")) + } } diff --git a/models/spacer/src/lib.rs b/models/spacer/src/lib.rs index 98724904d..51a86d41e 100644 --- a/models/spacer/src/lib.rs +++ b/models/spacer/src/lib.rs @@ -1,16 +1,56 @@ -use fj::syntax::*; +use fj::{ + syntax::*, ArgumentMetadata, ContextExt, HostExt, Model, ModelMetadata, + PluginMetadata, +}; -#[fj::model] -pub fn model( - #[param(default = 1.0, min = inner * 1.01)] outer: f64, - #[param(default = 0.5, max = outer * 0.99)] inner: f64, - #[param(default = 1.0)] height: f64, -) -> fj::Shape { - let outer_edge = fj::Sketch::from_circle(fj::Circle::from_radius(outer)); - let inner_edge = fj::Sketch::from_circle(fj::Circle::from_radius(inner)); +fj::register_model!(|host| { + host.register_model(Spacer); - let footprint = outer_edge.difference(&inner_edge); - let spacer = footprint.sweep([0., 0., height]); + Ok(PluginMetadata::new( + env!("CARGO_PKG_NAME"), + env!("CARGO_PKG_VERSION"), + )) +}); - spacer.into() +struct Spacer; + +impl Model for Spacer { + fn shape( + &self, + ctx: &dyn fj::Context, + ) -> Result> { + let outer: f64 = ctx.parse_optional_argument("outer")?.unwrap_or(1.0); + let inner: f64 = ctx.parse_optional_argument("inner")?.unwrap_or(0.5); + let height: f64 = ctx.parse_optional_argument("height")?.unwrap_or(1.0); + + if outer < inner * 1.01 { + todo!("Return a suitable error"); + } + if inner > outer * 0.99 { + todo!("Return a suitable error"); + } + + let outer_edge = + fj::Sketch::from_circle(fj::Circle::from_radius(outer)); + let inner_edge = + fj::Sketch::from_circle(fj::Circle::from_radius(inner)); + + let footprint = outer_edge.difference(&inner_edge); + let spacer = footprint.sweep([0., 0., height]); + + Ok(spacer.into()) + } + + fn metadata(&self) -> ModelMetadata { + ModelMetadata::new("Spacer") + .with_argument( + ArgumentMetadata::new("outer").with_default_value("1.0"), + ) + .with_argument( + ArgumentMetadata::new("inner").with_default_value("0.5"), + ) + .with_argument( + ArgumentMetadata::new("height").with_default_value("1.0"), + ) + } } diff --git a/models/star/src/lib.rs b/models/star/src/lib.rs index 3d1775680..787b92571 100644 --- a/models/star/src/lib.rs +++ b/models/star/src/lib.rs @@ -1,40 +1,85 @@ use std::f64::consts::PI; -#[fj::model] -pub fn model( - #[param(default = 5, min = 3)] num_points: u64, - #[param(default = 1.0, min = 1.0)] r1: f64, - #[param(default = 2.0, min = 2.0)] r2: f64, - #[param(default = 1.0)] h: f64, -) -> fj::Shape { - let num_vertices = num_points * 2; - let vertex_iter = (0..num_vertices).map(|i| { - let angle = - fj::Angle::from_rad(2. * PI / num_vertices as f64 * i as f64); - let radius = if i % 2 == 0 { r1 } else { r2 }; - (angle, radius) - }); - - // Now that we got that iterator prepared, generating the vertices is just a - // bit of trigonometry. - let mut outer = Vec::new(); - let mut inner = Vec::new(); - for (angle, radius) in vertex_iter { - let (sin, cos) = angle.rad().sin_cos(); - - let x = cos * radius; - let y = sin * radius; - - outer.push([x, y]); - inner.push([x / 2., y / 2.]); - } +use fj::{ + ArgumentMetadata, ContextExt, HostExt, Model, ModelMetadata, PluginMetadata, +}; + +fj::register_model!(|host| { + host.register_model(Star); + + Ok(PluginMetadata::new( + env!("CARGO_PKG_NAME"), + env!("CARGO_PKG_VERSION"), + )) +}); + +struct Star; + +impl Model for Star { + fn shape( + &self, + ctx: &dyn fj::Context, + ) -> Result> { + let num_points: u64 = + ctx.parse_optional_argument("num_points")?.unwrap_or(5); + let r1: f64 = ctx.parse_optional_argument("r1")?.unwrap_or(1.0); + let r2: f64 = ctx.parse_optional_argument("r2")?.unwrap_or(2.0); + let h: f64 = ctx.parse_optional_argument("h")?.unwrap_or(1.0); + + if num_points < 3 { + todo!(); + } + if r1 < 1.0 { + todo!(); + } + if r2 < 2.0 { + todo!(); + } + + let num_vertices = num_points * 2; + let vertex_iter = (0..num_vertices).map(|i| { + let angle = + fj::Angle::from_rad(2. * PI / num_vertices as f64 * i as f64); + let radius = if i % 2 == 0 { r1 } else { r2 }; + (angle, radius) + }); - let outer = fj::Sketch::from_points(outer); - let inner = fj::Sketch::from_points(inner); + // Now that we got that iterator prepared, generating the vertices is just a + // bit of trigonometry. + let mut outer = Vec::new(); + let mut inner = Vec::new(); + for (angle, radius) in vertex_iter { + let (sin, cos) = angle.rad().sin_cos(); - let footprint = fj::Difference2d::from_shapes([outer.into(), inner.into()]); + let x = cos * radius; + let y = sin * radius; - let star = fj::Sweep::from_path(footprint.into(), [0., 0., h]); + outer.push([x, y]); + inner.push([x / 2., y / 2.]); + } - star.into() + let outer = fj::Sketch::from_points(outer); + let inner = fj::Sketch::from_points(inner); + + let footprint = + fj::Difference2d::from_shapes([outer.into(), inner.into()]); + + let star = fj::Sweep::from_path(footprint.into(), [0., 0., h]); + + Ok(star.into()) + } + + fn metadata(&self) -> ModelMetadata { + ModelMetadata::new("Star") + .with_argument( + ArgumentMetadata::new("num_points").with_default_value("5"), + ) + .with_argument( + ArgumentMetadata::new("r1").with_default_value("1.0"), + ) + .with_argument( + ArgumentMetadata::new("r2").with_default_value("2.0"), + ) + .with_argument(ArgumentMetadata::new("h").with_default_value("1.0")) + } } diff --git a/models/test/Cargo.toml b/models/test/Cargo.toml index d6bd46854..dd9fa6bc0 100644 --- a/models/test/Cargo.toml +++ b/models/test/Cargo.toml @@ -2,6 +2,10 @@ name = "test" version = "0.1.0" edition = "2021" +description = "An example model" +homepage = "https://www.fornjot.app/" +repository = "https://github.com/hannobraun/fornjot" +license = "0BSD" [lib] crate-type = ["cdylib"] diff --git a/models/test/src/lib.rs b/models/test/src/lib.rs index 76f955cf6..d68df8662 100644 --- a/models/test/src/lib.rs +++ b/models/test/src/lib.rs @@ -1,18 +1,41 @@ use std::f64::consts::PI; -use fj::{syntax::*, Angle}; - -#[fj::model] -pub fn model() -> fj::Shape { - let a = star(4, 1., [0, 255, 0, 200]); - let b = star(5, -1., [255, 0, 0, 255]) - .rotate([1., 1., 1.], Angle::from_deg(45.)) - .translate([3., 3., 1.]); - let c = spacer().translate([6., 6., 1.]); - - let group = a.group(&b).group(&c); +use fj::{syntax::*, Angle, HostExt, ModelMetadata, PluginMetadata}; + +fj::register_model!(|host| { + host.register_model(Test); + + Ok( + PluginMetadata::new(env!("CARGO_PKG_NAME"), env!("CARGO_PKG_VERSION")) + .with_short_description(env!("CARGO_PKG_DESCRIPTION")) + .with_description(include_str!("../README.md")) + .with_homepage(env!("CARGO_PKG_HOMEPAGE")) + .with_repository(env!("CARGO_PKG_REPOSITORY")) + .with_license(env!("CARGO_PKG_LICENSE")), + ) +}); + +struct Test; + +impl fj::Model for Test { + fn shape( + &self, + _ctx: &dyn fj::Context, + ) -> Result> { + let a = star(4, 1., [0, 255, 0, 200]); + let b = star(5, -1., [255, 0, 0, 255]) + .rotate([1., 1., 1.], Angle::from_deg(45.)) + .translate([3., 3., 1.]); + let c = spacer().translate([6., 6., 1.]); + + let group = a.group(&b).group(&c); + + Ok(group.into()) + } - group.into() + fn metadata(&self) -> ModelMetadata { + ModelMetadata::new("Test") + } } fn star(num_points: u64, height: f64, color: [u8; 4]) -> fj::Shape { From 9e514d71e2775ce868385825c3b2e38a338fd497 Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Thu, 28 Jul 2022 23:59:51 +0800 Subject: [PATCH 09/14] remove the "plugin" terminology --- crates/fj/src/abi/metadata.rs | 18 +++++++------- crates/fj/src/abi/mod.rs | 14 +++++------ crates/fj/src/lib.rs | 2 +- crates/fj/src/metadata.rs | 44 +++++++++++++++++------------------ models/cuboid/src/lib.rs | 4 ++-- models/spacer/src/lib.rs | 6 ++--- models/star/src/lib.rs | 4 ++-- models/test/src/lib.rs | 4 ++-- 8 files changed, 48 insertions(+), 48 deletions(-) diff --git a/crates/fj/src/abi/metadata.rs b/crates/fj/src/abi/metadata.rs index 48c081104..56967a492 100644 --- a/crates/fj/src/abi/metadata.rs +++ b/crates/fj/src/abi/metadata.rs @@ -32,7 +32,7 @@ impl From for ModelMetadata { #[derive(Debug, Clone)] #[repr(C)] -pub struct PluginMetadata { +pub struct Metadata { name: ffi_safe::String, version: ffi_safe::String, short_description: ffi_safe::Option, @@ -42,9 +42,9 @@ pub struct PluginMetadata { license: ffi_safe::Option, } -impl From for crate::PluginMetadata { - fn from(m: PluginMetadata) -> Self { - let PluginMetadata { +impl From for crate::Metadata { + fn from(m: Metadata) -> Self { + let Metadata { name, version, short_description, @@ -54,7 +54,7 @@ impl From for crate::PluginMetadata { license, } = m; - crate::PluginMetadata { + crate::Metadata { name: name.into(), version: version.into(), short_description: short_description.map(Into::into).into(), @@ -66,9 +66,9 @@ impl From for crate::PluginMetadata { } } -impl From for PluginMetadata { - fn from(m: crate::PluginMetadata) -> Self { - let crate::PluginMetadata { +impl From for Metadata { + fn from(m: crate::Metadata) -> Self { + let crate::Metadata { name, version, short_description, @@ -78,7 +78,7 @@ impl From for PluginMetadata { license, } = m; - PluginMetadata { + Metadata { name: name.into(), version: version.into(), short_description: short_description.into(), diff --git a/crates/fj/src/abi/mod.rs b/crates/fj/src/abi/mod.rs index 6f9c2d49e..a882592e8 100644 --- a/crates/fj/src/abi/mod.rs +++ b/crates/fj/src/abi/mod.rs @@ -7,10 +7,10 @@ /// If the macro generated the wrong code, this doctest would fail. /// /// ```rust -/// use fj::{abi::INIT_FUNCTION_NAME, PluginMetadata}; +/// use fj::{abi::INIT_FUNCTION_NAME, Metadata}; /// /// fj::register_model!(|_| { -/// Ok(PluginMetadata::new("Plugin", "1.2.3")) +/// Ok(Metadata::new("My Model", "1.2.3")) /// }); /// /// mod x { @@ -24,13 +24,13 @@ /// /// // We can also make sure the unmangled name is correct by calling it. /// -/// let metadata: fj::PluginMetadata = unsafe { +/// let metadata: fj::Metadata = unsafe { /// let mut host = Host; /// let mut host = fj::abi::Host::from(&mut host); /// x::fj_model_init(&mut host).unwrap().into() /// }; /// -/// assert_eq!(metadata.name, "Plugin"); +/// assert_eq!(metadata.name, "My Model"); /// /// struct Host; /// impl fj::Host for Host { @@ -48,7 +48,7 @@ use std::any::Any; pub use self::{ context::Context, host::Host, - metadata::{ModelMetadata, PluginMetadata}, + metadata::{Metadata, ModelMetadata}, model::Model, }; @@ -62,7 +62,7 @@ macro_rules! register_model { let init: fn( &mut dyn $crate::Host, ) -> Result< - $crate::PluginMetadata, + $crate::Metadata, Box, > = $init; @@ -82,7 +82,7 @@ macro_rules! register_model { /// const _: fj::abi::InitFunction = fj_model_init; /// ``` pub type InitFunction = unsafe extern "C" fn(*mut Host<'_>) -> InitResult; -pub type InitResult = ffi_safe::Result; +pub type InitResult = ffi_safe::Result; pub type ShapeResult = ffi_safe::Result; /// The name of the function generated by [`register_model`]. diff --git a/crates/fj/src/lib.rs b/crates/fj/src/lib.rs index 3b0b2d54a..0ca79b7ee 100644 --- a/crates/fj/src/lib.rs +++ b/crates/fj/src/lib.rs @@ -39,7 +39,7 @@ pub use self::{ }, group::Group, host::{Host, HostExt}, - metadata::{ArgumentMetadata, ModelMetadata, PluginMetadata}, + metadata::{ArgumentMetadata, Metadata, ModelMetadata}, model::Model, shape_2d::*, sweep::Sweep, diff --git a/crates/fj/src/metadata.rs b/crates/fj/src/metadata.rs index b4fff0730..6a90a1d21 100644 --- a/crates/fj/src/metadata.rs +++ b/crates/fj/src/metadata.rs @@ -1,20 +1,20 @@ -/// Information about a particular plugin that can be used by the host for +/// Information about a particular module that can be used by the host for /// things like introspection and search. #[derive(Debug, Clone, PartialEq, Eq)] -pub struct PluginMetadata { - /// A short, human-friendly name used to identify this plugin. +pub struct Metadata { + /// A short, human-friendly name used to identify this module. pub name: String, - /// A semver-compliant version number for the plugin. + /// A semver-compliant version number. pub version: String, - /// A short, one-line description of what the plugin does. + /// A short, one-line description. pub short_description: Option, - /// A more elaborate description of what the plugin does. + /// A more elaborate description. pub description: Option, - /// A link to the plugin's homepage. + /// A link to the homepage. pub homepage: Option, - /// A link to the plugin's source code. + /// A link to the source code. pub repository: Option, - /// The name of the software license(s) this plugin is released under. + /// The name of the software license(s) this software is released under. /// /// This is interpreted as a SPDX license expression (e.g. `MIT OR /// Apache-2.0`). See [the SPDX site][spdx] for more information. @@ -23,8 +23,8 @@ pub struct PluginMetadata { pub license: Option, } -impl PluginMetadata { - /// Create metadata for a plugin. +impl Metadata { + /// Create a [`Metadata`] object with the bare minimum required fields. /// /// # Panics /// @@ -35,7 +35,7 @@ impl PluginMetadata { let version = version.into(); assert!(!version.is_empty()); - PluginMetadata { + Metadata { name, version, short_description: None, @@ -46,7 +46,7 @@ impl PluginMetadata { } } - /// Set the [`PluginMetadata::short_description`] field. + /// Set the [`Metadata::short_description`] field. pub fn with_short_description( self, short_description: impl Into, @@ -56,59 +56,59 @@ impl PluginMetadata { return self; } - PluginMetadata { + Metadata { short_description: Some(short_description), ..self } } - /// Set the [`PluginMetadata::description`] field. + /// Set the [`Metadata::description`] field. pub fn with_description(self, description: impl Into) -> Self { let description = description.into(); if description.is_empty() { return self; } - PluginMetadata { + Metadata { description: Some(description), ..self } } - /// Set the [`PluginMetadata::homepage`] field. + /// Set the [`Metadata::homepage`] field. pub fn with_homepage(self, homepage: impl Into) -> Self { let homepage = homepage.into(); if homepage.is_empty() { return self; } - PluginMetadata { + Metadata { homepage: Some(homepage), ..self } } - /// Set the [`PluginMetadata::repository`] field. + /// Set the [`Metadata::repository`] field. pub fn with_repository(self, repository: impl Into) -> Self { let repository = repository.into(); if repository.is_empty() { return self; } - PluginMetadata { + Metadata { repository: Some(repository), ..self } } - /// Set the [`PluginMetadata::license`] field. + /// Set the [`Metadata::license`] field. pub fn with_license(self, license: impl Into) -> Self { let license = license.into(); if license.is_empty() { return self; } - PluginMetadata { + Metadata { license: Some(license), ..self } diff --git a/models/cuboid/src/lib.rs b/models/cuboid/src/lib.rs index 2c2b3cd45..39800822e 100644 --- a/models/cuboid/src/lib.rs +++ b/models/cuboid/src/lib.rs @@ -1,11 +1,11 @@ use fj::{ - ArgumentMetadata, ContextExt, HostExt, Model, ModelMetadata, PluginMetadata, + ArgumentMetadata, ContextExt, HostExt, Metadata, Model, ModelMetadata, }; fj::register_model!(|host| { host.register_model(Cuboid); - Ok(PluginMetadata::new( + Ok(Metadata::new( env!("CARGO_PKG_NAME"), env!("CARGO_PKG_VERSION"), )) diff --git a/models/spacer/src/lib.rs b/models/spacer/src/lib.rs index 51a86d41e..7e9c691bf 100644 --- a/models/spacer/src/lib.rs +++ b/models/spacer/src/lib.rs @@ -1,12 +1,12 @@ use fj::{ - syntax::*, ArgumentMetadata, ContextExt, HostExt, Model, ModelMetadata, - PluginMetadata, + syntax::*, ArgumentMetadata, ContextExt, HostExt, Metadata, Model, + ModelMetadata, }; fj::register_model!(|host| { host.register_model(Spacer); - Ok(PluginMetadata::new( + Ok(Metadata::new( env!("CARGO_PKG_NAME"), env!("CARGO_PKG_VERSION"), )) diff --git a/models/star/src/lib.rs b/models/star/src/lib.rs index 787b92571..34fa9bab4 100644 --- a/models/star/src/lib.rs +++ b/models/star/src/lib.rs @@ -1,13 +1,13 @@ use std::f64::consts::PI; use fj::{ - ArgumentMetadata, ContextExt, HostExt, Model, ModelMetadata, PluginMetadata, + ArgumentMetadata, ContextExt, HostExt, Metadata, Model, ModelMetadata, }; fj::register_model!(|host| { host.register_model(Star); - Ok(PluginMetadata::new( + Ok(Metadata::new( env!("CARGO_PKG_NAME"), env!("CARGO_PKG_VERSION"), )) diff --git a/models/test/src/lib.rs b/models/test/src/lib.rs index d68df8662..11abd4883 100644 --- a/models/test/src/lib.rs +++ b/models/test/src/lib.rs @@ -1,12 +1,12 @@ use std::f64::consts::PI; -use fj::{syntax::*, Angle, HostExt, ModelMetadata, PluginMetadata}; +use fj::{syntax::*, Angle, HostExt, Metadata, ModelMetadata}; fj::register_model!(|host| { host.register_model(Test); Ok( - PluginMetadata::new(env!("CARGO_PKG_NAME"), env!("CARGO_PKG_VERSION")) + Metadata::new(env!("CARGO_PKG_NAME"), env!("CARGO_PKG_VERSION")) .with_short_description(env!("CARGO_PKG_DESCRIPTION")) .with_description(include_str!("../README.md")) .with_homepage(env!("CARGO_PKG_HOMEPAGE")) From c43bcfd1d76ac1ff686e0926c40d49d628d5c2f6 Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Tue, 9 Aug 2022 14:37:12 +0800 Subject: [PATCH 10/14] Reworked the #[fj::model] attribute to match the new ABI --- crates/fj-proc/src/expand.rs | 185 ++++++++++++++++ crates/fj-proc/src/lib.rs | 225 ++------------------ crates/fj-proc/src/parse.rs | 389 ++++++++++++++++++++++++++++++++++ crates/fj/src/abi/ffi_safe.rs | 6 +- crates/fj/src/abi/metadata.rs | 14 +- 5 files changed, 613 insertions(+), 206 deletions(-) create mode 100644 crates/fj-proc/src/expand.rs create mode 100644 crates/fj-proc/src/parse.rs diff --git a/crates/fj-proc/src/expand.rs b/crates/fj-proc/src/expand.rs new file mode 100644 index 000000000..bea51ad38 --- /dev/null +++ b/crates/fj-proc/src/expand.rs @@ -0,0 +1,185 @@ +use proc_macro2::TokenStream; +use quote::{quote, ToTokens}; + +use crate::parse::{ + ArgumentMetadata, Constraint, ConstraintKind, ExtractedArgument, + GeometryFunction, Initializer, Metadata, Model, +}; + +impl Initializer { + fn register(&self) -> TokenStream { + quote! { + const _: () = { + fj::register_model!(|host| { + fj::HostExt::register_model(host, Model); + + Ok( + fj::Metadata::new(env!("CARGO_PKG_NAME"), env!("CARGO_PKG_VERSION")) + .with_short_description(env!("CARGO_PKG_DESCRIPTION")) + .with_homepage(env!("CARGO_PKG_HOMEPAGE")) + .with_repository(env!("CARGO_PKG_REPOSITORY")) + .with_license(env!("CARGO_PKG_LICENSE")), + ) + }); + }; + } + } +} + +impl ToTokens for Initializer { + fn to_tokens(&self, tokens: &mut TokenStream) { + let Initializer { model } = self; + + tokens.extend(self.register()); + model.to_tokens(tokens); + } +} + +impl Model { + fn definition(&self) -> TokenStream { + quote! { struct Model; } + } + + fn trait_implementation(&self) -> TokenStream { + let Model { metadata, geometry } = self; + + quote! { + impl fj::Model for Model { + #metadata + #geometry + } + } + } +} + +impl ToTokens for Model { + fn to_tokens(&self, tokens: &mut TokenStream) { + tokens.extend(self.definition()); + tokens.extend(self.trait_implementation()); + } +} + +impl ToTokens for Metadata { + fn to_tokens(&self, tokens: &mut TokenStream) { + let Metadata { name, arguments } = self; + + tokens.extend(quote! { + fn metadata(&self) -> fj::ModelMetadata { + fj::ModelMetadata::new(#name) + #( .with_argument(#arguments) )* + } + }); + } +} + +impl ToTokens for ArgumentMetadata { + fn to_tokens(&self, tokens: &mut TokenStream) { + let ArgumentMetadata { + name, + default_value, + } = self; + + tokens.extend(quote! { fj::ArgumentMetadata::new(#name) }); + + if let Some(default_value) = default_value { + tokens.extend(quote! { + .with_default_value(stringify!(#default_value)) + }); + } + } +} + +impl ToTokens for GeometryFunction { + fn to_tokens(&self, tokens: &mut TokenStream) { + let GeometryFunction { + geometry_function, + arguments, + constraints, + fallible, + } = self; + + let argument_names = arguments.iter().map(|a| &a.ident); + + let invocation = quote! { + #geometry_function(#( #argument_names ),*) + }; + let invocation = if *fallible { + quote! { #invocation.map(fj::Shape::from).map_err(Into::into) } + } else { + quote! { Ok(#invocation.into()) } + }; + + tokens.extend(quote! { + fn shape( + &self, + ctx: &dyn fj::Context, + ) -> Result> { + #( #arguments )* + #( #constraints )* + #invocation + } + }); + } +} + +impl ToTokens for ExtractedArgument { + fn to_tokens(&self, tokens: &mut TokenStream) { + let ExtractedArgument { + ident, + ty, + default_value, + } = self; + + let name = ident.to_string(); + let t = match default_value { + Some(default) => quote! { + let #ident: #ty = match ctx.get_argument(#name) { + Some(value) => value.parse()?, + None => #default + }; + }, + None => { + let error_message = format!("Expected {name}"); + quote! { + let #ident: #ty = match ctx.get_argument(#name) { + Some(value) => value.parse()?, + None => return Err(#error_message.into()), + }; + } + } + }; + + tokens.extend(t); + } +} + +impl ToTokens for Constraint { + fn to_tokens(&self, tokens: &mut TokenStream) { + let Constraint { target, expr, kind } = self; + + let operator = match kind { + ConstraintKind::Max => quote!(<), + ConstraintKind::Min => quote!(>), + }; + let predicate = quote! { #target #operator #expr }; + // Note: this will cause `expr` to be evaluated twice. Predicates should + // be pure functions, so in theory this shouldn't be an issue. + let error_message = quote! { + format!( + "Expected {} {} {} (i.e. {} {} {})", + stringify!(#target), + stringify!(#operator), + stringify!(#expr), + #target, + stringify!(#operator), + #expr, + ) + }; + + tokens.extend(quote! { + if #predicate { + return Err(#error_message.into()); + } + }); + } +} diff --git a/crates/fj-proc/src/lib.rs b/crates/fj-proc/src/lib.rs index 7ad3179b3..bdec05655 100644 --- a/crates/fj-proc/src/lib.rs +++ b/crates/fj-proc/src/lib.rs @@ -1,8 +1,8 @@ +mod expand; +mod parse; + use proc_macro::TokenStream; -use quote::quote; -use syn::{ - bracketed, parenthesized, parse::Parse, parse_macro_input, parse_quote, -}; +use syn::{parse_macro_input, FnArg, ItemFn}; /// Define a function-based model. /// @@ -74,212 +74,31 @@ use syn::{ #[proc_macro_attribute] pub fn model(_: TokenStream, input: TokenStream) -> TokenStream { let item = parse_macro_input!(input as syn::ItemFn); - let inputs = &item.sig.inputs; - - let args: Vec = - inputs.iter().map(|inp| parse_quote!(#inp)).collect(); - - let mut parameter_extraction = Vec::new(); - - let mut min_checks = Vec::new(); - let mut max_checks = Vec::new(); - - for Argument { attr, ident, ty } in &args { - if let Some(attr) = attr { - if let Some(default) = attr.get_default() { - let def = default.val; - parameter_extraction.push(quote! { - let #ident: #ty = args.get(stringify!(#ident)) - .map(|arg| arg.parse().unwrap()) - .unwrap_or(#def); - }); - } else { - parameter_extraction.push(quote! { - let #ident: #ty = args.get(stringify!(#ident)) - .map(|arg| arg.parse().unwrap()) - .expect(format!("A value for `{}` has to be provided since no default is specified",stringify!(#ident)).as_str()); - }); - } - - if let Some(minimum) = attr.get_minimum() { - let min = minimum.val; - min_checks.push(quote! { - if #ident < #min { - panic!("Value of `{}` must not be smaller than: {}",stringify!(#ident), #min); - } - }); - } - if let Some(maximum) = attr.get_maximum() { - let max = maximum.val; - max_checks.push(quote! { - if #ident > #max { - panic!("Value of `{}` must not be larger than: {}", stringify!(#ident), #max); - } - }) - } - } else { - parameter_extraction.push(quote! { - let #ident: #ty = args.get(stringify!(#ident)) - .map(|arg| arg.parse().unwrap()) - .expect(format!("A value for `{}` has to be provided since no default is specified",stringify!(#ident)).as_str()); - }); - } - } - - let function_boilerplate = quote! { - #[no_mangle] - pub extern "C" fn model( - args: &std::collections::HashMap - ) -> fj::Shape - }; - - let function_name = &item.sig.ident; - let body = &item.block; - let arg_names: Vec<_> = args.iter().map(|a| &a.ident).collect(); - let arg_types: Vec<_> = args.iter().map(|a| &a.ty).collect(); - let return_type = &item.sig.output; - - quote! { - #function_boilerplate { - #( - #parameter_extraction - )* - #( - #min_checks - )* - #( - #max_checks - )* - - fn #function_name( - #( #arg_names : #arg_types ),* - ) #return_type { - #body - } - - #function_name(#( #arg_names),*).into() - } - } - .into() -} - -/// Represents one parameter given to the `model` -/// `#[param(default=3, min=4)] num_points: u64` -/// `^^^^^^^^^^^^^^^^^^^^^^^^^^ ~~~~~~~~~~ ^^^-- ty` -/// ` | |` -/// ` attr ident` -#[derive(Debug, Clone)] -struct Argument { - pub attr: Option, - pub ident: proc_macro2::Ident, - pub ty: proc_macro2::Ident, -} - -impl Parse for Argument { - fn parse(input: syn::parse::ParseStream) -> syn::Result { - let mut attr = None; - if input.peek(syn::token::Pound) { - attr = Some(input.parse()?); - } - let ident: proc_macro2::Ident = input.parse()?; - - let _: syn::token::Colon = input.parse()?; - - let ty: proc_macro2::Ident = input.parse()?; - Ok(Self { attr, ident, ty }) - } -} -/// Represents all arguments given to the `#[param]` attribute eg: -/// `#[param(default=3, min=4)]` -/// ` ^^^^^^^^^^^^^^^^` -#[derive(Debug, Clone)] -struct HelperAttribute { - pub param: - Option>, -} + match parse::parse(&item) { + Ok(init) => { + let item = without_param_attrs(item); -impl Parse for HelperAttribute { - fn parse(input: syn::parse::ParseStream) -> syn::Result { - let attr_content; - let param_content; - let _: syn::token::Pound = input.parse()?; - bracketed!(attr_content in input); - let ident: proc_macro2::Ident = attr_content.parse()?; - if ident != *"param" { - return Err(syn::Error::new_spanned( - ident.clone(), - format!( - "Unknown attribute \"{}\" found, expected \"param\"", - ident - ), - )); - } + let tokens = quote::quote! { + #item + #init + }; - if attr_content.peek(syn::token::Paren) { - parenthesized!(param_content in attr_content); - if param_content.is_empty() { - Ok(Self { param: None }) - } else { - Ok(Self { - param: Some( - syn::punctuated::Punctuated::parse_separated_nonempty_with( - ¶m_content, - DefaultParam::parse, - )?, - ), - }) - } - } else { - Ok(Self { param: None }) + tokens.into() } + Err(e) => e.into_compile_error().into(), } } -impl HelperAttribute { - fn get_parameter(&self, parameter_name: &str) -> Option { - if let Some(values) = self.param.clone() { - values.into_iter().find(|val| val.ident == *parameter_name) - } else { - None - } - } - - pub fn get_default(&self) -> Option { - self.get_parameter("default") +/// Strip out any of our `#[param(...)]` attributes so the item will compile. +fn without_param_attrs(mut item: ItemFn) -> ItemFn { + for input in &mut item.sig.inputs { + let attrs = match input { + FnArg::Receiver(r) => &mut r.attrs, + FnArg::Typed(t) => &mut t.attrs, + }; + attrs.retain(|attr| !attr.path.is_ident("param")); } - pub fn get_minimum(&self) -> Option { - self.get_parameter("min") - } - - pub fn get_maximum(&self) -> Option { - self.get_parameter("max") - } -} - -/// Represents one argument given to the `#[param]` attribute eg: -/// `#[param(default=3)]` -/// ` ^^^^^^^^^----- is parsed as DefaultParam{ ident: Some(default), val: 3 }` -#[derive(Debug, Clone)] -struct DefaultParam { - pub ident: proc_macro2::Ident, - pub val: syn::Expr, -} - -impl Parse for DefaultParam { - fn parse(input: syn::parse::ParseStream) -> syn::Result { - if input.peek(syn::Ident) { - let ident: proc_macro2::Ident = input.parse()?; - let _: syn::token::Eq = input.parse()?; - Ok(Self { - ident, - val: input.parse()?, - }) - } else { - Err(input - .parse::() - .expect_err("No identifier found")) - } - } + item } diff --git a/crates/fj-proc/src/parse.rs b/crates/fj-proc/src/parse.rs new file mode 100644 index 000000000..529659f6d --- /dev/null +++ b/crates/fj-proc/src/parse.rs @@ -0,0 +1,389 @@ +use proc_macro2::Ident; +use syn::{ + bracketed, parenthesized, parse::Parse, parse_quote, Expr, ItemFn, + ReturnType, Type, +}; + +/// The call to `fj::register_model!()`. +#[derive(Debug)] +pub(crate) struct Initializer { + pub(crate) model: Model, +} + +/// The generated `Model` struct and its `fj::Model` impl. +#[derive(Debug)] +pub(crate) struct Model { + pub(crate) metadata: Metadata, + pub(crate) geometry: GeometryFunction, +} + +/// The model metadata we return in `<_ as fj::Model>::metadata()`. +#[derive(Debug)] +pub(crate) struct Metadata { + pub(crate) name: String, + pub(crate) arguments: Vec, +} + +/// Metadata for a specific argument. +#[derive(Debug)] +pub(crate) struct ArgumentMetadata { + pub(crate) name: String, + pub(crate) default_value: Option, +} + +/// The `<_ as fj::Model>::shape()` function. +#[derive(Debug)] +pub(crate) struct GeometryFunction { + pub(crate) geometry_function: Ident, + pub(crate) arguments: Vec, + pub(crate) constraints: Vec, + pub(crate) fallible: bool, +} + +#[derive(Debug)] +pub(crate) struct ExtractedArgument { + pub(crate) ident: Ident, + pub(crate) ty: Type, + pub(crate) default_value: Option, +} + +#[derive(Debug)] +pub(crate) struct Constraint { + pub(crate) target: Ident, + pub(crate) expr: Expr, + pub(crate) kind: ConstraintKind, +} + +#[derive(Debug, Clone, Copy, PartialEq)] +pub(crate) enum ConstraintKind { + Min, + Max, +} + +pub(crate) fn parse(f: &ItemFn) -> syn::Result { + let model = parse_model(f)?; + + Ok(Initializer { model }) +} + +fn parse_model(item: &ItemFn) -> syn::Result { + let geometry_function = item.sig.ident.clone(); + + let args: Vec = item + .sig + .inputs + .iter() + .map(|inp| parse_quote!(#inp)) + .collect(); + + let metadata = Metadata { + name: geometry_function.to_string(), + arguments: args + .iter() + .map(|a| ArgumentMetadata { + name: a.ident.to_string(), + default_value: a.default(), + }) + .collect(), + }; + + let geometry = GeometryFunction { + geometry_function, + arguments: args + .iter() + .map(|a| ExtractedArgument { + ident: a.ident.clone(), + default_value: a.default(), + ty: a.ty.clone(), + }) + .collect(), + constraints: args.iter().flat_map(argument_constraints).collect(), + fallible: match &item.sig.output { + ReturnType::Default => false, + ReturnType::Type(_, ty) => contains_result(ty), + }, + }; + + Ok(Model { metadata, geometry }) +} + +fn contains_result(ty: &Type) -> bool { + match ty { + Type::Path(p) => p.path.segments.last().unwrap().ident == "Result", + _ => false, + } +} + +fn argument_constraints(arg: &Argument) -> Vec { + let attr = match arg.attr.as_ref() { + Some(a) => a, + None => return Vec::new(), + }; + + let mut constraints = Vec::new(); + + if let Some(min) = attr.get_minimum() { + constraints.push(Constraint { + target: arg.ident.clone(), + expr: min.val, + kind: ConstraintKind::Min, + }); + } + + if let Some(max) = attr.get_maximum() { + constraints.push(Constraint { + target: arg.ident.clone(), + expr: max.val, + kind: ConstraintKind::Max, + }); + } + + constraints +} + +/// Represents one parameter given to the `model`. +/// +/// ```text +/// #[param(default=3, min=4)] num_points: u64 +/// ^^^^^^^^^^^^^^^^^^^^^^^^^^ ~~~~~~~~~~ ^^^-- ty +/// | | +/// attr ident +/// ``` +#[derive(Debug, Clone)] +struct Argument { + attr: Option, + ident: Ident, + ty: Type, +} + +impl Argument { + fn default(&self) -> Option { + self.attr + .as_ref() + .and_then(|attr| attr.get_default()) + .map(|param| param.val) + } +} + +impl Parse for Argument { + fn parse(input: syn::parse::ParseStream) -> syn::Result { + let mut attr = None; + if input.peek(syn::token::Pound) { + attr = Some(input.parse()?); + } + let ident: Ident = input.parse()?; + + let _: syn::token::Colon = input.parse()?; + + let ty: Type = input.parse()?; + Ok(Self { attr, ident, ty }) + } +} + +/// Represents all arguments given to the `#[param]` attribute eg: +/// +/// ```text +/// #[param(default=3, min=4)] +/// ^^^^^^^^^^^^^^^^ +/// ``` +#[derive(Debug, Clone)] +struct HelperAttribute { + param: Option>, +} + +impl Parse for HelperAttribute { + fn parse(input: syn::parse::ParseStream) -> syn::Result { + let attr_content; + let param_content; + let _: syn::token::Pound = input.parse()?; + bracketed!(attr_content in input); + let ident: Ident = attr_content.parse()?; + if ident != *"param" { + return Err(syn::Error::new_spanned( + ident.clone(), + format!( + "Unknown attribute \"{}\" found, expected \"param\"", + ident + ), + )); + } + + if attr_content.peek(syn::token::Paren) { + parenthesized!(param_content in attr_content); + if param_content.is_empty() { + Ok(Self { param: None }) + } else { + Ok(Self { + param: Some( + syn::punctuated::Punctuated::parse_separated_nonempty_with( + ¶m_content, + DefaultParam::parse, + )?, + ), + }) + } + } else { + Ok(Self { param: None }) + } + } +} + +impl HelperAttribute { + fn get_parameter(&self, parameter_name: &str) -> Option { + if let Some(values) = self.param.clone() { + values.into_iter().find(|val| val.ident == *parameter_name) + } else { + None + } + } + + fn get_default(&self) -> Option { + self.get_parameter("default") + } + + fn get_minimum(&self) -> Option { + self.get_parameter("min") + } + + fn get_maximum(&self) -> Option { + self.get_parameter("max") + } +} + +/// Represents one argument given to the `#[param]` attribute eg: +/// +/// ```text +/// #[param(default=3)] +/// ^^^^^^^^^----- is parsed as DefaultParam{ ident: Some(default), val: 3 } +/// ``` +#[derive(Debug, Clone)] +struct DefaultParam { + ident: Ident, + val: syn::Expr, +} + +impl Parse for DefaultParam { + fn parse(input: syn::parse::ParseStream) -> syn::Result { + if input.peek(syn::Ident) { + let ident: Ident = input.parse()?; + let _: syn::token::Eq = input.parse()?; + Ok(Self { + ident, + val: input.parse()?, + }) + } else { + Err(input.parse::().expect_err("No identifier found")) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use quote::{quote, ToTokens}; + + #[test] + fn parse_a_typical_model_function() { + let tokens = quote! { + pub fn spacer( + #[param(default = 1.0, min = inner * 1.01)] outer: f64, + #[param(default = 0.5, max = outer * 0.99)] inner: f64, + height: f64, + ) -> fj::Shape { + let outer_edge = fj::Sketch::from_circle(fj::Circle::from_radius(outer)); + let inner_edge = fj::Sketch::from_circle(fj::Circle::from_radius(inner)); + + let footprint = outer_edge.difference(&inner_edge); + let spacer = footprint.sweep([0., 0., height]); + + spacer.into() + } + }; + let function: ItemFn = syn::parse2(tokens).unwrap(); + + let Initializer { + model: Model { metadata, geometry }, + } = parse(&function).unwrap(); + + // Note: we can't #[derive(PartialEq)] on our parsed structs because + // proc_macro2::Ident and friends don't implement PartialEq, so let's + // manually check everything parsed correctly. + let Metadata { name, arguments } = metadata; + assert_eq!(name, "spacer"); + let expected_meta = &[ + ("outer".to_string(), Some("1.0".to_string())), + ("inner".to_string(), Some("0.5".to_string())), + ("height".to_string(), None), + ]; + let meta: Vec<_> = arguments + .iter() + .map(|arg| { + ( + arg.name.clone(), + arg.default_value + .as_ref() + .map(|v| v.to_token_stream().to_string()), + ) + }) + .collect(); + assert_eq!(meta, expected_meta); + + let GeometryFunction { + geometry_function, + arguments, + constraints, + fallible, + } = geometry; + assert_eq!(geometry_function.to_string(), "spacer"); + assert!(!fallible); + let arguments: Vec<_> = arguments + .iter() + .map(|arg| { + ( + arg.ident.to_string(), + arg.default_value + .as_ref() + .map(|v| v.to_token_stream().to_string()), + ) + }) + .collect(); + assert_eq!(arguments, expected_meta); + let expected_constraints = &[ + ( + "outer".to_string(), + "inner * 1.01".to_string(), + ConstraintKind::Min, + ), + ( + "inner".to_string(), + "outer * 0.99".to_string(), + ConstraintKind::Max, + ), + ]; + let constraints: Vec<_> = constraints + .iter() + .map(|Constraint { kind, expr, target }| { + ( + target.to_string(), + expr.to_token_stream().to_string(), + *kind, + ) + }) + .collect(); + assert_eq!(constraints, expected_constraints); + } + + #[test] + fn parse_fallible_function() { + let tokens = quote! { + pub fn spacer() -> Result { + todo!() + } + }; + let function: ItemFn = syn::parse2(tokens).unwrap(); + + let init = parse(&function).unwrap(); + + assert!(init.model.geometry.fallible); + } +} diff --git a/crates/fj/src/abi/ffi_safe.rs b/crates/fj/src/abi/ffi_safe.rs index 957005e06..ce9dabe7d 100644 --- a/crates/fj/src/abi/ffi_safe.rs +++ b/crates/fj/src/abi/ffi_safe.rs @@ -314,7 +314,11 @@ impl std::error::Error for BoxedError {} impl From> for BoxedError { fn from(err: Box) -> Self { - // TODO: is it worth capturing the message from each source error, too? + // Open question: is it worth capturing the message from each source + // error, too? We could have some sort of `sources: Vec` field + // where `Source` is a private wrapper around String that implements + // std::error::Error, however then people will see what *looks* like a + // particular error type, but they won't be able to downcast to it. BoxedError { msg: err.to_string().into(), } diff --git a/crates/fj/src/abi/metadata.rs b/crates/fj/src/abi/metadata.rs index 56967a492..c0ac1947a 100644 --- a/crates/fj/src/abi/metadata.rs +++ b/crates/fj/src/abi/metadata.rs @@ -25,8 +25,18 @@ impl From for crate::ModelMetadata { } impl From for ModelMetadata { - fn from(_m: crate::ModelMetadata) -> Self { - todo!() + fn from(m: crate::ModelMetadata) -> Self { + let crate::ModelMetadata { + name, + description, + arguments, + } = m; + + ModelMetadata { + name: name.into(), + description: description.into(), + arguments: arguments.into_iter().map(Into::into).collect(), + } } } From dd356cc5161420642057ab9f6b6e458be604f696 Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Tue, 9 Aug 2022 14:39:21 +0800 Subject: [PATCH 11/14] Reverted model implementations to main --- models/cuboid/src/lib.rs | 60 ++++++--------------- models/spacer/src/lib.rs | 64 +++++----------------- models/star/src/lib.rs | 111 ++++++++++++--------------------------- models/test/src/lib.rs | 47 +++++------------ 4 files changed, 74 insertions(+), 208 deletions(-) diff --git a/models/cuboid/src/lib.rs b/models/cuboid/src/lib.rs index 39800822e..6505e3612 100644 --- a/models/cuboid/src/lib.rs +++ b/models/cuboid/src/lib.rs @@ -1,44 +1,18 @@ -use fj::{ - ArgumentMetadata, ContextExt, HostExt, Metadata, Model, ModelMetadata, -}; - -fj::register_model!(|host| { - host.register_model(Cuboid); - - Ok(Metadata::new( - env!("CARGO_PKG_NAME"), - env!("CARGO_PKG_VERSION"), - )) -}); - -pub struct Cuboid; - -impl Model for Cuboid { - fn shape( - &self, - ctx: &dyn fj::Context, - ) -> Result> { - let x: f64 = ctx.parse_optional_argument("x")?.unwrap_or(3.0); - let y: f64 = ctx.parse_optional_argument("y")?.unwrap_or(2.0); - let z: f64 = ctx.parse_optional_argument("z")?.unwrap_or(1.0); - - #[rustfmt::skip] - let rectangle = fj::Sketch::from_points(vec![ - [-x / 2., -y / 2.], - [ x / 2., -y / 2.], - [ x / 2., y / 2.], - [-x / 2., y / 2.], - ]).with_color([100,255,0,200]); - - let cuboid = fj::Sweep::from_path(rectangle.into(), [0., 0., z]); - - Ok(cuboid.into()) - } - - fn metadata(&self) -> ModelMetadata { - ModelMetadata::new("Cuboid") - .with_argument(ArgumentMetadata::new("x").with_default_value("3.0")) - .with_argument(ArgumentMetadata::new("y").with_default_value("2.0")) - .with_argument(ArgumentMetadata::new("z").with_default_value("1.0")) - } +#[fj::model] +pub fn model( + #[param(default = 3.0)] x: f64, + #[param(default = 2.0)] y: f64, + #[param(default = 1.0)] z: f64, +) -> fj::Shape { + #[rustfmt::skip] + let rectangle = fj::Sketch::from_points(vec![ + [-x / 2., -y / 2.], + [ x / 2., -y / 2.], + [ x / 2., y / 2.], + [-x / 2., y / 2.], + ]).with_color([100,255,0,200]); + + let cuboid = fj::Sweep::from_path(rectangle.into(), [0., 0., z]); + + cuboid.into() } diff --git a/models/spacer/src/lib.rs b/models/spacer/src/lib.rs index 7e9c691bf..98724904d 100644 --- a/models/spacer/src/lib.rs +++ b/models/spacer/src/lib.rs @@ -1,56 +1,16 @@ -use fj::{ - syntax::*, ArgumentMetadata, ContextExt, HostExt, Metadata, Model, - ModelMetadata, -}; +use fj::syntax::*; -fj::register_model!(|host| { - host.register_model(Spacer); +#[fj::model] +pub fn model( + #[param(default = 1.0, min = inner * 1.01)] outer: f64, + #[param(default = 0.5, max = outer * 0.99)] inner: f64, + #[param(default = 1.0)] height: f64, +) -> fj::Shape { + let outer_edge = fj::Sketch::from_circle(fj::Circle::from_radius(outer)); + let inner_edge = fj::Sketch::from_circle(fj::Circle::from_radius(inner)); - Ok(Metadata::new( - env!("CARGO_PKG_NAME"), - env!("CARGO_PKG_VERSION"), - )) -}); + let footprint = outer_edge.difference(&inner_edge); + let spacer = footprint.sweep([0., 0., height]); -struct Spacer; - -impl Model for Spacer { - fn shape( - &self, - ctx: &dyn fj::Context, - ) -> Result> { - let outer: f64 = ctx.parse_optional_argument("outer")?.unwrap_or(1.0); - let inner: f64 = ctx.parse_optional_argument("inner")?.unwrap_or(0.5); - let height: f64 = ctx.parse_optional_argument("height")?.unwrap_or(1.0); - - if outer < inner * 1.01 { - todo!("Return a suitable error"); - } - if inner > outer * 0.99 { - todo!("Return a suitable error"); - } - - let outer_edge = - fj::Sketch::from_circle(fj::Circle::from_radius(outer)); - let inner_edge = - fj::Sketch::from_circle(fj::Circle::from_radius(inner)); - - let footprint = outer_edge.difference(&inner_edge); - let spacer = footprint.sweep([0., 0., height]); - - Ok(spacer.into()) - } - - fn metadata(&self) -> ModelMetadata { - ModelMetadata::new("Spacer") - .with_argument( - ArgumentMetadata::new("outer").with_default_value("1.0"), - ) - .with_argument( - ArgumentMetadata::new("inner").with_default_value("0.5"), - ) - .with_argument( - ArgumentMetadata::new("height").with_default_value("1.0"), - ) - } + spacer.into() } diff --git a/models/star/src/lib.rs b/models/star/src/lib.rs index 34fa9bab4..3d1775680 100644 --- a/models/star/src/lib.rs +++ b/models/star/src/lib.rs @@ -1,85 +1,40 @@ use std::f64::consts::PI; -use fj::{ - ArgumentMetadata, ContextExt, HostExt, Metadata, Model, ModelMetadata, -}; - -fj::register_model!(|host| { - host.register_model(Star); - - Ok(Metadata::new( - env!("CARGO_PKG_NAME"), - env!("CARGO_PKG_VERSION"), - )) -}); - -struct Star; - -impl Model for Star { - fn shape( - &self, - ctx: &dyn fj::Context, - ) -> Result> { - let num_points: u64 = - ctx.parse_optional_argument("num_points")?.unwrap_or(5); - let r1: f64 = ctx.parse_optional_argument("r1")?.unwrap_or(1.0); - let r2: f64 = ctx.parse_optional_argument("r2")?.unwrap_or(2.0); - let h: f64 = ctx.parse_optional_argument("h")?.unwrap_or(1.0); - - if num_points < 3 { - todo!(); - } - if r1 < 1.0 { - todo!(); - } - if r2 < 2.0 { - todo!(); - } - - let num_vertices = num_points * 2; - let vertex_iter = (0..num_vertices).map(|i| { - let angle = - fj::Angle::from_rad(2. * PI / num_vertices as f64 * i as f64); - let radius = if i % 2 == 0 { r1 } else { r2 }; - (angle, radius) - }); - - // Now that we got that iterator prepared, generating the vertices is just a - // bit of trigonometry. - let mut outer = Vec::new(); - let mut inner = Vec::new(); - for (angle, radius) in vertex_iter { - let (sin, cos) = angle.rad().sin_cos(); - - let x = cos * radius; - let y = sin * radius; - - outer.push([x, y]); - inner.push([x / 2., y / 2.]); - } - - let outer = fj::Sketch::from_points(outer); - let inner = fj::Sketch::from_points(inner); +#[fj::model] +pub fn model( + #[param(default = 5, min = 3)] num_points: u64, + #[param(default = 1.0, min = 1.0)] r1: f64, + #[param(default = 2.0, min = 2.0)] r2: f64, + #[param(default = 1.0)] h: f64, +) -> fj::Shape { + let num_vertices = num_points * 2; + let vertex_iter = (0..num_vertices).map(|i| { + let angle = + fj::Angle::from_rad(2. * PI / num_vertices as f64 * i as f64); + let radius = if i % 2 == 0 { r1 } else { r2 }; + (angle, radius) + }); + + // Now that we got that iterator prepared, generating the vertices is just a + // bit of trigonometry. + let mut outer = Vec::new(); + let mut inner = Vec::new(); + for (angle, radius) in vertex_iter { + let (sin, cos) = angle.rad().sin_cos(); + + let x = cos * radius; + let y = sin * radius; + + outer.push([x, y]); + inner.push([x / 2., y / 2.]); + } - let footprint = - fj::Difference2d::from_shapes([outer.into(), inner.into()]); + let outer = fj::Sketch::from_points(outer); + let inner = fj::Sketch::from_points(inner); - let star = fj::Sweep::from_path(footprint.into(), [0., 0., h]); + let footprint = fj::Difference2d::from_shapes([outer.into(), inner.into()]); - Ok(star.into()) - } + let star = fj::Sweep::from_path(footprint.into(), [0., 0., h]); - fn metadata(&self) -> ModelMetadata { - ModelMetadata::new("Star") - .with_argument( - ArgumentMetadata::new("num_points").with_default_value("5"), - ) - .with_argument( - ArgumentMetadata::new("r1").with_default_value("1.0"), - ) - .with_argument( - ArgumentMetadata::new("r2").with_default_value("2.0"), - ) - .with_argument(ArgumentMetadata::new("h").with_default_value("1.0")) - } + star.into() } diff --git a/models/test/src/lib.rs b/models/test/src/lib.rs index 11abd4883..76f955cf6 100644 --- a/models/test/src/lib.rs +++ b/models/test/src/lib.rs @@ -1,41 +1,18 @@ use std::f64::consts::PI; -use fj::{syntax::*, Angle, HostExt, Metadata, ModelMetadata}; - -fj::register_model!(|host| { - host.register_model(Test); - - Ok( - Metadata::new(env!("CARGO_PKG_NAME"), env!("CARGO_PKG_VERSION")) - .with_short_description(env!("CARGO_PKG_DESCRIPTION")) - .with_description(include_str!("../README.md")) - .with_homepage(env!("CARGO_PKG_HOMEPAGE")) - .with_repository(env!("CARGO_PKG_REPOSITORY")) - .with_license(env!("CARGO_PKG_LICENSE")), - ) -}); - -struct Test; - -impl fj::Model for Test { - fn shape( - &self, - _ctx: &dyn fj::Context, - ) -> Result> { - let a = star(4, 1., [0, 255, 0, 200]); - let b = star(5, -1., [255, 0, 0, 255]) - .rotate([1., 1., 1.], Angle::from_deg(45.)) - .translate([3., 3., 1.]); - let c = spacer().translate([6., 6., 1.]); - - let group = a.group(&b).group(&c); - - Ok(group.into()) - } +use fj::{syntax::*, Angle}; - fn metadata(&self) -> ModelMetadata { - ModelMetadata::new("Test") - } +#[fj::model] +pub fn model() -> fj::Shape { + let a = star(4, 1., [0, 255, 0, 200]); + let b = star(5, -1., [255, 0, 0, 255]) + .rotate([1., 1., 1.], Angle::from_deg(45.)) + .translate([3., 3., 1.]); + let c = spacer().translate([6., 6., 1.]); + + let group = a.group(&b).group(&c); + + group.into() } fn star(num_points: u64, height: f64, color: [u8; 4]) -> fj::Shape { From cf588e88f3e95b1d62bb9d99376019d4341305c3 Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Tue, 9 Aug 2022 14:53:56 +0800 Subject: [PATCH 12/14] Predicates should be negated when checking if a constraint failed --- crates/fj-proc/src/expand.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-proc/src/expand.rs b/crates/fj-proc/src/expand.rs index bea51ad38..b3f886a08 100644 --- a/crates/fj-proc/src/expand.rs +++ b/crates/fj-proc/src/expand.rs @@ -177,7 +177,7 @@ impl ToTokens for Constraint { }; tokens.extend(quote! { - if #predicate { + if !#predicate { return Err(#error_message.into()); } }); From 559059f8aa12151cbcaf6ead94c49d703aae0ee4 Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Tue, 9 Aug 2022 14:55:05 +0800 Subject: [PATCH 13/14] Parens --- crates/fj-proc/src/expand.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-proc/src/expand.rs b/crates/fj-proc/src/expand.rs index b3f886a08..f19d2a4e1 100644 --- a/crates/fj-proc/src/expand.rs +++ b/crates/fj-proc/src/expand.rs @@ -177,7 +177,7 @@ impl ToTokens for Constraint { }; tokens.extend(quote! { - if !#predicate { + if !(#predicate) { return Err(#error_message.into()); } }); From 38c77d48d8853e28c3c4ea3a3777d16d3681301c Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Tue, 9 Aug 2022 15:00:13 +0800 Subject: [PATCH 14/14] Values are allowed to be equal to their max/min --- crates/fj-proc/src/expand.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-proc/src/expand.rs b/crates/fj-proc/src/expand.rs index f19d2a4e1..45be76958 100644 --- a/crates/fj-proc/src/expand.rs +++ b/crates/fj-proc/src/expand.rs @@ -158,8 +158,8 @@ impl ToTokens for Constraint { let Constraint { target, expr, kind } = self; let operator = match kind { - ConstraintKind::Max => quote!(<), - ConstraintKind::Min => quote!(>), + ConstraintKind::Max => quote!(<=), + ConstraintKind::Min => quote!(>=), }; let predicate = quote! { #target #operator #expr }; // Note: this will cause `expr` to be evaluated twice. Predicates should