Skip to content

Commit

Permalink
Merge branch 'master' into aztec-packages
Browse files Browse the repository at this point in the history
* master: (25 commits)
  fix(frontend): Ban type vars bound to a reference from passing the unconstrained boundary  (#5949)
  feat: add `StructDefinition::add_attribute` and `has_named_attribute` (#5945)
  feat: add `FunctionDef::set_return_visibility` (#5941)
  feat: add `FunctionDefinition::add_attribute` (#5944)
  fix: always place module attribute generated items inside module (#5943)
  feat: Add `Quoted::tokens` (#5942)
  feat(perf): Remove known store values that equal the store address in mem2reg (#5935)
  feat: remove blocks which consist of only a jump to another block (#5889)
  fix: use element_size() instead of computing it with division (#5939)
  feat: Add `StructDefinition::set_fields` (#5931)
  feat: Only check array bounds in brillig if index is unsafe (#5938)
  chore: error on false constraint (#5890)
  feat: warn on unused functions (#5892)
  feat: add `fmtstr::contents` (#5928)
  fix: collect functions generated by attributes (#5930)
  fix: Support debug comptime flag for attributes (#5929)
  feat: Allow inserting new structs and into programs from attributes (#5927)
  feat: module attributes (#5888)
  feat: unquote some value as tokens, not as unquote markers (#5924)
  feat: check argument count and types on attribute function callback (#5921)
  ...
  • Loading branch information
TomAFrench committed Sep 6, 2024
2 parents fd0d5ac + ce34fbd commit 2167743
Show file tree
Hide file tree
Showing 22 changed files with 512 additions and 157 deletions.
83 changes: 68 additions & 15 deletions compiler/noirc_frontend/src/elaborator/comptime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use crate::{
},
dc_mod,
},
def_map::ModuleId,
def_map::{LocalModuleId, ModuleId},
resolution::errors::ResolverError,
},
hir_def::expr::HirIdent,
Expand All @@ -30,6 +30,24 @@ use crate::{

use super::{Elaborator, FunctionContext, ResolverMeta};

#[derive(Debug, Copy, Clone)]
struct AttributeContext {
// The file where generated items should be added
file: FileId,
// The module where generated items should be added
module: LocalModuleId,
// The file where the attribute is located
attribute_file: FileId,
// The module where the attribute is located
attribute_module: LocalModuleId,
}

impl AttributeContext {
fn new(file: FileId, module: LocalModuleId) -> Self {
Self { file, module, attribute_file: file, attribute_module: module }
}
}

impl<'context> Elaborator<'context> {
/// Elaborate an expression from the middle of a comptime scope.
/// When this happens we require additional information to know
Expand Down Expand Up @@ -89,15 +107,22 @@ impl<'context> Elaborator<'context> {
}
}

pub(super) fn run_comptime_attributes_on_item(
fn run_comptime_attributes_on_item(
&mut self,
attributes: &[SecondaryAttribute],
item: Value,
span: Span,
attribute_context: AttributeContext,
generated_items: &mut CollectedItems,
) {
for attribute in attributes {
self.run_comptime_attribute_on_item(attribute, &item, span, generated_items);
self.run_comptime_attribute_on_item(
attribute,
&item,
span,
attribute_context,
generated_items,
);
}
}

Expand All @@ -106,6 +131,7 @@ impl<'context> Elaborator<'context> {
attribute: &SecondaryAttribute,
item: &Value,
span: Span,
attribute_context: AttributeContext,
generated_items: &mut CollectedItems,
) {
if let SecondaryAttribute::Custom(attribute) = attribute {
Expand All @@ -114,6 +140,7 @@ impl<'context> Elaborator<'context> {
item.clone(),
span,
attribute.contents_span,
attribute_context,
generated_items,
) {
self.errors.push(error);
Expand All @@ -127,8 +154,12 @@ impl<'context> Elaborator<'context> {
item: Value,
span: Span,
attribute_span: Span,
attribute_context: AttributeContext,
generated_items: &mut CollectedItems,
) -> Result<(), (CompilationError, FileId)> {
self.file = attribute_context.attribute_file;
self.local_module = attribute_context.attribute_module;

let location = Location::new(attribute_span, self.file);
let Some((function, arguments)) = Self::parse_attribute(attribute, location)? else {
// Do not issue an error if the attribute is unknown
Expand Down Expand Up @@ -156,6 +187,9 @@ impl<'context> Elaborator<'context> {
return Err((ResolverError::NonFunctionInAnnotation { span }.into(), self.file));
};

self.file = attribute_context.file;
self.local_module = attribute_context.module;

let mut interpreter = self.setup_interpreter();
let mut arguments = Self::handle_attribute_arguments(
&mut interpreter,
Expand Down Expand Up @@ -463,18 +497,28 @@ impl<'context> Elaborator<'context> {
let attributes = &trait_.trait_def.attributes;
let item = Value::TraitDefinition(*trait_id);
let span = trait_.trait_def.span;
self.local_module = trait_.module_id;
self.file = trait_.file_id;
self.run_comptime_attributes_on_item(attributes, item, span, &mut generated_items);
let context = AttributeContext::new(trait_.file_id, trait_.module_id);
self.run_comptime_attributes_on_item(
attributes,
item,
span,
context,
&mut generated_items,
);
}

for (struct_id, struct_def) in types {
let attributes = &struct_def.struct_def.attributes;
let item = Value::StructDefinition(*struct_id);
let span = struct_def.struct_def.span;
self.local_module = struct_def.module_id;
self.file = struct_def.file_id;
self.run_comptime_attributes_on_item(attributes, item, span, &mut generated_items);
let context = AttributeContext::new(struct_def.file_id, struct_def.module_id);
self.run_comptime_attributes_on_item(
attributes,
item,
span,
context,
&mut generated_items,
);
}

self.run_attributes_on_functions(functions, &mut generated_items);
Expand All @@ -496,10 +540,14 @@ impl<'context> Elaborator<'context> {
let attribute = &module_attribute.attribute;
let span = Span::default();

self.local_module = module_attribute.attribute_module_id;
self.file = module_attribute.attribute_file_id;
let context = AttributeContext {
file: module_attribute.file_id,
module: module_attribute.module_id,
attribute_file: module_attribute.attribute_file_id,
attribute_module: module_attribute.attribute_module_id,
};

self.run_comptime_attribute_on_item(attribute, &item, span, generated_items);
self.run_comptime_attribute_on_item(attribute, &item, span, context, generated_items);
}
}

Expand All @@ -509,15 +557,20 @@ impl<'context> Elaborator<'context> {
generated_items: &mut CollectedItems,
) {
for function_set in function_sets {
self.file = function_set.file_id;
self.self_type = function_set.self_type.clone();

for (local_module, function_id, function) in &function_set.functions {
self.local_module = *local_module;
let context = AttributeContext::new(function_set.file_id, *local_module);
let attributes = function.secondary_attributes();
let item = Value::FunctionDefinition(*function_id);
let span = function.span();
self.run_comptime_attributes_on_item(attributes, item, span, generated_items);
self.run_comptime_attributes_on_item(
attributes,
item,
span,
context,
generated_items,
);
}
}
}
Expand Down
87 changes: 49 additions & 38 deletions compiler/noirc_frontend/src/elaborator/lints.rs
Original file line number Diff line number Diff line change
@@ -1,20 +1,19 @@
use crate::{
ast::FunctionKind,
ast::{FunctionKind, Ident},
graph::CrateId,
hir::{
resolution::errors::{PubPosition, ResolverError},
type_check::TypeCheckError,
},
hir_def::expr::HirIdent,
hir_def::{expr::HirIdent, function::FuncMeta},
macros_api::{
HirExpression, HirLiteral, NodeInterner, NoirFunction, Signedness, UnaryOp,
UnresolvedTypeData, Visibility,
HirExpression, HirLiteral, NodeInterner, NoirFunction, Signedness, UnaryOp, Visibility,
},
node_interner::{DefinitionKind, ExprId, FuncId},
node_interner::{DefinitionKind, ExprId, FuncId, FunctionModifiers},
Type,
};

use noirc_errors::Span;
use noirc_errors::{Span, Spanned};

pub(super) fn deprecated_function(interner: &NodeInterner, expr: ExprId) -> Option<TypeCheckError> {
let HirExpression::Ident(HirIdent { location, id, impl_kind: _ }, _) =
Expand All @@ -39,16 +38,17 @@ pub(super) fn deprecated_function(interner: &NodeInterner, expr: ExprId) -> Opti
/// Inline attributes are only relevant for constrained functions
/// as all unconstrained functions are not inlined and so
/// associated attributes are disallowed.
pub(super) fn inlining_attributes(func: &NoirFunction) -> Option<ResolverError> {
if func.def.is_unconstrained {
let attributes = func.attributes().clone();

if attributes.is_no_predicates() {
Some(ResolverError::NoPredicatesAttributeOnUnconstrained {
ident: func.name_ident().clone(),
})
} else if attributes.is_foldable() {
Some(ResolverError::FoldAttributeOnUnconstrained { ident: func.name_ident().clone() })
pub(super) fn inlining_attributes(
func: &FuncMeta,
modifiers: &FunctionModifiers,
) -> Option<ResolverError> {
if modifiers.is_unconstrained {
if modifiers.attributes.is_no_predicates() {
let ident = func_meta_name_ident(func, modifiers);
Some(ResolverError::NoPredicatesAttributeOnUnconstrained { ident })
} else if modifiers.attributes.is_foldable() {
let ident = func_meta_name_ident(func, modifiers);
Some(ResolverError::FoldAttributeOnUnconstrained { ident })
} else {
None
}
Expand All @@ -59,24 +59,30 @@ pub(super) fn inlining_attributes(func: &NoirFunction) -> Option<ResolverError>

/// Attempting to define new low level (`#[builtin]` or `#[foreign]`) functions outside of the stdlib is disallowed.
pub(super) fn low_level_function_outside_stdlib(
func: &NoirFunction,
func: &FuncMeta,
modifiers: &FunctionModifiers,
crate_id: CrateId,
) -> Option<ResolverError> {
let is_low_level_function =
func.attributes().function.as_ref().map_or(false, |func| func.is_low_level());
modifiers.attributes.function.as_ref().map_or(false, |func| func.is_low_level());
if !crate_id.is_stdlib() && is_low_level_function {
Some(ResolverError::LowLevelFunctionOutsideOfStdlib { ident: func.name_ident().clone() })
let ident = func_meta_name_ident(func, modifiers);
Some(ResolverError::LowLevelFunctionOutsideOfStdlib { ident })
} else {
None
}
}

/// Oracle definitions (functions with the `#[oracle]` attribute) must be marked as unconstrained.
pub(super) fn oracle_not_marked_unconstrained(func: &NoirFunction) -> Option<ResolverError> {
pub(super) fn oracle_not_marked_unconstrained(
func: &FuncMeta,
modifiers: &FunctionModifiers,
) -> Option<ResolverError> {
let is_oracle_function =
func.attributes().function.as_ref().map_or(false, |func| func.is_oracle());
if is_oracle_function && !func.def.is_unconstrained {
Some(ResolverError::OracleMarkedAsConstrained { ident: func.name_ident().clone() })
modifiers.attributes.function.as_ref().map_or(false, |func| func.is_oracle());
if is_oracle_function && !modifiers.is_unconstrained {
let ident = func_meta_name_ident(func, modifiers);
Some(ResolverError::OracleMarkedAsConstrained { ident })
} else {
None
}
Expand Down Expand Up @@ -106,24 +112,26 @@ pub(super) fn oracle_called_from_constrained_function(
}

/// `pub` is required on return types for entry point functions
pub(super) fn missing_pub(func: &NoirFunction, is_entry_point: bool) -> Option<ResolverError> {
if is_entry_point
&& func.return_type().typ != UnresolvedTypeData::Unit
&& func.def.return_visibility == Visibility::Private
pub(super) fn missing_pub(func: &FuncMeta, modifiers: &FunctionModifiers) -> Option<ResolverError> {
if func.is_entry_point
&& func.return_type() != &Type::Unit
&& func.return_visibility == Visibility::Private
{
Some(ResolverError::NecessaryPub { ident: func.name_ident().clone() })
let ident = func_meta_name_ident(func, modifiers);
Some(ResolverError::NecessaryPub { ident })
} else {
None
}
}

/// `#[recursive]` attribute is only allowed for entry point functions
pub(super) fn recursive_non_entrypoint_function(
func: &NoirFunction,
is_entry_point: bool,
func: &FuncMeta,
modifiers: &FunctionModifiers,
) -> Option<ResolverError> {
if !is_entry_point && func.kind == FunctionKind::Recursive {
Some(ResolverError::MisplacedRecursiveAttribute { ident: func.name_ident().clone() })
if !func.is_entry_point && func.kind == FunctionKind::Recursive {
let ident = func_meta_name_ident(func, modifiers);
Some(ResolverError::MisplacedRecursiveAttribute { ident })
} else {
None
}
Expand Down Expand Up @@ -163,14 +171,13 @@ pub(super) fn unconstrained_function_return(
///
/// Application of `pub` to other functions is not meaningful and is a mistake.
pub(super) fn unnecessary_pub_return(
func: &NoirFunction,
func: &FuncMeta,
modifiers: &FunctionModifiers,
is_entry_point: bool,
) -> Option<ResolverError> {
if !is_entry_point && func.def.return_visibility == Visibility::Public {
Some(ResolverError::UnnecessaryPub {
ident: func.name_ident().clone(),
position: PubPosition::ReturnType,
})
if !is_entry_point && func.return_visibility == Visibility::Public {
let ident = func_meta_name_ident(func, modifiers);
Some(ResolverError::UnnecessaryPub { ident, position: PubPosition::ReturnType })
} else {
None
}
Expand Down Expand Up @@ -252,3 +259,7 @@ pub(crate) fn overflowing_int(

errors
}

fn func_meta_name_ident(func: &FuncMeta, modifiers: &FunctionModifiers) -> Ident {
Ident(Spanned::from(func.name.location.span, modifiers.name.clone()))
}
38 changes: 23 additions & 15 deletions compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ use crate::{
SecondaryAttribute, StructId,
},
node_interner::{
DefinitionKind, DependencyId, ExprId, FuncId, GlobalId, ReferenceId, TraitId, TypeAliasId,
DefinitionKind, DependencyId, ExprId, FuncId, FunctionModifiers, GlobalId, ReferenceId,
TraitId, TypeAliasId,
},
token::CustomAtrribute,

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

View workflow job for this annotation

GitHub Actions / Code

Unknown word (Atrribute)
Shared, Type, TypeVariable,
Expand Down Expand Up @@ -417,6 +418,10 @@ impl<'context> Elaborator<'context> {
self.trait_bounds = func_meta.trait_constraints.clone();
self.function_context.push(FunctionContext::default());

let modifiers = self.interner.function_modifiers(&id).clone();

self.run_function_lints(&func_meta, &modifiers);

self.introduce_generics_into_scope(func_meta.all_generics.clone());

// The DefinitionIds for each parameter were already created in define_function_meta
Expand Down Expand Up @@ -731,20 +736,6 @@ impl<'context> Elaborator<'context> {

let is_entry_point = self.is_entry_point_function(func, in_contract);

self.run_lint(|_| lints::inlining_attributes(func).map(Into::into));
self.run_lint(|_| lints::missing_pub(func, is_entry_point).map(Into::into));
self.run_lint(|elaborator| {
lints::unnecessary_pub_return(func, elaborator.pub_allowed(func, in_contract))
.map(Into::into)
});
self.run_lint(|_| lints::oracle_not_marked_unconstrained(func).map(Into::into));
self.run_lint(|elaborator| {
lints::low_level_function_outside_stdlib(func, elaborator.crate_id).map(Into::into)
});
self.run_lint(|_| {
lints::recursive_non_entrypoint_function(func, is_entry_point).map(Into::into)
});

// Both the #[fold] and #[no_predicates] alter a function's inline type and code generation in similar ways.
// In certain cases such as type checking (for which the following flag will be used) both attributes
// indicate we should code generate in the same way. Thus, we unify the attributes into one flag here.
Expand Down Expand Up @@ -858,6 +849,23 @@ impl<'context> Elaborator<'context> {
self.current_item = None;
}

fn run_function_lints(&mut self, func: &FuncMeta, modifiers: &FunctionModifiers) {
self.run_lint(|_| lints::inlining_attributes(func, modifiers).map(Into::into));
self.run_lint(|_| lints::missing_pub(func, modifiers).map(Into::into));
self.run_lint(|_| {
let pub_allowed = func.is_entry_point || modifiers.attributes.is_foldable();
lints::unnecessary_pub_return(func, modifiers, pub_allowed).map(Into::into)
});
self.run_lint(|_| lints::oracle_not_marked_unconstrained(func, modifiers).map(Into::into));
self.run_lint(|elaborator| {
lints::low_level_function_outside_stdlib(func, modifiers, elaborator.crate_id)
.map(Into::into)
});
self.run_lint(|_| {
lints::recursive_non_entrypoint_function(func, modifiers).map(Into::into)
});
}

/// Only sized types are valid to be used as main's parameters or the parameters to a contract
/// function. If the given type is not sized (e.g. contains a slice or NamedGeneric type), an
/// error is issued.
Expand Down
Loading

0 comments on commit 2167743

Please sign in to comment.