fix: ban self-referential structs (#4883)

# Description

## Problem\*

Resolves #3490

## Summary\*

Directly referencing the current struct within its definition currently
fails with a stack overflow.

This PR makes it a normal error, by keeping track of the
currently-resolving structs and ensuring no field resolves to them.

## Additional Context

Mutual recursion is already handled.

## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

---------

Co-authored-by: jfecher <jake@aztecprotocol.com>
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>

compiler/noirc_frontend/src/hir/resolution/errors.rs

@@ -86,6 +86,8 @@ pub enum ResolverError {
     JumpInConstrainedFn { is_break: bool, span: Span },
     #[error("break/continue are only allowed within loops")]
     JumpOutsideLoop { is_break: bool, span: Span },
+    #[error("Self-referential structs are not supported")]
+    SelfReferentialStruct { span: Span },
     #[error("#[inline(tag)] attribute is only allowed on constrained functions")]
     InlineAttributeOnUnconstrained { ident: Ident },
     #[error("#[fold] attribute is only allowed on constrained functions")]
@@ -344,6 +346,13 @@ impl From<ResolverError> for Diagnostic {
                     span,
                 )
             },
+            ResolverError::SelfReferentialStruct { span } => {
+                Diagnostic::simple_error(
+                    "Self-referential structs are not supported".into(),
+                    "".into(),
+                    span,
+                )
+            },
             ResolverError::InlineAttributeOnUnconstrained { ident } => {
                 let name = &ident.0.contents;
 

compiler/noirc_frontend/src/hir/resolution/resolver.rs

@@ -22,7 +22,7 @@ use crate::hir_def::traits::{Trait, TraitConstraint};
 use crate::macros_api::SecondaryAttribute;
 use crate::token::{Attributes, FunctionAttribute};
 use regex::Regex;
-use std::collections::{BTreeMap, HashSet};
+use std::collections::{BTreeMap, BTreeSet, HashSet};
 use std::rc::Rc;
 
 use crate::ast::{
@@ -96,6 +96,21 @@ pub struct Resolver<'a> {
     /// Used to link items to their dependencies in the dependency graph
     current_item: Option<DependencyId>,
 
+    /// In-resolution names
+    ///
+    /// This needs to be a set because we can have multiple in-resolution
+    /// names when resolving structs that are declared in reverse order of their
+    /// dependencies, such as in the following case:
+    ///
+    /// ```
+    /// struct Wrapper {
+    ///     value: Wrapped
+    /// }
+    /// struct Wrapped {
+    /// }
+    /// ```
+    resolving_ids: BTreeSet<StructId>,
+
     /// True if the current module is a contract.
     /// This is usually determined by self.path_resolver.module_id(), but it can
     /// be overridden for impls. Impls are an odd case since the methods within resolve
@@ -159,6 +174,7 @@ impl<'a> Resolver<'a> {
             lambda_stack: Vec::new(),
             current_trait_impl: None,
             current_item: None,
+            resolving_ids: BTreeSet::new(),
             file,
             in_contract,
             in_unconstrained_fn: false,
@@ -616,6 +632,14 @@ impl<'a> Resolver<'a> {
 
         match self.lookup_struct_or_error(path) {
             Some(struct_type) => {
+                if self.resolving_ids.contains(&struct_type.borrow().id) {
+                    self.push_err(ResolverError::SelfReferentialStruct {
+                        span: struct_type.borrow().name.span(),
+                    });
+
+                    return Type::Error;
+                }
+
                 let expected_generic_count = struct_type.borrow().generics.len();
                 if !self.in_contract
                     && self
@@ -886,7 +910,10 @@ impl<'a> Resolver<'a> {
         self.resolve_local_globals();
 
         self.current_item = Some(DependencyId::Struct(struct_id));
+
+        self.resolving_ids.insert(struct_id);
         let fields = vecmap(unresolved.fields, |(ident, typ)| (ident, self.resolve_type(typ)));
+        self.resolving_ids.remove(&struct_id);
 
         (generics, fields, self.errors)
     }

compiler/noirc_frontend/src/tests.rs

@@ -1167,7 +1167,7 @@ fn lambda$f1(mut env$l1: (Field)) -> Field {
     }
 
     #[test]
-    fn deny_cyclic_structs() {
+    fn deny_mutually_recursive_structs() {
         let src = r#"
             struct Foo { bar: Bar }
             struct Bar { foo: Foo }

test_programs/compile_failure/self_referential_struct/Nargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = "self_referential_struct"
+type = "bin"
+authors = [""]
+compiler_version = ">=0.27.0"
+
+[dependencies]

test_programs/compile_failure/self_referential_struct/src/main.nr

@@ -0,0 +1,11 @@
+struct Option2<T> {
+    _is_some: bool,
+    _value: T,
+}
+
+struct SelfReferential
+{
+    prop : Option2<SelfReferential>
+}
+
+fn main(x: SelfReferential) { assert(x._is_some); }