Skip to content

Commit

Permalink
Fixes stack overflow when using recursive trait.
Browse files Browse the repository at this point in the history
ReplaceDecls, CollectTypesMetadata, HashWithEngines now have a mechanism that interrupts recursive visiting.

The mechanism works by adding TypeIds or DeclIds to a HashMap or HashSet and no longer visiting TypeIds or DeclIds on the passed collection.

This was necessary because in some cases while visiting a declaration we can go back to the beginning because of recursive DeclIds.

I suspect these changes will also be useful to #3018

This partially fixes #5504, which is still failing because we have yet to fix a recursion problem in the IR side of the compiler.
  • Loading branch information
esdrubal committed Jan 24, 2024
1 parent 546bd99 commit 52ddd02
Show file tree
Hide file tree
Showing 46 changed files with 995 additions and 361 deletions.
63 changes: 51 additions & 12 deletions sway-core/src/decl_engine/ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@
//! `fn my_function() { .. }`, and to use [DeclRef] for cases like function
//! application `my_function()`.

use std::hash::{Hash, Hasher};
use std::{
collections::{HashMap, HashSet},
hash::{Hash, Hasher},
};

use sway_error::handler::{ErrorEmitted, Handler};
use sway_types::{Ident, Named, Span, Spanned};
Expand Down Expand Up @@ -159,20 +162,37 @@ impl<T> DeclRef<DeclId<T>>
where
AssociatedItemDeclId: From<DeclId<T>>,
DeclEngine: DeclEngineIndex<T>,
T: Named + Spanned + ReplaceDecls + std::fmt::Debug + Clone,
T: Named + Spanned + ReplaceDecls + std::fmt::Debug + Clone + 'static,
{
pub(crate) fn replace_decls_and_insert_new_with_parent(
&self,
decl_mapping: &DeclMapping,
handler: &Handler,
ctx: &mut TypeCheckContext,
decls_replaced: &mut HashMap<(usize, std::any::TypeId), (usize, Span)>,
) -> Result<Self, ErrorEmitted> {
let decl_engine = ctx.engines().de();
let key = (self.id.inner(), std::any::TypeId::of::<T>());
if let Some((ref_id, ref_span)) = decls_replaced.get(&key) {
return Ok(DeclRef::new(
Ident::new(ref_span.clone()),
DeclId::<T>::new(*ref_id),
ref_span.clone(),
));
}
let mut decl = (*decl_engine.get(&self.id)).clone();
decl.replace_decls(decl_mapping, handler, ctx)?;
Ok(decl_engine
.insert(decl)
.with_parent(decl_engine, self.id.into()))
let decl_replacement_ref = decl_engine
.insert(decl.clone())
.with_parent(decl_engine, self.id.into());

let mut decls_replaced_updated = decls_replaced.clone();
decls_replaced_updated.insert(key, (decl_replacement_ref.id().inner(), self.span()));

decl.replace_decls(decl_mapping, handler, ctx, &mut decls_replaced_updated)?;

decl_engine.replace(*decl_replacement_ref.id(), decl);

Ok(decl_replacement_ref)
}
}

