Skip to content

Commit

Permalink
feat: Allow globals to refer to any expression (#4293)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Resolves #1734

## Summary\*

Allows globals to be defined as any expression using the dependency
graph to ensure there are no cycles.

Note that globals defined via a function call are allowed: `global FOO =
bar();`, although the function will be
re-called each time the global is used since globals are just
substituted for their definitions where they are used.

Example program:

```rs
global A = B;
global B = foo();

fn foo() -> Field {
    A
}
```

Example error:

```
error: Dependency cycle found
  ┌─ /Users/user/Code/Noir/noir/short/src/main.nr:2:8
  │
2 │ global B = foo();
  │        - 'B' recursively depends on itself: B -> foo -> A -> B
  │

```

## Additional Context

This is a somewhat longer PR because it also includes a refactoring to
use a new `GlobalId` over `StmtId`. I found
this necessary to allow referring to globals in cycles where another
global may not be resolved yet.

WIP because I still need to:
- ~~Track dependencies within functions~~
- ~~Update documentation~~

## Documentation\*

Check one:
- [ ] No documentation needed.
- [x] Documentation included in this PR.
- [ ] **[Exceptional Case]** 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: José Pedro Sousa <[email protected]>
  • Loading branch information
jfecher and signorecello authored Feb 8, 2024
1 parent d86ff1a commit 479330e
Show file tree
Hide file tree
Showing 13 changed files with 286 additions and 184 deletions.
22 changes: 11 additions & 11 deletions compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use crate::hir::type_check::{type_check_func, TypeCheckError, TypeChecker};
use crate::hir::Context;

use crate::macros_api::{MacroError, MacroProcessor};
use crate::node_interner::{FuncId, NodeInterner, StmtId, StructId, TraitId, TypeAliasId};
use crate::node_interner::{FuncId, GlobalId, NodeInterner, StructId, TraitId, TypeAliasId};

use crate::parser::{ParserError, SortedModule};
use crate::{
Expand Down Expand Up @@ -109,7 +109,7 @@ pub struct UnresolvedTypeAlias {
pub struct UnresolvedGlobal {
pub file_id: FileId,
pub module_id: LocalModuleId,
pub stmt_id: StmtId,
pub global_id: GlobalId,
pub stmt_def: LetStatement,
}

Expand Down Expand Up @@ -317,7 +317,7 @@ impl DefCollector {
// Must resolve structs before we resolve globals.
errors.extend(resolve_structs(context, def_collector.collected_types, crate_id));

// We must wait to resolve non-integer globals until after we resolve structs since structs
// We must wait to resolve non-integer globals until after we resolve structs since struct
// globals will need to reference the struct type they're initialized to to ensure they are valid.
resolved_globals.extend(resolve_globals(context, other_globals, crate_id));
errors.extend(resolved_globals.errors);
Expand Down Expand Up @@ -436,15 +436,15 @@ fn filter_literal_globals(

fn type_check_globals(
interner: &mut NodeInterner,
global_ids: Vec<(FileId, StmtId)>,
global_ids: Vec<(FileId, GlobalId)>,
) -> Vec<(CompilationError, fm::FileId)> {
global_ids
.iter()
.flat_map(|(file_id, stmt_id)| {
TypeChecker::check_global(stmt_id, interner)
.into_iter()
.flat_map(|(file_id, global_id)| {
TypeChecker::check_global(global_id, interner)
.iter()
.cloned()
.map(|e| (e.into(), *file_id))
.map(|e| (e.into(), file_id))
.collect::<Vec<_>>()
})
.collect()
Expand All @@ -455,12 +455,12 @@ fn type_check_functions(
file_func_ids: Vec<(FileId, FuncId)>,
) -> Vec<(CompilationError, fm::FileId)> {
file_func_ids
.iter()
.into_iter()
.flat_map(|(file, func)| {
type_check_func(interner, *func)
type_check_func(interner, func)
.iter()
.cloned()
.map(|e| (e.into(), *file))
.map(|e| (e.into(), file))
.collect::<Vec<_>>()
})
.collect()
Expand Down
19 changes: 11 additions & 8 deletions compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,12 @@ impl<'a> ModCollector<'a> {
for global in globals {
let name = global.pattern.name_ident().clone();

// First create dummy function in the DefInterner
// So that we can get a StmtId
let stmt_id = context.def_interner.push_empty_global();
let global_id =
context.def_interner.push_empty_global(name.clone(), self.module_id, self.file_id);

// Add the statement to the scope so its path can be looked up later
let result =
self.def_collector.def_map.modules[self.module_id.0].declare_global(name, stmt_id);
let result = self.def_collector.def_map.modules[self.module_id.0]
.declare_global(name, global_id);

if let Err((first_def, second_def)) = result {
let err = DefCollectorErrorKind::Duplicate {
Expand All @@ -109,7 +108,7 @@ impl<'a> ModCollector<'a> {
self.def_collector.collected_globals.push(UnresolvedGlobal {
file_id: self.file_id,
module_id: self.module_id,
stmt_id,
global_id,
stmt_def: global,
});
}
Expand Down Expand Up @@ -440,11 +439,15 @@ impl<'a> ModCollector<'a> {
}
}
TraitItem::Constant { name, .. } => {
let stmt_id = context.def_interner.push_empty_global();
let global_id = context.def_interner.push_empty_global(
name.clone(),
trait_id.0.local_id,
self.file_id,
);

if let Err((first_def, second_def)) = self.def_collector.def_map.modules
[trait_id.0.local_id.0]
.declare_global(name.clone(), stmt_id)
.declare_global(name.clone(), global_id)
{
let error = DefCollectorErrorKind::Duplicate {
typ: DuplicateType::TraitAssociatedConst,
Expand Down
4 changes: 2 additions & 2 deletions compiler/noirc_frontend/src/hir/def_map/module_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::collections::HashMap;
use noirc_errors::Location;

use crate::{
node_interner::{FuncId, StmtId, StructId, TraitId, TypeAliasId},
node_interner::{FuncId, GlobalId, StructId, TraitId, TypeAliasId},
Ident,
};

Expand Down Expand Up @@ -76,7 +76,7 @@ impl ModuleData {
self.definitions.remove_definition(name);
}

pub fn declare_global(&mut self, name: Ident, id: StmtId) -> Result<(), (Ident, Ident)> {
pub fn declare_global(&mut self, name: Ident, id: GlobalId) -> Result<(), (Ident, Ident)> {
self.declare(name, id.into(), None)
}

Expand Down
18 changes: 9 additions & 9 deletions compiler/noirc_frontend/src/hir/def_map/module_def.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::node_interner::{FuncId, StmtId, StructId, TraitId, TypeAliasId};
use crate::node_interner::{FuncId, GlobalId, StructId, TraitId, TypeAliasId};

use super::ModuleId;

Expand All @@ -10,7 +10,7 @@ pub enum ModuleDefId {
TypeId(StructId),
TypeAliasId(TypeAliasId),
TraitId(TraitId),
GlobalId(StmtId),
GlobalId(GlobalId),
}

impl ModuleDefId {
Expand Down Expand Up @@ -42,9 +42,9 @@ impl ModuleDefId {
}
}

pub fn as_global(&self) -> Option<StmtId> {
pub fn as_global(&self) -> Option<GlobalId> {
match self {
ModuleDefId::GlobalId(stmt_id) => Some(*stmt_id),
ModuleDefId::GlobalId(global_id) => Some(*global_id),
_ => None,
}
}
Expand Down Expand Up @@ -88,9 +88,9 @@ impl From<TypeAliasId> for ModuleDefId {
}
}

impl From<StmtId> for ModuleDefId {
fn from(stmt_id: StmtId) -> Self {
ModuleDefId::GlobalId(stmt_id)
impl From<GlobalId> for ModuleDefId {
fn from(global_id: GlobalId) -> Self {
ModuleDefId::GlobalId(global_id)
}
}

Expand Down Expand Up @@ -162,13 +162,13 @@ impl TryFromModuleDefId for TraitId {
}
}

impl TryFromModuleDefId for StmtId {
impl TryFromModuleDefId for GlobalId {
fn try_from(id: ModuleDefId) -> Option<Self> {
id.as_global()
}

fn dummy_id() -> Self {
StmtId::dummy_id()
GlobalId::dummy_id()
}

fn description() -> String {
Expand Down
15 changes: 6 additions & 9 deletions compiler/noirc_frontend/src/hir/resolution/globals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ use crate::{
def_map::ModuleId,
Context,
},
node_interner::StmtId,
node_interner::GlobalId,
};
use fm::FileId;
use iter_extended::vecmap;

pub(crate) struct ResolvedGlobals {
pub(crate) globals: Vec<(FileId, StmtId)>,
pub(crate) globals: Vec<(FileId, GlobalId)>,
pub(crate) errors: Vec<(CompilationError, FileId)>,
}

Expand Down Expand Up @@ -40,16 +40,13 @@ pub(crate) fn resolve_globals(
global.file_id,
);

let name = global.stmt_def.pattern.name_ident().clone();

let hir_stmt = resolver.resolve_global_let(global.stmt_def);
let hir_stmt = resolver.resolve_global_let(global.stmt_def, global.global_id);
errors.extend(take_errors(global.file_id, resolver));

context.def_interner.update_global(global.stmt_id, hir_stmt);

context.def_interner.push_global(global.stmt_id, name, global.module_id);
let statement_id = context.def_interner.get_global(global.global_id).let_statement;
context.def_interner.replace_statement(statement_id, hir_stmt);

(global.file_id, global.stmt_id)
(global.file_id, global.global_id)
});
ResolvedGlobals { globals, errors }
}
Loading

0 comments on commit 479330e

Please sign in to comment.