Skip to content

Commit

Permalink
Introduce recursive analysis via cycle finding in dependency graph. (#…
Browse files Browse the repository at this point in the history
…5210)

## Description

This PR introduces a new recursive analysis pass, which is implemented
for for regular and impl traits functions.

It works computing a dependency graph, and then running the Johnson
graph cycle finding algorithm in the graph.

Regular functions still use the old recursive analysis pass due to
issues with disabling the old dependency pass. When disabling the old
recursion analysis code for regular functions, we get a stack overflow
in the existing dependency map building code, which is shared with the
existing type recursion analysis code.

Closes #4954.

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.

Co-authored-by: Joshua Batty <joshpbatty@gmail.com>
Co-authored-by: Sophie Dankel <47993817+sdankel@users.noreply.github.com>
  • Loading branch information
3 people authored Dec 1, 2023
1 parent 3ab743f commit e70cb77
Show file tree
Hide file tree
Showing 15 changed files with 394 additions and 56 deletions.
13 changes: 13 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions sway-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ etk-dasm = { package = "fuel-etk-dasm", version = "0.3.1-dev" }
etk-ops = { package = "fuel-etk-ops", version = "0.3.1-dev" }
fuel-abi-types = "0.1"
fuel-vm = { workspace = true, features = ["serde"] }
graph-cycles = "0.1.0"
hashbrown = "0.13.1"
hex = { version = "0.4", optional = true }
im = "15.0"
Expand Down
43 changes: 43 additions & 0 deletions sway-core/src/language/ty/ast_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,49 @@ impl TyAstNode {
TyAstNodeContent::SideEffect(_) | TyAstNodeContent::Error(_, _) => {}
}
}

pub(crate) fn check_recursive(
&self,
engines: &Engines,
handler: &Handler,
) -> Result<(), ErrorEmitted> {
handler.scope(|handler| {
match &self.content {
TyAstNodeContent::Declaration(node) => match node {
TyDecl::VariableDecl(_decl) => {}
TyDecl::ConstantDecl(_decl) => {}
TyDecl::TraitTypeDecl(_) => {}
TyDecl::FunctionDecl(decl) => {
let fn_decl_id = decl.decl_id;
let mut ctx = TypeCheckAnalysisContext::new(engines);
let _ = fn_decl_id.type_check_analyze(handler, &mut ctx);
let _ = ctx.check_recursive_calls(handler);
}
TyDecl::ImplTrait(decl) => {
let decl = engines.de().get(&decl.decl_id);
for item in decl.items.iter() {
let mut ctx = TypeCheckAnalysisContext::new(engines);
let _ = item.type_check_analyze(handler, &mut ctx);
let _ = ctx.check_recursive_calls(handler);
}
}
TyDecl::AbiDecl(_)
| TyDecl::GenericTypeForFunctionScope(_)
| TyDecl::ErrorRecovery(_, _)
| TyDecl::StorageDecl(_)
| TyDecl::TraitDecl(_)
| TyDecl::StructDecl(_)
| TyDecl::EnumDecl(_)
| TyDecl::EnumVariantDecl(_)
| TyDecl::TypeAliasDecl(_) => {}
},
TyAstNodeContent::Expression(_node) => {}
TyAstNodeContent::ImplicitReturnExpression(_node) => {}
TyAstNodeContent::SideEffect(_) | TyAstNodeContent::Error(_, _) => {}
};
Ok(())
})
}
}

