Skip to content

Commit

Permalink
fix: recurse only on referential nodes
Browse files Browse the repository at this point in the history
  • Loading branch information
ebastien committed Oct 3, 2023
1 parent 0a95a5d commit cf03327
Show file tree
Hide file tree
Showing 10 changed files with 223 additions and 136 deletions.
6 changes: 3 additions & 3 deletions examples/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ components:
- example: /some/path/_id_number_/template
type: string
format: uri-reference
- $ref: '#/components/schemas/hash-d60d1f8c1c82157cf19abfacf4d4aeae665becd17938a08d6436fbfc3ca277f2'
- $ref: '#/components/schemas/hash-949ca8b37ea72edb5d2a99579d7f8e77bab63f43a8e2dc6a7788ae9404b493bd'
obj2:
allOf:
- $ref: '#/components/schemas/obj1'
Expand All @@ -217,7 +217,7 @@ components:
type: integer
minimum: 0
maximum: 999
hash-d60d1f8c1c82157cf19abfacf4d4aeae665becd17938a08d6436fbfc3ca277f2:
hash-949ca8b37ea72edb5d2a99579d7f8e77bab63f43a8e2dc6a7788ae9404b493bd:
type: object
properties:
name:
Expand All @@ -227,4 +227,4 @@ components:
children:
type: array
items:
$ref: '#/components/schemas/hash-d60d1f8c1c82157cf19abfacf4d4aeae665becd17938a08d6436fbfc3ca277f2'
$ref: '#/components/schemas/hash-949ca8b37ea72edb5d2a99579d7f8e77bab63f43a8e2dc6a7788ae9404b493bd'
8 changes: 5 additions & 3 deletions oal-compiler/src/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@ use crate::errors::Result;
use crate::inference::{constrain, substitute, tag};
use crate::module::ModuleSet;
use crate::resolve::resolve;
use crate::typecheck::type_check;
use crate::typecheck::{cycles_check, type_check};
use oal_model::locator::Locator;

