Skip to content

Commit 29c82e4

Browse files
azoyanazoyan
committed
Fix PR drawbacks: use Arc instead Rc; add static_assert that MainEngine implement Synd+Sync traits
1 parent 106aff9 commit 29c82e4

File tree

6 files changed

+36
-51
lines changed

6 files changed

+36
-51
lines changed

‎crates/rsonpath-lib/Cargo.toml

-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ test-case = { workspace = true }
4747
default = ["simd"]
4848
arbitrary = ["dep:arbitrary"]
4949
simd = []
50-
multithread = []
5150

5251
[[example]]
5352
name = "approx_spans_usage"

‎crates/rsonpath-lib/src/automaton.rs

+2-7
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,7 @@ use crate::{automaton::error::CompilerError, debug, string_pattern::StringPatter
1212
use nfa::NondeterministicAutomaton;
1313
use rsonpath_syntax::{num::JsonUInt, JsonPathQuery};
1414
use smallvec::SmallVec;
15-
use std::{fmt::Display, ops::Index};
16-
17-
#[cfg(not(feature = "multithread"))]
18-
use std::rc::Rc;
19-
#[cfg(feature = "multithread")]
20-
use std::sync::Arc as Rc;
15+
use std::{fmt::Display, ops::Index, sync::Arc};
2116

2217
/// A minimal, deterministic automaton representing a JSONPath query.
2318
#[derive(Clone, Debug, PartialEq, Eq)]
@@ -26,7 +21,7 @@ pub struct Automaton {
2621
}
2722

2823
/// Transition when a JSON member name matches a [`StringPattern`].
29-
pub type MemberTransition = (Rc<StringPattern>, State);
24+
pub type MemberTransition = (Arc<StringPattern>, State);
3025

3126
/// Transition on elements of an array with indices specified by either a single index
3227
/// or a simple slice expression.

‎crates/rsonpath-lib/src/automaton/minimizer.rs

+22-25
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
//! Determinization and minimization of an NFA into the final DFA used by the engines.
22
3-
#[cfg(not(feature = "multithread"))]
4-
use std::rc::Rc;
5-
#[cfg(feature = "multithread")]
6-
use std::sync::Arc as Rc;
3+
use std::sync::Arc;
74

85
// NOTE: Some comments in this module are outdated, because the minimizer doesn't
96
// actually produce minimal automata as of now - see #91.
@@ -53,7 +50,7 @@ pub(super) struct Minimizer {
5350
#[derive(Debug)]
5451
struct SuperstateTransitionTable {
5552
array: ArrayTransitionSet,
56-
member: VecMap<Rc<StringPattern>, SmallSet256>,
53+
member: VecMap<Arc<StringPattern>, SmallSet256>,
5754
wildcard: SmallSet256,
5855
}
5956

@@ -180,7 +177,7 @@ impl Minimizer {
180177
&self,
181178
id: DfaStateId,
182179
array_transitions: &[ArrayTransition],
183-
member_transitions: &[(Rc<StringPattern>, DfaStateId)],
180+
member_transitions: &[(Arc<StringPattern>, DfaStateId)],
184181
fallback: DfaStateId,
185182
) -> StateAttributes {
186183
let mut attrs = StateAttributesBuilder::new();
@@ -557,8 +554,8 @@ mod tests {
557554
#[test]
558555
fn interstitial_descendant_wildcard() {
559556
// Query = $..a.b..*.a..b
560-
let label_a = Rc::new(StringPattern::new(&JsonString::new("a")));
561-
let label_b = Rc::new(StringPattern::new(&JsonString::new("b")));
557+
let label_a = Arc::new(StringPattern::new(&JsonString::new("a")));
558+
let label_b = Arc::new(StringPattern::new(&JsonString::new("b")));
562559

563560
let nfa = NondeterministicAutomaton {
564561
ordered_states: vec![
@@ -626,8 +623,8 @@ mod tests {
626623
#[test]
627624
fn interstitial_nondescendant_wildcard() {
628625
// Query = $..a.b.*.a..b
629-
let label_a = Rc::new(StringPattern::new(&JsonString::new("a")));
630-
let label_b = Rc::new(StringPattern::new(&JsonString::new("b")));
626+
let label_a = Arc::new(StringPattern::new(&JsonString::new("a")));
627+
let label_b = Arc::new(StringPattern::new(&JsonString::new("b")));
631628

632629
let nfa = NondeterministicAutomaton {
633630
ordered_states: vec![
@@ -701,7 +698,7 @@ mod tests {
701698
#[test]
702699
fn simple_multi_accepting() {
703700
// Query = $..a.*
704-
let label = Rc::new(StringPattern::new(&JsonString::new("a")));
701+
let label = Arc::new(StringPattern::new(&JsonString::new("a")));
705702

706703
let nfa = NondeterministicAutomaton {
707704
ordered_states: vec![
@@ -799,7 +796,7 @@ mod tests {
799796
#[test]
800797
fn chained_wildcard_children() {
801798
// Query = $.a.*.*.*
802-
let label = Rc::new(StringPattern::new(&JsonString::new("a")));
799+
let label = Arc::new(StringPattern::new(&JsonString::new("a")));
803800

804801
let nfa = NondeterministicAutomaton {
805802
ordered_states: vec![
@@ -860,7 +857,7 @@ mod tests {
860857
#[test]
861858
fn chained_wildcard_children_after_descendant() {
862859
// Query = $..a.*.*
863-
let label = Rc::new(StringPattern::new(&JsonString::new("a")));
860+
let label = Arc::new(StringPattern::new(&JsonString::new("a")));
864861

865862
let nfa = NondeterministicAutomaton {
866863
ordered_states: vec![
@@ -938,11 +935,11 @@ mod tests {
938935
#[test]
939936
fn child_and_descendant() {
940937
// Query = $.x..a.b.a.b.c..d
941-
let label_a = Rc::new(StringPattern::new(&JsonString::new("a")));
942-
let label_b = Rc::new(StringPattern::new(&JsonString::new("b")));
943-
let label_c = Rc::new(StringPattern::new(&JsonString::new("c")));
944-
let label_d = Rc::new(StringPattern::new(&JsonString::new("d")));
945-
let label_x = Rc::new(StringPattern::new(&JsonString::new("x")));
938+
let label_a = Arc::new(StringPattern::new(&JsonString::new("a")));
939+
let label_b = Arc::new(StringPattern::new(&JsonString::new("b")));
940+
let label_c = Arc::new(StringPattern::new(&JsonString::new("c")));
941+
let label_d = Arc::new(StringPattern::new(&JsonString::new("d")));
942+
let label_x = Arc::new(StringPattern::new(&JsonString::new("x")));
946943

947944
let nfa = NondeterministicAutomaton {
948945
ordered_states: vec![
@@ -1024,9 +1021,9 @@ mod tests {
10241021
#[test]
10251022
fn child_descendant_and_child_wildcard() {
10261023
// Query = $.x.*..a.*.b
1027-
let label_a = Rc::new(StringPattern::new(&JsonString::new("a")));
1028-
let label_b = Rc::new(StringPattern::new(&JsonString::new("b")));
1029-
let label_x = Rc::new(StringPattern::new(&JsonString::new("x")));
1024+
let label_a = Arc::new(StringPattern::new(&JsonString::new("a")));
1025+
let label_b = Arc::new(StringPattern::new(&JsonString::new("b")));
1026+
let label_x = Arc::new(StringPattern::new(&JsonString::new("x")));
10301027

10311028
let nfa = NondeterministicAutomaton {
10321029
ordered_states: vec![
@@ -1106,10 +1103,10 @@ mod tests {
11061103
#[test]
11071104
fn all_name_and_wildcard_selectors() {
11081105
// Query = $.a.b..c..d.*..*
1109-
let label_a = Rc::new(StringPattern::new(&JsonString::new("a")));
1110-
let label_b = Rc::new(StringPattern::new(&JsonString::new("b")));
1111-
let label_c = Rc::new(StringPattern::new(&JsonString::new("c")));
1112-
let label_d = Rc::new(StringPattern::new(&JsonString::new("d")));
1106+
let label_a = Arc::new(StringPattern::new(&JsonString::new("a")));
1107+
let label_b = Arc::new(StringPattern::new(&JsonString::new("b")));
1108+
let label_c = Arc::new(StringPattern::new(&JsonString::new("c")));
1109+
let label_d = Arc::new(StringPattern::new(&JsonString::new("d")));
11131110

11141111
let nfa = NondeterministicAutomaton {
11151112
ordered_states: vec![

‎crates/rsonpath-lib/src/automaton/nfa.rs

+8-13
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,7 @@ use crate::{automaton::SimpleSlice, error::UnsupportedFeatureError, string_patte
55

66
use super::{error::CompilerError, ArrayTransitionLabel};
77
use rsonpath_syntax::{str::JsonString, JsonPathQuery, Step};
8-
use std::{collections::HashMap, fmt::Display, ops::Index};
9-
10-
#[cfg(not(feature = "multithread"))]
11-
use std::rc::Rc;
12-
#[cfg(feature = "multithread")]
13-
use std::sync::Arc as Rc;
8+
use std::{collections::HashMap, fmt::Display, ops::Index, sync::Arc};
149

1510
/// An NFA representing a query. It is always a directed path
1611
/// from an initial state to the unique accepting state at the end,
@@ -39,7 +34,7 @@ pub(super) enum Transition {
3934
/// A transition matching array indices.
4035
Array(ArrayTransitionLabel),
4136
/// A transition matching a specific member.
42-
Member(Rc<StringPattern>),
37+
Member(Arc<StringPattern>),
4338
/// A transition matching anything.
4439
Wildcard,
4540
}
@@ -76,7 +71,7 @@ impl NondeterministicAutomaton {
7671
use rsonpath_syntax::{Index, Selector};
7772
use std::collections::hash_map::Entry;
7873

79-
let mut string_pattern_cache: HashMap<&'q JsonString, Rc<StringPattern>> = HashMap::new();
74+
let mut string_pattern_cache: HashMap<&'q JsonString, Arc<StringPattern>> = HashMap::new();
8075

8176
let states_result: Result<Vec<NfaState>, CompilerError> = query
8277
.segments()
@@ -92,7 +87,7 @@ impl NondeterministicAutomaton {
9287
let pattern = match string_pattern_cache.entry(name) {
9388
Entry::Occupied(pat) => pat.get().clone(),
9489
Entry::Vacant(entry) => {
95-
let pat = Rc::new(StringPattern::new(name));
90+
let pat = Arc::new(StringPattern::new(name));
9691
entry.insert(pat.clone());
9792
pat
9893
}
@@ -247,10 +242,10 @@ mod tests {
247242

248243
let expected_automaton = NondeterministicAutomaton {
249244
ordered_states: vec![
250-
NfaState::Direct(Transition::Member(Rc::new(StringPattern::new(&label_a)))),
251-
NfaState::Direct(Transition::Member(Rc::new(StringPattern::new(&label_b)))),
252-
NfaState::Recursive(Transition::Member(Rc::new(StringPattern::new(&label_c)))),
253-
NfaState::Recursive(Transition::Member(Rc::new(StringPattern::new(&label_d)))),
245+
NfaState::Direct(Transition::Member(Arc::new(StringPattern::new(&label_a)))),
246+
NfaState::Direct(Transition::Member(Arc::new(StringPattern::new(&label_b)))),
247+
NfaState::Recursive(Transition::Member(Arc::new(StringPattern::new(&label_c)))),
248+
NfaState::Recursive(Transition::Member(Arc::new(StringPattern::new(&label_d)))),
254249
NfaState::Direct(Transition::Wildcard),
255250
NfaState::Direct(Transition::Wildcard),
256251
NfaState::Recursive(Transition::Wildcard),

‎crates/rsonpath-lib/src/engine/head_skipping.rs

+2-5
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,7 @@
22
//! the first matching member name in a query starting with a self-looping state.
33
//! This happens in queries starting with a descendant selector.
44
5-
#[cfg(not(feature = "multithread"))]
6-
use std::rc::Rc;
7-
#[cfg(feature = "multithread")]
8-
use std::sync::Arc as Rc;
5+
use std::sync::Arc;
96

107
use crate::{
118
automaton::{Automaton, State},
@@ -71,7 +68,7 @@ pub(super) struct HeadSkip<'b, I, V, const N: usize> {
7168
bytes: &'b I,
7269
state: State,
7370
is_accepting: bool,
74-
member_name: Rc<StringPattern>,
71+
member_name: Arc<StringPattern>,
7572
simd: V,
7673
}
7774

‎crates/rsonpath-lib/src/engine/main.rs

+2
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,8 @@ pub struct MainEngine {
7676
simd: SimdConfiguration,
7777
}
7878

79+
static_assertions::assert_impl_all!(MainEngine: Send, Sync);
80+
7981
impl Compiler for MainEngine {
8082
type E = Self;
8183

0 commit comments

Comments
 (0)