#[derive(Clone, Debug)]
Expand Down
3 changes: 1 addition & 2 deletions sway-core/src/language/ty/declaration/trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,7 @@ impl TypeCheckAnalysis for TyTraitItem {

match self {
TyTraitItem::Fn(node) => {
let item_fn = decl_engine.get_function(node);
item_fn.type_check_analyze(handler, ctx)?;
node.type_check_analyze(handler, ctx)?;
}
TyTraitItem::Constant(node) => {
let item_const = decl_engine.get_constant(node);
Expand Down
8 changes: 7 additions & 1 deletion sway-core/src/language/ty/expression/expression_variant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -908,12 +908,18 @@ impl TypeCheckAnalysis for TyExpressionVariant {
match self {
TyExpressionVariant::Literal(_) => {}
TyExpressionVariant::FunctionApplication { fn_ref, .. } => {
let fn_node = ctx.get_node_from_impl_trait_fn_ref_app(fn_ref);
let fn_decl_id = ctx.get_normalized_fn_node_id(fn_ref.id());

let fn_node = ctx.get_node_for_fn_decl(&fn_decl_id);
if let Some(fn_node) = fn_node {
ctx.add_edge_from_current(
fn_node,
TyNodeDepGraphEdge(TyNodeDepGraphEdgeInfo::FnApp),
);

if !ctx.node_stack.contains(&fn_node) {
let _ = fn_decl_id.type_check_analyze(handler, ctx);
}
}
}
TyExpressionVariant::LazyOperator { lhs, rhs, .. } => {
Expand Down
20 changes: 19 additions & 1 deletion sway-core/src/language/ty/module.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use sway_error::handler::Handler;
use sway_error::handler::{ErrorEmitted, Handler};
use sway_types::Span;

use crate::{
Expand Down Expand Up @@ -86,6 +86,24 @@ impl TyModule {
node.check_deprecated(engines, handler, allow_deprecated);
}
}

pub(crate) fn check_recursive(
&self,
engines: &Engines,
handler: &Handler,
) -> Result<(), ErrorEmitted> {
handler.scope(|handler| {
for (_, submodule) in self.submodules.iter() {
let _ = submodule.module.check_recursive(engines, handler);
}

for node in self.all_nodes.iter() {
let _ = node.check_recursive(engines, handler);
}

Ok(())
})
}
}

impl<'module> Iterator for SubmodulesRecursive<'module> {
Expand Down
8 changes: 8 additions & 0 deletions sway-core/src/language/ty/program.rs
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,14 @@ impl TyProgram {
self.root
.check_deprecated(engines, handler, &mut allow_deprecated);
}

pub fn check_recursive(
&self,
engines: &Engines,
handler: &Handler,
) -> Result<(), ErrorEmitted> {
self.root.check_recursive(engines, handler)
}
}

impl CollectTypesMetadata for TyProgram {
Expand Down
11 changes: 7 additions & 4 deletions sway-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ pub use build_config::{BuildConfig, BuildTarget};
use control_flow_analysis::ControlFlowGraph;
use metadata::MetadataManager;
use query_engine::{ModuleCacheKey, ModulePath, ProgramsCacheEntry};
use semantic_analysis::{TypeCheckAnalysis, TypeCheckAnalysisContext};
use std::collections::hash_map::DefaultHasher;
use std::collections::HashMap;
use std::hash::{Hash, Hasher};
Expand Down Expand Up @@ -485,9 +484,13 @@ pub fn parsed_to_ast(

typed_program.check_deprecated(engines, handler);

// Analyze the AST for dependency information.
let mut ctx = TypeCheckAnalysisContext::new(engines);
typed_program.type_check_analyze(handler, &mut ctx)?;
match typed_program.check_recursive(engines, handler) {
Ok(()) => {}
Err(e) => {
handler.dedup();
return Err(e);
}
};

// Collect information about the types used in this program
let types_metadata_result = typed_program
Expand Down
45 changes: 44 additions & 1 deletion sway-core/src/semantic_analysis/ast_node/declaration/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ use sway_error::{
};

use crate::{
decl_engine::{DeclId, DeclRefFunction},
language::{
parsed::*,
ty::{self, TyCodeBlock},
ty::{self, TyCodeBlock, TyFunctionDecl},
CallPath, Visibility,
},
semantic_analysis::{type_check_context::EnforceTypeArguments, *},
Expand Down Expand Up @@ -235,6 +236,48 @@ fn unify_return_statements(
})
}

impl TypeCheckAnalysis for DeclId<TyFunctionDecl> {
fn type_check_analyze(
&self,
handler: &Handler,
ctx: &mut TypeCheckAnalysisContext,
) -> Result<(), ErrorEmitted> {
handler.scope(|handler| {
let node = ctx.get_node_for_fn_decl(self);
if let Some(node) = node {
ctx.node_stack.push(node);

let item_fn = ctx.engines.de().get_function(self);
let _ = item_fn.type_check_analyze(handler, ctx);

ctx.node_stack.pop();
}
Ok(())
})
}
}

impl TypeCheckAnalysis for DeclRefFunction {
fn type_check_analyze(
&self,
handler: &Handler,
ctx: &mut TypeCheckAnalysisContext,
) -> Result<(), ErrorEmitted> {
handler.scope(|handler| {
let node = ctx.get_node_for_fn_decl(self.id());
if let Some(node) = node {
ctx.node_stack.push(node);

let item_fn = ctx.engines.de().get_function(self);
let _ = item_fn.type_check_analyze(handler, ctx);

ctx.node_stack.pop();
}
Ok(())
})
}
}

impl TypeCheckAnalysis for ty::TyFunctionDecl {
fn type_check_analyze(
&self,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1499,18 +1499,15 @@ impl TypeCheckAnalysis for ty::ImplTrait {
let impl_trait = decl_engine.get_impl_trait(&self.decl_id);

// Lets create a graph node for the impl trait and for every item in the trait.
ctx.push_impl_trait(self);
ctx.push_nodes_for_impl_trait(self);

// Now lets analyze each impl trait item.
for (i, item) in impl_trait.items.iter().enumerate() {
let node = ctx.items_node_stack[i];
ctx.node_stack.push(node);
let _node = ctx.items_node_stack[i];
item.type_check_analyze(handler, ctx)?;
ctx.node_stack.pop();
}

// Clear the work-in-progress node stacks.
ctx.node_stack.clear();
// Clear the work-in-progress node stack.
ctx.items_node_stack.clear();

Ok(())
Expand Down
Loading

0 comments on commit e70cb77

Please sign in to comment.