Skip to content

Commit

Permalink
Simplify implementation and reduce duplicated logic & possibility to …
Browse files Browse the repository at this point in the history
…crash the compiler
  • Loading branch information
ymadzhunkov committed Oct 4, 2023
1 parent 3070392 commit be67315
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 52 deletions.
19 changes: 10 additions & 9 deletions compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ use crate::hir::type_check::{type_check_func, TypeCheckError, TypeChecker};
use crate::hir::Context;
use crate::hir_def::traits::{Trait, TraitConstant, TraitFunction, TraitImpl, TraitType};
use crate::node_interner::{
allow_trait_impl_for_type, FuncId, NodeInterner, StmtId, StructId, TraitId, TraitImplKey,
TypeAliasId,
FuncId, NodeInterner, StmtId, StructId, TraitId, TraitImplKey, TypeAliasId,
};

use crate::parser::ParserError;
Expand Down Expand Up @@ -551,12 +550,6 @@ fn collect_trait_impl(
errors.push((err.into(), trait_impl.file_id));
}
}
} else if !allow_trait_impl_for_type(&typ) {
let error = DefCollectorErrorKind::NonStructTraitImpl {
trait_path: trait_impl.trait_path.clone(),
span: trait_impl.trait_path.span(),
};
errors.push((error.into(), trait_impl.file_id));
}
}
}
Expand Down Expand Up @@ -1025,7 +1018,15 @@ fn resolve_trait_impls(
trait_id,
methods: vecmap(&impl_methods, |(_, func_id)| *func_id),
});
interner.add_trait_implementation(&key, resolved_trait_impl.clone());
if !interner.add_trait_implementation(&key, resolved_trait_impl.clone()) {
// error
// unreachable!("Cannot add a method to the unsupported type '{}'", key.typ)
let error = DefCollectorErrorKind::TraitImplNotAllowedFor {
trait_path: trait_impl.trait_path.clone(),
span: trait_impl.trait_path.span(),
};
errors.push((error.into(), trait_impl.file_id));
}
}

methods.append(&mut impl_methods);
Expand Down
10 changes: 5 additions & 5 deletions compiler/noirc_frontend/src/hir/def_collector/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ pub enum DefCollectorErrorKind {
PathResolutionError(PathResolutionError),
#[error("Non-struct type used in impl")]
NonStructTypeInImpl { span: Span },
#[error("Non-struct type used in trait impl")]
NonStructTraitImpl { trait_path: Path, span: Span },
#[error("Trait implementation is not allowed for this")]
TraitImplNotAllowedFor { trait_path: Path, span: Span },
#[error("Cannot `impl` a type defined outside the current crate")]
ForeignImpl { span: Span, type_name: String },
#[error("Mismatch number of parameters in of trait implementation")]
Expand Down Expand Up @@ -119,10 +119,10 @@ impl From<DefCollectorErrorKind> for Diagnostic {
"Only struct types may have implementation methods".into(),
span,
),
DefCollectorErrorKind::NonStructTraitImpl { trait_path, span } => {
DefCollectorErrorKind::TraitImplNotAllowedFor { trait_path, span } => {
Diagnostic::simple_error(
format!("Only struct types may implement trait `{trait_path}`"),
"Only struct types may implement traits".into(),
format!("Only limited types may implement trait `{trait_path}`"),
"Only limited types may implement traits".into(),
span,
)
}
Expand Down
69 changes: 31 additions & 38 deletions compiler/noirc_frontend/src/node_interner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -872,29 +872,45 @@ impl NodeInterner {
self.trait_implementations.get(key).cloned()
}

pub fn add_trait_implementation(&mut self, key: &TraitImplKey, trait_impl: Shared<TraitImpl>) {
pub fn add_trait_implementation(
&mut self,
key: &TraitImplKey,
trait_impl: Shared<TraitImpl>,
) -> bool {
self.trait_implementations.insert(key.clone(), trait_impl.clone());
for func_id in &trait_impl.borrow().methods {
let method_name = self.function_name(func_id).to_owned();
match &key.typ {
Type::Struct(struct_type, _generics) => {
match &key.typ {
Type::Struct(struct_type, _generics) => {
for func_id in &trait_impl.borrow().methods {
let method_name = self.function_name(func_id).to_owned();
let key = (struct_type.borrow().id, method_name);
self.struct_methods.insert(key, *func_id);
}
Type::FieldElement
| Type::Array(..)
| Type::Integer(..)
| Type::Bool
| Type::Tuple(..)
| Type::String(..) => {
true
}
Type::FieldElement
| Type::Unit
| Type::Array(..)
| Type::Integer(..)
| Type::Bool
| Type::Tuple(..)
| Type::String(..)
| Type::FmtString(..)
=> {
for func_id in &trait_impl.borrow().methods {
let method_name = self.function_name(func_id).to_owned();
let key = (key.typ.clone(), method_name);
self.primitive_trait_impls.insert(key, *func_id);
}
Type::Error => {}
_ => {
unreachable!("Cannot add a method to the unsupported type '{}'", key.typ)
}
true
}
Type::TypeVariable(..) |
Type::NamedGeneric(..) |
Type::Function(..) |
Type::MutableReference(..) |
Type::Forall(..) |
Type::Constant(..) |
Type::NotConstant |
Type::Error => false,
}
}

Expand Down Expand Up @@ -954,26 +970,3 @@ fn get_type_method_key(typ: &Type) -> Option<TypeMethodKey> {
| Type::FmtString(_, _) => None,
}
}

pub fn allow_trait_impl_for_type(typ: &Type) -> bool {
match &typ {
Type::FieldElement
| Type::Array(..)
| Type::Integer(..)
| Type::Bool
| Type::Struct(..)
| Type::Tuple(..)
| Type::String(..) => true,

Type::FmtString(..)
| Type::Unit
| Type::TypeVariable(..)
| Type::NamedGeneric(..)
| Type::Function(..)
| Type::MutableReference(..)
| Type::Forall(..)
| Type::Constant(..)
| Type::NotConstant
| Type::Error => false,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,28 @@ impl Fieldable for str<6> {
}
}

impl Fieldable for () {
fn to_field(self) -> Field {
0
}
}

type Point2D = [Field; 2];
type Point2DAlias = Point2D;

impl Fieldable for Point2DAlias {
fn to_field(self) -> Field {
self[0] + self[1]
}
}

impl Fieldable for fmtstr<14, (Field, Field)> {
fn to_field(self) -> Field {
52
}
}


// x = 15
fn main(x: u32) {
assert(x.to_field() == 15);
Expand All @@ -66,4 +88,11 @@ fn main(x: u32) {
assert(k_false.to_field() == 32);
let m = "String";
assert(m.to_field() == 6);
let unit = ();
assert(unit.to_field() == 0);
let point: Point2DAlias = [2, 3];
assert(point.to_field() == 5);
let i: Field = 2;
let j: Field = 6;
assert(f"i: {i}, j: {j}".to_field() == 52);
}

0 comments on commit be67315

Please sign in to comment.