Skip to content

Commit

Permalink
feat: warn on unused functions (noir-lang/noir#5892)
Browse files Browse the repository at this point in the history
feat: add `fmtstr::contents` (noir-lang/noir#5928)
fix: collect functions generated by attributes (noir-lang/noir#5930)
fix: Support debug comptime flag for attributes (noir-lang/noir#5929)
feat: Allow inserting new structs and into programs from attributes (noir-lang/noir#5927)
feat: module attributes (noir-lang/noir#5888)
feat: unquote some value as tokens, not as unquote markers (noir-lang/noir#5924)
feat: check argument count and types on attribute function callback (noir-lang/noir#5921)
feat: LSP will now suggest private items if they are visible (noir-lang/noir#5923)
  • Loading branch information
AztecBot committed Sep 5, 2024
2 parents 37df363 + 5b204a3 commit d7c39cb
Show file tree
Hide file tree
Showing 20 changed files with 357 additions and 144 deletions.
2 changes: 1 addition & 1 deletion .noir-sync-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
34f21c0eadfc8a03f5177d72de7958903de8ac98
af3db4bf2e8f7feba6d06c3095d7cdf17c8dde75
29 changes: 16 additions & 13 deletions noir/noir-repo/compiler/noirc_frontend/src/elaborator/comptime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -326,21 +326,24 @@ impl<'context> Elaborator<'context> {
) {
match item {
TopLevelStatement::Function(function) => {
let id = self.interner.push_empty_fn();
let module = self.module_id();
self.interner.push_function(id, &function.def, module, location);
let module_id = self.module_id();

if self.interner.is_in_lsp_mode() && !function.def.is_test() {
self.interner.register_function(id, &function.def);
if let Some(id) = dc_mod::collect_function(
self.interner,
self.def_maps.get_mut(&self.crate_id).unwrap(),
&function,
module_id,
self.file,
&mut self.errors,
) {
let functions = vec![(self.local_module, id, function)];
generated_items.functions.push(UnresolvedFunctions {
file_id: self.file,
functions,
trait_id: None,
self_type: None,
});
}

let functions = vec![(self.local_module, id, function)];
generated_items.functions.push(UnresolvedFunctions {
file_id: self.file,
functions,
trait_id: None,
self_type: None,
});
}
TopLevelStatement::TraitImpl(mut trait_impl) => {
let (methods, associated_types, associated_constants) =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,19 @@ impl<'local, 'interner> Interpreter<'local, 'interner> {
consuming = false;

if let Some(value) = values.pop_front() {
result.push_str(&value.display(self.elaborator.interner).to_string());
// When interpolating a quoted value inside a format string, we don't include the
// surrounding `quote {` ... `}` as if we are unquoting the quoted value inside the string.
if let Value::Quoted(tokens) = value {
for (index, token) in tokens.iter().enumerate() {
if index > 0 {
result.push(' ');
}
result
.push_str(&token.display(self.elaborator.interner).to_string());
}
} else {
result.push_str(&value.display(self.elaborator.interner).to_string());
}
}
}
other if !consuming => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ use acvm::{AcirField, FieldElement};
use builtin_helpers::{
block_expression_to_value, check_argument_count, check_function_not_yet_resolved,
check_one_argument, check_three_arguments, check_two_arguments, get_expr, get_field,
get_function_def, get_module, get_quoted, get_slice, get_struct, get_trait_constraint,
get_trait_def, get_trait_impl, get_tuple, get_type, get_typed_expr, get_u32,
get_unresolved_type, hir_pattern_to_tokens, mutate_func_meta_type, parse,
get_format_string, get_function_def, get_module, get_quoted, get_slice, get_struct,
get_trait_constraint, get_trait_def, get_trait_impl, get_tuple, get_type, get_typed_expr,
get_u32, get_unresolved_type, hir_pattern_to_tokens, mutate_func_meta_type, parse,
replace_func_meta_parameters, replace_func_meta_return_type,
};
use chumsky::{prelude::choice, Parser};
Expand All @@ -32,6 +32,7 @@ use crate::{
InterpreterError, Value,
},
hir_def::function::FunctionBody,
lexer::Lexer,
macros_api::{HirExpression, HirLiteral, ModuleDefId, NodeInterner, Signedness},
node_interner::{DefinitionKind, TraitImplKind},
parser::{self},
Expand Down Expand Up @@ -95,6 +96,7 @@ impl<'local, 'context> Interpreter<'local, 'context> {
"expr_is_continue" => expr_is_continue(interner, arguments, location),
"expr_resolve" => expr_resolve(self, arguments, location),
"is_unconstrained" => Ok(Value::Bool(true)),
"fmtstr_quoted_contents" => fmtstr_quoted_contents(interner, arguments, location),
"function_def_body" => function_def_body(interner, arguments, location),
"function_def_has_named_attribute" => {
function_def_has_named_attribute(interner, arguments, location)
Expand Down Expand Up @@ -1576,6 +1578,23 @@ fn unwrap_expr_value(interner: &NodeInterner, mut expr_value: ExprValue) -> Expr
expr_value
}

// fn quoted_contents(self) -> Quoted
fn fmtstr_quoted_contents(
interner: &NodeInterner,
arguments: Vec<(Value, Location)>,
location: Location,
) -> IResult<Value> {
let self_argument = check_one_argument(arguments, location)?;
let (string, _) = get_format_string(interner, self_argument)?;
let (tokens, _) = Lexer::lex(&string);
let mut tokens: Vec<_> = tokens.0.into_iter().map(|token| token.into_token()).collect();
if let Some(Token::EOF) = tokens.last() {
tokens.pop();
}

Ok(Value::Quoted(Rc::new(tokens)))
}

// fn body(self) -> Expr
fn function_def_body(
interner: &NodeInterner,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,20 @@ pub(crate) fn get_expr(
}
}

pub(crate) fn get_format_string(
interner: &NodeInterner,
(value, location): (Value, Location),
) -> IResult<(Rc<String>, Type)> {
match value {
Value::FormatString(value, typ) => Ok((value, typ)),
value => {
let n = Box::new(interner.next_type_variable());
let e = Box::new(interner.next_type_variable());
type_mismatch(value, Type::FmtString(n, e), location)
}
}
}

pub(crate) fn get_function_def((value, location): (Value, Location)) -> IResult<FuncId> {
match value {
Value::FunctionDefinition(id) => Ok(id),
Expand Down
73 changes: 46 additions & 27 deletions noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -605,33 +605,7 @@ impl<'value, 'interner> Display for ValuePrinter<'value, 'interner> {
write!(f, "quote {{")?;
for token in tokens.iter() {
write!(f, " ")?;

match token {
Token::QuotedType(id) => {
write!(f, "{}", self.interner.get_quoted_type(*id))?;
}
Token::InternedExpr(id) => {
let value = Value::expression(ExpressionKind::Interned(*id));
value.display(self.interner).fmt(f)?;
}
Token::InternedStatement(id) => {
let value = Value::statement(StatementKind::Interned(*id));
value.display(self.interner).fmt(f)?;
}
Token::InternedLValue(id) => {
let value = Value::lvalue(LValue::Interned(*id, Span::default()));
value.display(self.interner).fmt(f)?;
}
Token::InternedUnresolvedTypeData(id) => {
let value = Value::UnresolvedType(UnresolvedTypeData::Interned(*id));
value.display(self.interner).fmt(f)?;
}
Token::UnquoteMarker(id) => {
let value = Value::TypedExpr(TypedExpr::ExprId(*id));
value.display(self.interner).fmt(f)?;
}
other => write!(f, "{other}")?,
}
token.display(self.interner).fmt(f)?;
}
write!(f, " }}")
}
Expand Down Expand Up @@ -713,6 +687,51 @@ impl<'value, 'interner> Display for ValuePrinter<'value, 'interner> {
}
}

impl Token {
pub fn display<'token, 'interner>(
&'token self,
interner: &'interner NodeInterner,
) -> TokenPrinter<'token, 'interner> {
TokenPrinter { token: self, interner }
}
}

pub struct TokenPrinter<'token, 'interner> {
token: &'token Token,
interner: &'interner NodeInterner,
}

impl<'token, 'interner> Display for TokenPrinter<'token, 'interner> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self.token {
Token::QuotedType(id) => {
write!(f, "{}", self.interner.get_quoted_type(*id))
}
Token::InternedExpr(id) => {
let value = Value::expression(ExpressionKind::Interned(*id));
value.display(self.interner).fmt(f)
}
Token::InternedStatement(id) => {
let value = Value::statement(StatementKind::Interned(*id));
value.display(self.interner).fmt(f)
}
Token::InternedLValue(id) => {
let value = Value::lvalue(LValue::Interned(*id, Span::default()));
value.display(self.interner).fmt(f)
}
Token::InternedUnresolvedTypeData(id) => {
let value = Value::UnresolvedType(UnresolvedTypeData::Interned(*id));
value.display(self.interner).fmt(f)
}
Token::UnquoteMarker(id) => {
let value = Value::TypedExpr(TypedExpr::ExprId(*id));
value.display(self.interner).fmt(f)
}
other => write!(f, "{other}"),
}
}
}

fn display_trait_constraint(interner: &NodeInterner, trait_constraint: &TraitConstraint) -> String {
let trait_ = interner.get_trait(trait_constraint.trait_id);
format!("{}: {}{}", trait_constraint.typ, trait_.name, trait_constraint.trait_generics)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use crate::hir::resolution::errors::ResolverError;
use crate::hir::resolution::path_resolver;
use crate::hir::type_check::TypeCheckError;
use crate::token::SecondaryAttribute;
use crate::usage_tracker::UnusedItem;
use crate::{Generics, Type};

use crate::hir::resolution::import::{resolve_import, ImportDirective, PathResolution};
Expand Down Expand Up @@ -271,7 +272,7 @@ impl DefCollector {
root_file_id: FileId,
debug_comptime_in_file: Option<&str>,
enable_arithmetic_generics: bool,
error_on_usage_tracker: bool,
error_on_unused_items: bool,
macro_processors: &[&dyn MacroProcessor],
) -> Vec<(CompilationError, FileId)> {
let mut errors: Vec<(CompilationError, FileId)> = vec![];
Expand Down Expand Up @@ -406,20 +407,14 @@ impl DefCollector {
let result = current_def_map.modules[resolved_import.module_scope.0]
.import(name.clone(), visibility, module_def_id, is_prelude);

// Empty spans could come from implicitly injected imports, and we don't want to track those
if visibility != ItemVisibility::Public
&& name.span().start() < name.span().end()
{
let module_id = ModuleId {
krate: crate_id,
local_id: resolved_import.module_scope,
};

context
.def_interner
.usage_tracker
.add_unused_import(module_id, name.clone());
}
let module_id =
ModuleId { krate: crate_id, local_id: resolved_import.module_scope };
context.def_interner.usage_tracker.add_unused_item(
module_id,
name.clone(),
UnusedItem::Import,
visibility,
);

if visibility != ItemVisibility::Private {
let local_id = resolved_import.module_scope;
Expand Down Expand Up @@ -494,26 +489,29 @@ impl DefCollector {
);
}

if error_on_usage_tracker {
Self::check_usage_tracker(context, crate_id, &mut errors);
if error_on_unused_items {
Self::check_unused_items(context, crate_id, &mut errors);
}

errors
}

fn check_usage_tracker(
fn check_unused_items(
context: &Context,
crate_id: CrateId,
errors: &mut Vec<(CompilationError, FileId)>,
) {
let unused_imports = context.def_interner.usage_tracker.unused_imports().iter();
let unused_imports = context.def_interner.usage_tracker.unused_items().iter();
let unused_imports = unused_imports.filter(|(module_id, _)| module_id.krate == crate_id);

errors.extend(unused_imports.flat_map(|(module_id, usage_tracker)| {
let module = &context.def_maps[&crate_id].modules()[module_id.local_id.0];
usage_tracker.iter().map(|ident| {
usage_tracker.iter().map(|(ident, unused_item)| {
let ident = ident.clone();
let error = CompilationError::ResolverError(ResolverError::UnusedImport { ident });
let error = CompilationError::ResolverError(ResolverError::UnusedItem {
ident,
item_type: unused_item.item_type(),
});
(error, module.location.file)
})
}));
Expand Down
Loading

0 comments on commit d7c39cb

Please sign in to comment.