From ac6fbb26d0318dfc51677b4e2830cb3cbc84aae6 Mon Sep 17 00:00:00 2001 From: Niklas Dewally Date: Tue, 21 Nov 2023 20:01:20 +0000 Subject: [PATCH 1/4] conjure_oxide: basic CLI input for main --- Cargo.lock | 116 +++++++++++++++++++++++++++++++++++-- conjure_oxide/Cargo.toml | 2 + conjure_oxide/src/main.rs | 100 +++++++++++++++----------------- conjure_oxide/src/parse.rs | 2 +- 4 files changed, 161 insertions(+), 59 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 695f2eeb9..a4047d2d6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -26,6 +26,54 @@ dependencies = [ "libc", ] +[[package]] +name = "anstream" +version = "0.6.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2ab91ebe16eb252986481c5b62f6098f3b698a45e34b5b98200cf20dd2484a44" +dependencies = [ + "anstyle", + "anstyle-parse", + "anstyle-query", + "anstyle-wincon", + "colorchoice", + "utf8parse", +] + +[[package]] +name = "anstyle" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7079075b41f533b8c61d2a4d073c4676e1f8b249ff94a393b0595db304e0dd87" + +[[package]] +name = "anstyle-parse" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "317b9a89c1868f5ea6ff1d9539a69f45dffc21ce321ac1fd1160dfa48c8e2140" +dependencies = [ + "utf8parse", +] + +[[package]] +name = "anstyle-query" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5ca11d4be1bab0c8bc8734a9aa7bf4ee8316d462a08c6ac5052f888fef5b494b" +dependencies = [ + "windows-sys", +] + +[[package]] +name = "anstyle-wincon" +version = "3.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f0699d10d2f4d628a98ee7b57b289abbc98ff3bad977cb3152709d4bf2330628" +dependencies = [ + "anstyle", + "windows-sys", +] + [[package]] name = "anyhow" version = "1.0.75" @@ -164,10 +212,58 @@ dependencies = [ "libloading", ] +[[package]] +name = "clap" +version = "4.4.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2275f18819641850fa26c89acc84d465c1bf91ce57bc2748b28c420473352f64" +dependencies = [ + "clap_builder", + "clap_derive", +] + +[[package]] +name = "clap_builder" +version = "4.4.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "07cdf1b148b25c1e1f7a42225e30a0d99a615cd4637eae7365548dd4529b95bc" +dependencies = [ + "anstream", + "anstyle", + "clap_lex", + "strsim", +] + +[[package]] +name = "clap_derive" +version = "4.4.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cf9804afaaf59a91e75b022a30fb7229a7901f60c755489cc61c9b423b836442" +dependencies = [ + "heck", + "proc-macro2", + "quote", + "syn 2.0.39", +] + +[[package]] +name = "clap_lex" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "702fc72eb24e5a1e48ce58027a675bc24edd52096d5397d4aea7c6dd9eca0bd1" + +[[package]] +name = "colorchoice" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "acbf1af155f9b9ef647e42cdc158db4b64a1b61f743629225fde6f3e0be2a7c7" + [[package]] name = "conjure_oxide" version = "0.0.1" dependencies = [ + "anyhow", + "clap", "minion_rs", "serde", "serde_json", @@ -284,6 +380,12 @@ version = "0.14.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f93e7192158dbcda357bdec5fb5788eebf8bbac027f3f33e719d29135ae84156" +[[package]] +name = "heck" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "95505c38b4572b2d910cecb0281560f54b440a19336cbbcb27bf6ce6adc6f5a8" + [[package]] name = "hex" version = "0.4.3" @@ -592,18 +694,18 @@ dependencies = [ [[package]] name = "serde" -version = "1.0.192" +version = "1.0.193" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bca2a08484b285dcb282d0f67b26cadc0df8b19f8c12502c13d966bf9482f001" +checksum = "25dd9975e68d0cb5aa1120c288333fc98731bd1dd12f561e468ea4728c042b89" dependencies = [ "serde_derive", ] [[package]] name = "serde_derive" -version = "1.0.192" +version = "1.0.193" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d6c7207fbec9faa48073f3e3074cbe553af6ea512d7c21ba46e434e70ea9fbc1" +checksum = "43576ca501357b9b071ac53cdc7da8ef0cbd9493d8df094cd821777ea6e894d3" dependencies = [ "proc-macro2", "quote", @@ -739,6 +841,12 @@ version = "1.0.12" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3354b9ac3fae1ff6755cb6db53683adb661634f67557942dea4facebec0fee4b" +[[package]] +name = "utf8parse" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "711b9620af191e0cdc7468a8d14e709c3dcdb115b36f838e601583af800a370a" + [[package]] name = "walkdir" version = "2.4.0" diff --git a/conjure_oxide/Cargo.toml b/conjure_oxide/Cargo.toml index 5d53553b9..617458afd 100644 --- a/conjure_oxide/Cargo.toml +++ b/conjure_oxide/Cargo.toml @@ -14,6 +14,8 @@ serde_json = "1.0.108" serde_with = "3.4.0" thiserror = "1.0.50" minion_rs = {path = "../solvers/minion" } +anyhow = "1.0.75" +clap = { version = "4.4.8", features = ["derive"] } [lints] workspace = true diff --git a/conjure_oxide/src/main.rs b/conjure_oxide/src/main.rs index cc3937147..f20e7147f 100644 --- a/conjure_oxide/src/main.rs +++ b/conjure_oxide/src/main.rs @@ -1,55 +1,47 @@ -use std::collections::HashMap; - -use conjure_oxide::ast::*; - -fn main() { - // find a,b,c : int(1..3) - // such that a + b + c = 4 - // such that a >= b - - println!("Manual model:"); - - let a = Name::UserName(String::from("a")); - let b = Name::UserName(String::from("b")); - let c = Name::UserName(String::from("c")); - - let mut variables = HashMap::new(); - variables.insert( - a.clone(), - DecisionVariable { - domain: Domain::IntDomain(vec![Range::Bounded(1, 3)]), - }, - ); - variables.insert( - b.clone(), - DecisionVariable { - domain: Domain::IntDomain(vec![Range::Bounded(1, 3)]), - }, - ); - variables.insert( - c.clone(), - DecisionVariable { - domain: Domain::IntDomain(vec![Range::Bounded(1, 3)]), - }, - ); - - let m = Model { - variables, - constraints: vec![ - Expression::Eq( - Box::new(Expression::Sum(vec![ - Expression::Reference(a.clone()), - Expression::Reference(b.clone()), - Expression::Reference(c.clone()), - ])), - Box::new(Expression::ConstantInt(4)), - ), - Expression::Geq( - Box::new(Expression::Reference(a.clone())), - Box::new(Expression::Reference(b.clone())), - ), - ], - }; - - println!("\n{:?}\n", m); +// (niklasdewally): temporary, gut this if you want! + +use anyhow::{anyhow, bail}; +use std::path::PathBuf; + +use anyhow::Result as AnyhowResult; +use clap::{arg, command, Parser}; +use conjure_oxide::parse::parse_json; +#[derive(Parser)] +#[command(author, version, about, long_about = None)] +struct Cli { + #[arg(long, value_name = "SOLVER")] + solver: Option, + + #[arg(value_name = "INPUT_ESSENCE")] + input_file: PathBuf, +} + +pub fn main() -> AnyhowResult<()> { + let cli = Cli::parse(); + println!("Input file: {}", cli.input_file.display()); + let input_file: &str = cli.input_file.to_str().ok_or(anyhow!( + "Given input_file could not be converted to a string" + ))?; + + /******************************************************/ + /* Parse essence to json using Conjure */ + /******************************************************/ + + let mut cmd = std::process::Command::new("conjure"); + let output = cmd + .arg("pretty") + .arg("--output-format=astjson") + .arg(input_file) + .output()?; + + let conjure_stderr = String::from_utf8(output.stderr)?; + if !conjure_stderr.is_empty() { + bail!(conjure_stderr); + } + + let astjson = String::from_utf8(output.stdout)?; + + let model = parse_json(&astjson)?; + println!("{:?}", model); + Ok(()) } diff --git a/conjure_oxide/src/parse.rs b/conjure_oxide/src/parse.rs index d7095f404..25a1a06fc 100644 --- a/conjure_oxide/src/parse.rs +++ b/conjure_oxide/src/parse.rs @@ -4,7 +4,7 @@ use serde_json::Value as JsonValue; use Error::ModelConstructError as CError; -pub fn parse_json(str: &String) -> Result { +pub fn parse_json(str: &str) -> Result { let mut m = Model::new(); let v: JsonValue = serde_json::from_str(str)?; let constraints = v["mStatements"] From 50133a1bc9d995085c1b0b0ca4bd989db1686f49 Mon Sep 17 00:00:00 2001 From: Niklas Dewally Date: Tue, 21 Nov 2023 20:45:21 +0000 Subject: [PATCH 2/4] conjure_oxide: solver error type --- Cargo.lock | 27 +++++++++ conjure_oxide/Cargo.toml | 2 + conjure_oxide/src/solvers/error.rs | 15 +++++ conjure_oxide/src/solvers/minion.rs | 90 ++++++++++++++++++---------- conjure_oxide/src/solvers/mod.rs | 4 ++ conjure_oxide/src/solvers/solvers.rs | 13 ++++ 6 files changed, 119 insertions(+), 32 deletions(-) create mode 100644 conjure_oxide/src/solvers/error.rs create mode 100644 conjure_oxide/src/solvers/solvers.rs diff --git a/Cargo.lock b/Cargo.lock index a4047d2d6..8417c9b74 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -268,6 +268,8 @@ dependencies = [ "serde", "serde_json", "serde_with", + "strum", + "strum_macros", "thiserror", "walkdir", ] @@ -677,6 +679,12 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "rustversion" +version = "1.0.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7ffc183a10b4478d04cbbbfc96d0873219d962dd5accaff2ffbd4ceb7df837f4" + [[package]] name = "ryu" version = "1.0.15" @@ -764,6 +772,25 @@ version = "0.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "73473c0e59e6d5812c5dfe2a064a6444949f089e20eec9a2e5506596494e4623" +[[package]] +name = "strum" +version = "0.25.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "290d54ea6f91c969195bdbcd7442c8c2a2ba87da8bf60a7ee86a235d4bc1e125" + +[[package]] +name = "strum_macros" +version = "0.25.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "23dc1fa9ac9c169a78ba62f0b841814b7abae11bdd047b9c58f893439e309ea0" +dependencies = [ + "heck", + "proc-macro2", + "quote", + "rustversion", + "syn 2.0.39", +] + [[package]] name = "syn" version = "1.0.109" diff --git a/conjure_oxide/Cargo.toml b/conjure_oxide/Cargo.toml index 617458afd..6ad746f39 100644 --- a/conjure_oxide/Cargo.toml +++ b/conjure_oxide/Cargo.toml @@ -16,6 +16,8 @@ thiserror = "1.0.50" minion_rs = {path = "../solvers/minion" } anyhow = "1.0.75" clap = { version = "4.4.8", features = ["derive"] } +strum_macros = "0.25.3" +strum = "0.25.0" [lints] workspace = true diff --git a/conjure_oxide/src/solvers/error.rs b/conjure_oxide/src/solvers/error.rs new file mode 100644 index 000000000..b080a4acf --- /dev/null +++ b/conjure_oxide/src/solvers/error.rs @@ -0,0 +1,15 @@ +use thiserror::Error; + +use super::Solver; + +#[derive(Error, Debug)] +pub enum SolverError { + #[error("not supported in solver `{0}`: `{1}`.")] + NotSupported(Solver, String), + + #[error("invalid instance for solver `{0}`:`{1}`")] + InvalidInstance(Solver, String), + + #[error(transparent)] + Other(#[from] anyhow::Error), +} diff --git a/conjure_oxide/src/solvers/minion.rs b/conjure_oxide/src/solvers/minion.rs index e7b9fb529..de7fef8dd 100644 --- a/conjure_oxide/src/solvers/minion.rs +++ b/conjure_oxide/src/solvers/minion.rs @@ -1,5 +1,7 @@ //! Solver interface to minion_rs. +use anyhow::anyhow; + use crate::ast::{ DecisionVariable, Domain as ConjureDomain, Expression as ConjureExpression, Model as ConjureModel, Name as ConjureName, Range as ConjureRange, @@ -9,9 +11,12 @@ use minion_rs::ast::{ Var as MinionVar, VarDomain as MinionDomain, }; +use super::{Solver, SolverError}; + +const SOLVER: Solver = Solver::Minion; + impl TryFrom for MinionModel { - // TODO (nd60): set this to equal ConjureError once it is merged. - type Error = String; + type Error = SolverError; fn try_from(conjure_model: ConjureModel) -> Result { let mut minion_model = MinionModel::new(); @@ -25,7 +30,10 @@ impl TryFrom for MinionModel { } } -fn parse_vars(conjure_model: &ConjureModel, minion_model: &mut MinionModel) -> Result<(), String> { +fn parse_vars( + conjure_model: &ConjureModel, + minion_model: &mut MinionModel, +) -> Result<(), SolverError> { // TODO (nd60): remove unused vars? // TODO (nd60): ensure all vars references are used. @@ -39,12 +47,12 @@ fn parse_var( name: &ConjureName, variable: &DecisionVariable, minion_model: &mut MinionModel, -) -> Result<(), String> { +) -> Result<(), SolverError> { let str_name = name_to_string(name.to_owned()); let ranges = match &variable.domain { ConjureDomain::IntDomain(range) => Ok(range), - x => Err(format!("Not supported: {:?}", x)), + x => Err(SolverError::NotSupported(SOLVER, format!("{:?}", x))), }?; // TODO (nd60): Currently, Minion only supports the use of one range in the domain. @@ -52,44 +60,54 @@ fn parse_var( // See: https://github.com/conjure-cp/conjure-oxide/issues/84 if ranges.len() != 1 { - return Err(format!( - "Variable {:?} has {:?} ranges. Multiple ranges / SparseBound is not yet supported.", + return Err(SolverError::NotSupported( + SOLVER, + format!( + "variable {:?} has {:?} ranges. Multiple ranges / SparseBound is not yet supported.", str_name, ranges.len() + ), )); } - let range = ranges - .first() - .ok_or(format!("Variable {:?} has no range", str_name))?; + let range = ranges.first().ok_or(SolverError::InvalidInstance( + SOLVER, + format!("variable {:?} has no range", str_name), + ))?; let (low, high) = match range { ConjureRange::Bounded(x, y) => Ok((x.to_owned(), y.to_owned())), ConjureRange::Single(x) => Ok((x.to_owned(), x.to_owned())), - a => Err(format!("Not implemented {:?}", a)), + x => Err(SolverError::NotSupported(SOLVER, format!("{:?}", x))), }?; minion_model .named_variables .add_var(str_name.to_owned(), MinionDomain::Bound(low, high)) - .ok_or(format!("Variable {:?} is defined twice", str_name))?; + .ok_or(SolverError::InvalidInstance( + SOLVER, + format!("variable {:?} is defined twice", str_name), + ))?; Ok(()) } -fn parse_exprs(conjure_model: &ConjureModel, minion_model: &mut MinionModel) -> Result<(), String> { +fn parse_exprs( + conjure_model: &ConjureModel, + minion_model: &mut MinionModel, +) -> Result<(), SolverError> { for expr in conjure_model.constraints.iter() { parse_expr(expr.to_owned(), minion_model)?; } Ok(()) } -fn parse_expr(expr: ConjureExpression, minion_model: &mut MinionModel) -> Result<(), String> { +fn parse_expr(expr: ConjureExpression, minion_model: &mut MinionModel) -> Result<(), SolverError> { match expr { ConjureExpression::SumLeq(lhs, rhs) => parse_sumleq(lhs, *rhs, minion_model), ConjureExpression::SumGeq(lhs, rhs) => parse_sumgeq(lhs, *rhs, minion_model), ConjureExpression::Ineq(a, b, c) => parse_ineq(*a, *b, *c, minion_model), - x => Err(format!("Not supported: {:?}", x)), + x => Err(SolverError::NotSupported(SOLVER, format!("{:?}", x))), } } @@ -97,7 +115,7 @@ fn parse_sumleq( sum_vars: Vec, rhs: ConjureExpression, minion_model: &mut MinionModel, -) -> Result<(), String> { +) -> Result<(), SolverError> { let minion_vars = must_be_vars(sum_vars)?; let minion_rhs = must_be_var(rhs)?; minion_model @@ -111,7 +129,7 @@ fn parse_sumgeq( sum_vars: Vec, rhs: ConjureExpression, minion_model: &mut MinionModel, -) -> Result<(), String> { +) -> Result<(), SolverError> { let minion_vars = must_be_vars(sum_vars)?; let minion_rhs = must_be_var(rhs)?; minion_model @@ -126,7 +144,7 @@ fn parse_ineq( b: ConjureExpression, c: ConjureExpression, minion_model: &mut MinionModel, -) -> Result<(), String> { +) -> Result<(), SolverError> { let a_minion = must_be_var(a)?; let b_minion = must_be_var(b)?; let c_value = must_be_const(c)?; @@ -139,7 +157,7 @@ fn parse_ineq( Ok(()) } -fn must_be_vars(exprs: Vec) -> Result, String> { +fn must_be_vars(exprs: Vec) -> Result, SolverError> { let mut minion_vars: Vec = vec![]; for expr in exprs { let minion_var = must_be_var(expr)?; @@ -148,7 +166,7 @@ fn must_be_vars(exprs: Vec) -> Result, String> Ok(minion_vars) } -fn must_be_var(e: ConjureExpression) -> Result { +fn must_be_var(e: ConjureExpression) -> Result { // a minion var is either a reference or a "var as const" match must_be_ref(e.clone()) { Ok(name) => Ok(MinionVar::NameRef(name)), @@ -159,20 +177,26 @@ fn must_be_var(e: ConjureExpression) -> Result { } } -fn must_be_ref(e: ConjureExpression) -> Result { +fn must_be_ref(e: ConjureExpression) -> Result { let name = match e { ConjureExpression::Reference(n) => Ok(n), - _ => Err(""), + x => Err(SolverError::InvalidInstance( + SOLVER, + format!("expected a reference, but got `{0:?}`", x), + )), }?; let str_name = name_to_string(name); Ok(str_name) } -fn must_be_const(e: ConjureExpression) -> Result { +fn must_be_const(e: ConjureExpression) -> Result { match e { ConjureExpression::ConstantInt(n) => Ok(n), - _ => Err("".to_owned()), + x => Err(SolverError::InvalidInstance( + SOLVER, + format!("expected a constant, but got `{0:?}`", x), + )), } } @@ -192,7 +216,7 @@ mod tests { use super::*; #[test] - fn flat_xyz_model() -> Result<(), String> { + fn flat_xyz_model() -> Result<(), anyhow::Error> { // TODO: convert to use public interfaces when these exist. let mut model = ConjureModel { variables: HashMap::new(), @@ -223,7 +247,7 @@ mod tests { model.constraints.push(ineq); let minion_model = MinionModel::try_from(model)?; - minion_rs::run_minion(minion_model, xyz_callback).map_err(|x| x.to_string()) + Ok(minion_rs::run_minion(minion_model, xyz_callback)?) } #[allow(clippy::unwrap_used)] @@ -255,7 +279,7 @@ mod tests { name: &str, domain_low: i32, domain_high: i32, - ) -> Result<(), String> { + ) -> Result<(), SolverError> { // TODO: convert to use public interfaces when these exist. let res = model.variables.insert( ConjureName::UserName(name.to_owned()), @@ -266,11 +290,13 @@ mod tests { )]), }, ); - - match res { - // variable was not already present - None => Ok(()), - Some(_) => Err(format!("Variable {:?} was already present", name)), + if res.is_some() { + return Err(SolverError::Other(anyhow!( + "Tried to add variable {:?} to the symbol table, but it was already present", + name + ))); } + + Ok(()) } } diff --git a/conjure_oxide/src/solvers/mod.rs b/conjure_oxide/src/solvers/mod.rs index dad73cff9..a0f74d2b1 100644 --- a/conjure_oxide/src/solvers/mod.rs +++ b/conjure_oxide/src/solvers/mod.rs @@ -1 +1,5 @@ +mod error; pub mod minion; +pub use error::*; +mod solvers; +pub use solvers::*; diff --git a/conjure_oxide/src/solvers/solvers.rs b/conjure_oxide/src/solvers/solvers.rs new file mode 100644 index 000000000..991142aa3 --- /dev/null +++ b/conjure_oxide/src/solvers/solvers.rs @@ -0,0 +1,13 @@ +//! All supported solvers. + +use strum_macros::Display; +use strum_macros::{EnumIter, EnumString}; + +/// All supported solvers. +/// +/// This enum implements, Display and Iter, so can be used as a string, or iterated over. +#[derive(Debug, EnumString, EnumIter, Display)] +pub enum Solver { + Minion, + KissSAT, +} From 07238e42c258520845b9d54aeab8b8c17bbce6b8 Mon Sep 17 00:00:00 2001 From: Niklas Dewally Date: Tue, 21 Nov 2023 20:50:25 +0000 Subject: [PATCH 3/4] conjure_oxide: more error varients, and wildcard error handling --- conjure_oxide/src/error.rs | 17 ++++++--- conjure_oxide/src/parse.rs | 74 ++++++++++++++++++++++++-------------- 2 files changed, 61 insertions(+), 30 deletions(-) diff --git a/conjure_oxide/src/error.rs b/conjure_oxide/src/error.rs index 2d107bcce..f883466d5 100644 --- a/conjure_oxide/src/error.rs +++ b/conjure_oxide/src/error.rs @@ -1,3 +1,5 @@ +//! Top-level error types for Conjure-Oxide. + use serde_json::Error as JsonError; use thiserror::Error; @@ -5,8 +7,15 @@ pub type Result = std::result::Result; #[derive(Debug, Error)] pub enum Error { - #[error("JSON parsing error: {0}")] - JsonError(#[from] JsonError), - #[error("Error constructing model: {0}")] - ModelConstructError(String), + #[error("JSON error: {0}")] + JSON(#[from] JsonError), + + #[error("Error parsing model: {0}")] + Parse(String), + + #[error("{0} is not yet implemented.")] + NotImplemented(String), + + #[error(transparent)] + Other(#[from] anyhow::Error), } diff --git a/conjure_oxide/src/parse.rs b/conjure_oxide/src/parse.rs index 25a1a06fc..3cfae8e91 100644 --- a/conjure_oxide/src/parse.rs +++ b/conjure_oxide/src/parse.rs @@ -2,29 +2,33 @@ use crate::ast::{DecisionVariable, Domain, Model, Name, Range}; use crate::error::{Error, Result}; use serde_json::Value as JsonValue; -use Error::ModelConstructError as CError; - pub fn parse_json(str: &str) -> Result { let mut m = Model::new(); let v: JsonValue = serde_json::from_str(str)?; let constraints = v["mStatements"] .as_array() - .ok_or(CError("mStatements is not an array".to_owned()))?; + .ok_or(Error::Parse("mStatements is not an array".to_owned()))?; for con in constraints { let entry = con .as_object() - .ok_or(CError("mStatements contains a non-object".to_owned()))? + .ok_or(Error::Parse("mStatements contains a non-object".to_owned()))? .iter() .next() - .ok_or(CError("mStatements contains an empty object".to_owned()))?; + .ok_or(Error::Parse( + "mStatements contains an empty object".to_owned(), + ))?; match entry.0.as_str() { "Declaration" => { let (name, var) = parse_variable(entry.1)?; m.add_variable(name, var); } "SuchThat" => parse_constraint(entry.1)?, - _ => return Err(CError("mStatements contains an unknown object".to_owned())), + _ => { + return Err(Error::Parse( + "mStatements contains an unknown object".to_owned(), + )) + } } } @@ -34,25 +38,29 @@ pub fn parse_json(str: &str) -> Result { fn parse_variable(v: &JsonValue) -> Result<(Name, DecisionVariable)> { let arr = v .as_object() - .ok_or(CError("Declaration is not an object".to_owned()))?["FindOrGiven"] + .ok_or(Error::Parse("Declaration is not an object".to_owned()))?["FindOrGiven"] .as_array() - .ok_or(CError("FindOrGiven is not an array".to_owned()))?; + .ok_or(Error::Parse("FindOrGiven is not an array".to_owned()))?; let name = arr[1] .as_object() - .ok_or(CError("FindOrGiven[1] is not an object".to_owned()))?["Name"] + .ok_or(Error::Parse("FindOrGiven[1] is not an object".to_owned()))?["Name"] .as_str() - .ok_or(CError("FindOrGiven[1].Name is not a string".to_owned()))?; + .ok_or(Error::Parse( + "FindOrGiven[1].Name is not a string".to_owned(), + ))?; let name = Name::UserName(name.to_owned()); let domain = arr[2] .as_object() - .ok_or(CError("FindOrGiven[2] is not an object".to_owned()))? + .ok_or(Error::Parse("FindOrGiven[2] is not an object".to_owned()))? .iter() .next() - .ok_or(CError("FindOrGiven[2] is an empty object".to_owned()))?; + .ok_or(Error::Parse("FindOrGiven[2] is an empty object".to_owned()))?; let domain = match domain.0.as_str() { "DomainInt" => Ok(parse_int_domain(domain.1)?), "DomainBool" => Ok(Domain::BoolDomain), - _ => Err(CError("FindOrGiven[2] is an unknown object".to_owned())), + _ => Err(Error::Parse( + "FindOrGiven[2] is an unknown object".to_owned(), + )), }?; Ok((name, DecisionVariable { domain })) } @@ -61,29 +69,37 @@ fn parse_int_domain(v: &JsonValue) -> Result { let mut ranges = Vec::new(); let arr = v .as_array() - .ok_or(CError("DomainInt is not an array".to_owned()))?[1] + .ok_or(Error::Parse("DomainInt is not an array".to_owned()))?[1] .as_array() - .ok_or(CError("DomainInt[1] is not an array".to_owned()))?; + .ok_or(Error::Parse("DomainInt[1] is not an array".to_owned()))?; for range in arr { let range = range .as_object() - .ok_or(CError("DomainInt[1] contains a non-object".to_owned()))? + .ok_or(Error::Parse( + "DomainInt[1] contains a non-object".to_owned(), + ))? .iter() .next() - .ok_or(CError("DomainInt[1] contains an empty object".to_owned()))?; + .ok_or(Error::Parse( + "DomainInt[1] contains an empty object".to_owned(), + ))?; match range.0.as_str() { "RangeBounded" => { let arr = range .1 .as_array() - .ok_or(CError("RangeBounded is not an array".to_owned()))?; + .ok_or(Error::Parse("RangeBounded is not an array".to_owned()))?; let mut nums = Vec::new(); for i in 0..2 { - let num = &arr[i]["Constant"]["ConstantInt"][1] - .as_i64() - .ok_or(CError("Could not parse int domain constant".to_owned()))?; - let num32 = i32::try_from(*num) - .map_err(|_| CError("Could not parse int domain constant".to_owned()))?; + let num = + &arr[i]["Constant"]["ConstantInt"][1] + .as_i64() + .ok_or(Error::Parse( + "Could not parse int domain constant".to_owned(), + ))?; + let num32 = i32::try_from(*num).map_err(|_| { + Error::Parse("Could not parse int domain constant".to_owned()) + })?; nums.push(num32); } ranges.push(Range::Bounded(nums[0], nums[1])); @@ -91,12 +107,18 @@ fn parse_int_domain(v: &JsonValue) -> Result { "RangeSingle" => { let num = &range.1["Constant"]["ConstantInt"][1] .as_i64() - .ok_or(CError("Could not parse int domain constant".to_owned()))?; + .ok_or(Error::Parse( + "Could not parse int domain constant".to_owned(), + ))?; let num32 = i32::try_from(*num) - .map_err(|_| CError("Could not parse int domain constant".to_owned()))?; + .map_err(|_| Error::Parse("Could not parse int domain constant".to_owned()))?; ranges.push(Range::Single(num32)); } - _ => return Err(CError("DomainInt[1] contains an unknown object".to_owned())), + _ => { + return Err(Error::Parse( + "DomainInt[1] contains an unknown object".to_owned(), + )) + } } } Ok(Domain::IntDomain(ranges)) From 983eaf06d8925d1104ed09d94d3f0648229a6970 Mon Sep 17 00:00:00 2001 From: Niklas Dewally Date: Tue, 21 Nov 2023 21:16:29 +0000 Subject: [PATCH 4/4] fix some clippy warnings --- conjure_oxide/build.rs | 4 ++-- conjure_oxide/src/parse.rs | 2 +- conjure_oxide/src/solvers/minion.rs | 2 ++ conjure_oxide/src/solvers/mod.rs | 4 ++-- conjure_oxide/src/solvers/{solvers.rs => solver_list.rs} | 4 ---- solvers/chuffed/src/lib.rs | 4 +--- src/lib.rs | 6 ------ 7 files changed, 8 insertions(+), 18 deletions(-) rename conjure_oxide/src/solvers/{solvers.rs => solver_list.rs} (60%) delete mode 100644 src/lib.rs diff --git a/conjure_oxide/build.rs b/conjure_oxide/build.rs index 7a160cba8..d3d0b18aa 100644 --- a/conjure_oxide/build.rs +++ b/conjure_oxide/build.rs @@ -7,7 +7,7 @@ use walkdir::WalkDir; fn main() -> io::Result<()> { let out_dir = var("OUT_DIR").map_err(|e| io::Error::new(io::ErrorKind::Other, e))?; // wrapping in a std::io::Error to match main's error type let dest = Path::new(&out_dir).join("gen_tests.rs"); - let mut f = File::create(&dest)?; + let mut f = File::create(dest)?; let test_dir = "tests/integration"; @@ -45,7 +45,7 @@ fn write_test(file: &mut File, path: String, essence_files: Vec) -> io:: file, include_str!("./tests/gen_test_template"), // TODO: better sanitisation of paths to function names - test_name = path.replace("./", "").replace("/", "_").replace("-", "_"), + test_name = path.replace("./", "").replace(['/', '-'], "_"), test_dir = path, essence_file = essence_files[0] ) diff --git a/conjure_oxide/src/parse.rs b/conjure_oxide/src/parse.rs index 3cfae8e91..1ed89e24a 100644 --- a/conjure_oxide/src/parse.rs +++ b/conjure_oxide/src/parse.rs @@ -124,7 +124,7 @@ fn parse_int_domain(v: &JsonValue) -> Result { Ok(Domain::IntDomain(ranges)) } -fn parse_constraint(obj: &JsonValue) -> Result<()> { +fn parse_constraint(_obj: &JsonValue) -> Result<()> { Ok(()) } diff --git a/conjure_oxide/src/solvers/minion.rs b/conjure_oxide/src/solvers/minion.rs index de7fef8dd..9e2f43db5 100644 --- a/conjure_oxide/src/solvers/minion.rs +++ b/conjure_oxide/src/solvers/minion.rs @@ -1,5 +1,6 @@ //! Solver interface to minion_rs. +#[allow(unused_imports)] use anyhow::anyhow; use crate::ast::{ @@ -78,6 +79,7 @@ fn parse_var( let (low, high) = match range { ConjureRange::Bounded(x, y) => Ok((x.to_owned(), y.to_owned())), ConjureRange::Single(x) => Ok((x.to_owned(), x.to_owned())), + #[allow(unreachable_patterns)] x => Err(SolverError::NotSupported(SOLVER, format!("{:?}", x))), }?; diff --git a/conjure_oxide/src/solvers/mod.rs b/conjure_oxide/src/solvers/mod.rs index a0f74d2b1..733306ab9 100644 --- a/conjure_oxide/src/solvers/mod.rs +++ b/conjure_oxide/src/solvers/mod.rs @@ -1,5 +1,5 @@ mod error; pub mod minion; pub use error::*; -mod solvers; -pub use solvers::*; +mod solver_list; +pub use solver_list::*; diff --git a/conjure_oxide/src/solvers/solvers.rs b/conjure_oxide/src/solvers/solver_list.rs similarity index 60% rename from conjure_oxide/src/solvers/solvers.rs rename to conjure_oxide/src/solvers/solver_list.rs index 991142aa3..b987e7ff6 100644 --- a/conjure_oxide/src/solvers/solvers.rs +++ b/conjure_oxide/src/solvers/solver_list.rs @@ -1,11 +1,7 @@ -//! All supported solvers. - use strum_macros::Display; use strum_macros::{EnumIter, EnumString}; /// All supported solvers. -/// -/// This enum implements, Display and Iter, so can be used as a string, or iterated over. #[derive(Debug, EnumString, EnumIter, Display)] pub enum Solver { Minion, diff --git a/solvers/chuffed/src/lib.rs b/solvers/chuffed/src/lib.rs index 4e8fc2265..45b17a4a3 100644 --- a/solvers/chuffed/src/lib.rs +++ b/solvers/chuffed/src/lib.rs @@ -1,7 +1,5 @@ pub mod bindings { - #![allow(non_upper_case_globals)] - #![allow(non_camel_case_types)] - #![allow(non_snake_case)] + #![allow(warnings)] include!(concat!(env!("OUT_DIR"), "/chuffed_bindings.rs")); } diff --git a/src/lib.rs b/src/lib.rs deleted file mode 100644 index d9559cebb..000000000 --- a/src/lib.rs +++ /dev/null @@ -1,6 +0,0 @@ -pub mod ast; -pub mod error; -pub mod parse; - -pub use ast::Model; -pub use error::Error;