Skip to content

Commit

Permalink
Prevent recursive enums and structs from getting converted from parse…
Browse files Browse the repository at this point in the history
… nodes to AST nodes. (FuelLabs#1768)
  • Loading branch information
otrho authored May 31, 2022
1 parent 1544da8 commit e08bb30
Show file tree
Hide file tree
Showing 13 changed files with 218 additions and 41 deletions.
45 changes: 28 additions & 17 deletions sway-core/src/convert_parse_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,8 @@ pub enum ConvertParseTreeError {
ConstrainedNonExistentType { ty_name: Ident, span: Span },
#[error("__generate_uid does not take arguments")]
GenerateUidTooManyArgs { span: Span },
#[error("recursive types are not supported")]
RecursiveType { span: Span },
}

impl ConvertParseTreeError {
Expand Down Expand Up @@ -228,6 +230,7 @@ impl ConvertParseTreeError {
ConvertParseTreeError::InvalidAttributeArgument { span, .. } => span.clone(),
ConvertParseTreeError::ConstrainedNonExistentType { span, .. } => span.clone(),
ConvertParseTreeError::GenerateUidTooManyArgs { span, .. } => span.clone(),
ConvertParseTreeError::RecursiveType { span } => span.clone(),
}
}
}
Expand Down Expand Up @@ -479,16 +482,20 @@ fn item_struct_to_struct_declaration(
item_struct: ItemStruct,
) -> Result<StructDeclaration, ErrorEmitted> {
let span = item_struct.span();
let fields = item_struct
.fields
.into_inner()
.into_iter()
.map(|type_field| type_field_to_struct_field(ec, type_field))
.collect::<Result<Vec<_>, _>>()?;
if fields.iter().any(
|field| matches!(&field.r#type, TypeInfo::Custom { name, ..} if name == &item_struct.name),
) {
return Err(ec.error(ConvertParseTreeError::RecursiveType { span }));
}
let struct_declaration = StructDeclaration {
name: item_struct.name,
fields: {
item_struct
.fields
.into_inner()
.into_iter()
.map(|type_field| type_field_to_struct_field(ec, type_field))
.collect::<Result<_, _>>()?
},
fields,
type_parameters: generic_params_opt_to_type_parameters(
ec,
item_struct.generics,
Expand All @@ -505,22 +512,26 @@ fn item_enum_to_enum_declaration(
item_enum: ItemEnum,
) -> Result<EnumDeclaration, ErrorEmitted> {
let span = item_enum.span();
let variants = item_enum
.fields
.into_inner()
.into_iter()
.enumerate()
.map(|(tag, type_field)| type_field_to_enum_variant(ec, type_field, tag))
.collect::<Result<Vec<_>, _>>()?;
if variants.iter().any(|variant| {
matches!(&variant.r#type, TypeInfo::Custom { name, ..} if name == &item_enum.name)
}) {
return Err(ec.error(ConvertParseTreeError::RecursiveType { span }));
}
let enum_declaration = EnumDeclaration {
name: item_enum.name,
type_parameters: generic_params_opt_to_type_parameters(
ec,
item_enum.generics,
item_enum.where_clause_opt,
)?,
variants: {
item_enum
.fields
.into_inner()
.into_iter()
.enumerate()
.map(|(tag, type_field)| type_field_to_enum_variant(ec, type_field, tag))
.collect::<Result<_, _>>()?
},
variants,
span,
visibility: pub_token_opt_to_visibility(item_enum.visibility),
};
Expand Down
10 changes: 10 additions & 0 deletions sway-core/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -831,6 +831,14 @@ pub enum CompileError {
call_chain: String, // Pretty list of symbols, e.g., "a, b and c".
span: Span,
},
#[error("Type {name} is recursive, which is unsupported at this time.")]
RecursiveType { name: Ident, span: Span },
#[error("Type {name} is recursive via {type_chain}, which is unsupported at this time.")]
RecursiveTypeChain {
name: Ident,
type_chain: String, // Pretty list of symbols, e.g., "a, b and c".
span: Span,
},
#[error(
"The size of this type is not known. Try putting it on the heap or changing the type."
)]
Expand Down Expand Up @@ -1113,6 +1121,8 @@ impl CompileError {
ArgumentParameterTypeMismatch { span, .. } => span.clone(),
RecursiveCall { span, .. } => span.clone(),
RecursiveCallChain { span, .. } => span.clone(),
RecursiveType { span, .. } => span.clone(),
RecursiveTypeChain { span, .. } => span.clone(),
TypeWithUnknownSize { span, .. } => span.clone(),
InfiniteDependencies { span, .. } => span.clone(),
GMFromExternalContract { span, .. } => span.clone(),
Expand Down
107 changes: 83 additions & 24 deletions sway-core/src/semantic_analysis/node_dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub(crate) fn order_ast_nodes_by_dependency(nodes: Vec<AstNode>) -> CompileResul
DependencyMap::from_iter(nodes.iter().filter_map(Dependencies::gather_from_decl_node));

// Check here for recursive calls now that we have a nice map of the dependencies to help us.
let mut errors = find_recursive_calls(&decl_dependencies);
let mut errors = find_recursive_decls(&decl_dependencies);
if !errors.is_empty() {
// Because we're pulling these errors out of a HashMap they'll probably be in a funny
// order. Here we'll sort them by span start.
Expand All @@ -45,22 +45,27 @@ pub(crate) fn order_ast_nodes_by_dependency(nodes: Vec<AstNode>) -> CompileResul
// -------------------------------------------------------------------------------------------------
// Recursion detection.

fn find_recursive_calls(decl_dependencies: &DependencyMap) -> Vec<CompileError> {
fn find_recursive_decls(decl_dependencies: &DependencyMap) -> Vec<CompileError> {
decl_dependencies
.iter()
.filter_map(|(dep_sym, _)| find_recursive_call(decl_dependencies, dep_sym))
.filter_map(|(dep_sym, _)| find_recursive_decl(decl_dependencies, dep_sym))
.collect()
}

fn find_recursive_call(
fn find_recursive_decl(
decl_dependencies: &DependencyMap,
fn_sym: &DependentSymbol,
dep_sym: &DependentSymbol,
) -> Option<CompileError> {
if let DependentSymbol::Fn(_, Some(fn_span)) = fn_sym {
let mut chain = Vec::new();
find_recursive_call_chain(decl_dependencies, fn_sym, fn_span, &mut chain)
} else {
None
match dep_sym {
DependentSymbol::Fn(_, Some(fn_span)) => {
let mut chain = Vec::new();
find_recursive_call_chain(decl_dependencies, dep_sym, fn_span, &mut chain)
}
DependentSymbol::Symbol(_) => {
let mut chain = Vec::new();
find_recursive_type_chain(decl_dependencies, dep_sym, &mut chain)
}
_otherwise => None,
}
}

Expand Down Expand Up @@ -98,6 +103,34 @@ fn find_recursive_call_chain(
}
}

fn find_recursive_type_chain(
decl_dependencies: &DependencyMap,
dep_sym: &DependentSymbol,
chain: &mut Vec<Ident>,
) -> Option<CompileError> {
if let DependentSymbol::Symbol(sym_ident) = dep_sym {
if chain.iter().any(|seen_sym| seen_sym == sym_ident) {
// See above about it only being an error if we're referring back to the start.
return if &chain[0] != sym_ident {
None
} else {
Some(build_recursive_type_error(sym_ident.clone(), &chain[1..]))
};
}
decl_dependencies.get(dep_sym).and_then(|deps_set| {
chain.push(sym_ident.clone());
let result = deps_set
.deps
.iter()
.find_map(|dep_sym| find_recursive_type_chain(decl_dependencies, dep_sym, chain));
chain.pop();
result
})
} else {
None
}
}

fn build_recursion_error(fn_sym: Ident, span: Span, chain: &[Ident]) -> CompileError {
match chain.len() {
// An empty chain indicates immediate recursion.
Expand Down Expand Up @@ -128,6 +161,34 @@ fn build_recursion_error(fn_sym: Ident, span: Span, chain: &[Ident]) -> CompileE
}
}

fn build_recursive_type_error(name: Ident, chain: &[Ident]) -> CompileError {
let span = name.span().clone();
match chain.len() {
// An empty chain indicates immediate recursion.
0 => CompileError::RecursiveType { name, span },
// Chain entries indicate mutual recursion.
1 => CompileError::RecursiveTypeChain {
name,
type_chain: chain[0].as_str().to_string(),
span,
},
n => {
let mut msg = chain[0].as_str().to_string();
for ident in &chain[1..(n - 1)] {
msg.push_str(", ");
msg.push_str(ident.as_str());
}
msg.push_str(" and ");
msg.push_str(chain[n - 1].as_str());
CompileError::RecursiveTypeChain {
name,
type_chain: msg,
span,
}
}
}
}

// -------------------------------------------------------------------------------------------------
// Dependency gathering.

Expand Down Expand Up @@ -375,9 +436,8 @@ impl Dependencies {
fields,
..
} => {
self.deps.insert(DependentSymbol::Symbol(
struct_name.suffix.as_str().to_string(),
));
self.deps
.insert(DependentSymbol::Symbol(struct_name.suffix.clone()));
self.gather_from_iter(fields.iter(), |deps, field| {
deps.gather_from_expr(&field.value)
})
Expand Down Expand Up @@ -472,14 +532,13 @@ impl Dependencies {
self.deps.insert(if is_fn_app {
DependentSymbol::Fn(call_path.suffix.clone(), None)
} else {
DependentSymbol::Symbol(call_path.suffix.as_str().to_string())
DependentSymbol::Symbol(call_path.suffix.clone())
});
} else if use_prefix && call_path.prefixes.len() == 1 {
// Here we can use the prefix (e.g., for 'Enum::Variant' -> 'Enum') as long is it's
// only a single element.
self.deps.insert(DependentSymbol::Symbol(
call_path.prefixes[0].as_str().to_string(),
));
self.deps
.insert(DependentSymbol::Symbol(call_path.prefixes[0].clone()));
}
self
}
Expand Down Expand Up @@ -509,7 +568,7 @@ impl Dependencies {
name,
type_arguments,
} => {
self.deps.insert(DependentSymbol::Symbol(name.to_string()));
self.deps.insert(DependentSymbol::Symbol(name.clone()));
self.gather_from_type_arguments(type_arguments)
}
TypeInfo::Tuple(elems) => self.gather_from_iter(elems.iter(), |deps, elem| {
Expand Down Expand Up @@ -544,7 +603,7 @@ impl Dependencies {

#[derive(Debug, Eq)]
enum DependentSymbol {
Symbol(String),
Symbol(Ident),
Fn(Ident, Option<Span>),
Impl(Ident, String, String), // Trait or self, type implementing for, and method names concatenated.
}
Expand Down Expand Up @@ -599,11 +658,11 @@ fn decl_name(decl: &Declaration) -> Option<DependentSymbol> {
decl.name.clone(),
Some(decl.span.clone()),
)),
Declaration::ConstantDeclaration(decl) => dep_sym(decl.name.as_str().to_string()),
Declaration::StructDeclaration(decl) => dep_sym(decl.name.as_str().to_string()),
Declaration::EnumDeclaration(decl) => dep_sym(decl.name.as_str().to_string()),
Declaration::TraitDeclaration(decl) => dep_sym(decl.name.as_str().to_string()),
Declaration::AbiDeclaration(decl) => dep_sym(decl.name.as_str().to_string()),
Declaration::ConstantDeclaration(decl) => dep_sym(decl.name.clone()),
Declaration::StructDeclaration(decl) => dep_sym(decl.name.clone()),
Declaration::EnumDeclaration(decl) => dep_sym(decl.name.clone()),
Declaration::TraitDeclaration(decl) => dep_sym(decl.name.clone()),
Declaration::AbiDeclaration(decl) => dep_sym(decl.name.clone()),

// These have the added complexity of converting CallPath and/or TypeInfo into a name.
Declaration::ImplSelf(decl) => {
Expand Down
3 changes: 3 additions & 0 deletions test/src/e2e_vm_tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,9 @@ pub fn run(filter_regex: Option<regex::Regex>) {
"should_fail/match_expressions_empty_arms",
"should_fail/type_mismatch_error_message",
"should_fail/non_literal_const_decl",
"should_fail/recursive_enum",
"should_fail/recursive_struct",
"should_fail/recursive_type_chain",
];
number_of_tests_run += negative_project_names.iter().fold(0, |acc, name| {
if filter(name) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[[package]]
name = 'recursive_enum'
dependencies = []
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[project]
name = "recursive_enum"
authors = ["Fuel Labs <[email protected]>"]
entry = "main.sw"
license = "Apache-2.0"
implicit-std = false
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
script;

enum E {
Eins: bool,
Zwei: u64,
Drei: E,
}

fn main() {

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[[package]]
name = 'recursive_struct'
dependencies = []
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[project]
name = "recursive_struct"
authors = ["Fuel Labs <[email protected]>"]
entry = "main.sw"
license = "Apache-2.0"
implicit-std = false
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
script;

struct S {
Eins: bool,
Zwei: u64,
Drei: S,
}

fn main() {

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[[package]]
name = 'recursive_type_chain'
dependencies = []
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[project]
name = "recursive_type_chain"
authors = ["Fuel Labs <[email protected]>"]
entry = "main.sw"
license = "Apache-2.0"
implicit-std = false
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
script;

enum E {
Eins: F,
}

enum F {
Zwei: G,
}

enum G {
Drei: E,
}

enum H {
Vier: I,
}

enum I {
Funf: H,
}

struct S {
one: T,
}

struct T {
two: S,
}

struct X {
three: Y,
}

enum Y {
four: Z,
}

struct Z {
five: X,
}

fn main() {

}

0 comments on commit e08bb30

Please sign in to comment.