Skip to content

Commit

Permalink
Merge branch 'master' into tf/generic-to-radix
Browse files Browse the repository at this point in the history
* master:
  fix(sha256): Fix upper bound when building msg block and delay final block compression under certain cases  (#5838)
  feat: remove unnecessary copying of vector size during reversal (#5852)
  chore: Add missing cases to arithmetic generics (#5841)
  feat: warn on unused imports (#5847)
  • Loading branch information
TomAFrench committed Aug 28, 2024
2 parents 38d4185 + 130b7b6 commit 20fdd10
Show file tree
Hide file tree
Showing 28 changed files with 729 additions and 297 deletions.
26 changes: 23 additions & 3 deletions compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,18 @@ pub struct CompileOptions {
pub skip_underconstrained_check: bool,
}

#[derive(Clone, Debug, Default)]
pub struct CheckOptions {
pub compile_options: CompileOptions,
pub error_on_unused_imports: bool,
}

impl CheckOptions {
pub fn new(compile_options: &CompileOptions, error_on_unused_imports: bool) -> Self {
Self { compile_options: compile_options.clone(), error_on_unused_imports }
}
}

pub fn parse_expression_width(input: &str) -> Result<ExpressionWidth, std::io::Error> {
use std::io::{Error, ErrorKind};
let width = input
Expand Down Expand Up @@ -278,8 +290,10 @@ pub fn add_dep(
pub fn check_crate(
context: &mut Context,
crate_id: CrateId,
options: &CompileOptions,
check_options: &CheckOptions,
) -> CompilationResult<()> {
let options = &check_options.compile_options;

let macros: &[&dyn MacroProcessor] =
if options.disable_macros { &[] } else { &[&aztec_macros::AztecMacro] };

Expand All @@ -289,6 +303,7 @@ pub fn check_crate(
context,
options.debug_comptime_in_file.as_deref(),
options.arithmetic_generics,
check_options.error_on_unused_imports,
macros,
);
errors.extend(diagnostics.into_iter().map(|(error, file_id)| {
Expand Down Expand Up @@ -322,7 +337,10 @@ pub fn compile_main(
options: &CompileOptions,
cached_program: Option<CompiledProgram>,
) -> CompilationResult<CompiledProgram> {
let (_, mut warnings) = check_crate(context, crate_id, options)?;
let error_on_unused_imports = true;
let check_options = CheckOptions::new(options, error_on_unused_imports);

let (_, mut warnings) = check_crate(context, crate_id, &check_options)?;

let main = context.get_main_function(&crate_id).ok_or_else(|| {
// TODO(#2155): This error might be a better to exist in Nargo
Expand Down Expand Up @@ -357,7 +375,9 @@ pub fn compile_contract(
crate_id: CrateId,
options: &CompileOptions,
) -> CompilationResult<CompiledContract> {
let (_, warnings) = check_crate(context, crate_id, options)?;
let error_on_unused_imports = true;
let check_options = CheckOptions::new(options, error_on_unused_imports);
let (_, warnings) = check_crate(context, crate_id, &check_options)?;

// TODO: We probably want to error if contracts is empty
let contracts = context.get_all_contracts(&crate_id);
Expand Down
29 changes: 8 additions & 21 deletions compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,33 +263,20 @@ impl<F: AcirField + DebugToString> BrilligContext<F> {
let index_at_end_of_array = self.allocate_register();
let end_value_register = self.allocate_register();

self.codegen_loop(iteration_count, |ctx, iterator_register| {
// Load both values
ctx.codegen_array_get(array.pointer, iterator_register, start_value_register);
self.usize_const_instruction(index_at_end_of_array, array.size.into());

self.codegen_loop(iteration_count, |ctx, iterator_register| {
// The index at the end of array is size - 1 - iterator
ctx.usize_const_instruction(index_at_end_of_array, array.size.into());
ctx.codegen_usize_op_in_place(index_at_end_of_array, BrilligBinaryOp::Sub, 1);
ctx.memory_op_instruction(
index_at_end_of_array,
iterator_register.address,
index_at_end_of_array,
BrilligBinaryOp::Sub,
);

ctx.codegen_array_get(
array.pointer,
SingleAddrVariable::new_usize(index_at_end_of_array),
end_value_register,
);
let index_at_end_of_array_var = SingleAddrVariable::new_usize(index_at_end_of_array);

// Load both values
ctx.codegen_array_get(array.pointer, iterator_register, start_value_register);
ctx.codegen_array_get(array.pointer, index_at_end_of_array_var, end_value_register);

// Write both values
ctx.codegen_array_set(array.pointer, iterator_register, end_value_register);
ctx.codegen_array_set(
array.pointer,
SingleAddrVariable::new_usize(index_at_end_of_array),
start_value_register,
);
ctx.codegen_array_set(array.pointer, index_at_end_of_array_var, start_value_register);
});

self.deallocate_register(iteration_count);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1434,7 +1434,6 @@ impl<F: AcirField> AcirContext<F> {
name,
BlackBoxFunc::MultiScalarMul
| BlackBoxFunc::Keccakf1600
| BlackBoxFunc::Sha256Compression
| BlackBoxFunc::Blake2s
| BlackBoxFunc::Blake3
| BlackBoxFunc::AND
Expand Down
14 changes: 5 additions & 9 deletions compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::{
UnresolvedTypeAlias,
},
def_map::DefMaps,
resolution::{errors::ResolverError, path_resolver::PathResolver},
resolution::errors::ResolverError,
scope::ScopeForest as GenericScopeForest,
type_check::{generics::TraitGenerics, TypeCheckError},
},
Expand All @@ -36,7 +36,7 @@ use crate::{
hir::{
def_collector::{dc_crate::CollectedItems, errors::DefCollectorErrorKind},
def_map::{LocalModuleId, ModuleDefId, ModuleId, MAIN_FUNCTION},
resolution::{import::PathResolution, path_resolver::StandardPathResolver},
resolution::import::PathResolution,
Context,
},
hir_def::function::{FuncMeta, HirFunction},
Expand Down Expand Up @@ -630,10 +630,8 @@ impl<'context> Elaborator<'context> {
}
}

pub fn resolve_module_by_path(&self, path: Path) -> Option<ModuleId> {
let path_resolver = StandardPathResolver::new(self.module_id());

match path_resolver.resolve(self.def_maps, path.clone(), &mut None) {
pub fn resolve_module_by_path(&mut self, path: Path) -> Option<ModuleId> {
match self.resolve_path(path.clone()) {
Ok(PathResolution { module_def_id: ModuleDefId::ModuleId(module_id), error }) => {
if error.is_some() {
None
Expand All @@ -646,9 +644,7 @@ impl<'context> Elaborator<'context> {
}

fn resolve_trait_by_path(&mut self, path: Path) -> Option<TraitId> {
let path_resolver = StandardPathResolver::new(self.module_id());

let error = match path_resolver.resolve(self.def_maps, path.clone(), &mut None) {
let error = match self.resolve_path(path.clone()) {
Ok(PathResolution { module_def_id: ModuleDefId::TraitId(trait_id), error }) => {
if let Some(error) = error {
self.push_err(error);
Expand Down
43 changes: 29 additions & 14 deletions compiler/noirc_frontend/src/elaborator/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use noirc_errors::{Location, Spanned};

use crate::ast::{PathKind, ERROR_IDENT};
use crate::hir::def_map::{LocalModuleId, ModuleId};
use crate::hir::resolution::import::{PathResolution, PathResolutionResult};
use crate::hir::resolution::path_resolver::{PathResolver, StandardPathResolver};
use crate::hir::scope::{Scope as GenericScope, ScopeTree as GenericScopeTree};
use crate::macros_api::Ident;
Expand Down Expand Up @@ -29,7 +30,7 @@ type ScopeTree = GenericScopeTree<String, ResolverMeta>;
impl<'context> Elaborator<'context> {
pub(super) fn lookup<T: TryFromModuleDefId>(&mut self, path: Path) -> Result<T, ResolverError> {
let span = path.span();
let id = self.resolve_path(path)?;
let id = self.resolve_path_or_error(path)?;
T::try_from(id).ok_or_else(|| ResolverError::Expected {
expected: T::description(),
got: id.as_str().to_owned(),
Expand All @@ -42,15 +43,37 @@ impl<'context> Elaborator<'context> {
ModuleId { krate: self.crate_id, local_id: self.local_module }
}

pub(super) fn resolve_path(&mut self, path: Path) -> Result<ModuleDefId, ResolverError> {
pub(super) fn resolve_path_or_error(
&mut self,
path: Path,
) -> Result<ModuleDefId, ResolverError> {
let path_resolution = self.resolve_path(path)?;

if let Some(error) = path_resolution.error {
self.push_err(error);
}

Ok(path_resolution.module_def_id)
}

pub(super) fn resolve_path(&mut self, path: Path) -> PathResolutionResult {
let mut module_id = self.module_id();
let mut path = path;

if path.kind == PathKind::Plain {
let def_map = self.def_maps.get_mut(&self.crate_id).unwrap();
let module_data = &mut def_map.modules[module_id.local_id.0];
module_data.use_import(&path.segments[0].ident);
}

if path.kind == PathKind::Plain && path.first_name() == SELF_TYPE_NAME {
if let Some(Type::Struct(struct_type, _)) = &self.self_type {
let struct_type = struct_type.borrow();
if path.segments.len() == 1 {
return Ok(ModuleDefId::TypeId(struct_type.id));
return Ok(PathResolution {
module_def_id: ModuleDefId::TypeId(struct_type.id),
error: None,
});
}

module_id = struct_type.id.module_id();
Expand All @@ -65,11 +88,7 @@ impl<'context> Elaborator<'context> {
self.resolve_path_in_module(path, module_id)
}

fn resolve_path_in_module(
&mut self,
path: Path,
module_id: ModuleId,
) -> Result<ModuleDefId, ResolverError> {
fn resolve_path_in_module(&mut self, path: Path, module_id: ModuleId) -> PathResolutionResult {
let resolver = StandardPathResolver::new(module_id);
let path_resolution;

Expand Down Expand Up @@ -99,11 +118,7 @@ impl<'context> Elaborator<'context> {
path_resolution = resolver.resolve(self.def_maps, path, &mut None)?;
}

if let Some(error) = path_resolution.error {
self.push_err(error);
}

Ok(path_resolution.module_def_id)
Ok(path_resolution)
}

pub(super) fn get_struct(&self, type_id: StructId) -> Shared<StructType> {
Expand Down Expand Up @@ -150,7 +165,7 @@ impl<'context> Elaborator<'context> {

pub(super) fn lookup_global(&mut self, path: Path) -> Result<DefinitionId, ResolverError> {
let span = path.span();
let id = self.resolve_path(path)?;
let id = self.resolve_path_or_error(path)?;

if let Some(function) = TryFromModuleDefId::try_from(id) {
return Ok(self.interner.function_definition_id(function));
Expand Down
11 changes: 8 additions & 3 deletions compiler/noirc_frontend/src/elaborator/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ impl<'context> Elaborator<'context> {
}

// If we cannot find a local generic of the same name, try to look up a global
match self.resolve_path(path.clone()) {
match self.resolve_path_or_error(path.clone()) {
Ok(ModuleDefId::GlobalId(id)) => {
if let Some(current_item) = self.current_item {
self.interner.add_global_dependency(current_item, id);
Expand All @@ -445,14 +445,19 @@ impl<'context> Elaborator<'context> {
})
}
UnresolvedTypeExpression::Constant(int, _) => Type::Constant(int),
UnresolvedTypeExpression::BinaryOperation(lhs, op, rhs, _) => {
UnresolvedTypeExpression::BinaryOperation(lhs, op, rhs, span) => {
let (lhs_span, rhs_span) = (lhs.span(), rhs.span());
let lhs = self.convert_expression_type(*lhs);
let rhs = self.convert_expression_type(*rhs);

match (lhs, rhs) {
(Type::Constant(lhs), Type::Constant(rhs)) => {
Type::Constant(op.function(lhs, rhs))
if let Some(result) = op.function(lhs, rhs) {
Type::Constant(result)
} else {
self.push_err(ResolverError::OverflowInType { lhs, op, rhs, span });
Type::Error
}
}
(lhs, rhs) => {
if !self.enable_arithmetic_generics {
Expand Down
22 changes: 22 additions & 0 deletions compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,13 +245,15 @@ impl DefCollector {
/// Collect all of the definitions in a given crate into a CrateDefMap
/// Modules which are not a part of the module hierarchy starting with
/// the root module, will be ignored.
#[allow(clippy::too_many_arguments)]
pub fn collect_crate_and_dependencies(
mut def_map: CrateDefMap,
context: &mut Context,
ast: SortedModule,
root_file_id: FileId,
debug_comptime_in_file: Option<&str>,
enable_arithmetic_generics: bool,
error_on_unused_imports: bool,
macro_processors: &[&dyn MacroProcessor],
) -> Vec<(CompilationError, FileId)> {
let mut errors: Vec<(CompilationError, FileId)> = vec![];
Expand All @@ -265,11 +267,13 @@ impl DefCollector {
let crate_graph = &context.crate_graph[crate_id];

for dep in crate_graph.dependencies.clone() {
let error_on_unused_imports = false;
errors.extend(CrateDefMap::collect_defs(
dep.crate_id,
context,
debug_comptime_in_file,
enable_arithmetic_generics,
error_on_unused_imports,
macro_processors,
));

Expand Down Expand Up @@ -413,8 +417,26 @@ impl DefCollector {
);
}

if error_on_unused_imports {
Self::check_unused_imports(context, crate_id, &mut errors);
}

errors
}

fn check_unused_imports(
context: &Context,
crate_id: CrateId,
errors: &mut Vec<(CompilationError, FileId)>,
) {
errors.extend(context.def_maps[&crate_id].modules().iter().flat_map(|(_, module)| {
module.unused_imports().iter().map(|ident| {
let ident = ident.clone();
let error = CompilationError::ResolverError(ResolverError::UnusedImport { ident });
(error, module.location.file)
})
}));
}
}

fn add_import_reference(
Expand Down
3 changes: 3 additions & 0 deletions compiler/noirc_frontend/src/hir/def_map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ impl CrateDefMap {
context: &mut Context,
debug_comptime_in_file: Option<&str>,
enable_arithmetic_generics: bool,
error_on_unused_imports: bool,
macro_processors: &[&dyn MacroProcessor],
) -> Vec<(CompilationError, FileId)> {
// Check if this Crate has already been compiled
Expand Down Expand Up @@ -127,12 +128,14 @@ impl CrateDefMap {
root_file_id,
debug_comptime_in_file,
enable_arithmetic_generics,
error_on_unused_imports,
macro_processors,
));

errors.extend(
parsing_errors.iter().map(|e| (e.clone().into(), root_file_id)).collect::<Vec<_>>(),
);

errors
}

Expand Down
Loading

0 comments on commit 20fdd10

Please sign in to comment.