/// Runs all compilation phases.
pub fn compile(mods: &ModuleSet, loc: &Locator) -> Result<()> {
// Resolve variable and function references.
resolve(mods, loc)?;
// Resolve variable and function references. Returns the graph of definitions.
let graph = resolve(mods, loc)?;
// Tag expressions with concrete and variable types.
let _nvars = tag(mods, loc)?;
// Collect the set of type inference equations.
Expand All @@ -17,6 +17,8 @@ pub fn compile(mods: &ModuleSet, loc: &Locator) -> Result<()> {
let set = eqs.unify()?;
// Substitute tags in each class of equivalence with the representative tag.
substitute(mods, loc, &set)?;
// Validates points of recursion in the graph of definitions.
cycles_check(graph, mods)?;
// Check type tags against expectations.
type_check(mods, loc)?;
Ok(())
Expand Down
30 changes: 30 additions & 0 deletions oal-compiler/src/compile_tests.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use crate::compile::compile;
use crate::module::ModuleSet;
use crate::tests::mods_from;
use oal_model::grammar::AbstractSyntaxNode;
use oal_model::locator::Locator;
use oal_syntax::parser::Program;

#[test]
fn compile_modules() -> anyhow::Result<()> {
Expand All @@ -25,3 +28,30 @@ fn compile_modules() -> anyhow::Result<()> {

Ok(())
}

#[test]
fn compile_cycles() -> anyhow::Result<()> {
let mods = mods_from(
r#"
let a = { 'b b }; // mutually recursive
let b = { 'a a }; // ...
let c = { 'a a, 'b b }; // non-recursive
let d = { 'd d }; // self-recursive
"#,
)?;

compile(&mods, mods.base()).expect("should compile");

let prog = Program::cast(mods.main().root()).expect("expected a program");
let a = prog.declarations().nth(0).expect("expected a declaration");
let b = prog.declarations().nth(1).expect("expected a declaration");
let c = prog.declarations().nth(2).expect("expected a declaration");
let d = prog.declarations().nth(3).expect("expected a declaration");

assert!(a.node().syntax().core_ref().is_recursive);
assert!(b.node().syntax().core_ref().is_recursive);
assert!(!c.node().syntax().core_ref().is_recursive);
assert!(d.node().syntax().core_ref().is_recursive);

Ok(())
}
63 changes: 0 additions & 63 deletions oal-compiler/src/defgraph.rs

This file was deleted.

41 changes: 36 additions & 5 deletions oal-compiler/src/eval_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,21 @@ use crate::inference::{check_complete, constrain, substitute, tag};
use crate::resolve::resolve;
use crate::spec::{Object, Reference, SchemaExpr, Spec, UriSegment};
use crate::tests::mods_from;
use crate::typecheck::type_check;
use crate::typecheck::{cycles_check, type_check};
use oal_syntax::atom::{HttpStatus, Method, VariadicOperator};

fn eval(code: &str, check: bool) -> anyhow::Result<Spec> {
let mods = mods_from(code)?;
let loc = mods.base();
resolve(&mods, loc)?;
let graph = resolve(&mods, loc)?;
let _nvars = tag(&mods, loc)?;
let eqs = constrain(&mods, loc)?;
let set = eqs.unify()?;
substitute(&mods, loc, &set)?;
if check {
check_complete(&mods, loc)?;
}
cycles_check(graph, &mods)?;
type_check(&mods, loc)?;

// Uncomment for debugging purpose:
Expand Down Expand Up @@ -772,14 +773,44 @@ fn eval_unique_recursive_identifiers() -> anyhow::Result<()> {
#[test]
#[ignore = "not implemented"]
fn eval_recursive_uri() -> anyhow::Result<()> {
let code = r#"
let a = concat /a a;
res a;
"#;

assert!(matches!(
eval_check(code)
.expect_err(format!("expected error evaluating: {}", code).as_str())
.downcast_ref::<errors::Error>()
.expect("expected compiler error")
.kind,
errors::Kind::InvalidType
));

let s = eval_check(
r#"
let a = concat /a a; // Should not work
let x = /b?{ 'p b };
let b = concat /a x; // Should work
let a = /b?{ 'p b };
let b = concat /a a;
res b;
"#,
)?;
assert_eq!(s.rels.len(), 1);
assert_eq!(s.refs.len(), 1);

Ok(())
}

#[test]
fn eval_recursive_content() -> anyhow::Result<()> {
let s = eval_check(
r#"
let c = <headers={ 'Location r }, {}>;
let r = / on get -> c;
res r;
"#,
)?;
assert_eq!(s.rels.len(), 1);
assert_eq!(s.refs.len(), 1);
Ok(())
}

Expand Down
1 change: 0 additions & 1 deletion oal-compiler/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
mod annotation;
pub mod compile;
mod defgraph;
pub mod definition;
mod env;
pub mod errors;
Expand Down
71 changes: 59 additions & 12 deletions oal-compiler/src/resolve.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use crate::defgraph::DefGraph;
use crate::definition::{Definition, External};
use crate::env::{Entry, Env};
use crate::errors::{Error, Kind, Result};
Expand All @@ -8,8 +7,58 @@ use crate::tree::Core;
use oal_model::grammar::{AbstractSyntaxNode, NodeCursor};
use oal_model::locator::Locator;
use oal_syntax::parser::{Declaration, Import, Program, Recursion, Variable};
use petgraph::graph::NodeIndex;
use petgraph::stable_graph::StableDiGraph;
use std::collections::{hash_map, HashMap};

fn define_variable(env: &mut Env, defg: &mut DefGraph, var: Variable<'_, Core>) -> Result<()> {
pub type Graph = StableDiGraph<External, ()>;

/// A builder for the graph of dependencies between variable definitions.
#[derive(Debug, Default)]
pub struct Builder {
/// The current (i.e. opened) definition.
current: Option<External>,
/// The map from definitions to graph node indices.
externals: HashMap<External, NodeIndex>,
/// The graph of definitions.
graph: Graph,
}

impl Builder {
/// Inserts a new definition.
fn insert(&mut self, ext: External) -> NodeIndex {
match self.externals.entry(ext.clone()) {
hash_map::Entry::Occupied(e) => *e.get(),
hash_map::Entry::Vacant(e) => *e.insert(self.graph.add_node(ext)),
}
}

/// Opens a definition, becoming the current definition.
pub fn open(&mut self, from: External) {
self.current = Some(from);
}

/// Closes a definition.
pub fn close(&mut self) {
self.current = None;
}

/// Connects the current definition to another definition.
pub fn connect(&mut self, to: External) {
if let Some(from) = &self.current {
let from_idx = self.insert(from.clone());
let to_idx = self.insert(to);
self.graph.add_edge(from_idx, to_idx, ());
}
}

/// Returns the graph of definitions.
pub fn graph(self) -> Graph {
self.graph
}
}

fn define_variable(env: &mut Env, defg: &mut Builder, var: Variable<'_, Core>) -> Result<()> {
let qualifier = var.qualifier().map(|q| q.ident());
let entry = Entry::new(var.ident(), qualifier);
if let Some(definition) = env.lookup(&entry) {
Expand Down Expand Up @@ -57,7 +106,7 @@ fn declare_variable(env: &mut Env, decl: Declaration<'_, Core>) -> Result<()> {
}
}

fn open_declaration(env: &mut Env, defg: &mut DefGraph, decl: Declaration<'_, Core>) -> Result<()> {
fn open_declaration(env: &mut Env, defg: &mut Builder, decl: Declaration<'_, Core>) -> Result<()> {
env.open();
defg.open(External::new(decl.node()));
for binding in decl.bindings() {
Expand All @@ -68,7 +117,7 @@ fn open_declaration(env: &mut Env, defg: &mut DefGraph, decl: Declaration<'_, Co
Ok(())
}

fn close_declaration(env: &mut Env, defg: &mut DefGraph) -> Result<()> {
fn close_declaration(env: &mut Env, defg: &mut Builder) -> Result<()> {
defg.close();
env.close();
Ok(())
Expand All @@ -88,8 +137,8 @@ fn close_recursion(env: &mut Env) -> Result<()> {
Ok(())
}

pub fn resolve(mods: &ModuleSet, loc: &Locator) -> Result<()> {
let defg = &mut DefGraph::default();
pub fn resolve(mods: &ModuleSet, loc: &Locator) -> Result<Graph> {
let mut defg = Builder::default();

let env = &mut Env::new();
stdlib::import(env)?;
Expand All @@ -107,24 +156,22 @@ pub fn resolve(mods: &ModuleSet, loc: &Locator) -> Result<()> {
match cursor {
NodeCursor::Start(node) => {
if let Some(decl) = Declaration::cast(node) {
open_declaration(env, defg, decl)?;
open_declaration(env, &mut defg, decl)?;
} else if let Some(var) = Variable::cast(node) {
define_variable(env, defg, var)?;
define_variable(env, &mut defg, var)?;
} else if let Some(rec) = Recursion::cast(node) {
open_recursion(env, rec)?;
}
}
NodeCursor::End(node) => {
if Declaration::cast(node).is_some() {
close_declaration(env, defg)?;
close_declaration(env, &mut defg)?;
} else if Recursion::cast(node).is_some() {
close_recursion(env)?;
}
}
}
}

defg.identify_recursion(mods);

Ok(())
Ok(defg.graph())
}
Loading

0 comments on commit cf03327

Please sign in to comment.