Expand Down Expand Up @@ -214,9 +234,14 @@ where
impl<T> HashWithEngines for DeclRef<DeclId<T>>
where
DeclEngine: DeclEngineIndex<T>,
T: Named + Spanned + HashWithEngines,
T: Named + Spanned + HashWithEngines + 'static,
{
fn hash<H: Hasher>(&self, state: &mut H, engines: &Engines) {
fn hash<H: Hasher>(
&self,
state: &mut H,
engines: &Engines,
already_in_hash: &mut HashSet<(usize, std::any::TypeId)>,
) {
let decl_engine = engines.de();
let DeclRef {
name,
Expand All @@ -228,7 +253,15 @@ where
subst_list: _,
} = self;
name.hash(state);
decl_engine.get(id).hash(state, engines);
let key = (id.inner(), std::any::TypeId::of::<T>());
if already_in_hash.contains(&key) {
return;
}
let mut already_in_hash_updated = already_in_hash.clone();
already_in_hash_updated.insert(key);
decl_engine
.get(id)
.hash(state, engines, &mut already_in_hash_updated);
}
}

Expand All @@ -253,19 +286,24 @@ impl PartialEqWithEngines for DeclRefMixedInterface {
}

impl HashWithEngines for DeclRefMixedInterface {
fn hash<H: Hasher>(&self, state: &mut H, engines: &Engines) {
fn hash<H: Hasher>(
&self,
state: &mut H,
engines: &Engines,
already_in_hash: &mut HashSet<(usize, std::any::TypeId)>,
) {
match self.id {
InterfaceDeclId::Abi(id) => {
state.write_u8(0);
let decl_engine = engines.de();
let decl = decl_engine.get(&id);
decl.hash(state, engines);
decl.hash(state, engines, already_in_hash);
}
InterfaceDeclId::Trait(id) => {
state.write_u8(1);
let decl_engine = engines.de();
let decl = decl_engine.get(&id);
decl.hash(state, engines);
decl.hash(state, engines, already_in_hash);
}
}
}
Expand Down Expand Up @@ -296,6 +334,7 @@ impl ReplaceDecls for DeclRefFunction {
decl_mapping: &DeclMapping,
_handler: &Handler,
ctx: &mut TypeCheckContext,
_decls_replaced: &mut HashMap<(usize, std::any::TypeId), (usize, Span)>,
) -> Result<(), ErrorEmitted> {
let engines = ctx.engines();
let decl_engine = engines.de();
Expand Down
7 changes: 6 additions & 1 deletion sway-core/src/decl_engine/replace_decls.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use std::collections::HashMap;

use sway_error::handler::{ErrorEmitted, Handler};
use sway_types::Span;

use crate::{
engine_threading::Engines,
Expand All @@ -14,16 +17,18 @@ pub trait ReplaceDecls {
decl_mapping: &DeclMapping,
handler: &Handler,
ctx: &mut TypeCheckContext,
decls_replaced: &mut HashMap<(usize, std::any::TypeId), (usize, Span)>,
) -> Result<(), ErrorEmitted>;

fn replace_decls(
&mut self,
decl_mapping: &DeclMapping,
handler: &Handler,
ctx: &mut TypeCheckContext,
decls_replaced: &mut HashMap<(usize, std::any::TypeId), (usize, Span)>,
) -> Result<(), ErrorEmitted> {
if !decl_mapping.is_empty() {
self.replace_decls_inner(decl_mapping, handler, ctx)?;
self.replace_decls_inner(decl_mapping, handler, ctx, decls_replaced)?;
}

Ok(())
Expand Down
62 changes: 50 additions & 12 deletions sway-core/src/engine_threading.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
use crate::{decl_engine::DeclEngine, query_engine::QueryEngine, type_system::TypeEngine};
use crate::{
decl_engine::{DeclEngine, DeclIdIndexType},
query_engine::QueryEngine,
type_system::TypeEngine,
};
use std::{
cmp::Ordering,
collections::HashSet,
fmt,
hash::{BuildHasher, Hash, Hasher},
};
Expand Down Expand Up @@ -89,7 +94,11 @@ impl<T: DebugWithEngines> fmt::Debug for WithEngines<'_, T> {

impl<T: HashWithEngines> Hash for WithEngines<'_, T> {
fn hash<H: Hasher>(&self, state: &mut H) {
self.thing.hash(state, self.engines)
self.thing.hash(
state,
self.engines,
&mut HashSet::<(DeclIdIndexType, std::any::TypeId)>::new(),
)
}
}

Expand Down Expand Up @@ -194,35 +203,60 @@ impl<T: DebugWithEngines> DebugWithEngines for Vec<T> {
}

pub trait HashWithEngines {
fn hash<H: Hasher>(&self, state: &mut H, engines: &Engines);
fn hash<H: Hasher>(
&self,
state: &mut H,
engines: &Engines,
already_in_hash: &mut HashSet<(usize, std::any::TypeId)>,
);
}

impl<T: HashWithEngines + ?Sized> HashWithEngines for &T {
fn hash<H: Hasher>(&self, state: &mut H, engines: &Engines) {
(*self).hash(state, engines)
fn hash<H: Hasher>(
&self,
state: &mut H,
engines: &Engines,
already_in_hash: &mut HashSet<(usize, std::any::TypeId)>,
) {
(*self).hash(state, engines, already_in_hash)
}
}

impl<T: HashWithEngines> HashWithEngines for Option<T> {
fn hash<H: Hasher>(&self, state: &mut H, engines: &Engines) {
fn hash<H: Hasher>(
&self,
state: &mut H,
engines: &Engines,
already_in_hash: &mut HashSet<(usize, std::any::TypeId)>,
) {
match self {
None => state.write_u8(0),
Some(x) => x.hash(state, engines),
Some(x) => x.hash(state, engines, already_in_hash),
}
}
}

impl<T: HashWithEngines> HashWithEngines for [T] {
fn hash<H: Hasher>(&self, state: &mut H, engines: &Engines) {
fn hash<H: Hasher>(
&self,
state: &mut H,
engines: &Engines,
already_in_hash: &mut HashSet<(usize, std::any::TypeId)>,
) {
for x in self {
x.hash(state, engines)
x.hash(state, engines, already_in_hash)
}
}
}

impl<T: HashWithEngines> HashWithEngines for Box<T> {
fn hash<H: Hasher>(&self, state: &mut H, engines: &Engines) {
(**self).hash(state, engines)
fn hash<H: Hasher>(
&self,
state: &mut H,
engines: &Engines,
already_in_hash: &mut HashSet<(usize, std::any::TypeId)>,
) {
(**self).hash(state, engines, already_in_hash)
}
}

Expand Down Expand Up @@ -308,7 +342,11 @@ where
{
move |key: &K| {
let mut state = hash_builder.build_hasher();
key.hash(&mut state, engines);
key.hash(
&mut state,
engines,
&mut HashSet::<(DeclIdIndexType, std::any::TypeId)>::new(),
);
state.finish()
}
}
21 changes: 16 additions & 5 deletions sway-core/src/language/call_path.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::{
cmp::Ordering,
collections::HashSet,
fmt,
hash::{Hash, Hasher},
sync::Arc,
Expand Down Expand Up @@ -28,13 +29,18 @@ pub struct CallPathTree {
}

impl HashWithEngines for CallPathTree {
fn hash<H: Hasher>(&self, state: &mut H, engines: &Engines) {
fn hash<H: Hasher>(
&self,
state: &mut H,
engines: &Engines,
already_in_hash: &mut HashSet<(usize, std::any::TypeId)>,
) {
let CallPathTree {
qualified_call_path,
children,
} = self;
qualified_call_path.hash(state, engines);
children.hash(state, engines);
qualified_call_path.hash(state, engines, already_in_hash);
children.hash(state, engines, already_in_hash);
}
}

Expand Down Expand Up @@ -109,13 +115,18 @@ impl QualifiedCallPath {
}

impl HashWithEngines for QualifiedCallPath {
fn hash<H: Hasher>(&self, state: &mut H, engines: &Engines) {
fn hash<H: Hasher>(
&self,
state: &mut H,
engines: &Engines,
already_in_hash: &mut HashSet<(usize, std::any::TypeId)>,
) {
let QualifiedCallPath {
call_path,
qualified_path_root,
} = self;
call_path.hash(state);
qualified_path_root.hash(state, engines);
qualified_path_root.hash(state, engines, already_in_hash);
}
}

Expand Down
14 changes: 11 additions & 3 deletions sway-core/src/language/parsed/declaration/trait.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use std::hash::{Hash, Hasher};
use std::{
collections::HashSet,
hash::{Hash, Hasher},
};

use super::{ConstantDeclaration, FunctionDeclaration, FunctionParameter};

Expand Down Expand Up @@ -57,10 +60,15 @@ impl PartialEqWithEngines for Supertrait {
}

impl HashWithEngines for Supertrait {
fn hash<H: Hasher>(&self, state: &mut H, engines: &Engines) {
fn hash<H: Hasher>(
&self,
state: &mut H,
engines: &Engines,
already_in_hash: &mut HashSet<(usize, std::any::TypeId)>,
) {
let Supertrait { name, decl_ref } = self;
name.hash(state);
decl_ref.hash(state, engines);
decl_ref.hash(state, engines, already_in_hash);
}
}

Expand Down
16 changes: 12 additions & 4 deletions sway-core/src/language/parsed/expression/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{cmp::Ordering, fmt, hash::Hasher};
use std::{cmp::Ordering, collections::HashSet, fmt, hash::Hasher};

use crate::{
engine_threading::{
Expand Down Expand Up @@ -119,15 +119,23 @@ pub struct QualifiedPathRootTypes {
}

impl HashWithEngines for QualifiedPathRootTypes {
fn hash<H: Hasher>(&self, state: &mut H, engines: &Engines) {
fn hash<H: Hasher>(
&self,
state: &mut H,
engines: &Engines,
already_in_hash: &mut HashSet<(usize, std::any::TypeId)>,
) {
let QualifiedPathRootTypes {
ty,
as_trait,
// ignored fields
as_trait_span: _,
} = self;
ty.hash(state, engines);
engines.te().get(*as_trait).hash(state, engines);
ty.hash(state, engines, already_in_hash);
engines
.te()
.get(*as_trait)
.hash(state, engines, already_in_hash);
}
}

Expand Down
Loading

0 comments on commit 52ddd02

Please sign in to comment.