Skip to content

Commit

Permalink
chore: start moving lints into a separate linting directory (#5165)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Resolves <!-- Link to GitHub Issue -->

## Summary\*

This PR starts moving some diagnostics into a separate `lints` directory
and out of the main flow of the elaborator.

Eventually we can ideally move this to run on the node interner after
the elaborator has completed rather than being called during elaboration
but this can happen in a later PR

## Additional Context



## 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.
  • Loading branch information
TomAFrench authored Jun 4, 2024
1 parent 2f573ec commit 1afc6c2
Show file tree
Hide file tree
Showing 5 changed files with 277 additions and 153 deletions.
221 changes: 221 additions & 0 deletions compiler/noirc_frontend/src/elaborator/lints.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,221 @@
use crate::{
ast::FunctionKind,
graph::CrateId,
hir::{
resolution::errors::{PubPosition, ResolverError},
type_check::TypeCheckError,
},
hir_def::expr::HirIdent,
macros_api::{
HirExpression, HirLiteral, NodeInterner, NoirFunction, UnaryOp, UnresolvedTypeData,
Visibility,
},
node_interner::{DefinitionKind, ExprId},
Type,
};
use acvm::AcirField;

use noirc_errors::Span;

pub(super) fn deprecated_function(interner: &NodeInterner, expr: ExprId) -> Option<TypeCheckError> {
let HirExpression::Ident(HirIdent { location, id, impl_kind: _ }, _) =
interner.expression(&expr)
else {
return None;
};

let Some(DefinitionKind::Function(func_id)) = interner.try_definition(id).map(|def| &def.kind)
else {
return None;
};

let attributes = interner.function_attributes(func_id);
attributes.get_deprecated_note().map(|note| TypeCheckError::CallDeprecated {
name: interner.definition_name(id).to_string(),
note,
span: location.span,
})
}

/// 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() })
} else {
None
}
} else {
None
}
}

/// 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,
crate_id: CrateId,
) -> Option<ResolverError> {
let is_low_level_function =
func.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() })
} else {
None
}
}

/// `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
{
Some(ResolverError::NecessaryPub { ident: func.name_ident().clone() })
} else {
None
}
}

/// `#[recursive]` attribute is only allowed for entry point functions
pub(super) fn recursive_non_entrypoint_function(
func: &NoirFunction,
is_entry_point: bool,
) -> Option<ResolverError> {
if !is_entry_point && func.kind == FunctionKind::Recursive {
Some(ResolverError::MisplacedRecursiveAttribute { ident: func.name_ident().clone() })
} else {
None
}
}

/// Test functions cannot have arguments in order to be executable.
pub(super) fn test_function_with_args(func: &NoirFunction) -> Option<ResolverError> {
if func.attributes().is_test_function() && !func.parameters().is_empty() {
Some(ResolverError::TestFunctionHasParameters { span: func.name_ident().span() })
} else {
None
}
}

/// Check that we are not passing a mutable reference from a constrained runtime to an unconstrained runtime.
pub(super) fn unconstrained_function_args(
function_args: &[(Type, ExprId, Span)],
) -> Vec<TypeCheckError> {
function_args
.iter()
.filter_map(|(typ, _, span)| {
if type_contains_mutable_reference(typ) {
Some(TypeCheckError::ConstrainedReferenceToUnconstrained { span: *span })
} else {
None
}
})
.collect()
}

/// Check that we are not passing a slice from an unconstrained runtime to a constrained runtime.
pub(super) fn unconstrained_function_return(
return_type: &Type,
span: Span,
) -> Option<TypeCheckError> {
if return_type.contains_slice() {
Some(TypeCheckError::UnconstrainedSliceReturnToConstrained { span })
} else if type_contains_mutable_reference(return_type) {
Some(TypeCheckError::UnconstrainedReferenceToConstrained { span })
} else {
None
}
}

fn type_contains_mutable_reference(typ: &Type) -> bool {
matches!(&typ.follow_bindings(), Type::MutableReference(_))
}

/// Only entrypoint functions require a `pub` visibility modifier applied to their return types.
///
/// Application of `pub` to other functions is not meaningful and is a mistake.
pub(super) fn unnecessary_pub_return(
func: &NoirFunction,
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,
})
} else {
None
}
}

/// Only arguments to entrypoint functions may have a non-private visibility modifier applied to them.
///
/// Other functions are disallowed from declaring the visibility of their arguments.
pub(super) fn unnecessary_pub_argument(
func: &NoirFunction,
arg_visibility: Visibility,
is_entry_point: bool,
) -> Option<ResolverError> {
if arg_visibility == Visibility::Public && !is_entry_point {
Some(ResolverError::UnnecessaryPub {
ident: func.name_ident().clone(),
position: PubPosition::Parameter,
})
} else {
None
}
}

/// Check if an assignment is overflowing with respect to `annotated_type`
/// in a declaration statement where `annotated_type` is an unsigned integer
pub(crate) fn overflowing_uint(
interner: &NodeInterner,
rhs_expr: &ExprId,
annotated_type: &Type,
) -> Vec<TypeCheckError> {
let expr = interner.expression(rhs_expr);
let span = interner.expr_span(rhs_expr);

let mut errors = Vec::with_capacity(2);
match expr {
HirExpression::Literal(HirLiteral::Integer(value, false)) => {
let v = value.to_u128();
if let Type::Integer(_, bit_count) = annotated_type {
let bit_count: u32 = (*bit_count).into();
let max = 1 << bit_count;
if v >= max {
errors.push(TypeCheckError::OverflowingAssignment {
expr: value,
ty: annotated_type.clone(),
range: format!("0..={}", max - 1),
span,
});
};
};
}
HirExpression::Prefix(expr) => {
overflowing_uint(interner, &expr.rhs, annotated_type);
if expr.operator == UnaryOp::Minus {
errors.push(TypeCheckError::InvalidUnaryOp {
kind: "annotated_type".to_string(),
span,
});
}
}
HirExpression::Infix(expr) => {
errors.extend(overflowing_uint(interner, &expr.lhs, annotated_type));
errors.extend(overflowing_uint(interner, &expr.rhs, annotated_type));
}
_ => {}
}

errors
}
Loading

0 comments on commit 1afc6c2

Please sign in to comment.