From e25005b19729c3386db4405d9ec994ae454be81b Mon Sep 17 00:00:00 2001 From: Niklas Dewally Date: Fri, 27 Oct 2023 18:51:49 +0100 Subject: [PATCH 01/10] DROP ME: change submodule url for testing --- .gitmodules | 3 ++- solvers/minion/vendor | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.gitmodules b/.gitmodules index bb672fbb3..894ec4b3b 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,6 +1,7 @@ [submodule "minion-bindings/vendor"] path = solvers/minion/vendor - url = https://github.com/minion/minion + url = https://github.com/niklasdewally/minion + branch = conjure-oxide-devel ignore = dirty [submodule "solvers/chuffed/vendor"] diff --git a/solvers/minion/vendor b/solvers/minion/vendor index 9f4948fbb..7e408139e 160000 --- a/solvers/minion/vendor +++ b/solvers/minion/vendor @@ -1 +1 @@ -Subproject commit 9f4948fbb099a18663c37b76e6e7a568c585c126 +Subproject commit 7e408139e94c1b9de5c2ab81403006af4e81ffe4 From 641e0ebf600dc20091af02b3fdfe1d4e31d94761 Mon Sep 17 00:00:00 2001 From: Niklas Dewally Date: Fri, 27 Oct 2023 18:57:38 +0100 Subject: [PATCH 02/10] minion: rename things --- solvers/minion/src/lib.rs | 4 +--- solvers/minion/src/main.rs | 2 +- solvers/minion/src/raw_bindings.rs | 1 + solvers/minion/tests/dummy_test.rs | 2 +- 4 files changed, 4 insertions(+), 5 deletions(-) create mode 100644 solvers/minion/src/raw_bindings.rs diff --git a/solvers/minion/src/lib.rs b/solvers/minion/src/lib.rs index 8be9d8140..0f130578d 100644 --- a/solvers/minion/src/lib.rs +++ b/solvers/minion/src/lib.rs @@ -1,3 +1 @@ -pub mod bindings { - include!(concat!(env!("OUT_DIR"), "/bindings.rs")); -} +pub mod raw_bindings; diff --git a/solvers/minion/src/main.rs b/solvers/minion/src/main.rs index 19fcf8893..dca032a13 100644 --- a/solvers/minion/src/main.rs +++ b/solvers/minion/src/main.rs @@ -17,6 +17,6 @@ pub fn main() { .collect::>(); unsafe { - minion_rs::bindings::minion_main(c_args.len() as i32, c_args.as_ptr() as *mut *mut i8); + minion_rs::raw_bindings::minion_main(c_args.len() as i32, c_args.as_ptr() as *mut *mut i8); } } diff --git a/solvers/minion/src/raw_bindings.rs b/solvers/minion/src/raw_bindings.rs new file mode 100644 index 000000000..44ae78fca --- /dev/null +++ b/solvers/minion/src/raw_bindings.rs @@ -0,0 +1 @@ +include!(concat!(env!("OUT_DIR"), "/bindings.rs")); diff --git a/solvers/minion/tests/dummy_test.rs b/solvers/minion/tests/dummy_test.rs index b3e8fcf98..a8bf08d02 100644 --- a/solvers/minion/tests/dummy_test.rs +++ b/solvers/minion/tests/dummy_test.rs @@ -1,4 +1,4 @@ -use minion_rs::bindings; +use minion_rs::raw_bindings; #[test] fn dummy() { From ebbbb1afaae61d733eb3ed5a7cc427c77beb24f4 Mon Sep 17 00:00:00 2001 From: Niklas Dewally Date: Sat, 28 Oct 2023 11:02:40 +0100 Subject: [PATCH 03/10] minion: working raw bindings --- Cargo.lock | 12 ++-- solvers/minion/.gitignore | 1 + solvers/minion/build.rs | 33 +++++++-- solvers/minion/build.sh | 4 +- solvers/minion/src/main.rs | 22 ------ solvers/minion/src/raw_bindings.rs | 105 +++++++++++++++++++++++++++++ solvers/minion/tests/dummy_test.rs | 6 -- 7 files changed, 143 insertions(+), 40 deletions(-) delete mode 100644 solvers/minion/src/main.rs delete mode 100644 solvers/minion/tests/dummy_test.rs diff --git a/Cargo.lock b/Cargo.lock index 5c8c202fb..e79b4433c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -341,9 +341,9 @@ checksum = "08d43f7aa6b08d49f382cde6a7982047c3426db949b1424bc4b7ec9ae12c6ce2" [[package]] name = "rustix" -version = "0.38.19" +version = "0.38.21" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "745ecfa778e66b2b63c88a61cb36e0eea109e803b0b86bf9879fbc77c70e86ed" +checksum = "2b426b0506e5d50a7d8dafcf2e81471400deb602392c7dd110815afb4eaf02a3" dependencies = [ "bitflags 2.4.1", "errno", @@ -382,18 +382,18 @@ dependencies = [ [[package]] name = "thiserror" -version = "1.0.49" +version = "1.0.50" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1177e8c6d7ede7afde3585fd2513e611227efd6481bd78d2e82ba1ce16557ed4" +checksum = "f9a7210f5c9a7156bb50aa36aed4c95afb51df0df00713949448cf9e97d382d2" dependencies = [ "thiserror-impl", ] [[package]] name = "thiserror-impl" -version = "1.0.49" +version = "1.0.50" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "10712f02019e9288794769fba95cd6847df9874d49d871d062172f9dd41bc4cc" +checksum = "266b2e40bc00e5a6c09c3584011e08b06f123c00362c92b975ba9843aaaa14b8" dependencies = [ "proc-macro2", "quote", diff --git a/solvers/minion/.gitignore b/solvers/minion/.gitignore index a9853e79b..e328a97db 100644 --- a/solvers/minion/.gitignore +++ b/solvers/minion/.gitignore @@ -1,3 +1,4 @@ debug/ target/ vendor/build +*.log diff --git a/solvers/minion/build.rs b/solvers/minion/build.rs index d7eaa668c..953d239e5 100755 --- a/solvers/minion/build.rs +++ b/solvers/minion/build.rs @@ -1,7 +1,8 @@ // adapted from -// - https://github.com/gokberkkocak/rust_glucose/blob/master/build.rs +// e https://github.com/gokberkkocak/rust_glucose/blob/master/build.rs // - https://rust-lang.github.io/rust-bindgen/non-system-libraries.html // - https://doc.rust-lang.org/cargo/reference/build-scripts.html#rerun-if-changed +// use std::env; use std::path::PathBuf; @@ -61,11 +62,35 @@ fn bind() { // Tell cargo to invalidate the built crate whenever any of the // included header files changed. .parse_callbacks(Box::new(bindgen::CargoCallbacks)) - // Must manually give allow list to stop bindgen accidentally binding something complicated - // in C++ stdlib that will make it crash. - .allowlist_function("minion_main") + // Make all templates opaque as reccomended by bindgen + .opaque_type("std::.*") + // Manually allow C++ functions to stop bindgen getting confused. + .allowlist_function("resetMinion") + .allowlist_function("runMinion") + .allowlist_function("constantAsVar") + .allowlist_function("newSearchOptions") + .allowlist_function("newSearchMethod") + .allowlist_function("newInstance") + .allowlist_function("newConstraintBlob") + .allowlist_function("newSearchOrder") + .allowlist_function("getVarByName") + .allowlist_function("newVar_ffi") + .allowlist_function("instance-free") + .allowlist_function("instance_addSearchOrder") + .allowlist_function("instance_addConstraint") + .allowlist_function("printMatrix_addVar") + .allowlist_function("printMatrix_getValue") + .allowlist_function("constraint_addVarList") + .allowlist_function("constraint_addConstantList") + .allowlist_function("vec_var_new") + .allowlist_function("vec_var_push_back") + .allowlist_function("vec_var_free") + .allowlist_function("vec_int_new") + .allowlist_function("vec_int_push_back") + .allowlist_function("vec_int_free") .clang_arg("-Ivendor/build/src/") // generated from configure.py .clang_arg("-Ivendor/minion/") + .clang_arg("-DLIBMINION") .clang_arg(r"--std=gnu++11") .clang_arg(r"-xc++") // Finish the builder and generate the bindings. diff --git a/solvers/minion/build.sh b/solvers/minion/build.sh index 856233be0..5107bf508 100755 --- a/solvers/minion/build.sh +++ b/solvers/minion/build.sh @@ -14,10 +14,10 @@ if [ -d vendor/build ]; then else mkdir -p vendor/build cd vendor/build - python3 ../configure.py --library + python3 ../configure.py --lib fi echo "------ BUILD STEP ------" cd "$SCRIPT_DIR" cd vendor/build -make all +nice make -j8 diff --git a/solvers/minion/src/main.rs b/solvers/minion/src/main.rs deleted file mode 100644 index dca032a13..000000000 --- a/solvers/minion/src/main.rs +++ /dev/null @@ -1,22 +0,0 @@ -#![allow(non_upper_case_globals)] -#![allow(non_camel_case_types)] -#![allow(non_snake_case)] - -use std::ffi::c_char; -use std::ffi::CString; - -pub fn main() { - // https://stackoverflow.com/questions/34379641/how-do-i-convert-rust-args-into-the-argc-and-argv-c-equivalents - let args = std::env::args() - .map(|arg| CString::new(arg).unwrap()) - .collect::>(); - - let c_args = args - .iter() - .map(|arg| arg.as_ptr()) - .collect::>(); - - unsafe { - minion_rs::raw_bindings::minion_main(c_args.len() as i32, c_args.as_ptr() as *mut *mut i8); - } -} diff --git a/solvers/minion/src/raw_bindings.rs b/solvers/minion/src/raw_bindings.rs index 44ae78fca..b1c23a4a3 100644 --- a/solvers/minion/src/raw_bindings.rs +++ b/solvers/minion/src/raw_bindings.rs @@ -1 +1,106 @@ include!(concat!(env!("OUT_DIR"), "/bindings.rs")); + + +#[cfg(test)] +mod tests { + + use std::ffi::CString; + use super::*; + + // solutions + static mut X_VAL:i32 = 0; + static mut Y_VAL:i32 = 0; + static mut Z_VAL:i32 = 0; + + #[no_mangle] + pub extern "C" fn hello_from_rust() -> bool { + unsafe{ + X_VAL = printMatrix_getValue(0) as _; + Y_VAL = printMatrix_getValue(1) as _; + Z_VAL = printMatrix_getValue(2) as _; + return true; + } + } + + #[test] + fn xyz_raw() { + // A simple constraints model, manually written using FFI functions. + // Testing to see if it does not segfault. + // Results can be manually inspected in the outputted minion logs. + unsafe { + // See https://rust-lang.github.io/rust-bindgen/cpp.html + let options = newSearchOptions(); + let args = newSearchMethod(); + let instance = newInstance(); + + let x_str = CString::new("x").expect("bad x"); + let y_str = CString::new("y").expect("bad y"); + let z_str = CString::new("z").expect("bad z"); + + newVar_ffi(instance, x_str.as_ptr() as _, VariableType_VAR_BOUND, 1, 3); + newVar_ffi(instance, y_str.as_ptr() as _, VariableType_VAR_BOUND, 2, 4); + newVar_ffi(instance, z_str.as_ptr() as _, VariableType_VAR_BOUND, 1, 5); + + let x = getVarByName(instance, x_str.as_ptr() as _); + let y = getVarByName(instance, y_str.as_ptr() as _); + let z = getVarByName(instance, z_str.as_ptr() as _); + + // PRINT + printMatrix_addVar(instance, x); + printMatrix_addVar(instance, y); + printMatrix_addVar(instance, z); + + // VARORDER + let search_vars = vec_var_new(); + vec_var_push_back(search_vars as _, x); + vec_var_push_back(search_vars as _, y); + vec_var_push_back(search_vars as _, z); + let search_order = newSearchOrder(search_vars as _, VarOrderEnum_ORDER_STATIC, false); + instance_addSearchOrder(instance, search_order); + + // CONSTRAINTS + let leq = newConstraintBlob(ConstraintType_CT_LEQSUM); + let geq = newConstraintBlob(ConstraintType_CT_GEQSUM); + let ineq = newConstraintBlob(ConstraintType_CT_INEQ); + + let rhs_vars = vec_var_new(); + vec_var_push_back(rhs_vars, constantAsVar(4)); + + // leq / geq : [var] [var] + constraint_addVarList(leq, search_vars as _); + constraint_addVarList(leq, rhs_vars as _); + + constraint_addVarList(geq, search_vars as _); + constraint_addVarList(geq, rhs_vars as _); + + // ineq: [var] [var] [const] + let x_vec = vec_var_new(); + vec_var_push_back(x_vec,x); + + let y_vec = vec_var_new(); + vec_var_push_back(y_vec,y); + + let const_vec = vec_int_new(); + vec_int_push_back(const_vec,-1); + + constraint_addVarList(ineq,x_vec as _); + constraint_addVarList(ineq,y_vec as _); + constraint_addConstantList(ineq,const_vec as _); + + instance_addConstraint(instance, leq); + instance_addConstraint(instance, geq); + instance_addConstraint(instance, ineq); + + let res = runMinion(options, args, instance, Some(hello_from_rust)); + + // does it get this far? + assert_eq!(res,0); + + // test if solutions are correct + assert_eq!(X_VAL,1); + assert_eq!(Y_VAL,2); + assert_eq!(Z_VAL,1); + } + + } +} diff --git a/solvers/minion/tests/dummy_test.rs b/solvers/minion/tests/dummy_test.rs deleted file mode 100644 index a8bf08d02..000000000 --- a/solvers/minion/tests/dummy_test.rs +++ /dev/null @@ -1,6 +0,0 @@ -use minion_rs::raw_bindings; - -#[test] -fn dummy() { - assert_eq!(1, 1); -} From 42f23991e0770e848e67d6c1e10c167f66961818 Mon Sep 17 00:00:00 2001 From: Niklas Dewally Date: Sat, 28 Oct 2023 11:52:08 +0100 Subject: [PATCH 04/10] minion: AST --- solvers/minion/src/ast.rs | 94 ++++++++++++++++++++++++++++++ solvers/minion/src/lib.rs | 3 +- solvers/minion/src/raw_bindings.rs | 1 + 3 files changed, 97 insertions(+), 1 deletion(-) create mode 100644 solvers/minion/src/ast.rs diff --git a/solvers/minion/src/ast.rs b/solvers/minion/src/ast.rs new file mode 100644 index 000000000..920784a9d --- /dev/null +++ b/solvers/minion/src/ast.rs @@ -0,0 +1,94 @@ +//! The Model Syntax tree for the Minion bindings. + +use std::{collections::HashMap}; + +pub type VarName = String; + +pub struct Model { + /// A lookup table of all named variables. + pub named_variables : SymbolTable, + pub constraints: Vec +} + + +impl Model { + pub fn new() -> Model{ + Model { + named_variables: SymbolTable::new(), + constraints: Vec::new() + } + } +} + +pub enum Constraints { + SumLeq(Vec,Var), + SumGeq(Vec,Var), + Ineq(Var,Var,Constant) +} + + +/// A variable can either be a named variable, or an anomynous "constant as a variable". +/// +/// The latter is not stored in the symbol table, or counted in Minions internal list of all +/// variables, but is used to allow the use of a constant in the place of a variable in a +/// constraint. +pub enum Var{ + NameRef(VarName), + ConstantAsVar(Constant) +} + +pub enum Constant { + Bool(bool), + Discrete(i32) +} + +#[derive(Copy,Clone)] +pub enum VarType { + Bounded(i32,i32) +} + +pub struct SymbolTable { + table: HashMap, + + // for now doubles both as Minion's SearchOrder and print order + var_order: Vec +} + +impl SymbolTable { + fn new() -> SymbolTable { + SymbolTable { + table: HashMap::new(), + var_order: Vec::new() + } + } + + /// Creates a new variable and adds it to the symbol table. + /// If a variable already exists with the given name, an error is thrown. + pub fn add_var(&mut self,name: VarName, vartype: VarType) -> Option<()> { + if self.table.contains_key(&name){ + return None; + } + + self.table.insert(name.clone(),vartype); + self.var_order.push(name); + + return Some(()); + } + + pub fn get_vartype(&self,name: VarName) -> Option { + match self.table.get(&name) { + Some(m) => Some(*m), + None => None + } + } + + pub fn get_variable_order(self) -> Vec { + self.var_order + } + + pub fn contains(&self, name: VarName) -> bool { + self.table.contains_key(&name) + } + + +} diff --git a/solvers/minion/src/lib.rs b/solvers/minion/src/lib.rs index 0f130578d..731897c0f 100644 --- a/solvers/minion/src/lib.rs +++ b/solvers/minion/src/lib.rs @@ -1 +1,2 @@ -pub mod raw_bindings; +mod raw_bindings; +pub mod ast; diff --git a/solvers/minion/src/raw_bindings.rs b/solvers/minion/src/raw_bindings.rs index b1c23a4a3..32e3e9871 100644 --- a/solvers/minion/src/raw_bindings.rs +++ b/solvers/minion/src/raw_bindings.rs @@ -1,3 +1,4 @@ +#![allow(warnings)] include!(concat!(env!("OUT_DIR"), "/bindings.rs")); From 774538882bc3bea87baaeb8d1ae7182e7d76a02b Mon Sep 17 00:00:00 2001 From: Niklas Dewally Date: Sat, 28 Oct 2023 12:40:32 +0100 Subject: [PATCH 05/10] minion: rust model representation and running (with default settings) --- .github/workflows/minion-tests.yml | 2 +- solvers/minion/README.md | 6 +- solvers/minion/src/ast.rs | 13 +- solvers/minion/src/lib.rs | 4 + solvers/minion/src/run.rs | 203 +++++++++++++++++++++++++++++ 5 files changed, 217 insertions(+), 11 deletions(-) create mode 100644 solvers/minion/src/run.rs diff --git a/.github/workflows/minion-tests.yml b/.github/workflows/minion-tests.yml index d3a5f25a0..9aed25144 100644 --- a/.github/workflows/minion-tests.yml +++ b/.github/workflows/minion-tests.yml @@ -111,7 +111,7 @@ jobs: run: rustup update stable && rustup default stable - working-directory: ./solvers/minion - run: cargo test + run: cargo test -- --test-threads=1 -- --test-threads=1 diff --git a/solvers/minion/README.md b/solvers/minion/README.md index 3a17850e6..f2575c452 100644 --- a/solvers/minion/README.md +++ b/solvers/minion/README.md @@ -1,7 +1,5 @@ This directory contains in progress bindings for the [Minion solver](https://github.com/minion/minion). -Currently, this builds `minion`, but provides no functionality. -# Installation / Usage - -TODO +Note that minion is single threaded only so tests must be ran with +cargo test -- --test-threads=1 diff --git a/solvers/minion/src/ast.rs b/solvers/minion/src/ast.rs index 920784a9d..d598f4155 100644 --- a/solvers/minion/src/ast.rs +++ b/solvers/minion/src/ast.rs @@ -7,7 +7,7 @@ pub type VarName = String; pub struct Model { /// A lookup table of all named variables. pub named_variables : SymbolTable, - pub constraints: Vec + pub constraints: Vec } @@ -20,7 +20,7 @@ impl Model { } } -pub enum Constraints { +pub enum Constraint { SumLeq(Vec,Var), SumGeq(Vec,Var), Ineq(Var,Var,Constant) @@ -34,7 +34,7 @@ pub enum Constraints { /// constraint. pub enum Var{ NameRef(VarName), - ConstantAsVar(Constant) + ConstantAsVar(i32) } pub enum Constant { @@ -44,7 +44,8 @@ pub enum Constant { #[derive(Copy,Clone)] pub enum VarType { - Bounded(i32,i32) + Bounded(i32,i32), + Bool(bool) } pub struct SymbolTable { @@ -82,8 +83,8 @@ impl SymbolTable { } } - pub fn get_variable_order(self) -> Vec { - self.var_order + pub fn get_variable_order(&self) -> Vec { + self.var_order.clone() } pub fn contains(&self, name: VarName) -> bool { diff --git a/solvers/minion/src/lib.rs b/solvers/minion/src/lib.rs index 731897c0f..352453d27 100644 --- a/solvers/minion/src/lib.rs +++ b/solvers/minion/src/lib.rs @@ -1,2 +1,6 @@ mod raw_bindings; + +mod run; +pub use run::*; + pub mod ast; diff --git a/solvers/minion/src/run.rs b/solvers/minion/src/run.rs new file mode 100644 index 000000000..24bb97325 --- /dev/null +++ b/solvers/minion/src/run.rs @@ -0,0 +1,203 @@ + +use std::{ffi::CString, error::Error}; + +use crate::{ast::*, raw_bindings::{*, self}}; + +// TODO: allow passing of options. + + +/// Callback function used to capture results from minion as they are generated. +/// Should return true if search is to continue, false otherwise. +pub type Callback = fn(Vec<(VarName,Constant)>) -> bool; + +#[no_mangle] +extern "C" fn hello_from_rust2() -> bool { + return true; +} + +//TODO memory +pub fn run_minion(model: Model, callback: Callback) { + unsafe { + let options = newSearchOptions(); + let args = newSearchMethod(); + let instance = convert_model_to_raw(&model); + let res = runMinion(options, args, instance, Some(hello_from_rust2)); + } +} + +// TODO: memory cleanup +unsafe fn convert_model_to_raw(model: &Model) -> *mut ProbSpec_CSPInstance { + + + let instance = newInstance(); + + /*******************************/ + /* Add variables */ + /*******************************/ + // Add variables to: + // 1. symbol table + // 2. print matrix + // 3. search vars + // + // For now, use searchorder static only + // + // These are all done in the order saved in the Symboltable + + let search_vars = vec_var_new(); + + for var_name in model.named_variables.get_variable_order() { + + //TODO: make this return Result + let c_str = CString::new(var_name.clone()).expect(""); + let vartype = model.named_variables.get_vartype(var_name.clone()).expect(""); + + let (vartype_raw, domain_low,domain_high) = match vartype { + VarType::Bounded(a,b) => { (VariableType_VAR_BOUND,a,b) } + _ => panic!("NOT IMPLEMENTED") + }; + + + newVar_ffi(instance,c_str.as_ptr() as _, vartype_raw,domain_low,domain_high); + + let var = getVarByName(instance,c_str.as_ptr() as _); + + printMatrix_addVar(instance,var); + vec_var_push_back(search_vars,var); + } + + let search_order = newSearchOrder(search_vars,VarOrderEnum_ORDER_STATIC,false); + instance_addSearchOrder(instance,search_order); + + /*********************************/ + /* Add constraints */ + /*********************************/ + + for constraint in &model.constraints { + // 1. get constraint type and create C++ constraint object + // 2. run through arguments and add them to the constraint + // 3. add constraint to instance + + let constraint_type = get_constraint_type(constraint); + let raw_constraint = newConstraintBlob(constraint_type); + + constraint_add_args(instance,raw_constraint,constraint); + instance_addConstraint(instance,raw_constraint); + } + + return instance; +} + +unsafe fn get_constraint_type(constraint: &Constraint) -> u32{ + match constraint { + Constraint::SumGeq(_,_) => ConstraintType_CT_GEQSUM, + Constraint::SumLeq(_,_) => ConstraintType_CT_LEQSUM, + Constraint::Ineq(_,_,_) => ConstraintType_CT_INEQ, + #[allow(unreachable_patterns)] + _ => panic!("NOT IMPLEMENTED") + } +} + +unsafe fn constraint_add_args(i: *mut ProbSpec_CSPInstance, r_constr: *mut ProbSpec_ConstraintBlob,constr: &Constraint) { + match constr { + Constraint::SumGeq(lhs_vars,rhs_var) => {read_vars(i,r_constr,&lhs_vars);read_var(i,r_constr,rhs_var)}, + Constraint::SumLeq(lhs_vars,rhs_var) => {read_vars(i,r_constr,&lhs_vars);read_var(i,r_constr,rhs_var)}, + Constraint::Ineq(var1,var2,c) => {read_var(i,r_constr,&var1);read_var(i,r_constr,&var2);read_const(r_constr,c)}, + #[allow(unreachable_patterns)] + _ => panic!("NOT IMPLEMENTED") + }; +} + +// DO NOT call manually - this assumes that all needed vars are already in the symbol table. +// TODO not happy with this just assuming the name is in the symbol table +unsafe fn read_vars(instance: *mut ProbSpec_CSPInstance, raw_constraint: *mut ProbSpec_ConstraintBlob, vars: &Vec) { + let raw_vars = vec_var_new(); + for var in vars { + // TODO: could easily break and segfault and die and so on + let raw_var = match var { + Var::NameRef(name) => { + let c_str = CString::new(name.clone()).expect(""); + getVarByName(instance,c_str.as_ptr() as _) + } + Var::ConstantAsVar(n) => constantAsVar(*n), + }; + + vec_var_push_back(raw_vars,raw_var); + } + + constraint_addVarList(raw_constraint,raw_vars); + vec_var_free(raw_vars); +} + +unsafe fn read_var(instance: *mut ProbSpec_CSPInstance, raw_constraint: *mut ProbSpec_ConstraintBlob, var: &Var) { + let raw_vars = vec_var_new(); + let raw_var = match var { + Var::NameRef(name) => { + let c_str = CString::new(name.clone()).expect(""); + getVarByName(instance,c_str.as_ptr() as _) + } + Var::ConstantAsVar(n) => constantAsVar(*n), + }; + + vec_var_push_back(raw_vars,raw_var); + + constraint_addVarList(raw_constraint,raw_vars); + vec_var_free(raw_vars); +} + +unsafe fn read_const(raw_constraint: *mut ProbSpec_ConstraintBlob, constant: &Constant) { + let raw_consts = vec_int_new(); + + let val = match constant { + Constant::Discrete(n) => n, + _ => panic!("NOT IMPLEMENTED"), + }; + + vec_int_push_back(raw_consts,*val); + constraint_addConstantList(raw_constraint,raw_consts); + vec_int_free(raw_consts); +} + +#[cfg(test)] +mod tests { + use super::*; + + /// . + fn callback (_:Vec<(VarName,Constant)>) -> bool{ + return true; + } + + #[test] + fn basic_ast_test() { + let mut model = Model::new(); + model.named_variables.add_var("x".to_owned(),VarType::Bounded(1,3)); + model.named_variables.add_var("y".to_owned(),VarType::Bounded(2,4)); + model.named_variables.add_var("z".to_owned(),VarType::Bounded(1,5)); + + let leq = Constraint::SumLeq(vec![ + Var::NameRef("x".to_owned()), + Var::NameRef("y".to_owned()), + Var::NameRef("z".to_owned())], + Var::ConstantAsVar(4) + ); + + let geq = Constraint::SumGeq(vec![ + Var::NameRef("x".to_owned()), + Var::NameRef("y".to_owned()), + Var::NameRef("z".to_owned())], + Var::ConstantAsVar(4) + ); + + let ineq = Constraint::Ineq( + Var::NameRef("x".to_owned()), + Var::NameRef("y".to_owned()), + Constant::Discrete(-1)); + + + model.constraints.push(leq); + model.constraints.push(geq); + model.constraints.push(ineq); + + run_minion(model,callback); + } +} + From b78896884ff1bcfd3986aec043d506223cd86fa8 Mon Sep 17 00:00:00 2001 From: Niklas Dewally Date: Sat, 28 Oct 2023 15:53:46 +0100 Subject: [PATCH 06/10] minion: fix ci --- .github/workflows/minion-tests.yml | 2 +- solvers/minion/build.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/minion-tests.yml b/.github/workflows/minion-tests.yml index 9aed25144..917ee7bca 100644 --- a/.github/workflows/minion-tests.yml +++ b/.github/workflows/minion-tests.yml @@ -111,7 +111,7 @@ jobs: run: rustup update stable && rustup default stable - working-directory: ./solvers/minion - run: cargo test -- --test-threads=1 -- --test-threads=1 + run: cargo test -- --test-threads=1 diff --git a/solvers/minion/build.sh b/solvers/minion/build.sh index 5107bf508..fb85ecd73 100755 --- a/solvers/minion/build.sh +++ b/solvers/minion/build.sh @@ -20,4 +20,4 @@ fi echo "------ BUILD STEP ------" cd "$SCRIPT_DIR" cd vendor/build -nice make -j8 +make From 93f6fc30c1cc3902926ad18e537ca03bc43f6697 Mon Sep 17 00:00:00 2001 From: Niklas Dewally Date: Sat, 28 Oct 2023 15:54:35 +0100 Subject: [PATCH 07/10] minion: run cargo fmt --- solvers/minion/src/ast.rs | 48 ++++---- solvers/minion/src/raw_bindings.rs | 174 ++++++++++++++--------------- solvers/minion/src/run.rs | 167 ++++++++++++++++----------- tests/model_tests.rs | 17 +-- tests/rewrite_tests.rs | 2 +- 5 files changed, 215 insertions(+), 193 deletions(-) diff --git a/solvers/minion/src/ast.rs b/solvers/minion/src/ast.rs index d598f4155..a6d79cc4a 100644 --- a/solvers/minion/src/ast.rs +++ b/solvers/minion/src/ast.rs @@ -1,85 +1,83 @@ //! The Model Syntax tree for the Minion bindings. -use std::{collections::HashMap}; +use std::collections::HashMap; pub type VarName = String; pub struct Model { /// A lookup table of all named variables. - pub named_variables : SymbolTable, - pub constraints: Vec + pub named_variables: SymbolTable, + pub constraints: Vec, } - impl Model { - pub fn new() -> Model{ + pub fn new() -> Model { Model { named_variables: SymbolTable::new(), - constraints: Vec::new() + constraints: Vec::new(), } } } pub enum Constraint { - SumLeq(Vec,Var), - SumGeq(Vec,Var), - Ineq(Var,Var,Constant) + SumLeq(Vec, Var), + SumGeq(Vec, Var), + Ineq(Var, Var, Constant), } - /// A variable can either be a named variable, or an anomynous "constant as a variable". /// /// The latter is not stored in the symbol table, or counted in Minions internal list of all /// variables, but is used to allow the use of a constant in the place of a variable in a /// constraint. -pub enum Var{ +pub enum Var { NameRef(VarName), - ConstantAsVar(i32) + ConstantAsVar(i32), } pub enum Constant { Bool(bool), - Discrete(i32) + Discrete(i32), } -#[derive(Copy,Clone)] +#[derive(Copy, Clone)] pub enum VarType { - Bounded(i32,i32), - Bool(bool) + Bounded(i32, i32), + Bool(bool), } pub struct SymbolTable { - table: HashMap, + table: HashMap, // for now doubles both as Minion's SearchOrder and print order - var_order: Vec + var_order: Vec, } impl SymbolTable { fn new() -> SymbolTable { SymbolTable { table: HashMap::new(), - var_order: Vec::new() + var_order: Vec::new(), } } /// Creates a new variable and adds it to the symbol table. /// If a variable already exists with the given name, an error is thrown. - pub fn add_var(&mut self,name: VarName, vartype: VarType) -> Option<()> { - if self.table.contains_key(&name){ + pub fn add_var(&mut self, name: VarName, vartype: VarType) -> Option<()> { + if self.table.contains_key(&name) { return None; } - self.table.insert(name.clone(),vartype); + self.table.insert(name.clone(), vartype); self.var_order.push(name); return Some(()); } - pub fn get_vartype(&self,name: VarName) -> Option { + pub fn get_vartype(&self, name: VarName) -> Option { match self.table.get(&name) { Some(m) => Some(*m), - None => None + None => None, } } @@ -90,6 +88,4 @@ impl SymbolTable { pub fn contains(&self, name: VarName) -> bool { self.table.contains_key(&name) } - - } diff --git a/solvers/minion/src/raw_bindings.rs b/solvers/minion/src/raw_bindings.rs index 32e3e9871..4fac11941 100644 --- a/solvers/minion/src/raw_bindings.rs +++ b/solvers/minion/src/raw_bindings.rs @@ -1,107 +1,105 @@ #![allow(warnings)] include!(concat!(env!("OUT_DIR"), "/bindings.rs")); - #[cfg(test)] mod tests { - use std::ffi::CString; use super::*; + use std::ffi::CString; // solutions - static mut X_VAL:i32 = 0; - static mut Y_VAL:i32 = 0; - static mut Z_VAL:i32 = 0; + static mut X_VAL: i32 = 0; + static mut Y_VAL: i32 = 0; + static mut Z_VAL: i32 = 0; #[no_mangle] pub extern "C" fn hello_from_rust() -> bool { - unsafe{ - X_VAL = printMatrix_getValue(0) as _; - Y_VAL = printMatrix_getValue(1) as _; - Z_VAL = printMatrix_getValue(2) as _; - return true; + unsafe { + X_VAL = printMatrix_getValue(0) as _; + Y_VAL = printMatrix_getValue(1) as _; + Z_VAL = printMatrix_getValue(2) as _; + return true; } } #[test] fn xyz_raw() { - // A simple constraints model, manually written using FFI functions. - // Testing to see if it does not segfault. - // Results can be manually inspected in the outputted minion logs. - unsafe { - // See https://rust-lang.github.io/rust-bindgen/cpp.html - let options = newSearchOptions(); - let args = newSearchMethod(); - let instance = newInstance(); - - let x_str = CString::new("x").expect("bad x"); - let y_str = CString::new("y").expect("bad y"); - let z_str = CString::new("z").expect("bad z"); - - newVar_ffi(instance, x_str.as_ptr() as _, VariableType_VAR_BOUND, 1, 3); - newVar_ffi(instance, y_str.as_ptr() as _, VariableType_VAR_BOUND, 2, 4); - newVar_ffi(instance, z_str.as_ptr() as _, VariableType_VAR_BOUND, 1, 5); - - let x = getVarByName(instance, x_str.as_ptr() as _); - let y = getVarByName(instance, y_str.as_ptr() as _); - let z = getVarByName(instance, z_str.as_ptr() as _); - - // PRINT - printMatrix_addVar(instance, x); - printMatrix_addVar(instance, y); - printMatrix_addVar(instance, z); - - // VARORDER - let search_vars = vec_var_new(); - vec_var_push_back(search_vars as _, x); - vec_var_push_back(search_vars as _, y); - vec_var_push_back(search_vars as _, z); - let search_order = newSearchOrder(search_vars as _, VarOrderEnum_ORDER_STATIC, false); - instance_addSearchOrder(instance, search_order); - - // CONSTRAINTS - let leq = newConstraintBlob(ConstraintType_CT_LEQSUM); - let geq = newConstraintBlob(ConstraintType_CT_GEQSUM); - let ineq = newConstraintBlob(ConstraintType_CT_INEQ); - - let rhs_vars = vec_var_new(); - vec_var_push_back(rhs_vars, constantAsVar(4)); - - // leq / geq : [var] [var] - constraint_addVarList(leq, search_vars as _); - constraint_addVarList(leq, rhs_vars as _); - - constraint_addVarList(geq, search_vars as _); - constraint_addVarList(geq, rhs_vars as _); - - // ineq: [var] [var] [const] - let x_vec = vec_var_new(); - vec_var_push_back(x_vec,x); - - let y_vec = vec_var_new(); - vec_var_push_back(y_vec,y); - - let const_vec = vec_int_new(); - vec_int_push_back(const_vec,-1); - - constraint_addVarList(ineq,x_vec as _); - constraint_addVarList(ineq,y_vec as _); - constraint_addConstantList(ineq,const_vec as _); - - instance_addConstraint(instance, leq); - instance_addConstraint(instance, geq); - instance_addConstraint(instance, ineq); - - let res = runMinion(options, args, instance, Some(hello_from_rust)); - - // does it get this far? - assert_eq!(res,0); - - // test if solutions are correct - assert_eq!(X_VAL,1); - assert_eq!(Y_VAL,2); - assert_eq!(Z_VAL,1); - } - + // A simple constraints model, manually written using FFI functions. + // Testing to see if it does not segfault. + // Results can be manually inspected in the outputted minion logs. + unsafe { + // See https://rust-lang.github.io/rust-bindgen/cpp.html + let options = newSearchOptions(); + let args = newSearchMethod(); + let instance = newInstance(); + + let x_str = CString::new("x").expect("bad x"); + let y_str = CString::new("y").expect("bad y"); + let z_str = CString::new("z").expect("bad z"); + + newVar_ffi(instance, x_str.as_ptr() as _, VariableType_VAR_BOUND, 1, 3); + newVar_ffi(instance, y_str.as_ptr() as _, VariableType_VAR_BOUND, 2, 4); + newVar_ffi(instance, z_str.as_ptr() as _, VariableType_VAR_BOUND, 1, 5); + + let x = getVarByName(instance, x_str.as_ptr() as _); + let y = getVarByName(instance, y_str.as_ptr() as _); + let z = getVarByName(instance, z_str.as_ptr() as _); + + // PRINT + printMatrix_addVar(instance, x); + printMatrix_addVar(instance, y); + printMatrix_addVar(instance, z); + + // VARORDER + let search_vars = vec_var_new(); + vec_var_push_back(search_vars as _, x); + vec_var_push_back(search_vars as _, y); + vec_var_push_back(search_vars as _, z); + let search_order = newSearchOrder(search_vars as _, VarOrderEnum_ORDER_STATIC, false); + instance_addSearchOrder(instance, search_order); + + // CONSTRAINTS + let leq = newConstraintBlob(ConstraintType_CT_LEQSUM); + let geq = newConstraintBlob(ConstraintType_CT_GEQSUM); + let ineq = newConstraintBlob(ConstraintType_CT_INEQ); + + let rhs_vars = vec_var_new(); + vec_var_push_back(rhs_vars, constantAsVar(4)); + + // leq / geq : [var] [var] + constraint_addVarList(leq, search_vars as _); + constraint_addVarList(leq, rhs_vars as _); + + constraint_addVarList(geq, search_vars as _); + constraint_addVarList(geq, rhs_vars as _); + + // ineq: [var] [var] [const] + let x_vec = vec_var_new(); + vec_var_push_back(x_vec, x); + + let y_vec = vec_var_new(); + vec_var_push_back(y_vec, y); + + let const_vec = vec_int_new(); + vec_int_push_back(const_vec, -1); + + constraint_addVarList(ineq, x_vec as _); + constraint_addVarList(ineq, y_vec as _); + constraint_addConstantList(ineq, const_vec as _); + + instance_addConstraint(instance, leq); + instance_addConstraint(instance, geq); + instance_addConstraint(instance, ineq); + + let res = runMinion(options, args, instance, Some(hello_from_rust)); + + // does it get this far? + assert_eq!(res, 0); + + // test if solutions are correct + assert_eq!(X_VAL, 1); + assert_eq!(Y_VAL, 2); + assert_eq!(Z_VAL, 1); + } } } diff --git a/solvers/minion/src/run.rs b/solvers/minion/src/run.rs index 24bb97325..ea9b4664d 100644 --- a/solvers/minion/src/run.rs +++ b/solvers/minion/src/run.rs @@ -1,14 +1,15 @@ +use std::{error::Error, ffi::CString}; -use std::{ffi::CString, error::Error}; - -use crate::{ast::*, raw_bindings::{*, self}}; +use crate::{ + ast::*, + raw_bindings::{self, *}, +}; // TODO: allow passing of options. - /// Callback function used to capture results from minion as they are generated. /// Should return true if search is to continue, false otherwise. -pub type Callback = fn(Vec<(VarName,Constant)>) -> bool; +pub type Callback = fn(Vec<(VarName, Constant)>) -> bool; #[no_mangle] extern "C" fn hello_from_rust2() -> bool { @@ -27,8 +28,6 @@ pub fn run_minion(model: Model, callback: Callback) { // TODO: memory cleanup unsafe fn convert_model_to_raw(model: &Model) -> *mut ProbSpec_CSPInstance { - - let instance = newInstance(); /*******************************/ @@ -46,27 +45,34 @@ unsafe fn convert_model_to_raw(model: &Model) -> *mut ProbSpec_CSPInstance { let search_vars = vec_var_new(); for var_name in model.named_variables.get_variable_order() { - //TODO: make this return Result let c_str = CString::new(var_name.clone()).expect(""); - let vartype = model.named_variables.get_vartype(var_name.clone()).expect(""); - - let (vartype_raw, domain_low,domain_high) = match vartype { - VarType::Bounded(a,b) => { (VariableType_VAR_BOUND,a,b) } - _ => panic!("NOT IMPLEMENTED") + let vartype = model + .named_variables + .get_vartype(var_name.clone()) + .expect(""); + + let (vartype_raw, domain_low, domain_high) = match vartype { + VarType::Bounded(a, b) => (VariableType_VAR_BOUND, a, b), + _ => panic!("NOT IMPLEMENTED"), }; + newVar_ffi( + instance, + c_str.as_ptr() as _, + vartype_raw, + domain_low, + domain_high, + ); - newVar_ffi(instance,c_str.as_ptr() as _, vartype_raw,domain_low,domain_high); + let var = getVarByName(instance, c_str.as_ptr() as _); - let var = getVarByName(instance,c_str.as_ptr() as _); - - printMatrix_addVar(instance,var); - vec_var_push_back(search_vars,var); + printMatrix_addVar(instance, var); + vec_var_push_back(search_vars, var); } - let search_order = newSearchOrder(search_vars,VarOrderEnum_ORDER_STATIC,false); - instance_addSearchOrder(instance,search_order); + let search_order = newSearchOrder(search_vars, VarOrderEnum_ORDER_STATIC, false); + instance_addSearchOrder(instance, search_order); /*********************************/ /* Add constraints */ @@ -76,71 +82,93 @@ unsafe fn convert_model_to_raw(model: &Model) -> *mut ProbSpec_CSPInstance { // 1. get constraint type and create C++ constraint object // 2. run through arguments and add them to the constraint // 3. add constraint to instance - + let constraint_type = get_constraint_type(constraint); let raw_constraint = newConstraintBlob(constraint_type); - constraint_add_args(instance,raw_constraint,constraint); - instance_addConstraint(instance,raw_constraint); + constraint_add_args(instance, raw_constraint, constraint); + instance_addConstraint(instance, raw_constraint); } return instance; } -unsafe fn get_constraint_type(constraint: &Constraint) -> u32{ +unsafe fn get_constraint_type(constraint: &Constraint) -> u32 { match constraint { - Constraint::SumGeq(_,_) => ConstraintType_CT_GEQSUM, - Constraint::SumLeq(_,_) => ConstraintType_CT_LEQSUM, - Constraint::Ineq(_,_,_) => ConstraintType_CT_INEQ, + Constraint::SumGeq(_, _) => ConstraintType_CT_GEQSUM, + Constraint::SumLeq(_, _) => ConstraintType_CT_LEQSUM, + Constraint::Ineq(_, _, _) => ConstraintType_CT_INEQ, #[allow(unreachable_patterns)] - _ => panic!("NOT IMPLEMENTED") + _ => panic!("NOT IMPLEMENTED"), } } -unsafe fn constraint_add_args(i: *mut ProbSpec_CSPInstance, r_constr: *mut ProbSpec_ConstraintBlob,constr: &Constraint) { +unsafe fn constraint_add_args( + i: *mut ProbSpec_CSPInstance, + r_constr: *mut ProbSpec_ConstraintBlob, + constr: &Constraint, +) { match constr { - Constraint::SumGeq(lhs_vars,rhs_var) => {read_vars(i,r_constr,&lhs_vars);read_var(i,r_constr,rhs_var)}, - Constraint::SumLeq(lhs_vars,rhs_var) => {read_vars(i,r_constr,&lhs_vars);read_var(i,r_constr,rhs_var)}, - Constraint::Ineq(var1,var2,c) => {read_var(i,r_constr,&var1);read_var(i,r_constr,&var2);read_const(r_constr,c)}, + Constraint::SumGeq(lhs_vars, rhs_var) => { + read_vars(i, r_constr, &lhs_vars); + read_var(i, r_constr, rhs_var) + } + Constraint::SumLeq(lhs_vars, rhs_var) => { + read_vars(i, r_constr, &lhs_vars); + read_var(i, r_constr, rhs_var) + } + Constraint::Ineq(var1, var2, c) => { + read_var(i, r_constr, &var1); + read_var(i, r_constr, &var2); + read_const(r_constr, c) + } #[allow(unreachable_patterns)] - _ => panic!("NOT IMPLEMENTED") + _ => panic!("NOT IMPLEMENTED"), }; } // DO NOT call manually - this assumes that all needed vars are already in the symbol table. // TODO not happy with this just assuming the name is in the symbol table -unsafe fn read_vars(instance: *mut ProbSpec_CSPInstance, raw_constraint: *mut ProbSpec_ConstraintBlob, vars: &Vec) { +unsafe fn read_vars( + instance: *mut ProbSpec_CSPInstance, + raw_constraint: *mut ProbSpec_ConstraintBlob, + vars: &Vec, +) { let raw_vars = vec_var_new(); for var in vars { // TODO: could easily break and segfault and die and so on let raw_var = match var { Var::NameRef(name) => { let c_str = CString::new(name.clone()).expect(""); - getVarByName(instance,c_str.as_ptr() as _) + getVarByName(instance, c_str.as_ptr() as _) } Var::ConstantAsVar(n) => constantAsVar(*n), }; - vec_var_push_back(raw_vars,raw_var); + vec_var_push_back(raw_vars, raw_var); } - constraint_addVarList(raw_constraint,raw_vars); + constraint_addVarList(raw_constraint, raw_vars); vec_var_free(raw_vars); } -unsafe fn read_var(instance: *mut ProbSpec_CSPInstance, raw_constraint: *mut ProbSpec_ConstraintBlob, var: &Var) { +unsafe fn read_var( + instance: *mut ProbSpec_CSPInstance, + raw_constraint: *mut ProbSpec_ConstraintBlob, + var: &Var, +) { let raw_vars = vec_var_new(); let raw_var = match var { Var::NameRef(name) => { let c_str = CString::new(name.clone()).expect(""); - getVarByName(instance,c_str.as_ptr() as _) + getVarByName(instance, c_str.as_ptr() as _) } Var::ConstantAsVar(n) => constantAsVar(*n), }; - vec_var_push_back(raw_vars,raw_var); + vec_var_push_back(raw_vars, raw_var); - constraint_addVarList(raw_constraint,raw_vars); + constraint_addVarList(raw_constraint, raw_vars); vec_var_free(raw_vars); } @@ -152,8 +180,8 @@ unsafe fn read_const(raw_constraint: *mut ProbSpec_ConstraintBlob, constant: &Co _ => panic!("NOT IMPLEMENTED"), }; - vec_int_push_back(raw_consts,*val); - constraint_addConstantList(raw_constraint,raw_consts); + vec_int_push_back(raw_consts, *val); + constraint_addConstantList(raw_constraint, raw_consts); vec_int_free(raw_consts); } @@ -162,42 +190,51 @@ mod tests { use super::*; /// . - fn callback (_:Vec<(VarName,Constant)>) -> bool{ + fn callback(_: Vec<(VarName, Constant)>) -> bool { return true; } #[test] fn basic_ast_test() { let mut model = Model::new(); - model.named_variables.add_var("x".to_owned(),VarType::Bounded(1,3)); - model.named_variables.add_var("y".to_owned(),VarType::Bounded(2,4)); - model.named_variables.add_var("z".to_owned(),VarType::Bounded(1,5)); - - let leq = Constraint::SumLeq(vec![ - Var::NameRef("x".to_owned()), - Var::NameRef("y".to_owned()), - Var::NameRef("z".to_owned())], - Var::ConstantAsVar(4) - ); - - let geq = Constraint::SumGeq(vec![ - Var::NameRef("x".to_owned()), - Var::NameRef("y".to_owned()), - Var::NameRef("z".to_owned())], - Var::ConstantAsVar(4) - ); + model + .named_variables + .add_var("x".to_owned(), VarType::Bounded(1, 3)); + model + .named_variables + .add_var("y".to_owned(), VarType::Bounded(2, 4)); + model + .named_variables + .add_var("z".to_owned(), VarType::Bounded(1, 5)); + + let leq = Constraint::SumLeq( + vec![ + Var::NameRef("x".to_owned()), + Var::NameRef("y".to_owned()), + Var::NameRef("z".to_owned()), + ], + Var::ConstantAsVar(4), + ); + + let geq = Constraint::SumGeq( + vec![ + Var::NameRef("x".to_owned()), + Var::NameRef("y".to_owned()), + Var::NameRef("z".to_owned()), + ], + Var::ConstantAsVar(4), + ); let ineq = Constraint::Ineq( Var::NameRef("x".to_owned()), Var::NameRef("y".to_owned()), - Constant::Discrete(-1)); - + Constant::Discrete(-1), + ); model.constraints.push(leq); model.constraints.push(geq); model.constraints.push(ineq); - run_minion(model,callback); + run_minion(model, callback); } } - diff --git a/tests/model_tests.rs b/tests/model_tests.rs index 9f56392e3..edcf36890 100644 --- a/tests/model_tests.rs +++ b/tests/model_tests.rs @@ -1,7 +1,7 @@ // Tests for various functionalities of the Model -use std::collections::HashMap; use conjure_oxide::ast::*; +use std::collections::HashMap; #[test] fn modify_domain() { @@ -11,25 +11,16 @@ fn modify_domain() { let d2 = Domain::IntDomain(vec![Range::Bounded(1, 2)]); let mut variables = HashMap::new(); - variables.insert( - a.clone(), - DecisionVariable { - domain: d1.clone(), - }, - ); + variables.insert(a.clone(), DecisionVariable { domain: d1.clone() }); let mut m = Model { variables, constraints: Vec::new(), }; - assert!( - (*m.variables.get(&a).unwrap()).domain == d1 - ); + assert!((*m.variables.get(&a).unwrap()).domain == d1); m.update_domain(&a, d2.clone()); - assert!( - (*m.variables.get(&a).unwrap()).domain == d2 - ); + assert!((*m.variables.get(&a).unwrap()).domain == d2); } diff --git a/tests/rewrite_tests.rs b/tests/rewrite_tests.rs index de2e7dc52..13e4a6779 100644 --- a/tests/rewrite_tests.rs +++ b/tests/rewrite_tests.rs @@ -90,4 +90,4 @@ fn simplify_expression(expr: Expression) -> Expression { ), _ => expr, } -} \ No newline at end of file +} From a0cf992bb472c15fde31297798df4d62223b8ff7 Mon Sep 17 00:00:00 2001 From: Niklas Dewally Date: Sun, 29 Oct 2023 10:43:26 +0000 Subject: [PATCH 08/10] minion: call destructors when out of scope --- solvers/minion/build.rs | 6 +++- solvers/minion/src/lib.rs | 2 ++ solvers/minion/src/run.rs | 56 ++++++++++++++++++-------------- solvers/minion/src/scoped_ptr.rs | 28 ++++++++++++++++ 4 files changed, 66 insertions(+), 26 deletions(-) create mode 100644 solvers/minion/src/scoped_ptr.rs diff --git a/solvers/minion/build.rs b/solvers/minion/build.rs index 953d239e5..02d01f1f7 100755 --- a/solvers/minion/build.rs +++ b/solvers/minion/build.rs @@ -74,8 +74,12 @@ fn bind() { .allowlist_function("newConstraintBlob") .allowlist_function("newSearchOrder") .allowlist_function("getVarByName") + .allowlist_function("searchOptions_free") + .allowlist_function("searchMethod_free") + .allowlist_function("instance_free") + .allowlist_function("constraint_free") + .allowlist_function("searchOrder_free") .allowlist_function("newVar_ffi") - .allowlist_function("instance-free") .allowlist_function("instance_addSearchOrder") .allowlist_function("instance_addConstraint") .allowlist_function("printMatrix_addVar") diff --git a/solvers/minion/src/lib.rs b/solvers/minion/src/lib.rs index 352453d27..cd403d68a 100644 --- a/solvers/minion/src/lib.rs +++ b/solvers/minion/src/lib.rs @@ -4,3 +4,5 @@ mod run; pub use run::*; pub mod ast; + +mod scoped_ptr; diff --git a/solvers/minion/src/run.rs b/solvers/minion/src/run.rs index ea9b4664d..72ed0bf31 100644 --- a/solvers/minion/src/run.rs +++ b/solvers/minion/src/run.rs @@ -3,6 +3,7 @@ use std::{error::Error, ffi::CString}; use crate::{ ast::*, raw_bindings::{self, *}, + scoped_ptr::Scoped, }; // TODO: allow passing of options. @@ -19,15 +20,16 @@ extern "C" fn hello_from_rust2() -> bool { //TODO memory pub fn run_minion(model: Model, callback: Callback) { unsafe { - let options = newSearchOptions(); - let args = newSearchMethod(); - let instance = convert_model_to_raw(&model); - let res = runMinion(options, args, instance, Some(hello_from_rust2)); + let options = Scoped::new(newSearchOptions(), |x| searchOptions_free(x as _)); + let args = Scoped::new(newSearchMethod(), |x| searchMethod_free(x as _)); + let instance = Scoped::new(convert_model_to_raw(&model), |x| instance_free(x as _)); + let res = runMinion(options.ptr, args.ptr, instance.ptr, Some(hello_from_rust2)); } } -// TODO: memory cleanup +/// Callee owns the returned instance unsafe fn convert_model_to_raw(model: &Model) -> *mut ProbSpec_CSPInstance { + // This is managed in scope by the callee let instance = newInstance(); /*******************************/ @@ -42,7 +44,7 @@ unsafe fn convert_model_to_raw(model: &Model) -> *mut ProbSpec_CSPInstance { // // These are all done in the order saved in the Symboltable - let search_vars = vec_var_new(); + let search_vars = Scoped::new(vec_var_new(), |x| vec_var_free(x as _)); for var_name in model.named_variables.get_variable_order() { //TODO: make this return Result @@ -68,11 +70,17 @@ unsafe fn convert_model_to_raw(model: &Model) -> *mut ProbSpec_CSPInstance { let var = getVarByName(instance, c_str.as_ptr() as _); printMatrix_addVar(instance, var); - vec_var_push_back(search_vars, var); + vec_var_push_back(search_vars.ptr, var); } - let search_order = newSearchOrder(search_vars, VarOrderEnum_ORDER_STATIC, false); - instance_addSearchOrder(instance, search_order); + let search_order = Scoped::new( + newSearchOrder(search_vars.ptr, VarOrderEnum_ORDER_STATIC, false), + |x| searchOrder_free(x as _), + ); + + // this and other instance_ functions does not move so my use of ptrs are ok + // TODO (nd60): document this + instance_addSearchOrder(instance, search_order.ptr); /*********************************/ /* Add constraints */ @@ -84,10 +92,12 @@ unsafe fn convert_model_to_raw(model: &Model) -> *mut ProbSpec_CSPInstance { // 3. add constraint to instance let constraint_type = get_constraint_type(constraint); - let raw_constraint = newConstraintBlob(constraint_type); + let raw_constraint = Scoped::new(newConstraintBlob(constraint_type), |x| { + constraint_free(x as _) + }); - constraint_add_args(instance, raw_constraint, constraint); - instance_addConstraint(instance, raw_constraint); + constraint_add_args(instance, raw_constraint.ptr, constraint); + instance_addConstraint(instance, raw_constraint.ptr); } return instance; @@ -134,7 +144,7 @@ unsafe fn read_vars( raw_constraint: *mut ProbSpec_ConstraintBlob, vars: &Vec, ) { - let raw_vars = vec_var_new(); + let raw_vars = Scoped::new(vec_var_new(), |x| vec_var_free(x as _)); for var in vars { // TODO: could easily break and segfault and die and so on let raw_var = match var { @@ -145,11 +155,10 @@ unsafe fn read_vars( Var::ConstantAsVar(n) => constantAsVar(*n), }; - vec_var_push_back(raw_vars, raw_var); + vec_var_push_back(raw_vars.ptr, raw_var); } - constraint_addVarList(raw_constraint, raw_vars); - vec_var_free(raw_vars); + constraint_addVarList(raw_constraint, raw_vars.ptr); } unsafe fn read_var( @@ -157,7 +166,7 @@ unsafe fn read_var( raw_constraint: *mut ProbSpec_ConstraintBlob, var: &Var, ) { - let raw_vars = vec_var_new(); + let raw_vars = Scoped::new(vec_var_new(), |x| vec_var_free(x as _)); let raw_var = match var { Var::NameRef(name) => { let c_str = CString::new(name.clone()).expect(""); @@ -166,23 +175,20 @@ unsafe fn read_var( Var::ConstantAsVar(n) => constantAsVar(*n), }; - vec_var_push_back(raw_vars, raw_var); - - constraint_addVarList(raw_constraint, raw_vars); - vec_var_free(raw_vars); + vec_var_push_back(raw_vars.ptr, raw_var); + constraint_addVarList(raw_constraint, raw_vars.ptr); } unsafe fn read_const(raw_constraint: *mut ProbSpec_ConstraintBlob, constant: &Constant) { - let raw_consts = vec_int_new(); + let raw_consts = Scoped::new(vec_int_new(), |x| vec_var_free(x as _)); let val = match constant { Constant::Discrete(n) => n, _ => panic!("NOT IMPLEMENTED"), }; - vec_int_push_back(raw_consts, *val); - constraint_addConstantList(raw_constraint, raw_consts); - vec_int_free(raw_consts); + vec_int_push_back(raw_consts.ptr, *val); + constraint_addConstantList(raw_constraint, raw_consts.ptr); } #[cfg(test)] diff --git a/solvers/minion/src/scoped_ptr.rs b/solvers/minion/src/scoped_ptr.rs new file mode 100644 index 000000000..a7b328894 --- /dev/null +++ b/solvers/minion/src/scoped_ptr.rs @@ -0,0 +1,28 @@ +/// A light scoped wrapper over a raw *mut pointer. +/// +/// Implements destruction of the pointer when it goes out of scope, but provides no other +/// guarentees. +pub struct Scoped { + pub ptr: *mut T, + destructor: fn(*mut T), +} + +// Could use +// https://doc.rust-lang.org/std/alloc/trait.Allocator.html with box (in nightly only) +// or +// https://docs.rs/scopeguard/latest/scopeguard/ +// instead +impl Scoped { + pub unsafe fn new(ptr: *mut T, destructor: fn(*mut T)) -> Scoped { + return Scoped { ptr, destructor }; + } +} + +// https://doc.rust-lang.org/nomicon/destructors.html +impl Drop for Scoped { + fn drop(&mut self) { + unsafe { + (self.destructor)(self.ptr); + } + } +} From cc1e6b72888bb924553c2ab99844769c254bebdf Mon Sep 17 00:00:00 2001 From: Niklas Dewally Date: Mon, 30 Oct 2023 13:53:41 +0000 Subject: [PATCH 09/10] Update vendor --- solvers/minion/vendor | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solvers/minion/vendor b/solvers/minion/vendor index 7e408139e..5a53c9065 160000 --- a/solvers/minion/vendor +++ b/solvers/minion/vendor @@ -1 +1 @@ -Subproject commit 7e408139e94c1b9de5c2ab81403006af4e81ffe4 +Subproject commit 5a53c90653700bb66aa052ed1830d970a442f3ef From 3e3e0a9cbd2c93938793c3578201a3b1569bd602 Mon Sep 17 00:00:00 2001 From: Niklas Dewally Date: Mon, 30 Oct 2023 17:12:53 +0000 Subject: [PATCH 10/10] minion: rename things --- solvers/minion/src/ast.rs | 14 ++++++++------ solvers/minion/src/run.rs | 12 ++++++------ 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/solvers/minion/src/ast.rs b/solvers/minion/src/ast.rs index a6d79cc4a..9c57353a6 100644 --- a/solvers/minion/src/ast.rs +++ b/solvers/minion/src/ast.rs @@ -37,17 +37,19 @@ pub enum Var { pub enum Constant { Bool(bool), - Discrete(i32), + Integer(i32), } #[derive(Copy, Clone)] -pub enum VarType { - Bounded(i32, i32), +pub enum VarDomain { + Bound(i32, i32), + Discrete(i32, i32), + SparseBound(i32, i32), Bool(bool), } pub struct SymbolTable { - table: HashMap, + table: HashMap, // for now doubles both as Minion's SearchOrder and print order var_order: Vec, @@ -63,7 +65,7 @@ impl SymbolTable { /// Creates a new variable and adds it to the symbol table. /// If a variable already exists with the given name, an error is thrown. - pub fn add_var(&mut self, name: VarName, vartype: VarType) -> Option<()> { + pub fn add_var(&mut self, name: VarName, vartype: VarDomain) -> Option<()> { if self.table.contains_key(&name) { return None; } @@ -74,7 +76,7 @@ impl SymbolTable { return Some(()); } - pub fn get_vartype(&self, name: VarName) -> Option { + pub fn get_vartype(&self, name: VarName) -> Option { match self.table.get(&name) { Some(m) => Some(*m), None => None, diff --git a/solvers/minion/src/run.rs b/solvers/minion/src/run.rs index 72ed0bf31..65badad78 100644 --- a/solvers/minion/src/run.rs +++ b/solvers/minion/src/run.rs @@ -55,7 +55,7 @@ unsafe fn convert_model_to_raw(model: &Model) -> *mut ProbSpec_CSPInstance { .expect(""); let (vartype_raw, domain_low, domain_high) = match vartype { - VarType::Bounded(a, b) => (VariableType_VAR_BOUND, a, b), + VarDomain::Bound(a, b) => (VariableType_VAR_BOUND, a, b), _ => panic!("NOT IMPLEMENTED"), }; @@ -183,7 +183,7 @@ unsafe fn read_const(raw_constraint: *mut ProbSpec_ConstraintBlob, constant: &Co let raw_consts = Scoped::new(vec_int_new(), |x| vec_var_free(x as _)); let val = match constant { - Constant::Discrete(n) => n, + Constant::Integer(n) => n, _ => panic!("NOT IMPLEMENTED"), }; @@ -205,13 +205,13 @@ mod tests { let mut model = Model::new(); model .named_variables - .add_var("x".to_owned(), VarType::Bounded(1, 3)); + .add_var("x".to_owned(), VarDomain::Bound(1, 3)); model .named_variables - .add_var("y".to_owned(), VarType::Bounded(2, 4)); + .add_var("y".to_owned(), VarDomain::Bound(2, 4)); model .named_variables - .add_var("z".to_owned(), VarType::Bounded(1, 5)); + .add_var("z".to_owned(), VarDomain::Bound(1, 5)); let leq = Constraint::SumLeq( vec![ @@ -234,7 +234,7 @@ mod tests { let ineq = Constraint::Ineq( Var::NameRef("x".to_owned()), Var::NameRef("y".to_owned()), - Constant::Discrete(-1), + Constant::Integer(-1), ); model.constraints.push(leq);