Only add to the precondition if this is actually possible.

Author: Hendrik van Antwerpen

stack-graphs/src/partial.rs

@@ -2311,9 +2311,9 @@ impl Node {
                         None => return Err(PathResolutionError::MissingAttachedScopeList),
                     };
                     *scope_stack_postcondition = new_scope_stack;
-                } else {
-                    // If the symbol stack postcondition is empty, then we need to update the
-                    // _precondition_ to indicate that the symbol stack needs to contain this scoped
+                } else if symbol_stack_postcondition.has_variable() {
+                    // If the symbol stack postcondition is empty but has a variable, then we can update
+                    // the _precondition_ to indicate that the symbol stack needs to contain this scoped
                     // symbol in order to successfully use this partial path.
                     let scope_stack_variable =
                         PartialPath::fresh_scope_stack_variable_for_partial_stack(
@@ -2327,9 +2327,18 @@ impl Node {
                             scope_stack_variable,
                         )),
                     };
+                    // We simply push to the precondition. The official procedure here
+                    // is to bind the postcondition symbol stack variable to the symbol
+                    // and a fresh variable, and apply that. However, because the variable
+                    // can only be bound in the precondition symbol stack, this amounts to
+                    // pushing the symbol there directly.
                     symbol_stack_precondition.push_back(partials, precondition_symbol);
                     *scope_stack_postcondition =
                         PartialScopeStack::from_variable(scope_stack_variable);
+                } else {
+                    // The symbol stack postcondition is empty and has no variable, so we cannot
+                    // perform the operation.
+                    return Err(PathResolutionError::SymbolStackUnsatisfied);
                 }
             }
             Self::PopSymbol(sink) => {
@@ -2341,15 +2350,24 @@ impl Node {
                     if top.scopes.is_some() {
                         return Err(PathResolutionError::UnexpectedAttachedScopeList);
                     }
-                } else {
-                    // If the symbol stack postcondition is empty, then we need to update the
-                    // _precondition_ to indicate that the symbol stack needs to contain this symbol in
-                    // order to successfully use this partial path.
+                } else if symbol_stack_postcondition.has_variable() {
+                    // If the symbol stack postcondition is empty but has a variable, then we can update
+                    // the _precondition_ to indicate that the symbol stack needs to contain this symbol
+                    // in order to successfully use this partial path.
                     let precondition_symbol = PartialScopedSymbol {
                         symbol: sink.symbol,
                         scopes: ControlledOption::none(),
                     };
+                    // We simply push to the precondition. The official procedure here
+                    // is to bind the postcondition symbol stack variable to the symbol
+                    // and a fresh variable, and apply that. However, because the variable
+                    // can only be bound in the precondition symbol stack, this amounts to
+                    // pushing the symbol there directly.
                     symbol_stack_precondition.push_back(partials, precondition_symbol);
+                } else {
+                    // The symbol stack postcondition is empty and has no variable, so we cannot
+                    // perform the operation.
+                    return Err(PathResolutionError::SymbolStackUnsatisfied);
                 }
             }
             Self::PushScopedSymbol(sink) => {

stack-graphs/tests/it/partial.rs

@@ -519,6 +519,192 @@ fn can_create_partial_path_from_node() {
     );
 }
 
