diff options
Diffstat (limited to 'crates/shirabe/src')
13 files changed, 399 insertions, 492 deletions
diff --git a/crates/shirabe/src/dependency_resolver/decisions.rs b/crates/shirabe/src/dependency_resolver/decisions.rs index f0842d9..3b82883 100644 --- a/crates/shirabe/src/dependency_resolver/decisions.rs +++ b/crates/shirabe/src/dependency_resolver/decisions.rs @@ -10,7 +10,7 @@ use std::fmt; pub struct Decisions { pub(crate) pool: std::rc::Rc<std::cell::RefCell<Pool>>, pub(crate) decision_map: IndexMap<i64, i64>, - pub(crate) decision_queue: Vec<(i64, Box<dyn Rule>)>, + pub(crate) decision_queue: Vec<(i64, std::rc::Rc<std::cell::RefCell<Rule>>)>, iterator_cursor: Option<usize>, } @@ -36,7 +36,7 @@ impl Decisions { } } - pub fn decide(&mut self, literal: i64, level: i64, why: Box<dyn Rule>) { + pub fn decide(&mut self, literal: i64, level: i64, why: std::rc::Rc<std::cell::RefCell<Rule>>) { self.add_decision(literal, level); self.decision_queue.push((literal, why)); } @@ -90,12 +90,15 @@ impl Decisions { 0 } - pub fn decision_rule(&self, literal_or_package_id: i64) -> &dyn Rule { + pub fn decision_rule( + &self, + literal_or_package_id: i64, + ) -> std::rc::Rc<std::cell::RefCell<Rule>> { let package_id = literal_or_package_id.abs(); for decision in &self.decision_queue { if package_id == decision.0.abs() { - return &*decision.1; + return decision.1.clone(); } } @@ -112,7 +115,7 @@ impl Decisions { ); } - pub fn at_offset(&self, queue_offset: usize) -> &(i64, Box<dyn Rule>) { + pub fn at_offset(&self, queue_offset: usize) -> &(i64, std::rc::Rc<std::cell::RefCell<Rule>>) { &self.decision_queue[queue_offset] } @@ -120,8 +123,8 @@ impl Decisions { queue_offset >= 0 && queue_offset < self.decision_queue.len() as i64 } - pub fn last_reason(&self) -> &dyn Rule { - &*self.decision_queue[self.decision_queue.len() - 1].1 + pub fn last_reason(&self) -> std::rc::Rc<std::cell::RefCell<Rule>> { + self.decision_queue[self.decision_queue.len() - 1].1.clone() } pub fn last_literal(&self) -> i64 { @@ -159,7 +162,7 @@ impl Decisions { } } - pub fn current(&self) -> Option<&(i64, Box<dyn Rule>)> { + pub fn current(&self) -> Option<&(i64, std::rc::Rc<std::cell::RefCell<Rule>>)> { self.iterator_cursor .and_then(|cursor| self.decision_queue.get(cursor)) } diff --git a/crates/shirabe/src/dependency_resolver/generic_rule.rs b/crates/shirabe/src/dependency_resolver/generic_rule.rs index 11cff06..eada1c3 100644 --- a/crates/shirabe/src/dependency_resolver/generic_rule.rs +++ b/crates/shirabe/src/dependency_resolver/generic_rule.rs @@ -2,9 +2,9 @@ use crate::dependency_resolver::{Rule, RuleBase}; use anyhow::Result; -use shirabe_php_shim::{PHP_VERSION_ID, PhpMixed, RuntimeException, hash_raw, implode, unpack}; +use shirabe_php_shim::{PHP_VERSION_ID, PhpMixed, RuntimeException, hash_raw, unpack}; -use super::{request::Request, rule::ReasonData}; +use super::rule::ReasonData; #[derive(Debug)] pub struct GenericRule { @@ -19,6 +19,14 @@ impl GenericRule { Self { inner, literals } } + pub(crate) fn base(&self) -> &RuleBase { + &self.inner + } + + pub(crate) fn base_mut(&mut self) -> &mut RuleBase { + &mut self.inner + } + pub fn get_literals(&self) -> &Vec<i64> { &self.literals } @@ -57,8 +65,8 @@ impl GenericRule { } } - pub fn equals(&self, rule: &dyn RuleLiterals) -> bool { - self.literals == *rule.get_literals() + pub fn equals(&self, rule: &Rule) -> bool { + self.literals == rule.get_literals() } pub fn is_assertion(&self) -> bool { @@ -66,75 +74,6 @@ impl GenericRule { } } -pub trait RuleLiterals { - fn get_literals(&self) -> &Vec<i64>; - fn is_multi_conflict_rule(&self) -> bool { - false - } - fn is_assertion(&self) -> bool { - false - } - fn is_disabled(&self) -> bool { - false - } - fn as_any(&self) -> &dyn std::any::Any { - todo!() - } - /// Clone this rule into an owned `Box<dyn Rule>` so callers like - /// `RuleWatchGraph::propagate_literal` can hand it to `Decisions::decide`. - fn clone_rule_box(&self) -> Box<dyn Rule> { - todo!() - } -} - -impl RuleLiterals for GenericRule { - fn get_literals(&self) -> &Vec<i64> { - &self.literals - } -} - -impl Rule for GenericRule { - fn bitfield(&self) -> i64 { - todo!() - } - - fn bitfield_mut(&mut self) -> &mut i64 { - todo!() - } - - fn request(&self) -> Option<&Request> { - todo!() - } - - fn request_mut(&mut self) -> Option<&mut Request> { - todo!() - } - - fn reason_data(&self) -> Option<&ReasonData> { - todo!() - } - - fn reason_data_mut(&mut self) -> Option<&mut ReasonData> { - todo!() - } - - fn get_literals(&self) -> Vec<i64> { - todo!() - } - - fn get_hash(&self) -> PhpMixed { - todo!() - } - - fn equals(&self, rule: &dyn Rule) -> bool { - todo!() - } - - fn is_assertion(&self) -> bool { - todo!() - } -} - impl std::fmt::Display for GenericRule { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!( diff --git a/crates/shirabe/src/dependency_resolver/multi_conflict_rule.rs b/crates/shirabe/src/dependency_resolver/multi_conflict_rule.rs index 6447be0..5b2e107 100644 --- a/crates/shirabe/src/dependency_resolver/multi_conflict_rule.rs +++ b/crates/shirabe/src/dependency_resolver/multi_conflict_rule.rs @@ -1,12 +1,8 @@ //! ref: composer/src/Composer/DependencyResolver/MultiConflictRule.php -use shirabe_php_shim::PhpMixed; - -use crate::dependency_resolver::Request; -use crate::dependency_resolver::RuleLiterals; use crate::dependency_resolver::{ReasonData, Rule, RuleBase}; use anyhow::Result; -use shirabe_php_shim::{PHP_VERSION_ID, RuntimeException, hash_raw}; +use shirabe_php_shim::{PHP_VERSION_ID, PhpMixed, RuntimeException, hash_raw}; #[derive(Debug)] pub struct MultiConflictRule { @@ -15,11 +11,7 @@ pub struct MultiConflictRule { } impl MultiConflictRule { - pub fn new( - mut literals: Vec<i64>, - reason: shirabe_php_shim::PhpMixed, - reason_data: shirabe_php_shim::PhpMixed, - ) -> Result<Self> { + pub fn new(mut literals: Vec<i64>, reason: PhpMixed, reason_data: PhpMixed) -> Result<Self> { if literals.len() < 3 { return Err(RuntimeException { message: "multi conflict rule requires at least 3 literals".to_string(), @@ -37,6 +29,14 @@ impl MultiConflictRule { }) } + pub(crate) fn base(&self) -> &RuleBase { + &self.inner + } + + pub(crate) fn base_mut(&mut self) -> &mut RuleBase { + &mut self.inner + } + pub fn get_literals(&self) -> &Vec<i64> { &self.literals } @@ -75,78 +75,17 @@ impl MultiConflictRule { } } - pub fn equals(&self, rule: &dyn RuleLiterals) -> bool { - // PHP: if ($rule instanceof MultiConflictRule) { ... } return false; - // Phase A: instanceof check not representable via RuleLiterals trait; literals-only comparison used - self.literals == *rule.get_literals() + pub fn equals(&self, rule: &Rule) -> bool { + if let Rule::MultiConflict(other) = rule { + self.literals == other.literals + } else { + false + } } pub fn is_assertion(&self) -> bool { false } - - pub fn disable(&mut self) -> Result<()> { - Err(RuntimeException { - message: "Disabling multi conflict rules is not possible. Please contact composer at https://github.com/composer/composer to let us debug what lead to this situation.".to_string(), - code: 0, - }.into()) - } -} - -impl RuleLiterals for MultiConflictRule { - fn get_literals(&self) -> &Vec<i64> { - &self.literals - } - - fn is_multi_conflict_rule(&self) -> bool { - true - } -} - -impl Rule for MultiConflictRule { - fn bitfield(&self) -> i64 { - todo!() - } - - fn bitfield_mut(&mut self) -> &mut i64 { - todo!() - } - - fn request(&self) -> Option<&Request> { - todo!() - } - - fn request_mut(&mut self) -> Option<&mut Request> { - todo!() - } - - fn reason_data(&self) -> Option<&ReasonData> { - todo!() - } - - fn reason_data_mut(&mut self) -> Option<&mut ReasonData> { - todo!() - } - - fn get_literals(&self) -> Vec<i64> { - todo!() - } - - fn get_hash(&self) -> PhpMixed { - todo!() - } - - fn equals(&self, rule: &dyn Rule) -> bool { - todo!() - } - - fn is_assertion(&self) -> bool { - todo!() - } - - fn as_multi_conflict(&self) -> Option<&MultiConflictRule> { - Some(self) - } } impl std::fmt::Display for MultiConflictRule { diff --git a/crates/shirabe/src/dependency_resolver/problem.rs b/crates/shirabe/src/dependency_resolver/problem.rs index 4bc5fe8..a69a4a2 100644 --- a/crates/shirabe/src/dependency_resolver/problem.rs +++ b/crates/shirabe/src/dependency_resolver/problem.rs @@ -35,7 +35,7 @@ pub struct Problem { pub(crate) reason_seen: IndexMap<String, bool>, /// A set of reasons for the problem, each is a rule or a root require and a rule - pub(crate) reasons: IndexMap<i64, Vec<Box<dyn Rule>>>, + pub(crate) reasons: IndexMap<i64, Vec<std::rc::Rc<std::cell::RefCell<Rule>>>>, pub(crate) section: i64, } @@ -50,13 +50,13 @@ impl Problem { } /// Add a rule as a reason - pub fn add_rule(&mut self, rule: Box<dyn Rule>) { - let id = spl_object_hash(&rule); + pub fn add_rule(&mut self, rule: std::rc::Rc<std::cell::RefCell<Rule>>) { + let id = spl_object_hash(&*rule.borrow()); self.add_reason(id, rule); } /// Retrieve all reasons for this problem - pub fn get_reasons(&self) -> &IndexMap<i64, Vec<Box<dyn Rule>>> { + pub fn get_reasons(&self) -> &IndexMap<i64, Vec<std::rc::Rc<std::cell::RefCell<Rule>>>> { &self.reasons } @@ -68,20 +68,21 @@ impl Problem { pool: &mut Pool, is_verbose: bool, installed_map: &IndexMap<String, Box<dyn BasePackage>>, - learned_pool: &Vec<Vec<Box<dyn Rule>>>, + learned_pool: &Vec<Vec<std::rc::Rc<std::cell::RefCell<Rule>>>>, ) -> anyhow::Result<String> { // TODO doesn't this entirely defeat the purpose of the problem sections? what's the point of sections? - let mut reasons: Vec<Box<dyn Rule>> = Vec::new(); + let mut reasons: Vec<std::rc::Rc<std::cell::RefCell<Rule>>> = Vec::new(); for section_rules in self.reasons.values().rev() { for rule in section_rules { - reasons.push(rule.clone_box()); + reasons.push(rule.clone()); } } if reasons.len() == 1 { - let rule = reasons[0].clone_box(); + let rule = reasons[0].clone(); + let rule_ref = rule.borrow(); - if rule.get_reason() != rule::RULE_ROOT_REQUIRE { + if rule_ref.get_reason() != rule::RULE_ROOT_REQUIRE { return Err(LogicException { message: "Single reason problems must contain a root require rule.".to_string(), code: 0, @@ -89,7 +90,7 @@ impl Problem { .into()); } - let reason_data = rule.get_reason_data(); + let reason_data = rule_ref.get_reason_data(); // TODO(phase-b): reason_data for RULE_ROOT_REQUIRE; extract via ReasonData::RootRequire variant. let (package_name, constraint): (String, Option<&dyn ConstraintInterface>) = match reason_data { @@ -115,14 +116,14 @@ impl Problem { } reasons.sort_by(|rule1, rule2| { - let rule1_prio = self.get_rule_priority(rule1.as_ref()); - let rule2_prio = self.get_rule_priority(rule2.as_ref()); + let rule1_prio = self.get_rule_priority(&rule1.borrow()); + let rule2_prio = self.get_rule_priority(&rule2.borrow()); if rule1_prio != rule2_prio { return rule2_prio.cmp(&rule1_prio); } - self.get_sortable_string(pool, rule1.as_ref()) - .cmp(&self.get_sortable_string(pool, rule2.as_ref())) + self.get_sortable_string(pool, &rule1.borrow()) + .cmp(&self.get_sortable_string(pool, &rule2.borrow())) }); Ok(Self::format_deduplicated_rules( @@ -137,7 +138,7 @@ impl Problem { )) } - fn get_sortable_string(&self, pool: &Pool, rule: &dyn Rule) -> String { + fn get_sortable_string(&self, pool: &Pool, rule: &Rule) -> String { match rule.get_reason() { rule::RULE_ROOT_REQUIRE => match rule.get_reason_data() { rule::ReasonData::RootRequire { package_name, .. } => package_name.clone(), @@ -181,7 +182,7 @@ impl Problem { } } - fn get_rule_priority(&self, rule: &dyn Rule) -> i64 { + fn get_rule_priority(&self, rule: &Rule) -> i64 { match rule.get_reason() { rule::RULE_FIXED => 3, rule::RULE_ROOT_REQUIRE => 2, @@ -199,14 +200,14 @@ impl Problem { /// @internal pub fn format_deduplicated_rules( - rules: &Vec<Box<dyn Rule>>, + rules: &Vec<std::rc::Rc<std::cell::RefCell<Rule>>>, indent: &str, repository_set: &RepositorySet, request: &Request, pool: &mut Pool, is_verbose: bool, installed_map: &IndexMap<String, Box<dyn BasePackage>>, - learned_pool: &Vec<Vec<Box<dyn Rule>>>, + learned_pool: &Vec<Vec<std::rc::Rc<std::cell::RefCell<Rule>>>>, ) -> String { let mut messages: Vec<String> = Vec::new(); let mut templates: IndexMap<String, IndexMap<String, IndexMap<String, String>>> = @@ -215,7 +216,8 @@ impl Problem { let deduplicatable_rule_types = vec![rule::RULE_PACKAGE_REQUIRES, rule::RULE_PACKAGE_CONFLICT]; for rule in rules { - let mut message = rule.get_pretty_string( + let rule_ref = rule.borrow(); + let mut message = rule_ref.get_pretty_string( repository_set, request, pool, @@ -225,7 +227,7 @@ impl Problem { ); let mut m: IndexMap<CaptureKey, String> = IndexMap::new(); let matched = if in_array( - PhpMixed::Int(rule.get_reason()), + PhpMixed::Int(rule_ref.get_reason()), &PhpMixed::List( deduplicatable_rule_types .iter() @@ -257,7 +259,7 @@ impl Problem { .entry(pkg_key.clone()) .or_insert_with(IndexMap::new) .insert(version_key, m2.clone()); - let source_package = rule.get_source_package(pool); + let source_package = rule_ref.get_source_package(pool); for (version, pretty_version) in pool.get_removed_versions_by_package(&spl_object_hash(&source_package)) { @@ -354,7 +356,10 @@ impl Problem { ) -> bool { for section_rules in self.reasons.values() { for rule in section_rules { - if rule.is_caused_by_lock(repository_set, request, pool) { + if rule + .borrow() + .is_caused_by_lock(repository_set, request, pool) + { return true; } } @@ -364,7 +369,7 @@ impl Problem { } /// Store a reason descriptor but ignore duplicates - pub(crate) fn add_reason(&mut self, id: String, reason: Box<dyn Rule>) { + pub(crate) fn add_reason(&mut self, id: String, reason: std::rc::Rc<std::cell::RefCell<Rule>>) { // TODO: if a rule is part of a problem description in two sections, isn't this going to remove a message // that is important to understand the issue? diff --git a/crates/shirabe/src/dependency_resolver/rule.rs b/crates/shirabe/src/dependency_resolver/rule.rs index bf117ad..90223e2 100644 --- a/crates/shirabe/src/dependency_resolver/rule.rs +++ b/crates/shirabe/src/dependency_resolver/rule.rs @@ -1,19 +1,24 @@ //! ref: composer/src/Composer/DependencyResolver/Rule.php use std::any::Any; +use std::cell::RefCell; +use std::rc::Rc; use anyhow::Result; use indexmap::IndexMap; use shirabe_php_shim::{ - LogicException, PhpMixed, abs, array_filter, array_keys, array_shift, array_values, implode, - is_object, + LogicException, PhpMixed, RuntimeException, abs, array_filter, array_keys, array_shift, + array_values, implode, is_object, }; use shirabe_semver::constraint::Constraint; use shirabe_semver::constraint::ConstraintInterface; +use crate::dependency_resolver::GenericRule; +use crate::dependency_resolver::MultiConflictRule; use crate::dependency_resolver::Pool; use crate::dependency_resolver::Problem; use crate::dependency_resolver::Request; +use crate::dependency_resolver::Rule2Literals; use crate::dependency_resolver::RuleSet; use crate::package::AliasPackage; use crate::package::BasePackage; @@ -23,8 +28,6 @@ use crate::package::version::VersionParser; use crate::repository::PlatformRepository; use crate::repository::RepositorySet; -/// PHP: @phpstan-type ReasonData = Link|BasePackage|string|int|array{...}|array{...} -/// We model this as an enum. #[derive(Debug)] pub enum ReasonData { Link(Link), @@ -70,41 +73,98 @@ pub const BITFIELD_TYPE: i64 = 0; pub const BITFIELD_REASON: i64 = 8; pub const BITFIELD_DISABLED: i64 = 16; -pub trait Rule: std::fmt::Display + std::fmt::Debug { - fn bitfield(&self) -> i64; - fn bitfield_mut(&mut self) -> &mut i64; - fn request(&self) -> Option<&Request>; - fn request_mut(&mut self) -> Option<&mut Request>; - fn reason_data(&self) -> Option<&ReasonData>; - fn reason_data_mut(&mut self) -> Option<&mut ReasonData>; +#[derive(Debug)] +pub enum Rule { + Generic(GenericRule), + MultiConflict(MultiConflictRule), + TwoLiterals(Rule2Literals), +} + +impl Rule { + fn base(&self) -> &RuleBase { + match self { + Rule::Generic(r) => r.base(), + Rule::MultiConflict(r) => r.base(), + Rule::TwoLiterals(r) => r.base(), + } + } + + fn base_mut(&mut self) -> &mut RuleBase { + match self { + Rule::Generic(r) => r.base_mut(), + Rule::MultiConflict(r) => r.base_mut(), + Rule::TwoLiterals(r) => r.base_mut(), + } + } + + fn bitfield(&self) -> i64 { + self.base().bitfield + } + + fn bitfield_mut(&mut self) -> &mut i64 { + &mut self.base_mut().bitfield + } + + fn reason_data(&self) -> Option<&ReasonData> { + self.base().reason_data.as_ref() + } + + pub fn get_literals(&self) -> Vec<i64> { + match self { + Rule::Generic(r) => r.get_literals().clone(), + Rule::MultiConflict(r) => r.get_literals().clone(), + // PHP Rule2Literals::getLiterals returns [$literal1, $literal2]. + Rule::TwoLiterals(r) => vec![r.literal1, r.literal2], + } + } + + pub fn get_hash(&self) -> Result<PhpMixed> { + match self { + Rule::Generic(r) => Ok(PhpMixed::Int(r.get_hash()?)), + Rule::MultiConflict(r) => Ok(PhpMixed::Int(r.get_hash()?)), + Rule::TwoLiterals(r) => Ok(PhpMixed::String(r.get_hash())), + } + } - fn get_literals(&self) -> Vec<i64>; - fn get_hash(&self) -> PhpMixed; - fn equals(&self, rule: &dyn Rule) -> bool; - fn is_assertion(&self) -> bool; + pub fn equals(&self, rule: &Rule) -> bool { + match self { + Rule::Generic(r) => r.equals(rule), + Rule::MultiConflict(r) => r.equals(rule), + Rule::TwoLiterals(r) => r.equals(rule), + } + } + + pub fn is_assertion(&self) -> bool { + match self { + Rule::Generic(r) => r.is_assertion(), + Rule::MultiConflict(r) => r.is_assertion(), + Rule::TwoLiterals(r) => r.is_assertion(), + } + } - fn clone_box(&self) -> Box<dyn Rule> { - todo!() + pub fn is_multi_conflict_rule(&self) -> bool { + matches!(self, Rule::MultiConflict(_)) } - /// PHP: `$rule instanceof MultiConflictRule`. Returns a borrow of the - /// underlying `MultiConflictRule` when this rule is one, otherwise `None`. - fn as_multi_conflict(&self) -> Option<&crate::dependency_resolver::MultiConflictRule> { - None + pub fn as_multi_conflict(&self) -> Option<&MultiConflictRule> { + match self { + Rule::MultiConflict(r) => Some(r), + _ => None, + } } /// @return self::RULE_* - fn get_reason(&self) -> i64 { + pub fn get_reason(&self) -> i64 { (self.bitfield() & (255 << BITFIELD_REASON)) >> BITFIELD_REASON } /// @phpstan-return ReasonData - fn get_reason_data(&self) -> &ReasonData { + pub fn get_reason_data(&self) -> &ReasonData { // TODO(phase-b): reason_data() returns Option; PHP getReasonData unconditional self.reason_data().unwrap() } - fn get_required_package(&self) -> Option<String> { + pub fn get_required_package(&self) -> Option<String> { match self.get_reason() { r if r == RULE_ROOT_REQUIRE => match self.get_reason_data() { ReasonData::RootRequire { package_name, .. } => Some(package_name.clone()), @@ -123,33 +183,41 @@ pub trait Rule: std::fmt::Display + std::fmt::Debug { } /// @param RuleSet::TYPE_* $type - fn set_type(&mut self, r#type: i64) { + pub fn set_type(&mut self, r#type: i64) { *self.bitfield_mut() = (self.bitfield() & !(255i64 << BITFIELD_TYPE)) | ((255 & r#type) << BITFIELD_TYPE); } - fn get_type(&self) -> i64 { + pub fn get_type(&self) -> i64 { (self.bitfield() & (255 << BITFIELD_TYPE)) >> BITFIELD_TYPE } - fn disable(&mut self) { + pub fn disable(&mut self) -> Result<()> { + if let Rule::MultiConflict(_) = self { + return Err(RuntimeException { + message: "Disabling multi conflict rules is not possible. Please contact composer at https://github.com/composer/composer to let us debug what lead to this situation.".to_string(), + code: 0, + } + .into()); + } *self.bitfield_mut() = (self.bitfield() & !(255i64 << BITFIELD_DISABLED)) | (1i64 << BITFIELD_DISABLED); + Ok(()) } - fn enable(&mut self) { + pub fn enable(&mut self) { *self.bitfield_mut() &= !(255i64 << BITFIELD_DISABLED); } - fn is_disabled(&self) -> bool { + pub fn is_disabled(&self) -> bool { 0 != ((self.bitfield() & (255 << BITFIELD_DISABLED)) >> BITFIELD_DISABLED) } - fn is_enabled(&self) -> bool { + pub fn is_enabled(&self) -> bool { 0 == ((self.bitfield() & (255 << BITFIELD_DISABLED)) >> BITFIELD_DISABLED) } - fn is_caused_by_lock( + pub fn is_caused_by_lock( &self, _repository_set: &RepositorySet, request: &Request, @@ -221,7 +289,7 @@ pub trait Rule: std::fmt::Display + std::fmt::Debug { } /// @internal - fn get_source_package(&self, pool: &Pool) -> Result<Box<dyn BasePackage>> { + pub fn get_source_package(&self, pool: &Pool) -> Result<Box<dyn BasePackage>> { let literals = self.get_literals(); match self.get_reason() { @@ -263,14 +331,14 @@ pub trait Rule: std::fmt::Display + std::fmt::Debug { /// @param BasePackage[] $installedMap /// @param array<Rule[]> $learnedPool - fn get_pretty_string( + pub fn get_pretty_string( &self, repository_set: &RepositorySet, request: &Request, pool: &mut Pool, is_verbose: bool, installed_map: &IndexMap<String, Box<dyn BasePackage>>, - _learned_pool: &Vec<Vec<Box<dyn Rule>>>, + _learned_pool: &Vec<Vec<Rc<RefCell<Rule>>>>, ) -> String { let mut literals = self.get_literals(); @@ -295,7 +363,6 @@ pub trait Rule: std::fmt::Display + std::fmt::Debug { ); } - // PHP: array_values(array_filter($packages, fn ($p) => !($p instanceof AliasPackage))) let packages_non_alias: Vec<Box<dyn BasePackage>> = packages .iter() .filter(|p| p.as_any().downcast_ref::<AliasPackage>().is_none()) @@ -317,7 +384,7 @@ pub trait Rule: std::fmt::Display + std::fmt::Debug { "Root composer.json requires {} {} -> satisfiable by {}.", package_name, constraint.get_pretty_string(), - self.format_packages_unique( + self.format_packages_unique_from_packages( pool, packages, is_verbose, @@ -437,7 +504,7 @@ pub trait Rule: std::fmt::Display + std::fmt::Debug { format!( "{} -> satisfiable by {}.", text, - self.format_packages_unique( + self.format_packages_unique_from_packages( pool, requires, is_verbose, @@ -467,7 +534,6 @@ pub trait Rule: std::fmt::Display + std::fmt::Debug { let package = pool.literal_to_package(*literal); package_names.insert(package.get_name().to_string(), true); } - // PHP: unset($literal); let replaced_name = match self.get_reason_data() { ReasonData::String(s) => s.clone(), _ => String::new(), @@ -510,14 +576,14 @@ pub trait Rule: std::fmt::Display + std::fmt::Debug { if installed_packages.len() > 0 && removable_packages.len() > 0 { return format!( "{} cannot be installed as that would require removing {}. {}", - self.format_packages_unique( + self.format_packages_unique_from_packages( pool, removable_packages, is_verbose, None, true, ), - self.format_packages_unique( + self.format_packages_unique_from_packages( pool, installed_packages, is_verbose, @@ -576,7 +642,7 @@ pub trait Rule: std::fmt::Display + std::fmt::Debug { "{}{} {}", group, if packages.len() > 1 { " one of" } else { "" }, - self.format_packages_unique( + self.format_packages_unique_from_packages( pool, packages.iter().map(|p| p.clone_box()).collect(), is_verbose, @@ -640,22 +706,15 @@ pub trait Rule: std::fmt::Display + std::fmt::Debug { } } - /// @param array<int|BasePackage> $literalsOrPackages An array containing packages or literals - fn format_packages_unique( + // Corresponds the variant formatPackagesUnique() that takes an array of BasePackages. + fn format_packages_unique_from_packages( &self, pool: &Pool, - literals_or_packages: Vec<Box<dyn BasePackage>>, + packages: Vec<Box<dyn BasePackage>>, is_verbose: bool, constraint: Option<&dyn ConstraintInterface>, use_removed_version_group: bool, ) -> String { - let mut packages: Vec<Box<dyn BasePackage>> = vec![]; - for package in literals_or_packages { - // PHP: \is_object($package) ? $package : $pool->literalToPackage($package); - // In Rust we already have BasePackage, so no conversion needed. - packages.push(package); - } - Problem::get_package_list( &packages, is_verbose, @@ -665,7 +724,7 @@ pub trait Rule: std::fmt::Display + std::fmt::Debug { ) } - /// Helper for cases where literals come as int IDs (PHP supports both via union). + // Corresponds the variant formatPackagesUnique() that takes an array of integers. fn format_packages_unique_from_literals( &self, pool: &Pool, @@ -701,6 +760,16 @@ pub trait Rule: std::fmt::Display + std::fmt::Debug { } } +impl std::fmt::Display for Rule { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Rule::Generic(r) => write!(f, "{}", r), + Rule::MultiConflict(r) => write!(f, "{}", r), + Rule::TwoLiterals(r) => write!(f, "{}", r), + } + } +} + #[derive(Debug)] pub struct RuleBase { pub(crate) bitfield: i64, diff --git a/crates/shirabe/src/dependency_resolver/rule2_literals.rs b/crates/shirabe/src/dependency_resolver/rule2_literals.rs index 3642f8a..7db9024 100644 --- a/crates/shirabe/src/dependency_resolver/rule2_literals.rs +++ b/crates/shirabe/src/dependency_resolver/rule2_literals.rs @@ -2,8 +2,6 @@ use shirabe_php_shim::PhpMixed; -use crate::dependency_resolver::Request; -use crate::dependency_resolver::RuleLiterals; use crate::dependency_resolver::{ReasonData, Rule, RuleBase}; #[derive(Debug)] @@ -11,16 +9,10 @@ pub struct Rule2Literals { inner: RuleBase, pub(crate) literal1: i64, pub(crate) literal2: i64, - literals: Vec<i64>, } impl Rule2Literals { - pub fn new( - literal1: i64, - literal2: i64, - reason: shirabe_php_shim::PhpMixed, - reason_data: shirabe_php_shim::PhpMixed, - ) -> Self { + pub fn new(literal1: i64, literal2: i64, reason: PhpMixed, reason_data: PhpMixed) -> Self { let (literal1, literal2) = if literal1 < literal2 { (literal1, literal2) } else { @@ -31,17 +23,33 @@ impl Rule2Literals { inner: RuleBase::new(reason.as_int().unwrap_or(0), ReasonData::from(reason_data)), literal1, literal2, - literals: vec![literal1, literal2], } } + pub(crate) fn base(&self) -> &RuleBase { + &self.inner + } + + pub(crate) fn base_mut(&mut self) -> &mut RuleBase { + &mut self.inner + } + pub fn get_hash(&self) -> String { format!("{},{}", self.literal1, self.literal2) } - pub fn equals(&self, rule: &dyn RuleLiterals) -> bool { - // PHP: specialized fast-case for instanceof self, then fallback to literal comparison - // In Rust: use get_literals() for all cases (semantically equivalent) + pub fn equals(&self, rule: &Rule) -> bool { + // PHP: specialized fast-case when `$rule instanceof self`. + if let Rule::TwoLiterals(other) = rule { + if self.literal1 != other.literal1 { + return false; + } + if self.literal2 != other.literal2 { + return false; + } + return true; + } + let literals = rule.get_literals(); if literals.len() != 2 { return false; @@ -60,54 +68,6 @@ impl Rule2Literals { } } -impl RuleLiterals for Rule2Literals { - fn get_literals(&self) -> &Vec<i64> { - &self.literals - } -} - -impl Rule for Rule2Literals { - fn bitfield(&self) -> i64 { - todo!() - } - - fn bitfield_mut(&mut self) -> &mut i64 { - todo!() - } - - fn request(&self) -> Option<&Request> { - todo!() - } - - fn request_mut(&mut self) -> Option<&mut Request> { - todo!() - } - - fn reason_data(&self) -> Option<&ReasonData> { - todo!() - } - - fn reason_data_mut(&mut self) -> Option<&mut ReasonData> { - todo!() - } - - fn get_literals(&self) -> Vec<i64> { - todo!() - } - - fn get_hash(&self) -> PhpMixed { - todo!() - } - - fn equals(&self, rule: &dyn Rule) -> bool { - todo!() - } - - fn is_assertion(&self) -> bool { - todo!() - } -} - impl std::fmt::Display for Rule2Literals { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!( diff --git a/crates/shirabe/src/dependency_resolver/rule_set.rs b/crates/shirabe/src/dependency_resolver/rule_set.rs index fa49bec..8a44385 100644 --- a/crates/shirabe/src/dependency_resolver/rule_set.rs +++ b/crates/shirabe/src/dependency_resolver/rule_set.rs @@ -1,5 +1,8 @@ //! ref: composer/src/Composer/DependencyResolver/RuleSet.php +use std::cell::RefCell; +use std::rc::Rc; + use indexmap::IndexMap; use shirabe_php_shim::OutOfBoundsException; @@ -11,10 +14,10 @@ use crate::repository::RepositorySet; #[derive(Debug)] pub struct RuleSet { - pub rule_by_id: IndexMap<i64, Box<dyn Rule>>, - pub(crate) rules: IndexMap<i64, Vec<Box<dyn Rule>>>, + pub rule_by_id: IndexMap<i64, Rc<RefCell<Rule>>>, + pub(crate) rules: IndexMap<i64, Vec<Rc<RefCell<Rule>>>>, pub(crate) next_rule_id: i64, - pub(crate) rules_by_hash: IndexMap<String, Vec<Box<dyn Rule>>>, + pub(crate) rules_by_hash: IndexMap<String, Vec<Rc<RefCell<Rule>>>>, } impl RuleSet { @@ -47,7 +50,7 @@ impl RuleSet { Self::types().into_keys().collect() } - pub fn add(&mut self, rule: Box<dyn Rule>, r#type: i64) -> anyhow::Result<()> { + pub fn add(&mut self, rule: Rc<RefCell<Rule>>, r#type: i64) -> anyhow::Result<()> { let types = Self::types(); if !types.contains_key(&r#type) { return Err(OutOfBoundsException { @@ -57,26 +60,25 @@ impl RuleSet { .into()); } - let hash = rule.get_hash().to_string(); + let hash = rule.borrow().get_hash()?.to_string(); + // Skip exact duplicates that share the same hash. if let Some(potential_duplicates) = self.rules_by_hash.get(&hash) { for potential_duplicate in potential_duplicates { - if rule.equals(potential_duplicate.as_ref()) { + if rule.borrow().equals(&potential_duplicate.borrow()) { return Ok(()); } } } - // TODO(phase-b): Rule is a PHP class with shared ownership; should be Rc<dyn Rule> - // so the same instance can be inserted in rules, rule_by_id, and rules_by_hash. - // Box<dyn Rule> cannot be cloned; storing placeholders for now. + // The same rule instance is referenced from `rules`, `rule_by_id`, and + // `rules_by_hash` (PHP shares one object across all three). self.rules .entry(r#type) .or_insert_with(Vec::new) - .push(todo!("share rule via Rc")); - rule.set_type(r#type); - self.rule_by_id - .insert(self.next_rule_id, todo!("share rule via Rc")); + .push(rule.clone()); + self.rule_by_id.insert(self.next_rule_id, rule.clone()); + rule.borrow_mut().set_type(r#type); self.next_rule_id += 1; @@ -92,34 +94,37 @@ impl RuleSet { self.next_rule_id } - pub fn rule_by_id(&self, id: i64) -> &dyn Rule { - &*self.rule_by_id[&id] - } - - pub fn rule_by_id_mut(&mut self, id: i64) -> &mut dyn Rule { - self.rule_by_id.get_mut(&id).unwrap().as_mut() + pub fn rule_by_id(&self, id: i64) -> Rc<RefCell<Rule>> { + self.rule_by_id[&id].clone() } - pub fn get_rules(&self) -> &IndexMap<i64, Vec<Box<dyn Rule>>> { + pub fn get_rules(&self) -> &IndexMap<i64, Vec<Rc<RefCell<Rule>>>> { &self.rules } pub fn get_iterator(&self) -> RuleSetIterator { - // TODO(phase-b): same Rule-clone concern as get_iterator_for. - RuleSetIterator::new(IndexMap::new()) + RuleSetIterator::new(self.rules.clone()) } pub fn get_iterator_for(&self, types: Vec<i64>) -> RuleSetIterator { - // TODO(phase-b): Rule is a PHP class with shared ownership; should be Rc<dyn Rule> - // before this can compile. Returning an empty iterator placeholder for now. - let _ = (self, types); - RuleSetIterator::new(IndexMap::new()) + let mut rules: IndexMap<i64, Vec<Rc<RefCell<Rule>>>> = IndexMap::new(); + for r#type in types { + if let Some(rules_for_type) = self.rules.get(&r#type) { + rules.insert(r#type, rules_for_type.clone()); + } + } + RuleSetIterator::new(rules) } pub fn get_iterator_without(&self, types: Vec<i64>) -> RuleSetIterator { - // TODO(phase-b): same as above; Box<dyn Rule> cannot be cloned. - let _ = (self, types); - RuleSetIterator::new(IndexMap::new()) + let mut rules: IndexMap<i64, Vec<Rc<RefCell<Rule>>>> = IndexMap::new(); + for (r#type, rules_for_type) in &self.rules { + if types.contains(r#type) { + continue; + } + rules.insert(*r#type, rules_for_type.clone()); + } + RuleSetIterator::new(rules) } pub fn get_types(&self) -> Vec<i64> { @@ -141,10 +146,10 @@ impl RuleSet { for rule in rules { if repository_set.is_some() && request.is_some() && pool.is_some() { // TODO(phase-b): get_pretty_string needs &mut Pool plus installed_map and learned_pool. - let _ = (repository_set, request, pool, is_verbose, rule); - string.push_str(&rule.to_string()); + let _ = (repository_set, request, pool, is_verbose); + string.push_str(&rule.borrow().to_string()); } else { - string.push_str(&rule.to_string()); + string.push_str(&rule.borrow().to_string()); } string.push('\n'); } diff --git a/crates/shirabe/src/dependency_resolver/rule_set_generator.rs b/crates/shirabe/src/dependency_resolver/rule_set_generator.rs index 2d99d7f..45f1331 100644 --- a/crates/shirabe/src/dependency_resolver/rule_set_generator.rs +++ b/crates/shirabe/src/dependency_resolver/rule_set_generator.rs @@ -1,7 +1,9 @@ //! ref: composer/src/Composer/DependencyResolver/RuleSetGenerator.php use std::any::Any; +use std::cell::RefCell; use std::collections::VecDeque; +use std::rc::Rc; use indexmap::IndexMap; use shirabe_php_shim::PhpMixed; @@ -121,20 +123,20 @@ impl RuleSetGenerator { packages: &[Box<dyn PackageInterface>], reason: i64, reason_data: PhpMixed, - ) -> Box<dyn Rule> { + ) -> Rule { let literals: Vec<i64> = packages.iter().map(|p| -p.get_id()).collect(); if literals.len() == 2 { - // Rule2Literals and MultiConflictRule both implement Rule (Phase B: define Rule type) - Box::new(Rule2Literals::new( + Rule::TwoLiterals(Rule2Literals::new( literals[0], literals[1], PhpMixed::Int(reason), reason_data, - )) as Box<dyn Rule> + )) } else { - Box::new(MultiConflictRule::new(literals, PhpMixed::Int(reason), reason_data).unwrap()) - as Box<dyn Rule> + Rule::MultiConflict( + MultiConflictRule::new(literals, PhpMixed::Int(reason), reason_data).unwrap(), + ) } } @@ -142,9 +144,9 @@ impl RuleSetGenerator { /// /// To be able to directly pass in the result of one of the rule creation /// methods null is allowed which will not insert a rule. - fn add_rule(&mut self, r#type: i64, new_rule: Option<Box<dyn Rule>>) { + fn add_rule(&mut self, r#type: i64, new_rule: Option<Rule>) { if let Some(rule) = new_rule { - self.rules.add(rule, r#type).ok(); + self.rules.add(Rc::new(RefCell::new(rule)), r#type).ok(); } } @@ -184,10 +186,7 @@ impl RuleSetGenerator { rule::RULE_PACKAGE_ALIAS, PhpMixed::Null, // reasonData: $package (BasePackage) ); - self.add_rule( - RuleSet::TYPE_PACKAGE, - rule.map(|r| Box::new(r) as Box<dyn Rule>), - ); + self.add_rule(RuleSet::TYPE_PACKAGE, rule.map(Rule::Generic)); // aliases must be installed with their main package, so create a rule the other way around as well let inverse_rule = self.create_require_rule( @@ -196,10 +195,7 @@ impl RuleSetGenerator { rule::RULE_PACKAGE_INVERSE_ALIAS, PhpMixed::Null, // reasonData: $package->getAliasOf() (BasePackage) ); - self.add_rule( - RuleSet::TYPE_PACKAGE, - inverse_rule.map(|r| Box::new(r) as Box<dyn Rule>), - ); + self.add_rule(RuleSet::TYPE_PACKAGE, inverse_rule.map(Rule::Generic)); // if alias package has no self.version requires, its requirements do not // need to be added as the aliased package processing will take care of it @@ -236,10 +232,7 @@ impl RuleSetGenerator { rule::RULE_PACKAGE_REQUIRES, PhpMixed::Null, // reasonData: $link (Link) ); - self.add_rule( - RuleSet::TYPE_PACKAGE, - rule.map(|r| Box::new(r) as Box<dyn Rule>), - ); + self.add_rule(RuleSet::TYPE_PACKAGE, rule.map(Rule::Generic)); for require in possible_requires { work_queue.push_back(require); @@ -297,10 +290,7 @@ impl RuleSetGenerator { rule::RULE_PACKAGE_CONFLICT, PhpMixed::Null, // reasonData: $link (Link) ); - self.add_rule( - RuleSet::TYPE_PACKAGE, - rule.map(|r| Box::new(r) as Box<dyn Rule>), - ); + self.add_rule(RuleSet::TYPE_PACKAGE, rule.map(Rule::TwoLiterals)); } } } @@ -360,7 +350,7 @@ impl RuleSetGenerator { rule::RULE_FIXED, PhpMixed::Array(reason_data), ); - self.add_rule(RuleSet::TYPE_REQUEST, Some(Box::new(rule) as Box<dyn Rule>)); + self.add_rule(RuleSet::TYPE_REQUEST, Some(Rule::Generic(rule))); } for (package_name, constraint) in request.get_requires() { @@ -406,7 +396,7 @@ impl RuleSetGenerator { rule::RULE_ROOT_REQUIRE, PhpMixed::Array(reason_data), ); - self.add_rule(RuleSet::TYPE_REQUEST, Some(Box::new(rule) as Box<dyn Rule>)); + self.add_rule(RuleSet::TYPE_REQUEST, Some(Rule::Generic(rule))); } } diff --git a/crates/shirabe/src/dependency_resolver/rule_set_iterator.rs b/crates/shirabe/src/dependency_resolver/rule_set_iterator.rs index 73c406f..c1b8657 100644 --- a/crates/shirabe/src/dependency_resolver/rule_set_iterator.rs +++ b/crates/shirabe/src/dependency_resolver/rule_set_iterator.rs @@ -1,12 +1,15 @@ //! ref: composer/src/Composer/DependencyResolver/RuleSetIterator.php +use std::cell::RefCell; +use std::rc::Rc; + use crate::dependency_resolver::Rule; use indexmap::IndexMap; /// Implements PHP \Iterator over a grouped rule set. #[derive(Debug)] pub struct RuleSetIterator { - pub(crate) rules: IndexMap<i64, Vec<Box<dyn Rule>>>, + pub(crate) rules: IndexMap<i64, Vec<Rc<RefCell<Rule>>>>, pub(crate) types: Vec<i64>, pub(crate) current_offset: i64, pub(crate) current_type: i64, @@ -14,7 +17,7 @@ pub struct RuleSetIterator { } impl RuleSetIterator { - pub fn new(rules: IndexMap<i64, Vec<Box<dyn Rule>>>) -> Self { + pub fn new(rules: IndexMap<i64, Vec<Rc<RefCell<Rule>>>>) -> Self { let mut types: Vec<i64> = rules.keys().copied().collect(); types.sort(); let mut iter = Self { @@ -28,8 +31,8 @@ impl RuleSetIterator { iter } - pub fn current(&self) -> &dyn Rule { - &*self.rules[&self.current_type][self.current_offset as usize] + pub fn current(&self) -> Rc<RefCell<Rule>> { + self.rules[&self.current_type][self.current_offset as usize].clone() } pub fn key(&self) -> i64 { diff --git a/crates/shirabe/src/dependency_resolver/rule_watch_graph.rs b/crates/shirabe/src/dependency_resolver/rule_watch_graph.rs index 6c41e62..f5f66df 100644 --- a/crates/shirabe/src/dependency_resolver/rule_watch_graph.rs +++ b/crates/shirabe/src/dependency_resolver/rule_watch_graph.rs @@ -1,11 +1,11 @@ //! ref: composer/src/Composer/DependencyResolver/RuleWatchGraph.php -use std::any::Any; +use std::cell::RefCell; +use std::rc::Rc; use indexmap::IndexMap; use crate::dependency_resolver::Decisions; -use crate::dependency_resolver::MultiConflictRule; use crate::dependency_resolver::Rule; use crate::dependency_resolver::RuleWatchChain; use crate::dependency_resolver::RuleWatchNode; @@ -22,17 +22,12 @@ impl RuleWatchGraph { } } - pub fn insert(&mut self, node: std::rc::Rc<std::cell::RefCell<RuleWatchNode>>) { - if node.borrow().get_rule().is_assertion() { + pub fn insert(&mut self, node: Rc<RefCell<RuleWatchNode>>) { + if node.borrow().get_rule().borrow().is_assertion() { return; } - let is_multi_conflict = node - .borrow() - .get_rule() - .as_any() - .downcast_ref::<MultiConflictRule>() - .is_some(); + let is_multi_conflict = node.borrow().get_rule().borrow().is_multi_conflict_rule(); if !is_multi_conflict { let watch1 = node.borrow().watch1; @@ -47,7 +42,7 @@ impl RuleWatchGraph { .unshift(node.clone()); } } else { - let literals: Vec<i64> = node.borrow().get_rule().get_literals().clone(); + let literals: Vec<i64> = node.borrow().get_rule().borrow().get_literals(); for literal in literals { if !self.watch_chains.contains_key(&literal) { self.watch_chains.insert(literal, RuleWatchChain::new()); @@ -65,7 +60,7 @@ impl RuleWatchGraph { decided_literal: i64, level: i64, decisions: &mut Decisions, - ) -> Option<Box<dyn Rule>> { + ) -> Option<Rc<RefCell<Rule>>> { let literal = -decided_literal; if !self.watch_chains.contains_key(&literal) { @@ -75,17 +70,14 @@ impl RuleWatchGraph { self.watch_chains.get_mut(&literal).unwrap().rewind(); while self.watch_chains.get(&literal).unwrap().valid() { let node = self.watch_chains.get(&literal).unwrap().current().clone(); - let is_multi_conflict = node - .borrow() - .get_rule() - .as_any() - .downcast_ref::<MultiConflictRule>() - .is_some(); + let is_multi_conflict = node.borrow().get_rule().borrow().is_multi_conflict_rule(); if !is_multi_conflict { let other_watch = node.borrow().get_other_watch(literal); - if !node.borrow().get_rule().is_disabled() && !decisions.satisfy(other_watch) { - let rule_literals: Vec<i64> = node.borrow().get_rule().get_literals().clone(); + if !node.borrow().get_rule().borrow().is_disabled() + && !decisions.satisfy(other_watch) + { + let rule_literals: Vec<i64> = node.borrow().get_rule().borrow().get_literals(); let alternative_literals: Vec<i64> = rule_literals .into_iter() @@ -103,20 +95,20 @@ impl RuleWatchGraph { } if decisions.conflict(other_watch) { - return Some(node.borrow().get_rule_boxed()); + return Some(node.borrow().get_rule()); } - decisions.decide(other_watch, level, node.borrow().get_rule_boxed()); + decisions.decide(other_watch, level, node.borrow().get_rule()); } } else { - let literals: Vec<i64> = node.borrow().get_rule().get_literals().clone(); + let literals: Vec<i64> = node.borrow().get_rule().borrow().get_literals(); for other_literal in literals { if literal != other_literal && !decisions.satisfy(other_literal) { if decisions.conflict(other_literal) { - return Some(node.borrow().get_rule_boxed()); + return Some(node.borrow().get_rule()); } - decisions.decide(other_literal, level, node.borrow().get_rule_boxed()); + decisions.decide(other_literal, level, node.borrow().get_rule()); } } } @@ -131,7 +123,7 @@ impl RuleWatchGraph { &mut self, from_literal: i64, to_literal: i64, - node: std::rc::Rc<std::cell::RefCell<RuleWatchNode>>, + node: Rc<RefCell<RuleWatchNode>>, ) { if !self.watch_chains.contains_key(&to_literal) { self.watch_chains.insert(to_literal, RuleWatchChain::new()); diff --git a/crates/shirabe/src/dependency_resolver/rule_watch_node.rs b/crates/shirabe/src/dependency_resolver/rule_watch_node.rs index a643d3e..7292ae0 100644 --- a/crates/shirabe/src/dependency_resolver/rule_watch_node.rs +++ b/crates/shirabe/src/dependency_resolver/rule_watch_node.rs @@ -1,12 +1,15 @@ //! ref: composer/src/Composer/DependencyResolver/RuleWatchNode.php +use std::cell::RefCell; +use std::rc::Rc; + use crate::dependency_resolver::Decisions; -use crate::dependency_resolver::RuleLiterals; +use crate::dependency_resolver::Rule; pub struct RuleWatchNode { pub watch1: i64, pub watch2: i64, - pub(crate) rule: Box<dyn RuleLiterals>, + pub(crate) rule: Rc<RefCell<Rule>>, } impl std::fmt::Debug for RuleWatchNode { @@ -19,8 +22,8 @@ impl std::fmt::Debug for RuleWatchNode { } impl RuleWatchNode { - pub fn new(rule: Box<dyn RuleLiterals>) -> Self { - let literals = rule.get_literals(); + pub fn new(rule: Rc<RefCell<Rule>>) -> Self { + let literals = rule.borrow().get_literals(); let literal_count = literals.len(); let watch1 = if literal_count > 0 { literals[0] } else { 0 }; let watch2 = if literal_count > 1 { literals[1] } else { 0 }; @@ -33,14 +36,13 @@ impl RuleWatchNode { } pub fn watch2_on_highest(&mut self, decisions: &Decisions) { - let literals = self.rule.get_literals(); + let literals = self.rule.borrow().get_literals(); // if there are only 2 elements, both are being watched anyway - if literals.len() < 3 || self.rule.is_multi_conflict_rule() { + if literals.len() < 3 || self.rule.borrow().is_multi_conflict_rule() { return; } - let literals: Vec<i64> = literals.clone(); let mut watch_level: i64 = 0; for literal in &literals { @@ -53,14 +55,8 @@ impl RuleWatchNode { } } - pub fn get_rule(&self) -> &dyn RuleLiterals { - self.rule.as_ref() - } - - /// Owned clone for callers that need a `Box<dyn Rule>`. Default impl in - /// `RuleLiterals` returns `todo!()`; concrete rule impls override it. - pub fn get_rule_boxed(&self) -> Box<dyn crate::dependency_resolver::Rule> { - self.rule.clone_rule_box() + pub fn get_rule(&self) -> Rc<RefCell<Rule>> { + self.rule.clone() } pub fn get_other_watch(&self, literal: i64) -> i64 { diff --git a/crates/shirabe/src/dependency_resolver/solver.rs b/crates/shirabe/src/dependency_resolver/solver.rs index fd30165..e4539d2 100644 --- a/crates/shirabe/src/dependency_resolver/solver.rs +++ b/crates/shirabe/src/dependency_resolver/solver.rs @@ -1,5 +1,8 @@ //! ref: composer/src/Composer/DependencyResolver/Solver.php +use std::cell::RefCell; +use std::rc::Rc; + use indexmap::IndexMap; use shirabe_php_shim::{ @@ -43,7 +46,7 @@ pub struct Solver { /// Pairs of `(literals, level)` — PHP indexes into these with the BRANCH_* constants. pub(crate) branches: Vec<(Vec<i64>, i64)>, pub(crate) problems: Vec<Problem>, - pub(crate) learned_pool: Vec<Vec<Box<dyn Rule>>>, + pub(crate) learned_pool: Vec<Vec<Rc<RefCell<Rule>>>>, pub(crate) learned_why: IndexMap<String, i64>, pub test_flag_learned_positive_literal: bool, @@ -94,18 +97,18 @@ impl Solver { let rules_count = self.rules.count(); let mut rule_index = 0_i64; while rule_index < rules_count { - let rule = self.rules.rule_by_id(rule_index).clone_box(); + let rule = self.rules.rule_by_id(rule_index); - if !rule.is_assertion() || rule.is_disabled() { + if !rule.borrow().is_assertion() || rule.borrow().is_disabled() { rule_index += 1; continue; } - let literals = rule.get_literals(); + let literals = rule.borrow().get_literals(); let literal = literals[0]; if !self.decisions.decided(literal) { - self.decisions.decide(literal, 1, rule.clone_box()); + self.decisions.decide(literal, 1, rule.clone()); rule_index += 1; continue; } @@ -116,24 +119,20 @@ impl Solver { } // found a conflict - if RuleSet::TYPE_LEARNED == rule.get_type() { - let rule_mut = self.rules.rule_by_id_mut(rule_index); - // TODO(phase-b): PHP `disable()` may throw for MultiConflictRule. - // The Rule trait method returns `()`; the special case isn't surfaced. - rule_mut.disable(); + if RuleSet::TYPE_LEARNED == rule.borrow().get_type() { + rule.borrow_mut().disable()?; rule_index += 1; continue; } - let conflict = self.decisions.decision_rule(literal).clone_box(); + let conflict = self.decisions.decision_rule(literal); - if RuleSet::TYPE_PACKAGE == conflict.get_type() { + if RuleSet::TYPE_PACKAGE == conflict.borrow().get_type() { let mut problem = Problem::new(); - problem.add_rule(rule.clone_box()); + problem.add_rule(rule.clone()); problem.add_rule(conflict); - // TODO(phase-b): PHP `disable()` may throw for MultiConflictRule. - self.rules.rule_by_id_mut(rule_index).disable(); + rule.borrow_mut().disable()?; self.problems.push(problem); rule_index += 1; continue; @@ -141,33 +140,29 @@ impl Solver { // conflict with another root require/fixed package let mut problem = Problem::new(); - problem.add_rule(rule.clone_box()); + problem.add_rule(rule.clone()); problem.add_rule(conflict); // push all of our rules (can only be root require/fixed package rules) // asserting this literal on the problem stack - // TODO(phase-b): RuleSetIterator does not expose an `ids()` method matching - // PHP's `array_keys($iterator->rules())`. Returning an empty Vec until the - // iterator surfaces the underlying rule ids. - let request_rules: Vec<i64> = { - let _iter = self.rules.get_iterator_for(vec![RuleSet::TYPE_REQUEST]); - Vec::new() - }; - for assert_rule_id in request_rules { - let assert_rule = self.rules.rule_by_id(assert_rule_id).clone_box(); - if assert_rule.is_disabled() || !assert_rule.is_assertion() { + let mut assert_iterator = self.rules.get_iterator_for(vec![RuleSet::TYPE_REQUEST]); + while assert_iterator.valid() { + let assert_rule = assert_iterator.current(); + if assert_rule.borrow().is_disabled() || !assert_rule.borrow().is_assertion() { + assert_iterator.next(); continue; } - let assert_rule_literals = assert_rule.get_literals(); + let assert_rule_literals = assert_rule.borrow().get_literals(); let assert_rule_literal = assert_rule_literals[0]; if literal.abs() != assert_rule_literal.abs() { + assert_iterator.next(); continue; } - problem.add_rule(assert_rule); - // TODO(phase-b): PHP `disable()` may throw for MultiConflictRule. - self.rules.rule_by_id_mut(assert_rule_id).disable(); + problem.add_rule(assert_rule.clone()); + assert_rule.borrow_mut().disable()?; + assert_iterator.next(); } self.problems.push(problem); @@ -227,7 +222,7 @@ impl Solver { // TODO(phase-b): store the constraint inside reason_data; PhpMixed needs to // accept a `dyn ConstraintInterface` wrapper. reason_data.insert("constraint".to_string(), PhpMixed::Null); - problem.add_rule(Box::new(GenericRule::new( + problem.add_rule(Rc::new(RefCell::new(Rule::Generic(GenericRule::new( Vec::new(), PhpMixed::Int(rule::RULE_ROOT_REQUIRE), PhpMixed::Array( @@ -236,7 +231,7 @@ impl Solver { .map(|(k, v)| (k, Box::new(v))) .collect(), ), - )) as Box<dyn Rule>); + ))))); self.problems.push(problem); } } @@ -265,10 +260,13 @@ impl Solver { self.decisions = Decisions::new(self.pool.clone()); self.watch_graph = RuleWatchGraph::new(); - // TODO(phase-b): RuleSet does not expose `iter()`; RuleWatchNode expects - // Box<dyn RuleLiterals>. Skipping watch-graph seeding until rule storage is - // refactored to share rules between RuleSet and RuleWatchGraph. - let _ = &mut self.watch_graph; + let mut iterator = self.rules.get_iterator(); + while iterator.valid() { + let rule = iterator.current(); + self.watch_graph + .insert(Rc::new(RefCell::new(RuleWatchNode::new(rule)))); + iterator.next(); + } // make decisions based on root require/fix assertions self.make_assertion_rule_decisions()?; @@ -288,10 +286,10 @@ impl Solver { ); if self.problems.len() > 0 { - // TODO(phase-b): SolverProblemsException stores `Box<dyn Rule>` which is not + // TODO(phase-b): SolverProblemsException stores `Rc<RefCell<Rule>>` which is not // `Send + Sync`, so it cannot satisfy `anyhow::Error`'s bounds. Returning a - // placeholder error preserves control flow until Rule gains thread-safety - // requirements or the exception type is reworked. + // placeholder error preserves control flow until the `Send + Sync` requirement + // is removed (single-threaded `Rc` model) or the exception type is reworked. let _ = SolverProblemsException::new( std::mem::take(&mut self.problems), std::mem::take(&mut self.learned_pool), @@ -316,7 +314,7 @@ impl Solver { /// If we find unit rules we make new decisions based on them /// /// Returns a `Rule` on conflict, otherwise `None`. - fn propagate(&mut self, level: i64) -> Option<Box<dyn Rule>> { + fn propagate(&mut self, level: i64) -> Option<Rc<RefCell<Rule>>> { while self.decisions.valid_offset(self.propagate_index) { let decision = self .decisions @@ -377,7 +375,7 @@ impl Solver { &mut self, level: i64, literal: i64, - rule: Box<dyn Rule>, + rule: Rc<RefCell<Rule>>, ) -> anyhow::Result<i64> { let mut level = level + 1; @@ -392,7 +390,7 @@ impl Solver { }; if level == 1 { - self.analyze_unsolvable(rule.as_ref()); + self.analyze_unsolvable(rule); return Ok(0); } @@ -411,14 +409,19 @@ impl Solver { self.revert(level); - // TODO(phase-b): GenericRule is a PHP class — Composer shares the same - // instance between RuleSet, RuleWatchGraph, and Decisions. Without shared - // ownership we can't add the rule once and reference it later; the watch - // graph and decisions hand-off are stubbed. - let _ = new_rule; - let _ = learn_literal; - let _ = why; - todo!("share learned GenericRule across RuleSet, RuleWatchGraph, and Decisions"); + // The same learned rule instance is shared between RuleSet, + // RuleWatchGraph, and Decisions (PHP shares one object). + let new_rule = Rc::new(RefCell::new(Rule::Generic(new_rule))); + self.rules.add(new_rule.clone(), RuleSet::TYPE_LEARNED)?; + + self.learned_why + .insert(spl_object_hash(&*new_rule.borrow()), why); + + let rule_node = Rc::new(RefCell::new(RuleWatchNode::new(new_rule.clone()))); + rule_node.borrow_mut().watch2_on_highest(&self.decisions); + self.watch_graph.insert(rule_node); + + self.decisions.decide(learn_literal, level, new_rule); } Ok(level) @@ -428,13 +431,13 @@ impl Solver { &mut self, level: i64, decision_queue: Vec<i64>, - rule: Box<dyn Rule>, + rule: Rc<RefCell<Rule>>, ) -> anyhow::Result<i64> { // choose best package to install from decisionQueue let mut literals = self.policy.select_preferred_packages( &*self.pool.borrow(), decision_queue, - rule.get_required_package(), + rule.borrow().get_required_package(), ); let selected_literal = array_shift::<i64>(&mut literals) @@ -451,9 +454,9 @@ impl Solver { fn analyze( &mut self, level: i64, - rule: Box<dyn Rule>, + rule: Rc<RefCell<Rule>>, ) -> anyhow::Result<(i64, i64, GenericRule, i64)> { - let analyzed_rule = rule.clone_box(); + let analyzed_rule = rule.clone(); let mut rule = rule; let mut rule_level = 1_i64; let mut num = 0_i64; @@ -468,11 +471,11 @@ impl Solver { 'outer: loop { let last = self.learned_pool.len() - 1; - self.learned_pool[last].push(rule.clone_box()); + self.learned_pool[last].push(rule.clone()); - for literal in rule.get_literals().clone() { + for literal in rule.borrow().get_literals() { // multiconflictrule is really a bunch of rules in one, so some may not have finished propagating yet - if rule.as_multi_conflict().is_some() && !self.decisions.decided(literal) { + if rule.borrow().as_multi_conflict().is_some() && !self.decisions.decided(literal) { continue; } @@ -521,8 +524,8 @@ impl Solver { return Err(anyhow::anyhow!(SolverBugException::new(format!( "Reached invalid decision id {} while looking through {} for a literal present in the analyzed rule {}.", decision_id, - rule.to_string(), - analyzed_rule.to_string() + rule.borrow().to_string(), + analyzed_rule.borrow().to_string() )))); } @@ -557,16 +560,16 @@ impl Solver { l1num += 1; l1retry = true; } else { - rule = self.decisions.at_offset(decision_id as usize).1.clone_box(); + rule = self.decisions.at_offset(decision_id as usize).1.clone(); - if rule.as_multi_conflict().is_some() { + if rule.borrow().as_multi_conflict().is_some() { // there is only ever exactly one positive decision in a MultiConflictRule - for rule_literal in rule.get_literals().clone() { + for rule_literal in rule.borrow().get_literals() { if !seen.contains_key(&rule_literal.abs()) && self.decisions.satisfy(-rule_literal) { let last = self.learned_pool.len() - 1; - self.learned_pool[last].push(rule.clone_box()); + self.learned_pool[last].push(rule.clone()); let l = self.decisions.decision_level(rule_literal); if 1 == l { l1num += 1; @@ -592,7 +595,7 @@ impl Solver { } let _ = literal_for_outer; - rule = self.decisions.at_offset(decision_id as usize).1.clone_box(); + rule = self.decisions.at_offset(decision_id as usize).1.clone(); } let why = (self.learned_pool.len() as i64) - 1; @@ -602,7 +605,7 @@ impl Solver { None => { return Err(anyhow::anyhow!(SolverBugException::new(format!( "Did not find a learnable literal in analyzed rule {}.", - analyzed_rule.to_string() + analyzed_rule.borrow().to_string() )))); } }; @@ -620,44 +623,44 @@ impl Solver { fn analyze_unsolvable_rule( &self, problem: &mut Problem, - conflict_rule: &dyn Rule, + conflict_rule: Rc<RefCell<Rule>>, rule_seen: &mut IndexMap<String, bool>, ) { - let why = spl_object_hash(conflict_rule); + let why = spl_object_hash(&*conflict_rule.borrow()); rule_seen.insert(why.clone(), true); - if conflict_rule.get_type() == RuleSet::TYPE_LEARNED { + if conflict_rule.borrow().get_type() == RuleSet::TYPE_LEARNED { let learned_why = self.learned_why[&why]; - let problem_rules = &self.learned_pool[learned_why as usize]; + let problem_rules = self.learned_pool[learned_why as usize].clone(); for problem_rule in problem_rules { - if !rule_seen.contains_key(&spl_object_hash(problem_rule)) { - self.analyze_unsolvable_rule(problem, problem_rule.as_ref(), rule_seen); + if !rule_seen.contains_key(&spl_object_hash(&*problem_rule.borrow())) { + self.analyze_unsolvable_rule(problem, problem_rule, rule_seen); } } return; } - if conflict_rule.get_type() == RuleSet::TYPE_PACKAGE { + if conflict_rule.borrow().get_type() == RuleSet::TYPE_PACKAGE { // package rules cannot be part of a problem return; } problem.next_section(); - problem.add_rule(conflict_rule.clone_box()); + problem.add_rule(conflict_rule); } - fn analyze_unsolvable(&mut self, conflict_rule: &dyn Rule) { + fn analyze_unsolvable(&mut self, conflict_rule: Rc<RefCell<Rule>>) { let mut problem = Problem::new(); - problem.add_rule(conflict_rule.clone_box()); + problem.add_rule(conflict_rule.clone()); let mut rule_seen: IndexMap<String, bool> = IndexMap::new(); - self.analyze_unsolvable_rule(&mut problem, conflict_rule, &mut rule_seen); + self.analyze_unsolvable_rule(&mut problem, conflict_rule.clone(), &mut rule_seen); let mut seen: IndexMap<i64, bool> = IndexMap::new(); - let literals = conflict_rule.get_literals().clone(); + let literals = conflict_rule.borrow().get_literals(); for literal in &literals { // skip the one true literal @@ -681,12 +684,12 @@ impl Solver { continue; } - let why = self.decisions.at_offset(offset - 1).1.clone_box(); + let why = self.decisions.at_offset(offset - 1).1.clone(); - problem.add_rule(why.clone_box()); - self.analyze_unsolvable_rule(&mut problem, why.as_ref(), &mut rule_seen); + problem.add_rule(why.clone()); + self.analyze_unsolvable_rule(&mut problem, why.clone(), &mut rule_seen); - let literals = why.get_literals().clone(); + let literals = why.borrow().get_literals(); for literal in &literals { // skip the one true literal if self.decisions.satisfy(*literal) { @@ -717,7 +720,7 @@ impl Solver { if 1 == level { let conflict_rule = self.propagate(level); if let Some(cr) = conflict_rule { - self.analyze_unsolvable(cr.as_ref()); + self.analyze_unsolvable(cr); return Ok(()); } @@ -728,12 +731,12 @@ impl Solver { let mut iterator = self.rules.get_iterator_for(vec![RuleSet::TYPE_REQUEST]); let mut broke_inner = false; while iterator.valid() { - let rule = iterator.current().clone_box(); - if rule.is_enabled() { + let rule = iterator.current(); + if rule.borrow().is_enabled() { let mut decision_queue: Vec<i64> = Vec::new(); let mut none_satisfied = true; - for literal in rule.get_literals().clone() { + for literal in rule.borrow().get_literals() { if self.decisions.satisfy(literal) { none_satisfied = false; break; @@ -820,10 +823,10 @@ impl Solver { pass += 1; } - let rule = self.rules.rule_by_id(i).clone_box(); - let literals = rule.get_literals().clone(); + let rule = self.rules.rule_by_id(i); + let literals = rule.borrow().get_literals(); - if rule.is_disabled() { + if rule.borrow().is_disabled() { i += 1; n += 1; continue; @@ -918,7 +921,7 @@ impl Solver { level = last_level_v; self.revert(level); - let why = self.decisions.last_reason().clone_box(); + let why = self.decisions.last_reason(); level = self.set_propagate_learn(level, last_literal_v, why)?; diff --git a/crates/shirabe/src/dependency_resolver/solver_problems_exception.rs b/crates/shirabe/src/dependency_resolver/solver_problems_exception.rs index 8c9e488..17ebaf2 100644 --- a/crates/shirabe/src/dependency_resolver/solver_problems_exception.rs +++ b/crates/shirabe/src/dependency_resolver/solver_problems_exception.rs @@ -1,5 +1,8 @@ //! ref: composer/src/Composer/DependencyResolver/SolverProblemsException.php +use std::cell::RefCell; +use std::rc::Rc; + use shirabe_php_shim::RuntimeException; use crate::dependency_resolver::Pool; @@ -13,7 +16,7 @@ use crate::util::IniHelper; pub struct SolverProblemsException { inner: RuntimeException, pub(crate) problems: Vec<Problem>, - pub(crate) learned_pool: Vec<Vec<Box<dyn Rule>>>, + pub(crate) learned_pool: Vec<Vec<Rc<RefCell<Rule>>>>, } impl SolverProblemsException { @@ -27,7 +30,7 @@ impl SolverProblemsException { &self.inner.message } - pub fn new(problems: Vec<Problem>, learned_pool: Vec<Vec<Box<dyn Rule>>>) -> Self { + pub fn new(problems: Vec<Problem>, learned_pool: Vec<Vec<Rc<RefCell<Rule>>>>) -> Self { let message = format!( "Failed resolving dependencies with {} problems, call getPrettyString to get formatted details", problems.len() @@ -70,10 +73,10 @@ impl SolverProblemsException { .unwrap_or_default() )); // TODO(phase-b): get_reasons returns an IndexMap; flatten its values into Vec<Vec<...>>. - let reasons_vec: Vec<Vec<Box<dyn crate::dependency_resolver::Rule>>> = problem + let reasons_vec: Vec<Vec<Rc<RefCell<Rule>>>> = problem .get_reasons() .values() - .map(|v| v.iter().map(|r| r.clone_box()).collect()) + .map(|v| v.iter().map(|r| r.clone()).collect()) .collect(); missing_extensions.extend(self.get_extension_problems(reasons_vec)); is_caused_by_lock = @@ -159,11 +162,11 @@ impl SolverProblemsException { text } - fn get_extension_problems(&self, reason_sets: Vec<Vec<Box<dyn Rule>>>) -> Vec<String> { + fn get_extension_problems(&self, reason_sets: Vec<Vec<Rc<RefCell<Rule>>>>) -> Vec<String> { let mut missing_extensions: indexmap::IndexMap<String, i64> = indexmap::IndexMap::new(); for reason_set in reason_sets { for rule in reason_set { - let required = rule.get_required_package(); + let required = rule.borrow().get_required_package(); if let Some(req) = required { if req.starts_with("ext-") { missing_extensions.insert(req.to_string(), 1); |