Skip to content

Commit

Permalink
Merge pull request #477 from ozgurakgun/warnings
Browse files Browse the repository at this point in the history
deal with warnings
  • Loading branch information
ozgurakgun authored Nov 21, 2024
2 parents 6a2656a + 9b35d7e commit 4e70610
Show file tree
Hide file tree
Showing 13 changed files with 95 additions and 26 deletions.
77 changes: 77 additions & 0 deletions .github/workflows/warnings.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
name: "Builds without any warnings"

on:
push:
branches:
- main # run for pushes to the main branch. other branches need to create a PR if they want testing.
paths:
- conjure_oxide/**
- solvers/**
- crates/**
- Cargo.*
- .github/workflows/warnings.yml
pull_request:
paths:
- conjure_oxide/**
- solvers/**
- crates/**
- Cargo.*
- .github/workflows/warnings.yml
workflow_dispatch:

env:
SCCACHE_GHA_ENABLED: "true"
RUSTC_WRAPPER: "sccache"
SCCACHE_GHA_VERSION: 4

jobs:
build-and-test:
name: "Build and Test"
timeout-minutes: 10
strategy:
# run all combinations of the matrix even if one combination fails.
fail-fast: false
matrix:
rust_release:
- stable
conjure_version:
- 2.5.1
os:
- ubuntu-latest
include:
- os: ubuntu-latest
release_suffix: linux

runs-on: ${{ matrix.os }}

steps:
- uses: actions/checkout@v3
- name: Run sccache-cache
uses: mozilla-actions/[email protected]

- run: rustup update ${{ matrix.rust_release }} && rustup default ${{ matrix.rust_release }}

- run: RUSTFLAGS="-D warnings" cargo build -vv --workspace

examples:
name: "Build Examples"
timeout-minutes: 10
strategy:
# run all combinations of the matrix even if one combination fails.
fail-fast: false
matrix:
rust_release:
- stable
os:
- ubuntu-latest

runs-on: ${{ matrix.os }}

steps:
- uses: actions/checkout@v3
- name: Run sccache-cache
uses: mozilla-actions/[email protected]

- run: rustup update ${{ matrix.rust_release }} && rustup default ${{ matrix.rust_release }}

- run: RUSTFLAGS="-D warnings" cargo build -vv --workspace --examples
2 changes: 1 addition & 1 deletion conjure_oxide/tests/generated_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ where
});

// Write the JSON log
write!(writer, "{}\n", log)
writeln!(writer, "{}", log)
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/conjure_core/src/ast/variables.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::ast::domains::{Domain, Range};
/// - `domain`:
/// - Type: `Domain`
/// - Represents the set of possible values that this decision variable can assume. The domain can be a range of integers
/// (IntDomain) or a boolean domain (BoolDomain).
/// (IntDomain) or a boolean domain (BoolDomain).
///
/// # Example
///
Expand Down
1 change: 0 additions & 1 deletion crates/conjure_core/src/model.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use std::borrow::{Borrow, BorrowMut};
use std::cell::RefCell;
use std::collections::HashSet;
use std::fmt::Debug;
Expand Down
2 changes: 1 addition & 1 deletion crates/conjure_core/src/parse/parse_model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ fn parse_constant(constant: &serde_json::Map<String, Value>) -> Option<Expressio
}

Some(Value::Object(b)) if b.contains_key("ConstantBool") => {
let b: bool = b["ConstantBool"].as_bool().unwrap();
let b: bool = b["ConstantBool"].as_bool()?;
Some(Expression::Atomic(
Metadata::new(),
Atom::Literal(Literal::Bool(b)),
Expand Down
13 changes: 5 additions & 8 deletions crates/conjure_core/src/rule_engine/rewrite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -357,16 +357,13 @@ fn apply_all_rules<'a>(
/// Process the chosen reduction
/// }
///
fn choose_rewrite<'a>(
results: &[RuleResult],
initial_expression: &'a Expression,
) -> Option<Reduction> {
fn choose_rewrite(results: &[RuleResult], initial_expression: &Expression) -> Option<Reduction> {
//in the case where multiple rules are applicable
if results.len() > 1 {
let expr = results[0].reduction.new_expression.clone();
let rules: Vec<_> = results.iter().map(|result| &result.rule).collect();

check_priority(rules.clone(), &initial_expression, &expr);
check_priority(rules.clone(), initial_expression, &expr);
}

if results.is_empty() {
Expand Down Expand Up @@ -408,10 +405,10 @@ fn check_priority<'a>(

//iterates over each rule_set and groups by the rule priority
for rule_set in &rule_sets {
if let Some((name, priority)) = rule_set.get(0) {
if let Some((name, priority)) = rule_set.first() {
rules_by_priorities
.entry(*priority)
.or_insert(Vec::new())
.or_default()
.push(*name);
}
}
Expand All @@ -424,7 +421,7 @@ fn check_priority<'a>(

if !duplicate_rules.is_empty() {
//accumulates all duplicates into a formatted message
let mut message = String::from(format!("Found multiple rules of the same priority applicable to to expression: {:?} \n resulting in expression: {:?}", initial_expr, new_expr));
let mut message = format!("Found multiple rules of the same priority applicable to to expression: {:?} \n resulting in expression: {:?}", initial_expr, new_expr);
for (priority, rules) in &duplicate_rules {
message.push_str(&format!("Priority {:?} \n Rules: {:?}", priority, rules));
}
Expand Down
3 changes: 1 addition & 2 deletions crates/conjure_core/src/rule_engine/rule.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
use std::collections::HashSet;
use std::fmt::{self, Display, Formatter};
use std::hash::Hash;

use thiserror::Error;

use crate::ast::{Expression, Name, SymbolTable};
use crate::ast::{Expression, SymbolTable};
use crate::metadata::Metadata;
use crate::model::Model;

Expand Down
2 changes: 1 addition & 1 deletion crates/conjure_core/src/rules/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ fn apply_eval_constant(expr: &Expr, _: &Model) -> ApplicationResult {
pub fn eval_constant(expr: &Expr) -> Option<Lit> {
match expr {
Expr::Atomic(_, Atom::Literal(c)) => Some(c.clone()),
Expr::Atomic(_, Atom::Reference(c)) => None,
Expr::Atomic(_, Atom::Reference(_c)) => None,
Expr::Eq(_, a, b) => bin_op::<i32, bool>(|a, b| a == b, a, b)
.or_else(|| bin_op::<bool, bool>(|a, b| a == b, a, b))
.map(Lit::Bool),
Expand Down
5 changes: 2 additions & 3 deletions crates/conjure_core/src/rules/minion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,12 @@ use crate::rules::extra_check;
use crate::{
ast::{
Atom::{self, *},
DecisionVariable, Domain,
Domain,
Expression::{self as Expr, *},
Literal::*,
SymbolTable,
},
bug,
rules::utils::{exprs_to_conjunction, is_atom},
rules::utils::exprs_to_conjunction,
};

use crate::solver::SolverFamily;
Expand Down
6 changes: 2 additions & 4 deletions crates/conjure_core/src/rules/partial_eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ use crate::ast::{Atom, Expression as Expr, Literal::*};
use crate::rule_engine::{ApplicationResult, Reduction};
use crate::Model;

use super::utils::ToAuxVarOutput;

#[register_rule(("Base",9000))]
fn partial_evaluator(expr: &Expr, _: &Model) -> ApplicationResult {
use conjure_core::rule_engine::ApplicationError::RuleNotApplicable;
Expand Down Expand Up @@ -286,8 +284,8 @@ fn partial_evaluator(expr: &Expr, _: &Model) -> ApplicationResult {
WatchedLiteral(_, _, _) => Err(RuleNotApplicable),
Reify(_, _, _) => Err(RuleNotApplicable),
AuxDeclaration(_, _, _) => Err(RuleNotApplicable),
UnsafeMod(_, a, b) => Err(RuleNotApplicable),
SafeMod(_, a, b) => Err(RuleNotApplicable),
UnsafeMod(_, _, _) => Err(RuleNotApplicable),
SafeMod(_, _, _) => Err(RuleNotApplicable),
ModuloEqUndefZero(_, _, _, _) => Err(RuleNotApplicable),
}
}
4 changes: 2 additions & 2 deletions crates/conjure_core/src/rules/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use uniplate::{Biplate, Uniplate};

use crate::{
ast::{Atom, DecisionVariable, Domain, Expression as Expr, Name, ReturnType},
bug,
metadata::Metadata,
Model,
};
Expand Down Expand Up @@ -85,7 +84,7 @@ pub fn to_aux_var(expr: &Expr, m: &Model) -> Option<ToAuxVarOutput> {
}

// Anything that should be bubbled, bubble
if (expr.can_be_undefined()) {
if expr.can_be_undefined() {
return None;
}

Expand All @@ -111,6 +110,7 @@ pub fn to_aux_var(expr: &Expr, m: &Model) -> Option<ToAuxVarOutput> {
pub struct ToAuxVarOutput {
aux_name: Name,
aux_decl: Expr,
#[allow(dead_code)] // TODO: aux_domain should be used soon, try removing this pragma
aux_domain: Domain,
new_model: Model,
_unconstructable: (),
Expand Down
2 changes: 1 addition & 1 deletion crates/conjure_macros/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ version = "0.1.0"
edition = "2021"

[lib]
proc_macro = true
proc-macro = true

[dependencies]
quote = "1.0.36"
Expand Down
2 changes: 1 addition & 1 deletion crates/enum_compatability_macro/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
[lib]
proc_macro = true
proc-macro = true

[dependencies]
quote = "1.0.36"
Expand Down

0 comments on commit 4e70610

Please sign in to comment.