+#[test]
+fn can_append_edges() -> Result<(), PathResolutionError> {
+    let mut graph = StackGraph::new();
+    let file = graph.add_file("test").expect("");
+    let jump_to_scope_node = StackGraph::jump_to_node();
+    let scope0 = create_scope_node(&mut graph, file, false);
+    let scope1 = create_scope_node(&mut graph, file, false);
+    let foo_ref = create_push_symbol_node(&mut graph, file, "foo", false);
+    let foo_def = create_pop_symbol_node(&mut graph, file, "foo", false);
+    let bar_ref = create_push_symbol_node(&mut graph, file, "bar", false);
+    let bar_def = create_pop_symbol_node(&mut graph, file, "bar", false);
+    let exported_scope = create_scope_node(&mut graph, file, true);
+    let exported_scope_id = graph[exported_scope].id();
+    let baz_ref = create_push_scoped_symbol_node(&mut graph, file, "baz", exported_scope_id, false);
+    let baz_def = create_pop_scoped_symbol_node(&mut graph, file, "baz", false);
+    let drop_scopes = create_drop_scopes_node(&mut graph, file);
+
+    fn run(
+        graph: &StackGraph,
+        left: NicePartialPath,
+        right: NiceEdge,
+        expected: &str,
+    ) -> Result<(), PathResolutionError> {
+        let mut g = StackGraph::new();
+        g.add_from_graph(graph).expect("");
+
+        let mut ps = PartialPaths::new();
+
+        let mut l = create_partial_path_and_edges(&mut g, &mut ps, left).expect("");
+        let r = create_edge(&mut g, right);
+
+        l.append(graph, &mut ps, r)?;
+        let actual = l.display(&g, &mut ps).to_string();
+        assert_eq!(expected, actual);
+
+        Ok(())
+    }
+
+    fn verify(graph: &StackGraph, left: NicePartialPath, right: NiceEdge, expected: &str) {
+        run(graph, left, right, expected).expect("");
+    }
+
+    fn verify_not(graph: &StackGraph, left: NicePartialPath, right: NiceEdge) {
+        run(graph, left, right, "").expect_err("");
+    }
+
+    verify(
+        &graph,
+        &[foo_ref, scope0],
+        (scope0, foo_def),
+        "<%1> ($1) [test(2) push foo] -> [test(3) pop foo] <%1> ($1)",
+    );
+
+    verify(
+        &graph,
+        &[foo_ref, scope0],
+        (scope0, scope1),
+        "<%1> ($1) [test(2) push foo] -> [test(1) scope] <foo,%1> ($1)",
+    );
+
+    verify(
+        &graph,
+        &[scope0, scope1],
+        (scope1, foo_ref),
+        "<%1> ($1) [test(0) scope] -> [test(2) push foo] <foo,%1> ($1)",
+    );
+
+    verify(
+        &graph,
+        &[foo_def, scope0],
+        (scope0, bar_ref),
+        "<foo,%1> ($1) [test(3) pop foo] -> [test(4) push bar] <bar,%1> ($1)",
+    );
+
+    verify(
+        &graph,
+        &[foo_ref, scope0, foo_def],
+        (foo_def, bar_ref),
+        "<%1> ($1) [test(2) push foo] -> [test(4) push bar] <bar,%1> ($1)",
+    );
+
+    verify(
+        &graph,
+        &[foo_def, scope0, bar_ref],
+        (bar_ref, bar_def),
+        "<foo,%1> ($1) [test(3) pop foo] -> [test(5) pop bar] <%1> ($1)",
+    );
+
+    verify(
+        &graph,
+        &[baz_ref, scope0, baz_def],
+        (baz_def, bar_ref),
+        "<%1> ($1) [test(7) push scoped baz test(6)] -> [test(4) push bar] <bar,%1> ([test(6)],$1)",
+    );
+
+    verify(
+        &graph,
+        &[foo_def, scope0, baz_ref],
+        (baz_ref, baz_def),
+        "<foo,%1> ($1) [test(3) pop foo] -> [test(8) pop scoped baz] <%1> ([test(6)],$1)",
+    );
+
+    verify(
+        &graph,
+        &[scope0, drop_scopes],
+        (drop_scopes, scope1),
+        "<%1> ($1) [test(0) scope] -> [test(1) scope] <%1> ()",
+    );
+
+    verify_not(
+        &graph,
+        &[scope0, drop_scopes, scope1],
+        (scope1, jump_to_scope_node),
+    );
+
+    verify(
+        &graph,
+        &[baz_def, scope0],
+        (scope0, jump_to_scope_node),
+        "<baz/($2),%1> ($1) [test(8) pop scoped baz] -> [jump to scope] <%1> ($2)",
+    );
+
+    verify(
+        &graph,
+        &[baz_ref, scope0],
+        (scope0, jump_to_scope_node),
+        "<%1> ($1) [test(7) push scoped baz test(6)] -> [jump to scope] <baz/([test(6)],$1),%1> ($1)",
+    );
+
+    Ok(())
+}
+
+#[test]
+fn can_append_edges_without_precondition_variables() -> Result<(), PathResolutionError> {
+    let mut graph = StackGraph::new();
+    let file = graph.add_file("test").expect("");
+    let scope0 = create_scope_node(&mut graph, file, false);
+    let foo_ref = create_push_symbol_node(&mut graph, file, "foo", false);
+    let foo_def = create_pop_symbol_node(&mut graph, file, "foo", false);
+    let bar_ref = create_push_symbol_node(&mut graph, file, "bar", false);
+    let bar_def = create_pop_symbol_node(&mut graph, file, "bar", false);
+
+    fn run(
+        graph: &StackGraph,
+        left: NicePartialPath,
+        right: NiceEdge,
+        expected: &str,
+    ) -> Result<(), PathResolutionError> {
+        let mut g = StackGraph::new();
+        g.add_from_graph(graph).expect("");
+
+        let mut ps = PartialPaths::new();
+
+        let mut l = create_partial_path_and_edges(&mut g, &mut ps, left).expect("");
+        l.eliminate_precondition_stack_variables(&mut ps);
+        let r = create_edge(&mut g, right);
+
+        l.append(graph, &mut ps, r)?;
+        let actual = l.display(&g, &mut ps).to_string();
+        assert_eq!(expected, actual);
+
+        Ok(())
+    }
+
+    fn verify(graph: &StackGraph, left: NicePartialPath, right: NiceEdge, expected: &str) {
+        run(graph, left, right, expected).expect("");
+    }
+
+    fn verify_not(graph: &StackGraph, left: NicePartialPath, right: NiceEdge) {
+        run(graph, left, right, "").expect_err("");
+    }
+
+    verify(
+        &graph,
+        &[foo_ref, scope0, foo_def],
+        (foo_def, bar_ref),
+        "<> () [test(1) push foo] -> [test(3) push bar] <bar> ()",
+    );
+
+    verify_not(&graph, &[scope0], (scope0, bar_def));
+
+    verify_not(&graph, &[foo_ref, scope0, foo_def], (foo_def, bar_def));
+
+    Ok(())
+}
+
 #[test]
 fn can_append_partial_paths() -> Result<(), PathResolutionError> {
     let mut graph = StackGraph::new();
@@ -658,24 +844,61 @@ fn can_append_partial_paths() -> Result<(), PathResolutionError> {
         "<%1> ($1) [test(7) push scoped baz test(6)] -> [jump to scope] <baz/([test(6)],$1),%1> ($1)",
     );
 
-    // verify that without stack variables in the precondition, the precondition cannot grow because of concatenation
-    {
-        let left = &[scope0];
-        let right = &[scope0, bar_def];
+    Ok(())
+}
 
+#[test]
+fn can_append_partial_paths_without_precondition_variables() -> Result<(), PathResolutionError> {
+    let mut graph = StackGraph::new();
+    let file = graph.add_file("test").expect("");
+    let scope0 = create_scope_node(&mut graph, file, false);
+    let foo_ref = create_push_symbol_node(&mut graph, file, "foo", false);
+    let foo_def = create_pop_symbol_node(&mut graph, file, "foo", false);
+    let bar_ref = create_push_symbol_node(&mut graph, file, "bar", false);
+    let bar_def = create_pop_symbol_node(&mut graph, file, "bar", false);
+
+    fn run(
+        graph: &StackGraph,
+        left: NicePartialPath,
+        right: NicePartialPath,
+        expected: &str,
+    ) -> Result<(), PathResolutionError> {
         let mut g = StackGraph::new();
-        g.add_from_graph(&graph).expect("");
+        g.add_from_graph(graph).expect("");
 
         let mut ps = PartialPaths::new();
+
         let mut l = create_partial_path_and_edges(&mut g, &mut ps, left).expect("");
         l.eliminate_precondition_stack_variables(&mut ps);
         let mut r = create_partial_path_and_edges(&mut g, &mut ps, right).expect("");
 
         r.ensure_no_overlapping_variables(&mut ps, &l);
-        let result = l.concatenate(&g, &mut ps, &r);
-        result.expect_err("");
+        l.concatenate(&g, &mut ps, &r)?;
+        let actual = l.display(&g, &mut ps).to_string();
+        assert_eq!(expected, actual);
+
+        Ok(())
+    }
+
+    fn verify(graph: &StackGraph, left: NicePartialPath, right: NicePartialPath, expected: &str) {
+        run(graph, left, right, expected).expect("");
+    }
+
+    fn verify_not(graph: &StackGraph, left: NicePartialPath, right: NicePartialPath) {
+        run(graph, left, right, "").expect_err("");
     }
 
+    verify(
+        &graph,
+        &[foo_ref, scope0],
+        &[scope0, foo_def, bar_ref],
+        "<> () [test(1) push foo] -> [test(3) push bar] <bar> ()",
+    );
+
+    verify_not(&graph, &[scope0], &[scope0, bar_def]);
+
+    verify_not(&graph, &[foo_ref, scope0, foo_def], &[foo_def, bar_def]);
+
     Ok(())
 }
 

stack-graphs/tests/it/util.rs

@@ -25,6 +25,7 @@ pub(crate) type NiceSymbolStack<'a> = (&'a [NiceScopedSymbol<'a>], Option<Symbol
 pub(crate) type NiceScopedSymbol<'a> = (&'a str, Option<NiceScopeStack<'a>>);
 pub(crate) type NiceScopeStack<'a> = (&'a [u32], Option<ScopeStackVariable>);
 pub(crate) type NicePartialPath<'a> = &'a [Handle<Node>];
+pub(crate) type NiceEdge = (Handle<Node>, Handle<Node>);
 
 pub(crate) fn create_drop_scopes_node(graph: &mut StackGraph, file: Handle<File>) -> Handle<Node> {
     let id = graph.new_node_id(file);
@@ -164,6 +165,12 @@ pub(crate) fn create_partial_path_and_edges(
     Ok(path)
 }
 
+pub(crate) fn create_edge(graph: &mut StackGraph, contents: NiceEdge) -> Edge {
+    let edge = edge(contents.0, contents.1, 0);
+    graph.add_edge(edge.source, edge.sink, edge.precedence);
+    edge
+}
+
 pub(crate) fn edge(source: Handle<Node>, sink: Handle<Node>, precedence: i32) -> Edge {
     Edge {
         source,