Skip to content
This repository was archived by the owner on Sep 9, 2025. It is now read-only.

Commit 5f440a7

Browse files
author
Hendrik van Antwerpen
committed
Replace should_extend predicate with preconditions without stack variables in ForwardPartialPathStitcher
Forward partial path stitching allowed a `should_extend` predicate to control which paths were considered for extension. This was used to cut off paths where the precondition was strengthened during extension. However, it is easy for a user to forget to set it corectly, possibly resulting in non-termination. This patch removed that predicate, and instead ensures that all initial paths have no stack variables in the precondition. This has the same effect as what the predicate was used for, and cannot be forgotten by the user.
1 parent 50fabc3 commit 5f440a7

File tree

3 files changed

+45
-60
lines changed

3 files changed

+45
-60
lines changed

‎stack-graphs/src/stitching.rs‎

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -769,7 +769,6 @@ pub struct ForwardPartialPathStitcher {
769769
max_work_per_phase: usize,
770770
#[cfg(feature = "copious-debugging")]
771771
phase_number: usize,
772-
should_extend: fn(&StackGraph, &mut PartialPaths, &PartialPath) -> bool,
773772
}
774773

775774
impl ForwardPartialPathStitcher {
@@ -819,10 +818,10 @@ impl ForwardPartialPathStitcher {
819818
.iter()
820819
.copied()
821820
.map(|handle| {
822-
(
823-
db[handle].clone(),
824-
AppendingCycleDetector::from(&mut appended_paths, handle.into()),
825-
)
821+
let mut p = db[handle].clone();
822+
p.eliminate_precondition_stack_variables(partials);
823+
let c = AppendingCycleDetector::from(&mut appended_paths, handle.into());
824+
(p, c)
826825
})
827826
.unzip();
828827
copious_debugging!("==> End phase 0");
@@ -836,7 +835,6 @@ impl ForwardPartialPathStitcher {
836835
max_work_per_phase: usize::MAX,
837836
#[cfg(feature = "copious-debugging")]
838837
phase_number: 1,
839-
should_extend: |_, _, _| true,
840838
}
841839
}
842840

@@ -851,14 +849,15 @@ impl ForwardPartialPathStitcher {
851849
/// [`process_next_phase`]: #method.process_next_phase
852850
pub fn from_partial_paths(
853851
_graph: &StackGraph,
854-
_partials: &mut PartialPaths,
852+
partials: &mut PartialPaths,
855853
_db: &mut Database,
856854
initial_partial_paths: Vec<PartialPath>,
857855
) -> ForwardPartialPathStitcher {
858856
let mut appended_paths = Appendables::new();
859857
let next_iteration = initial_partial_paths
860858
.into_iter()
861-
.map(|p| {
859+
.map(|mut p| {
860+
p.eliminate_precondition_stack_variables(partials);
862861
let c = AppendingCycleDetector::from(&mut appended_paths, p.clone().into());
863862
(p, c)
864863
})
@@ -873,7 +872,6 @@ impl ForwardPartialPathStitcher {
873872
max_work_per_phase: usize::MAX,
874873
#[cfg(feature = "copious-debugging")]
875874
phase_number: 1,
876-
should_extend: |_, _, _| true,
877875
}
878876
}
879877

@@ -906,14 +904,6 @@ impl ForwardPartialPathStitcher {
906904
self.max_work_per_phase = max_work_per_phase;
907905
}
908906

909-
/// Sets a condition that determines if a partial path is a candidate that should be extended.
910-
pub fn set_should_extend(
911-
&mut self,
912-
should_extend: fn(&StackGraph, &mut PartialPaths, &PartialPath) -> bool,
913-
) {
914-
self.should_extend = should_extend;
915-
}
916-
917907
/// Attempts to extend one partial path as part of the algorithm. When calling this function,
918908
/// you are responsible for ensuring that `db` already contains all of the possible partial
919909
/// paths that we might want to extend `partial_path` with.
@@ -1022,10 +1012,6 @@ impl ForwardPartialPathStitcher {
10221012
"--> Candidate partial path {}",
10231013
partial_path.display(graph, partials)
10241014
);
1025-
if !(self.should_extend)(graph, partials, &partial_path) {
1026-
copious_debugging!(" Should not extend");
1027-
continue;
1028-
}
10291015
if !self
10301016
.similar_path_detector
10311017
.should_process_path(&partial_path, |probe| {
@@ -1073,7 +1059,6 @@ impl ForwardPartialPathStitcher {
10731059
{
10741060
let mut stitcher =
10751061
ForwardPartialPathStitcher::from_nodes(graph, partials, db, starting_nodes);
1076-
stitcher.set_should_extend(|g, _, p| p.starts_at_reference(g));
10771062
while !stitcher.is_complete() {
10781063
cancellation_flag.check("finding complete partial paths")?;
10791064
let complete_partial_paths = stitcher

‎stack-graphs/tests/it/c/can_jump_to_definition_with_phased_partial_path_stitching.rs‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -242,23 +242,23 @@ fn class_field_through_function_parameter() {
242242
"main.py",
243243
&[
244244
// reference to `a` in import statement
245-
"<%1> ($1) [main.py(17) reference a] -> [a.py(0) definition a] <%1> ($1)",
245+
"<> () [main.py(17) reference a] -> [a.py(0) definition a] <> ()",
246246
// reference to `b` in import statement
247-
"<%1> ($1) [main.py(15) reference b] -> [b.py(0) definition b] <%1> ($1)",
247+
"<> () [main.py(15) reference b] -> [b.py(0) definition b] <> ()",
248248
// reference to `foo` in function call resolves to function definition
249-
"<%1> ($1) [main.py(13) reference foo] -> [a.py(5) definition foo] <%1> ($1)",
249+
"<> () [main.py(13) reference foo] -> [a.py(5) definition foo] <> ()",
250250
// reference to `A` as function parameter resolves to class definition
251-
"<%1> ($1) [main.py(9) reference A] -> [b.py(5) definition A] <%1> ($1)",
251+
"<> () [main.py(9) reference A] -> [b.py(5) definition A] <> ()",
252252
// reference to `bar` on result flows through body of `foo` to find `A.bar`
253-
"<%1> ($1) [main.py(10) reference bar] -> [b.py(8) definition bar] <%1> ($1)",
253+
"<> () [main.py(10) reference bar] -> [b.py(8) definition bar] <> ()",
254254
],
255255
);
256256
check_jump_to_definition(
257257
&graph,
258258
"a.py",
259259
&[
260260
// reference to `x` in function body resolves to formal parameter
261-
"<%1> ($1) [a.py(8) reference x] -> [a.py(14) definition x] <%1> ()",
261+
"<> () [a.py(8) reference x] -> [a.py(14) definition x] <> ()",
262262
],
263263
);
264264
check_jump_to_definition(
@@ -278,25 +278,25 @@ fn cyclic_imports_python() {
278278
"main.py",
279279
&[
280280
// reference to `a` in import statement
281-
"<%1> ($1) [main.py(8) reference a] -> [a.py(0) definition a] <%1> ($1)",
281+
"<> () [main.py(8) reference a] -> [a.py(0) definition a] <> ()",
282282
// reference to `foo` resolves through intermediate file to find `b.foo`
283-
"<%1> ($1) [main.py(6) reference foo] -> [b.py(6) definition foo] <%1> ($1)",
283+
"<> () [main.py(6) reference foo] -> [b.py(6) definition foo] <> ()",
284284
],
285285
);
286286
check_jump_to_definition(
287287
&graph,
288288
"a.py",
289289
&[
290290
// reference to `b` in import statement
291-
"<%1> ($1) [a.py(6) reference b] -> [b.py(0) definition b] <%1> ($1)",
291+
"<> () [a.py(6) reference b] -> [b.py(0) definition b] <> ()",
292292
],
293293
);
294294
check_jump_to_definition(
295295
&graph,
296296
"b.py",
297297
&[
298298
// reference to `a` in import statement
299-
"<%1> ($1) [b.py(8) reference a] -> [a.py(0) definition a] <%1> ($1)",
299+
"<> () [b.py(8) reference a] -> [a.py(0) definition a] <> ()",
300300
],
301301
);
302302
}
@@ -309,16 +309,16 @@ fn cyclic_imports_rust() {
309309
"test.rs",
310310
&[
311311
// reference to `a` in `a::FOO` resolves to module definition
312-
"<%1> ($1) [test.rs(103) reference a] -> [test.rs(201) definition a] <%1> ($1)",
312+
"<> () [test.rs(103) reference a] -> [test.rs(201) definition a] <> ()",
313313
// reference to `a::FOO` in `main` can resolve either to `a::BAR` or `b::FOO`
314-
"<%1> ($1) [test.rs(101) reference FOO] -> [test.rs(304) definition FOO] <%1> ($1)",
315-
"<%1> ($1) [test.rs(101) reference FOO] -> [test.rs(204) definition BAR] <%1> ($1)",
314+
"<> () [test.rs(101) reference FOO] -> [test.rs(304) definition FOO] <> ()",
315+
"<> () [test.rs(101) reference FOO] -> [test.rs(204) definition BAR] <> ()",
316316
// reference to `b` in use statement resolves to module definition
317-
"<%1> ($1) [test.rs(206) reference b] -> [test.rs(301) definition b] <%1> ($1)",
317+
"<> () [test.rs(206) reference b] -> [test.rs(301) definition b] <> ()",
318318
// reference to `a` in use statement resolves to module definition
319-
"<%1> ($1) [test.rs(307) reference a] -> [test.rs(201) definition a] <%1> ($1)",
319+
"<> () [test.rs(307) reference a] -> [test.rs(201) definition a] <> ()",
320320
// reference to `BAR` in module `b` can _only_ resolve to `a::BAR`
321-
"<%1> ($1) [test.rs(305) reference BAR] -> [test.rs(204) definition BAR] <%1> ($1)",
321+
"<> () [test.rs(305) reference BAR] -> [test.rs(204) definition BAR] <> ()",
322322
],
323323
);
324324
}
@@ -331,17 +331,17 @@ fn sequenced_import_star() {
331331
"main.py",
332332
&[
333333
// reference to `a` in import statement
334-
"<%1> ($1) [main.py(8) reference a] -> [a.py(0) definition a] <%1> ($1)",
334+
"<> () [main.py(8) reference a] -> [a.py(0) definition a] <> ()",
335335
// reference to `foo` resolves through intermediate file to find `b.foo`
336-
"<%1> ($1) [main.py(6) reference foo] -> [b.py(5) definition foo] <%1> ($1)",
336+
"<> () [main.py(6) reference foo] -> [b.py(5) definition foo] <> ()",
337337
],
338338
);
339339
check_jump_to_definition(
340340
&graph,
341341
"a.py",
342342
&[
343343
// reference to `b` in import statement
344-
"<%1> ($1) [a.py(6) reference b] -> [b.py(0) definition b] <%1> ($1)",
344+
"<> () [a.py(6) reference b] -> [b.py(0) definition b] <> ()",
345345
],
346346
);
347347
check_jump_to_definition(

‎stack-graphs/tests/it/can_jump_to_definition_with_forward_partial_path_stitching.rs‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -68,17 +68,17 @@ fn class_field_through_function_parameter() {
6868
&graph,
6969
&[
7070
// reference to `a` in import statement
71-
"<%1> ($1) [main.py(17) reference a] -> [a.py(0) definition a] <%1> ($1)",
71+
"<> () [main.py(17) reference a] -> [a.py(0) definition a] <> ()",
7272
// reference to `b` in import statement
73-
"<%1> ($1) [main.py(15) reference b] -> [b.py(0) definition b] <%1> ($1)",
73+
"<> () [main.py(15) reference b] -> [b.py(0) definition b] <> ()",
7474
// reference to `foo` in function call resolves to function definition
75-
"<%1> ($1) [main.py(13) reference foo] -> [a.py(5) definition foo] <%1> ($1)",
75+
"<> () [main.py(13) reference foo] -> [a.py(5) definition foo] <> ()",
7676
// reference to `A` as function parameter resolves to class definition
77-
"<%1> ($1) [main.py(9) reference A] -> [b.py(5) definition A] <%1> ($1)",
77+
"<> () [main.py(9) reference A] -> [b.py(5) definition A] <> ()",
7878
// reference to `bar` on result flows through body of `foo` to find `A.bar`
79-
"<%1> ($1) [main.py(10) reference bar] -> [b.py(8) definition bar] <%1> ($1)",
79+
"<> () [main.py(10) reference bar] -> [b.py(8) definition bar] <> ()",
8080
// reference to `x` in function body resolves to formal parameter
81-
"<%1> ($1) [a.py(8) reference x] -> [a.py(14) definition x] <%1> ()",
81+
"<> () [a.py(8) reference x] -> [a.py(14) definition x] <> ()",
8282
],
8383
);
8484
}
@@ -90,13 +90,13 @@ fn cyclic_imports_python() {
9090
&graph,
9191
&[
9292
// reference to `a` in import statement
93-
"<%1> ($1) [main.py(8) reference a] -> [a.py(0) definition a] <%1> ($1)",
93+
"<> () [main.py(8) reference a] -> [a.py(0) definition a] <> ()",
9494
// reference to `foo` resolves through intermediate file to find `b.foo`
95-
"<%1> ($1) [main.py(6) reference foo] -> [b.py(6) definition foo] <%1> ($1)",
95+
"<> () [main.py(6) reference foo] -> [b.py(6) definition foo] <> ()",
9696
// reference to `b` in import statement
97-
"<%1> ($1) [a.py(6) reference b] -> [b.py(0) definition b] <%1> ($1)",
97+
"<> () [a.py(6) reference b] -> [b.py(0) definition b] <> ()",
9898
// reference to `a` in import statement
99-
"<%1> ($1) [b.py(8) reference a] -> [a.py(0) definition a] <%1> ($1)",
99+
"<> () [b.py(8) reference a] -> [a.py(0) definition a] <> ()",
100100
],
101101
);
102102
}
@@ -108,16 +108,16 @@ fn cyclic_imports_rust() {
108108
&graph,
109109
&[
110110
// reference to `a` in `a::FOO` resolves to module definition
111-
"<%1> ($1) [test.rs(103) reference a] -> [test.rs(201) definition a] <%1> ($1)",
111+
"<> () [test.rs(103) reference a] -> [test.rs(201) definition a] <> ()",
112112
// reference to `a::FOO` in `main` can resolve either to `a::BAR` or `b::FOO`
113-
"<%1> ($1) [test.rs(101) reference FOO] -> [test.rs(304) definition FOO] <%1> ($1)",
114-
"<%1> ($1) [test.rs(101) reference FOO] -> [test.rs(204) definition BAR] <%1> ($1)",
113+
"<> () [test.rs(101) reference FOO] -> [test.rs(304) definition FOO] <> ()",
114+
"<> () [test.rs(101) reference FOO] -> [test.rs(204) definition BAR] <> ()",
115115
// reference to `b` in use statement resolves to module definition
116-
"<%1> ($1) [test.rs(206) reference b] -> [test.rs(301) definition b] <%1> ($1)",
116+
"<> () [test.rs(206) reference b] -> [test.rs(301) definition b] <> ()",
117117
// reference to `a` in use statement resolves to module definition
118-
"<%1> ($1) [test.rs(307) reference a] -> [test.rs(201) definition a] <%1> ($1)",
118+
"<> () [test.rs(307) reference a] -> [test.rs(201) definition a] <> ()",
119119
// reference to `BAR` in module `b` can _only_ resolve to `a::BAR`
120-
"<%1> ($1) [test.rs(305) reference BAR] -> [test.rs(204) definition BAR] <%1> ($1)",
120+
"<> () [test.rs(305) reference BAR] -> [test.rs(204) definition BAR] <> ()",
121121
],
122122
);
123123
}
@@ -129,11 +129,11 @@ fn sequenced_import_star() {
129129
&graph,
130130
&[
131131
// reference to `a` in import statement
132-
"<%1> ($1) [main.py(8) reference a] -> [a.py(0) definition a] <%1> ($1)",
132+
"<> () [main.py(8) reference a] -> [a.py(0) definition a] <> ()",
133133
// reference to `foo` resolves through intermediate file to find `b.foo`
134-
"<%1> ($1) [main.py(6) reference foo] -> [b.py(5) definition foo] <%1> ($1)",
134+
"<> () [main.py(6) reference foo] -> [b.py(5) definition foo] <> ()",
135135
// reference to `b` in import statement
136-
"<%1> ($1) [a.py(6) reference b] -> [b.py(0) definition b] <%1> ($1)",
136+
"<> () [a.py(6) reference b] -> [b.py(0) definition b] <> ()",
137137
],
138138
);
139139
}

0 commit comments

Comments
 (0)