Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(experimental elaborator): Fix frontend tests when --use-elaborator flag is specified #5145

Merged
merged 2 commits into from
May 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
},
resolution::{errors::ResolverError, path_resolver::PathResolver, resolver::LambdaContext},
scope::ScopeForest as GenericScopeForest,
type_check::TypeCheckError,
type_check::{check_trait_impl_method_matches_declaration, TypeCheckError},
},
hir_def::{expr::HirIdent, function::Parameters, traits::TraitConstraint},
macros_api::{
Expand Down Expand Up @@ -329,7 +329,7 @@
// when multiple impls are available. Instead we default first to choose the Field or u64 impl.
for typ in &self.type_variables {
if let Type::TypeVariable(variable, kind) = typ.follow_bindings() {
let msg = "TypeChecker should only track defaultable type vars";

Check warning on line 332 in compiler/noirc_frontend/src/elaborator/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (defaultable)
variable.bind(kind.default_type().expect(msg));
}
}
Expand Down Expand Up @@ -851,6 +851,12 @@
self.generics = trait_impl.resolved_generics;
self.current_trait_impl = trait_impl.impl_id;

for (module, function, _) in &trait_impl.methods.functions {
self.local_module = *module;
let errors = check_trait_impl_method_matches_declaration(self.interner, *function);
self.errors.extend(errors.into_iter().map(|error| (error.into(), self.file)));
}

self.elaborate_functions(trait_impl.methods);

self.self_type = None;
Expand All @@ -877,25 +883,26 @@
fn collect_trait_impl(&mut self, trait_impl: &mut UnresolvedTraitImpl) {
self.local_module = trait_impl.module_id;
self.file = trait_impl.file_id;
self.current_trait_impl = trait_impl.impl_id;
trait_impl.trait_id = self.resolve_trait_by_path(trait_impl.trait_path.clone());

let self_type = trait_impl.methods.self_type.clone();
let self_type =
self_type.expect("Expected struct type to be set before collect_trait_impl");

self.self_type = Some(self_type.clone());
let self_type_span = trait_impl.object_type.span;

if matches!(self_type, Type::MutableReference(_)) {
let span = self_type_span.unwrap_or_else(|| trait_impl.trait_path.span());
self.push_err(DefCollectorErrorKind::MutableReferenceInTraitImpl { span });
}

assert!(trait_impl.trait_id.is_some());
if let Some(trait_id) = trait_impl.trait_id {
self.generics = trait_impl.resolved_generics.clone();
self.collect_trait_impl_methods(trait_id, trait_impl);

let span = trait_impl.object_type.span.expect("All trait self types should have spans");
self.generics = trait_impl.resolved_generics.clone();
self.declare_methods_on_struct(true, &mut trait_impl.methods, span);

let methods = trait_impl.methods.function_ids();
Expand Down Expand Up @@ -945,6 +952,8 @@
}

self.generics.clear();
self.current_trait_impl = None;
self.self_type = None;
}

fn get_module_mut(&mut self, module: ModuleId) -> &mut ModuleData {
Expand Down Expand Up @@ -1059,6 +1068,7 @@
let module = self.module_id();
let location = Location::new(default_impl.def.span, trait_impl.file_id);
self.interner.push_function(func_id, &default_impl.def, module, location);
self.define_function_meta(&mut default_impl_clone, func_id, false);
func_ids_in_trait.insert(func_id);
ordered_methods.push((
method.default_impl_module_id,
Expand Down
8 changes: 7 additions & 1 deletion compiler/noirc_frontend/src/hir/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@
// when multiple impls are available. Instead we default first to choose the Field or u64 impl.
for typ in &type_checker.type_variables {
if let Type::TypeVariable(variable, kind) = typ.follow_bindings() {
let msg = "TypeChecker should only track defaultable type vars";

Check warning on line 138 in compiler/noirc_frontend/src/hir/type_check/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (defaultable)
variable.bind(kind.default_type().expect(msg));
}
}
Expand Down Expand Up @@ -237,7 +237,13 @@

let impl_ =
meta.trait_impl.expect("Trait impl function should have a corresponding trait impl");
let impl_ = interner.get_trait_implementation(impl_);

// If the trait implementation is not defined in the interner then there was a previous
// error in resolving the trait path and there is likely no trait for this impl.
let Some(impl_) = interner.try_get_trait_implementation(impl_) else {
return errors;
};

let impl_ = impl_.borrow();
let trait_info = interner.get_trait(impl_.trait_id);

Expand Down
4 changes: 4 additions & 0 deletions compiler/noirc_frontend/src/node_interner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
use iter_extended::vecmap;
use noirc_arena::{Arena, Index};
use noirc_errors::{Location, Span, Spanned};
use petgraph::algo::tarjan_scc;

Check warning on line 10 in compiler/noirc_frontend/src/node_interner.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (petgraph)

Check warning on line 10 in compiler/noirc_frontend/src/node_interner.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (tarjan)
use petgraph::prelude::DiGraph;

Check warning on line 11 in compiler/noirc_frontend/src/node_interner.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (petgraph)
use petgraph::prelude::NodeIndex as PetGraphIndex;

Check warning on line 12 in compiler/noirc_frontend/src/node_interner.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (petgraph)

use crate::ast::Ident;
use crate::graph::CrateId;
Expand Down Expand Up @@ -61,7 +61,7 @@
function_modules: HashMap<FuncId, ModuleId>,

/// This graph tracks dependencies between different global definitions.
/// This is used to ensure the absense of dependency cycles for globals and types.

Check warning on line 64 in compiler/noirc_frontend/src/node_interner.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (absense)
dependency_graph: DiGraph<DependencyId, ()>,

/// To keep track of where each DependencyId is in `dependency_graph`, we need
Expand Down Expand Up @@ -477,7 +477,7 @@
function_modifiers: HashMap::new(),
function_modules: HashMap::new(),
func_id_to_trait: HashMap::new(),
dependency_graph: petgraph::graph::DiGraph::new(),

Check warning on line 480 in compiler/noirc_frontend/src/node_interner.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (petgraph)
dependency_graph_indices: HashMap::new(),
id_to_location: HashMap::new(),
definitions: vec![],
Expand Down Expand Up @@ -1145,6 +1145,10 @@
}
}

pub fn try_get_trait_implementation(&self, id: TraitImplId) -> Option<Shared<TraitImpl>> {
self.trait_implementations.get(&id).cloned()
}

pub fn get_trait_implementation(&self, id: TraitImplId) -> Shared<TraitImpl> {
self.trait_implementations[&id].clone()
}
Expand Down
13 changes: 7 additions & 6 deletions compiler/noirc_frontend/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,10 @@ fn check_trait_wrong_parameter_type() {
}";
let errors = get_program_errors(src);
assert!(!has_parser_error(&errors));
assert!(errors.len() == 2, "Expected 2 errors, got: {:?}", errors);

// This is a duplicate error in the name resolver & type checker.
// In the elaborator there is no duplicate and only 1 error is issued
assert!(errors.len() <= 2, "Expected 1 or 2 errors, got: {:?}", errors);

for (err, _file_id) in errors {
match &err {
Expand Down Expand Up @@ -593,22 +596,20 @@ fn check_impl_struct_not_trait() {
bar: Field,
array: [Field; 2],
}

struct Default {
x: Field,
z: Field,
}

// Default is struct not a trait
// Default is a struct not a trait
impl Default for Foo {
fn default(x: Field, y: Field) -> Self {
Self { bar: x, array: [x,y] }
}
}

fn main() {
}

fn main() {}
";
let errors = get_program_errors(src);
assert!(!has_parser_error(&errors));
Expand Down
Loading