Skip to content

Commit

Permalink
Make generated identifiers of private fields deterministic (#219)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Veetaha
Veetaha authored Nov 27, 2024
1 parent 9cb599b commit 928ded9
Show file tree
Hide file tree
Showing 11 changed files with 90 additions and 149 deletions.
15 changes: 6 additions & 9 deletions bon-macros/src/builder/builder_gen/builder_decl.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,6 @@ impl super::BuilderGenCtx {
let where_clause = &self.generics.where_clause;
let phantom_data = self.phantom_data();
let state_mod = &self.state_mod.ident;
let phantom_field = &self.ident_pool.phantom;
let receiver_field = &self.ident_pool.receiver;
let start_fn_args_field = &self.ident_pool.start_fn_args;
let named_members_field = &self.ident_pool.named_members;

// The fields can't be hidden using Rust's privacy syntax.
// The details about this are described in the blog post:
Expand All @@ -27,7 +23,8 @@ impl super::BuilderGenCtx {
// generated code. This simplifies the task of removing unnecessary
// attributes from the generated code when preparing for demo purposes.
let deprecated_msg = "\
this field should not be used directly; it's an implementation detail \
this field should not be used directly; it's an implementation detail, and \
if you access it directly, you may break some internal unsafe invariants; \
if you found yourself needing it, then you are probably doing something wrong; \
feel free to open an issue/discussion in our GitHub repository \
(https://github.com/elastio/bon) or ask for help in our Discord server \
Expand All @@ -43,7 +40,7 @@ impl super::BuilderGenCtx {
let ty = &receiver.without_self_keyword;
quote! {
#private_field_attrs
#receiver_field: #ty,
__unsafe_private_receiver: #ty,
}
});

Expand All @@ -62,7 +59,7 @@ impl super::BuilderGenCtx {
let start_fn_args_field = start_fn_arg_types.peek().is_some().then(|| {
quote! {
#private_field_attrs
#start_fn_args_field: (#(#start_fn_arg_types,)*),
__unsafe_private_start_fn_args: (#(#start_fn_arg_types,)*),
}
});

Expand Down Expand Up @@ -104,15 +101,15 @@ impl super::BuilderGenCtx {
#where_clause
{
#private_field_attrs
#phantom_field: #phantom_data,
__unsafe_private_phantom: #phantom_data,

#receiver_field
#start_fn_args_field

#( #custom_fields_idents: #custom_fields_types, )*

#private_field_attrs
#named_members_field: (
__unsafe_private_named: (
#(
::core::option::Option<#named_members_types>,
)*
Expand Down
26 changes: 9 additions & 17 deletions bon-macros/src/builder/builder_gen/builder_derives.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,17 +70,12 @@ impl BuilderGenCtx {
let generic_args = &self.generics.args;
let builder_ident = &self.builder_type.ident;

let phantom_field = &self.ident_pool.phantom;
let receiver_field = &self.ident_pool.receiver;
let start_fn_args_field = &self.ident_pool.start_fn_args;
let named_members_field = &self.ident_pool.named_members;

let clone = quote!(::core::clone::Clone);

let clone_receiver = self.receiver().map(|receiver| {
let ty = &receiver.without_self_keyword;
quote! {
#receiver_field: <#ty as #clone>::clone(&self.#receiver_field),
__unsafe_private_receiver: <#ty as #clone>::clone(&self.__unsafe_private_receiver),
}
});

Expand All @@ -96,8 +91,8 @@ impl BuilderGenCtx {
// ```
// required for `(...big type...)` to implement `Clone`
// ```
#start_fn_args_field: (
#( <#types as #clone>::clone(&self.#start_fn_args_field.#indices), )*
__unsafe_private_start_fn_args: (
#( <#types as #clone>::clone(&self.__unsafe_private_start_fn_args.#indices), )*
),
}
});
Expand All @@ -115,7 +110,7 @@ impl BuilderGenCtx {

quote! {
#bon::__::derives::clone_member::<#ty>(
&self.#named_members_field.#member_index
&self.__unsafe_private_named.#member_index
)
}
});
Expand Down Expand Up @@ -148,7 +143,7 @@ impl BuilderGenCtx {
{
fn clone(&self) -> Self {
Self {
#phantom_field: ::core::marker::PhantomData,
__unsafe_private_phantom: ::core::marker::PhantomData,
#clone_receiver
#clone_start_fn_args
#( #clone_fields, )*
Expand All @@ -160,17 +155,14 @@ impl BuilderGenCtx {
// ```
// required for `(...big type...)` to implement `Clone`
// ```
#named_members_field: ( #( #clone_named_members, )* ),
__unsafe_private_named: ( #( #clone_named_members, )* ),
}
}
}
}
}

fn derive_debug(&self, derive: &DeriveConfig) -> TokenStream {
let receiver_field = &self.ident_pool.receiver;
let start_fn_args_field = &self.ident_pool.start_fn_args;
let named_members_field = &self.ident_pool.named_members;
let bon = &self.bon;

let format_members = self.members.iter().filter_map(|member| {
Expand All @@ -183,7 +175,7 @@ impl BuilderGenCtx {
output.field(
#member_ident_str,
#bon::__::derives::as_dyn_debug::<#member_ty>(
&self.#start_fn_args_field.#member_index
&self.__unsafe_private_start_fn_args.#member_index
)
);
})
Expand All @@ -206,7 +198,7 @@ impl BuilderGenCtx {
let member_ident_str = &member.name.snake_raw_str;
let member_ty = member.underlying_norm_ty();
Some(quote! {
if let Some(value) = &self.#named_members_field.#member_index {
if let Some(value) = &self.__unsafe_private_named.#member_index {
output.field(
#member_ident_str,
#bon::__::derives::as_dyn_debug::<#member_ty>(value)
Expand All @@ -228,7 +220,7 @@ impl BuilderGenCtx {
output.field(
"self",
#bon::__::derives::as_dyn_debug::<#ty>(
&self.#receiver_field
&self.__unsafe_private_receiver
)
);
}
Expand Down
10 changes: 4 additions & 6 deletions bon-macros/src/builder/builder_gen/finish_fn.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use super::member::{Member, PosFnMember};
use crate::util::prelude::*;

impl super::BuilderGenCtx {
fn finish_fn_member_expr(&self, member: &Member) -> TokenStream {
fn finish_fn_member_expr(member: &Member) -> TokenStream {
let member = match member {
Member::Named(member) => member,
Member::Skip(member) => {
Expand All @@ -14,9 +14,8 @@ impl super::BuilderGenCtx {
}
Member::StartFn(member) => {
let index = &member.index;
let start_fn_args_field = &self.ident_pool.start_fn_args;

return quote! { self.#start_fn_args_field.#index };
return quote! { self.__unsafe_private_start_fn_args.#index };
}
Member::FinishFn(member) => {
return member
Expand All @@ -31,9 +30,8 @@ impl super::BuilderGenCtx {

let index = &member.index;

let named_members_field = &self.ident_pool.named_members;
let member_field = quote! {
self.#named_members_field.#index
self.__unsafe_private_named.#index
};

let default = member
Expand Down Expand Up @@ -88,7 +86,7 @@ impl super::BuilderGenCtx {

pub(super) fn finish_fn(&self) -> TokenStream {
let members_vars_decls = self.members.iter().map(|member| {
let expr = self.finish_fn_member_expr(member);
let expr = Self::finish_fn_member_expr(member);
let var_ident = member.orig_ident();

// The type hint is necessary in some cases to assist the compiler
Expand Down
5 changes: 1 addition & 4 deletions bon-macros/src/builder/builder_gen/input_fn.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -454,10 +454,7 @@ impl FinishFnBody for FnCallBody {
let prefix = self
.sig
.receiver()
.map(|_| {
let receiver_field = &ctx.ident_pool.receiver;
quote!(self.#receiver_field.)
})
.map(|_| quote!(self.__unsafe_private_receiver.))
.or_else(|| {
let self_ty = &self.impl_ctx.as_deref()?.self_ty;
Some(quote!(<#self_ty>::))
Expand Down
84 changes: 0 additions & 84 deletions bon-macros/src/builder/builder_gen/models.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,6 @@ pub(crate) struct BuilderGenCtx {
/// Path to the `bon` crate.
pub(super) bon: syn::Path,

/// Private identifiers that are used in the builder implementation.
/// They are intentionally randomized to prevent users from accessing them.
pub(super) ident_pool: PrivateIdentsPool,

/// Name of the generic variable that holds the builder's state.
pub(super) state_var: syn::Ident,

Expand All @@ -160,22 +156,6 @@ pub(crate) struct BuilderGenCtx {
pub(super) finish_fn: FinishFn,
}

/// Identifiers that are private to the builder implementation. The users should
/// not use them directly. They are randomly generated to prevent users from
/// using them. Even if they try to reference them, they won't be able to re-compile
/// their code because the names of these identifiers are regenerated on every
/// macro run.
///
/// This is an unfortunate workaround due to the limitations of defining the
/// builder type inside of a nested module. See more details on this problem in
/// <https://bon-rs.com/blog/the-weird-of-function-local-types-in-rust>
pub(super) struct PrivateIdentsPool {
pub(super) phantom: syn::Ident,
pub(super) receiver: syn::Ident,
pub(super) start_fn_args: syn::Ident,
pub(super) named_members: syn::Ident,
}

pub(super) struct BuilderGenCtxParams<'a> {
pub(crate) bon: Option<syn::Path>,
pub(super) namespace: Cow<'a, GenericsNamespace>,
Expand Down Expand Up @@ -333,7 +313,6 @@ impl BuilderGenCtx {
Ok(Self {
bon: bon.unwrap_or_else(|| syn::parse_quote!(::bon)),
state_var,
ident_pool: PrivateIdentsPool::new(),
members,
allow_attrs,
on,
Expand All @@ -347,69 +326,6 @@ impl BuilderGenCtx {
}
}

impl PrivateIdentsPool {
fn new() -> Self {
use std::collections::hash_map::RandomState;
use std::hash::{BuildHasher, Hasher};

// Thanks @orhun for the article https://blog.orhun.dev/zero-deps-random-in-rust/
let random = RandomState::new().build_hasher().finish();

// Totally random words. All coincidences are purely accidental 😸
let random_words = [
"amethyst",
"applejack",
"blackjack",
"bon",
"cadance",
"celestia",
"cheerilee",
"derpy",
"fleetfoot",
"flitter",
"fluttershy",
"izzy",
"lilly",
"littlepip",
"luna",
"lyra",
"maud",
"minuette",
"octavia",
"pinkie",
"pipp",
"rainbow",
"rampage",
"rarity",
"roseluck",
"scootaloo",
"seaswirl",
"spitfire",
"starlight",
"sunset",
"sweetie",
"trixie",
"twilight",
"twinkleshine",
"twist",
"velvet",
"vinyl",
];

#[allow(clippy::cast_possible_truncation)]
let random_word = random_words[(random % (random_words.len() as u64)) as usize];

let ident = |name: &str| format_ident!("__private_{random_word}_{name}");

Self {
phantom: ident("phantom"),
receiver: ident("receiver"),
start_fn_args: ident("start_fn_args"),
named_members: ident("named_members"),
}
}
}

impl Generics {
pub(super) fn new(
decl_with_defaults: Vec<syn::GenericParam>,
Expand Down
28 changes: 10 additions & 18 deletions bon-macros/src/builder/builder_gen/setters/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -342,39 +342,31 @@ impl<'a> SettersCtx<'a> {
let body = match imp.body {
SetterBody::Forward { body } => body,
SetterBody::SetMember { expr } => {
let idents = &self.base.ident_pool;
let phantom_field = &idents.phantom;
let receiver_field = &idents.receiver;
let start_fn_args_field = &idents.start_fn_args;
let named_members_field = &idents.named_members;

let mut output = if !self.member.is_stateful() {
quote! {
self
}
} else {
let builder_ident = &self.base.builder_type.ident;

let maybe_receiver_field = self
.base
.receiver()
.map(|_| quote!(#receiver_field: self.#receiver_field,));
let maybe_receiver_field = self.base.receiver().map(
|_| quote!(__unsafe_private_receiver: self.__unsafe_private_receiver,),
);

let maybe_start_fn_args_field = self
.base
.start_fn_args()
.next()
.map(|_| quote!(#start_fn_args_field: self.#start_fn_args_field,));
let maybe_start_fn_args_field =
self.base.start_fn_args().next().map(
|_| quote!(__unsafe_private_start_fn_args: self.__unsafe_private_start_fn_args,),
);

let custom_fields_idents = self.base.custom_fields().map(|field| &field.ident);

quote! {
#builder_ident {
#phantom_field: ::core::marker::PhantomData,
__unsafe_private_phantom: ::core::marker::PhantomData,
#( #custom_fields_idents: self.#custom_fields_idents, )*
#maybe_receiver_field
#maybe_start_fn_args_field
#named_members_field: self.#named_members_field,
__unsafe_private_named: self.__unsafe_private_named,
}
}
};
Expand All @@ -393,7 +385,7 @@ impl<'a> SettersCtx<'a> {

let index = &self.member.index;
quote! {
self.#named_members_field.#index = #expr;
self.__unsafe_private_named.#index = #expr;
#output
}
}
Expand Down
Loading

0 comments on commit 928ded9

Please sign in to comment.