emproof-com / nyxstone

Nyxstone: assembly / disassembly library based on LLVM, implemented in C++ with Rust and Python bindings, maintained by emproof.com
https://www.emproof.com
MIT License
311 stars 14 forks source link

[Rust] Revise the builder #31

Closed seritools closed 11 months ago

seritools commented 11 months ago

It doesn't seem like the derive_builder Builder pattern works all that well as used here:

Instead, I'd propose to use an init/options struct instead, with the build/new function taking the triple directly:

let config = NyxstoneInit {
    immediate_style: IntegerBase::HexPrefix,
    ...Default::default()
};
let nyxstone = Nyxstone::new("x86_64", config)?;
#[non_exhaustive] // optional, but would allow you to add more init options in the future by forcing the instantiator to use NyxstoneInit { field, ...Default::default() } to create the struct
#[derive(Debug, Default)] // all fields impl Default!
pub struct NyxstoneInit<'a> {
   pub cpu: Option<&'a str>, // .unwrap_or("") in `Nyxstone::new` to get the current behavior
   pub enabled_features: Vec<String>,
   pub disabled_features: Vec<String>,
   pub immediate_style: IntegerBase,
}

impl NyxstoneInit<'_> {
    pub fn with_feature(mut self, feature: &str) -> Self { ... } // note how these can still take self by value if you want
    pub fn without_feature(mut self, feature: &str) -> Self { ... }

    pub fn init(self, target_triple: &str) -> Result<Nyxstone, NyxstoneInitError> {
        Nyxstone::new(target_triple, self)
    }
}

pub struct Nyxstone {
    // all the useless fields gone

    inner: cxx::UniquePtr<ffi::NyxstoneFFI>,
}

impl Nyxstone { 
   pub fn new(target_triple: &str, options: NyxstoneInit<'_>) -> Result<Self, NyxstoneInitError> {
       // do the init
   }
}