Skip to content

Commit

Permalink
Refactored feature gating
Browse files Browse the repository at this point in the history
Applied the requested changes to be more sane and heavily simplify the feature gating for `dynamic_fields`.
  • Loading branch information
LoopyAshy committed Oct 23, 2022
1 parent f05d7ed commit 540331d
Show file tree
Hide file tree
Showing 15 changed files with 72 additions and 106 deletions.
4 changes: 2 additions & 2 deletions crates/rune-cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
//! [rune]: https://github.com/rune-rs/rune
use anyhow::{anyhow, Result};
use rune::compile::ParseOptionError;
use rune::compile::ParseOptionsErrors;
use rune::termcolor::{Color, ColorChoice, ColorSpec, StandardStream, WriteColor};
use rune::workspace::WorkspaceFilter;
use rune::{Context, ContextError, Options};
Expand Down Expand Up @@ -329,7 +329,7 @@ struct Args {

impl Args {
/// Construct compiler options from cli arguments.
fn options(&self) -> Result<Options, ParseOptionError> {
fn options(&self) -> Result<Options, ParseOptionsErrors> {
let mut options = Options::default();

// Command-specific override defaults.
Expand Down
3 changes: 0 additions & 3 deletions crates/rune-macros/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@ description = """
Helper macros for Rune.
"""

[features]
dynamic_fields = []

[dependencies]
syn = { version = "1.0.82", features = ["full"] }
quote = "1.0.10"
Expand Down
47 changes: 18 additions & 29 deletions crates/rune-macros/src/any.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,7 @@ impl InternalCall {
Ok(())
};

#[cfg(feature = "dynamic_fields")]
let meta_fields = Some(quote! {Never});
#[cfg(not(feature = "dynamic_fields"))]
let meta_fields = None;
let meta_fields = quote! {Never};

let generics = syn::Generics::default();
expand_any(
Expand All @@ -60,7 +57,7 @@ impl InternalCall {
&expand_into,
&tokens,
&generics,
meta_fields,
&meta_fields,
)
}
}
Expand Down Expand Up @@ -104,24 +101,24 @@ impl Derive {
let meta_fields = match attrs.meta_fields {
Some(meta_fields) => {
if cfg!(feature = "dynamic_fields") {
Some(quote! {#meta_fields})
quote! {#meta_fields}
} else {
None
}
}
None => {
if cfg!(feature = "dynamic_fields") {
Some(quote! {Never})
} else {
None
return Err(vec![syn::Error::new(
meta_fields.span(),
format!(
"attempted to set dynamic_fields to `{}` while the feature `dynamic_fields` is disabled.",
meta_fields
),
)]);
}
}
None => quote! {Never},
};

let name = &quote!(#name);
let ident = &self.input.ident;

expand_any(ident, name, &install_with, &tokens, generics, meta_fields)
expand_any(ident, name, &install_with, &tokens, generics, &meta_fields)
}
}

Expand Down Expand Up @@ -400,16 +397,16 @@ pub(super) fn expand_any<T>(
installers: &TokenStream,
tokens: &Tokens,
generics: &syn::Generics,
meta_fields: Option<TokenStream>,
meta_fields: &TokenStream,
) -> Result<TokenStream, Vec<syn::Error>>
where
T: Copy + ToTokens,
{
let Tokens {
any,
context_error,
field_search: field_mode,
field_mode: field_search,
field_mode,
field_search,
hash,
module,
named,
Expand All @@ -432,16 +429,6 @@ where

let generic_names = generics.type_params().map(|v| &v.ident).collect::<Vec<_>>();

let impl_field_mode = if meta_fields.is_some() {
quote! {
impl #impl_generics #field_mode for #ident #ty_generics #where_clause {
const FIELD_MODE: #field_search = #field_search::#meta_fields;
}
}
} else {
quote! {}
};

let impl_named = if !generic_names.is_empty() {
quote! {
impl #impl_generics #named for #ident #ty_generics #where_clause {
Expand Down Expand Up @@ -477,7 +464,9 @@ where

#impl_named

#impl_field_mode
impl #impl_generics #field_search for #ident #ty_generics #where_clause {
const DYNAMIC_FIELD_MODE: #field_mode = #field_mode::#meta_fields;
}

impl #impl_generics #type_of for #ident #ty_generics #where_clause {
fn type_hash() -> #hash {
Expand Down
2 changes: 1 addition & 1 deletion crates/rune/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ default = ["emit"]
emit = ["codespan-reporting"]
bench = []
workspace = ["toml", "toml-spanned-value", "semver", "relative-path", "serde-hashkey"]
dynamic_fields = ["rune-macros/dynamic_fields"]
dynamic_fields = []

[dependencies]
thiserror = "1.0.30"
Expand Down
26 changes: 0 additions & 26 deletions crates/rune/src/any.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
#[cfg(feature = "dynamic_fields")]
use crate::compile::DynamicFieldSearch;

use crate::compile::Named;
Expand All @@ -22,31 +21,6 @@ pub use rune_macros::Any;
/// name: String,
/// }
/// ```
#[cfg(not(feature = "dynamic_fields"))]
pub trait Any: Named + std::any::Any {
/// The type hash of the type.
///
/// TODO: make const field when `TypeId::of` is const.
fn type_hash() -> Hash;
}

/// A trait which can be stored inside of an [AnyObj](crate::runtime::AnyObj).
///
/// We use our own marker trait that must be explicitly derived to prevent other
/// VM native types (like strings) which also implement `std::any::Any` from
/// being stored as an `AnyObj`.
///
/// This means, that only types which derive `Any` can be used inside of the VM:
///
/// ```
/// use rune::Any;
///
/// #[derive(Any)]
/// struct Npc {
/// name: String,
/// }
/// ```
#[cfg(feature = "dynamic_fields")]
pub trait Any: Named + DynamicFieldSearch + std::any::Any {
/// The type hash of the type.
///
Expand Down
8 changes: 5 additions & 3 deletions crates/rune/src/compile/dynamic_field_mode.rs
Original file line number Diff line number Diff line change
@@ -1,24 +1,26 @@
#[cfg(feature = "dynamic_fields")]
/// The trait used to identify when to search for fields
/// within a `AnyObj` via `Protocol::DYNAMIC_FIELD_GET` and
/// `Protocol::DYNAMIC_FIELD_SET`
pub trait DynamicFieldSearch {
/// When to use Protocol::DYNAMIC_FIELD_SET/Protocol::DYNAMIC_FIELD_GET over Protocol::SET/Protocol::GET.
const FIELD_MODE: DynamicFieldMode;
const DYNAMIC_FIELD_MODE: DynamicFieldMode;
}

#[cfg(feature = "dynamic_fields")]
/// The possible values for the `MetaFieldMode` trait.
#[derive(Debug, Clone, Copy)]
#[non_exhaustive]
pub enum DynamicFieldMode {
/// Never use `Protocol::DYNAMIC_FIELD_GET` or `Protocol::DYNAMIC_FIELD_SET`
Never,
/// Use `Protocol::DYNAMIC_FIELD_GET` or `Protocol::DYNAMIC_FIELD_SET` before
/// `Protocol::GET` and `Protocol::SET` respectively.
#[cfg(feature = "dynamic_fields")]
First,
/// Use `Protocol::GET` or `Protocol::SET` before
/// `Protocol::DYNAMIC_FIELD_GET` and `Protocol::DYNAMIC_FIELD_SET` respectively.
#[cfg(feature = "dynamic_fields")]
Last,
/// Use `Protocol::DYNAMIC_FIELD_GET` or `Protocol::DYNAMIC_FIELD_SET` exclusively.
#[cfg(feature = "dynamic_fields")]
Only,
}
4 changes: 1 addition & 3 deletions crates/rune/src/compile/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ pub(crate) use self::unit_builder::UnitBuilder;
mod v1;

mod options;
pub use self::options::{Options, ParseOptionError};
pub use self::options::{Options, ParseOptionsErrors};

mod location;
pub use self::location::Location;
Expand Down Expand Up @@ -77,9 +77,7 @@ pub(crate) use self::names::Names;
mod visibility;
pub(crate) use self::visibility::Visibility;

#[cfg(feature = "dynamic_fields")]
mod dynamic_field_mode;
#[cfg(feature = "dynamic_fields")]
pub use self::dynamic_field_mode::{DynamicFieldMode, DynamicFieldSearch};

/// A compile result alias.
Expand Down
40 changes: 32 additions & 8 deletions crates/rune/src/compile/options.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,22 @@
use thiserror::Error;

/// Error raised when trying to parse an invalid option.
/// Errors possibly returned while parsing options.
#[derive(Debug, Clone, Error)]
#[error("unsupported compile option `{option}`")]
pub struct ParseOptionError {
option: Box<str>,
pub enum ParseOptionsErrors {
/// Error raised when trying to parse an invalid option.
#[error("unsupported compile option `{option}`")]
UnsupportedOption {
/// The unsupported option.
option: Box<str>,
},
/// Error raised when trying to enable a option which is not enabled by a feature gate.
#[error("Featured gated compile option `{option}` requires feature `{required_feature}`")]
DisabledFeatureOptionError {
/// The disabled option.
option: Box<str>,
/// The required feature to enable this option.
required_feature: &'static str,
},
}

/// Options that can be provided to the compiler.
Expand Down Expand Up @@ -40,7 +52,7 @@ impl Options {
///
/// It can be used to consistenly parse a collection of options by other
/// programs as well.
pub fn parse_option(&mut self, option: &str) -> Result<(), ParseOptionError> {
pub fn parse_option(&mut self, option: &str) -> Result<(), ParseOptionsErrors> {
let mut it = option.split('=');

match it.next() {
Expand All @@ -65,12 +77,24 @@ impl Options {
Some("v2") => {
self.v2 = it.next() != Some("false");
}
#[cfg(feature = "dynamic_fields")]
Some("dynamic_fields") => {
self.dynamic_fields = it.next() != Some("false");
let result = it.next() != Some("false");
if result {
#[cfg(feature = "dynamic_fields")]
{
self.dynamic_fields = result;
}
#[cfg(not(feature = "dynamic_fields"))]
{
return Err(ParseOptionsErrors::DisabledFeatureOptionError {
option: option.into(),
required_feature: "dynamic_fields",
});
}
}
}
_ => {
return Err(ParseOptionError {
return Err(ParseOptionsErrors::UnsupportedOption {
option: option.into(),
});
}
Expand Down
21 changes: 6 additions & 15 deletions crates/rune/src/runtime/any_obj.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
//! Helper types for a holder of data.
#[cfg(feature = "dynamic_fields")]
use crate::compile::{DynamicFieldMode, DynamicFieldSearch};

use crate::runtime::RawStr;
Expand Down Expand Up @@ -59,8 +58,7 @@ impl AnyObj {
debug: debug_impl::<T>,
type_name: type_name_impl::<T>,
type_hash: type_hash_impl::<T>,
#[cfg(feature = "dynamic_fields")]
field_mode: T::FIELD_MODE,
field_mode: T::DYNAMIC_FIELD_MODE,
},
data: data as *mut (),
}
Expand Down Expand Up @@ -117,8 +115,7 @@ impl AnyObj {
debug: debug_ref_impl::<T>,
type_name: type_name_impl::<T>,
type_hash: type_hash_impl::<T>,
#[cfg(feature = "dynamic_fields")]
field_mode: T::FIELD_MODE,
field_mode: T::DYNAMIC_FIELD_MODE,
},
data: data as *const _ as *const (),
}
Expand Down Expand Up @@ -165,8 +162,7 @@ impl AnyObj {
debug: debug_ref_impl::<T::Target>,
type_name: type_name_impl::<T::Target>,
type_hash: type_hash_impl::<T::Target>,
#[cfg(feature = "dynamic_fields")]
field_mode: T::Target::FIELD_MODE,
field_mode: T::Target::DYNAMIC_FIELD_MODE,
},
data: boxed_guard as *const _ as *const (),
}
Expand Down Expand Up @@ -229,8 +225,7 @@ impl AnyObj {
debug: debug_mut_impl::<T>,
type_name: type_name_impl::<T>,
type_hash: type_hash_impl::<T>,
#[cfg(feature = "dynamic_fields")]
field_mode: T::FIELD_MODE,
field_mode: T::DYNAMIC_FIELD_MODE,
},
data: data as *const _ as *const (),
}
Expand Down Expand Up @@ -277,8 +272,7 @@ impl AnyObj {
debug: debug_mut_impl::<T::Target>,
type_name: type_name_impl::<T::Target>,
type_hash: type_hash_impl::<T::Target>,
#[cfg(feature = "dynamic_fields")]
field_mode: T::Target::FIELD_MODE,
field_mode: T::Target::DYNAMIC_FIELD_MODE,
},
data: boxed_guard as *const _ as *const (),
}
Expand Down Expand Up @@ -462,7 +456,6 @@ impl AnyObj {
(self.vtable.type_hash)()
}

#[cfg(feature = "dynamic_fields")]
pub(crate) fn field_mode(&self) -> DynamicFieldMode {
self.vtable.field_mode
}
Expand Down Expand Up @@ -522,9 +515,7 @@ pub struct AnyObjVtable {
type_name: TypeNameFn,
/// Get the type hash of the stored type.
type_hash: TypeHashFn,

#[cfg(feature = "dynamic_fields")]
/// Get the method of searching fields.
/// When to search for dynamic fields.
field_mode: DynamicFieldMode,
}

Expand Down
4 changes: 0 additions & 4 deletions crates/rune/src/runtime/inst.rs
Original file line number Diff line number Diff line change
Expand Up @@ -997,7 +997,6 @@ pub enum Inst {
/// <object>
/// => <value>
/// ```
#[cfg(feature = "dynamic_fields")]
ObjectIndexDynamicGet {
/// The static string slot corresponding to the index to fetch.
slot: usize,
Expand All @@ -1015,7 +1014,6 @@ pub enum Inst {
/// <value>
/// =>
/// ```
#[cfg(feature = "dynamic_fields")]
ObjectIndexDynamicSet {
/// The static string slot corresponding to the index to set.
slot: usize,
Expand Down Expand Up @@ -1322,11 +1320,9 @@ impl fmt::Display for Inst {
Self::Panic { reason } => {
write!(fmt, "panic reason={}", reason.ident())?;
}
#[cfg(feature = "dynamic_fields")]
Self::ObjectIndexDynamicGet { slot } => {
write!(fmt, "object-index-dynamic-get slot={}", slot)?;
}
#[cfg(feature = "dynamic_fields")]
Self::ObjectIndexDynamicSet { slot } => {
write!(fmt, "object-index-dynamic-set slot={}", slot)?;
}
Expand Down
Loading

0 comments on commit 540331d

Please sign in to comment.