Skip to content

Commit

Permalink
Merge branch 'master' into mv/ssa-globals-1
Browse files Browse the repository at this point in the history
  • Loading branch information
vezenovm authored Jan 10, 2025
2 parents df5eef0 + 7b7d7c6 commit 584cc92
Show file tree
Hide file tree
Showing 16 changed files with 471 additions and 379 deletions.
23 changes: 20 additions & 3 deletions compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,11 @@ pub struct CompileOptions {
/// Only show SSA passes whose name contains the provided string.
/// This setting takes precedence over `show_ssa` if it's not empty.
#[arg(long, hide = true)]
pub show_ssa_pass_name: Option<String>,
pub show_ssa_pass: Option<String>,

/// Only show the SSA and ACIR for the contract function with a given name.
#[arg(long, hide = true)]
pub show_contract_fn: Option<String>,

/// Emit the unoptimized SSA IR to file.
/// The IR will be dumped into the workspace target directory,
Expand Down Expand Up @@ -442,6 +446,11 @@ pub fn compile_contract(

if options.print_acir {
for contract_function in &compiled_contract.functions {
if let Some(ref name) = options.show_contract_fn {
if name != &contract_function.name {
continue;
}
}
println!(
"Compiled ACIR for {}::{} (unoptimized):",
compiled_contract.name, contract_function.name
Expand Down Expand Up @@ -486,7 +495,15 @@ fn compile_contract_inner(
continue;
}

let function = match compile_no_check(context, options, function_id, None, true) {
let mut options = options.clone();

if let Some(ref name_filter) = options.show_contract_fn {
let show = name == *name_filter;
options.show_ssa &= show;
options.show_ssa_pass = options.show_ssa_pass.filter(|_| show);
};

let function = match compile_no_check(context, &options, function_id, None, true) {
Ok(function) => function,
Err(new_error) => {
errors.push(FileDiagnostic::from(new_error));
Expand Down Expand Up @@ -642,7 +659,7 @@ pub fn compile_no_check(

let return_visibility = program.return_visibility;
let ssa_evaluator_options = noirc_evaluator::ssa::SsaEvaluatorOptions {
ssa_logging: match &options.show_ssa_pass_name {
ssa_logging: match &options.show_ssa_pass {
Some(string) => SsaLogging::Contains(string.clone()),
None => {
if options.show_ssa {
Expand Down
6 changes: 3 additions & 3 deletions compiler/noirc_evaluator/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ pub enum RuntimeError {
StaticAssertDynamicMessage { call_stack: CallStack },
#[error("Argument is dynamic")]
StaticAssertDynamicPredicate { call_stack: CallStack },
#[error("Argument is false")]
StaticAssertFailed { call_stack: CallStack },
#[error("{message}")]
StaticAssertFailed { message: String, call_stack: CallStack },
#[error("Nested slices, i.e. slices within an array or slice, are not supported")]
NestedSlice { call_stack: CallStack },
#[error("Big Integer modulus do no match")]
Expand Down Expand Up @@ -165,7 +165,7 @@ impl RuntimeError {
| RuntimeError::AssertConstantFailed { call_stack }
| RuntimeError::StaticAssertDynamicMessage { call_stack }
| RuntimeError::StaticAssertDynamicPredicate { call_stack }
| RuntimeError::StaticAssertFailed { call_stack }
| RuntimeError::StaticAssertFailed { call_stack, .. }
| RuntimeError::IntegerOutOfBounds { call_stack, .. }
| RuntimeError::UnsupportedIntegerSize { call_stack, .. }
| RuntimeError::InvalidBlackBoxInputBitSize { call_stack, .. }
Expand Down
18 changes: 18 additions & 0 deletions compiler/noirc_evaluator/src/ssa/ir/dfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,24 @@ impl DataFlowGraph {
}
}

/// If this value points to an array of constant bytes, returns a string
/// consisting of those bytes if they form a valid UTF-8 string.
pub(crate) fn get_string(&self, value: ValueId) -> Option<String> {
let (value_ids, _typ) = self.get_array_constant(value)?;

let mut bytes = Vec::new();
for value_id in value_ids {
let field_value = self.get_numeric_constant(value_id)?;
let u64_value = field_value.try_to_u64()?;
if u64_value > 255 {
return None;
};
let byte = u64_value as u8;
bytes.push(byte);
}
String::from_utf8(bytes).ok()
}

/// A constant index less than the array length is safe
pub(crate) fn is_safe_index(&self, index: ValueId, array: ValueId) -> bool {
#[allow(clippy::match_like_matches_macro)]
Expand Down
21 changes: 5 additions & 16 deletions compiler/noirc_evaluator/src/ssa/ir/printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,22 +317,11 @@ pub(crate) fn try_to_extract_string_from_error_payload(
values: &[ValueId],
dfg: &DataFlowGraph,
) -> Option<String> {
(is_string_type && (values.len() == 1))
.then_some(())
.and_then(|()| {
let (values, _) = &dfg.get_array_constant(values[0])?;
let values = values.iter().map(|value_id| dfg.get_numeric_constant(*value_id));
values.collect::<Option<Vec<_>>>()
})
.map(|fields| {
fields
.iter()
.map(|field| {
let as_u8 = field.try_to_u64().unwrap_or_default() as u8;
as_u8 as char
})
.collect()
})
if is_string_type && values.len() == 1 {
dfg.get_string(values[0])
} else {
None
}
}

fn display_constrain_error(
Expand Down
6 changes: 5 additions & 1 deletion compiler/noirc_evaluator/src/ssa/opt/assert_constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,11 @@ fn evaluate_static_assert(
} else {
let call_stack = function.dfg.get_instruction_call_stack(instruction);
if function.dfg.is_constant(arguments[0]) {
Err(RuntimeError::StaticAssertFailed { call_stack })
let message = function
.dfg
.get_string(arguments[1])
.expect("Expected second argument to be a string");
Err(RuntimeError::StaticAssertFailed { message, call_stack })
} else {
Err(RuntimeError::StaticAssertDynamicPredicate { call_stack })
}
Expand Down
80 changes: 46 additions & 34 deletions compiler/noirc_frontend/src/elaborator/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1361,7 +1361,7 @@ impl<'context> Elaborator<'context> {
span: Span,
has_self_arg: bool,
) -> Option<HirMethodReference> {
// First search in the struct methods
// First search in the type methods. If there is one, that's the one.
if let Some(method_id) =
self.interner.lookup_direct_method(object_type, method_name, has_self_arg)
{
Expand All @@ -1372,43 +1372,55 @@ impl<'context> Elaborator<'context> {
let trait_methods =
self.interner.lookup_trait_methods(object_type, method_name, has_self_arg);

if trait_methods.is_empty() {
// If we couldn't find any trait methods, search in
// impls for all types `T`, e.g. `impl<T> Foo for T`
if let Some(func_id) =
self.interner.lookup_generic_method(object_type, method_name, has_self_arg)
{
return Some(HirMethodReference::FuncId(func_id));
}
// If there's at least one matching trait method we need to see if only one is in scope.
if !trait_methods.is_empty() {
return self.return_trait_method_in_scope(&trait_methods, method_name, span);
}

if let Type::Struct(struct_type, _) = object_type {
let has_field_with_function_type =
struct_type.borrow().get_fields_as_written().into_iter().any(|field| {
field.name.0.contents == method_name && field.typ.is_function()
});
if has_field_with_function_type {
self.push_err(TypeCheckError::CannotInvokeStructFieldFunctionType {
method_name: method_name.to_string(),
object_type: object_type.clone(),
span,
});
} else {
self.push_err(TypeCheckError::UnresolvedMethodCall {
method_name: method_name.to_string(),
object_type: object_type.clone(),
span,
});
}
return None;
// If we couldn't find any trait methods, search in
// impls for all types `T`, e.g. `impl<T> Foo for T`
let generic_methods =
self.interner.lookup_generic_methods(object_type, method_name, has_self_arg);
if !generic_methods.is_empty() {
return self.return_trait_method_in_scope(&generic_methods, method_name, span);
}

if let Type::Struct(struct_type, _) = object_type {
let has_field_with_function_type = struct_type
.borrow()
.get_fields_as_written()
.into_iter()
.any(|field| field.name.0.contents == method_name && field.typ.is_function());
if has_field_with_function_type {
self.push_err(TypeCheckError::CannotInvokeStructFieldFunctionType {
method_name: method_name.to_string(),
object_type: object_type.clone(),
span,
});
} else {
// It could be that this type is a composite type that is bound to a trait,
// for example `x: (T, U) ... where (T, U): SomeTrait`
// (so this case is a generalization of the NamedGeneric case)
return self.lookup_method_in_trait_constraints(object_type, method_name, span);
self.push_err(TypeCheckError::UnresolvedMethodCall {
method_name: method_name.to_string(),
object_type: object_type.clone(),
span,
});
}
None
} else {
// It could be that this type is a composite type that is bound to a trait,
// for example `x: (T, U) ... where (T, U): SomeTrait`
// (so this case is a generalization of the NamedGeneric case)
self.lookup_method_in_trait_constraints(object_type, method_name, span)
}
}

// We found some trait methods... but is only one of them currently in scope?
/// Given a list of functions and the trait they belong to, returns the one function
/// that is in scope.
fn return_trait_method_in_scope(
&mut self,
trait_methods: &[(FuncId, TraitId)],
method_name: &str,
span: Span,
) -> Option<HirMethodReference> {
let module_id = self.module_id();
let module_data = self.get_module(module_id);

Expand Down Expand Up @@ -1490,7 +1502,7 @@ impl<'context> Elaborator<'context> {
fn trait_hir_method_reference(
&self,
trait_id: TraitId,
trait_methods: Vec<(FuncId, TraitId)>,
trait_methods: &[(FuncId, TraitId)],
method_name: &str,
span: Span,
) -> HirMethodReference {
Expand Down
45 changes: 10 additions & 35 deletions compiler/noirc_frontend/src/node_interner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1746,7 +1746,7 @@ impl NodeInterner {
Ok(())
}

/// Looks up a method that's directly defined in the given struct.
/// Looks up a method that's directly defined in the given type.
pub fn lookup_direct_method(
&self,
typ: &Type,
Expand All @@ -1761,7 +1761,7 @@ impl NodeInterner {
.and_then(|methods| methods.find_direct_method(typ, has_self_arg, self))
}

/// Looks up a methods that apply to the given struct but are defined in traits.
/// Looks up a methods that apply to the given type but are defined in traits.
pub fn lookup_trait_methods(
&self,
typ: &Type,
Expand All @@ -1780,43 +1780,18 @@ impl NodeInterner {
}
}

/// Select the 1 matching method with an object type matching `typ`
fn find_matching_method(
&self,
typ: &Type,
methods: Option<&Methods>,
method_name: &str,
has_self_arg: bool,
) -> Option<FuncId> {
if let Some(method) = methods.and_then(|m| m.find_matching_method(typ, has_self_arg, self))
{
Some(method)
} else {
self.lookup_generic_method(typ, method_name, has_self_arg)
}
}

/// Looks up a method at impls for all types `T`, e.g. `impl<T> Foo for T`
pub fn lookup_generic_method(
/// Looks up methods at impls for all types `T`, e.g. `impl<T> Foo for T`
pub fn lookup_generic_methods(
&self,
typ: &Type,
method_name: &str,
has_self_arg: bool,
) -> Option<FuncId> {
let global_methods = self.methods.get(&TypeMethodKey::Generic)?.get(method_name)?;
global_methods.find_matching_method(typ, has_self_arg, self)
}

/// Looks up a given method name on the given primitive type.
pub fn lookup_primitive_method(
&self,
typ: &Type,
method_name: &str,
has_self_arg: bool,
) -> Option<FuncId> {
let key = get_type_method_key(typ)?;
let methods = self.methods.get(&key)?.get(method_name)?;
self.find_matching_method(typ, Some(methods), method_name, has_self_arg)
) -> Vec<(FuncId, TraitId)> {
self.methods
.get(&TypeMethodKey::Generic)
.and_then(|h| h.get(method_name))
.map(|methods| methods.find_trait_methods(typ, has_self_arg, self))
.unwrap_or_default()
}

/// Returns what the next trait impl id is expected to be.
Expand Down
70 changes: 70 additions & 0 deletions compiler/noirc_frontend/src/tests/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -879,6 +879,43 @@ fn errors_if_multiple_trait_methods_are_in_scope_for_function_call() {
assert_eq!(traits, vec!["private_mod::Foo", "private_mod::Foo2"]);
}

#[test]
fn warns_if_trait_is_not_in_scope_for_method_call_and_there_is_only_one_trait_method() {
let src = r#"
fn main() {
let bar = Bar { x: 42 };
let _ = bar.foo();
}
pub struct Bar {
x: i32,
}
mod private_mod {
pub trait Foo {
fn foo(self) -> i32;
}
impl Foo for super::Bar {
fn foo(self) -> i32 {
self.x
}
}
}
"#;
let errors = get_program_errors(src);
assert_eq!(errors.len(), 1);

let CompilationError::ResolverError(ResolverError::PathResolutionError(
PathResolutionError::TraitMethodNotInScope { ident, trait_name },
)) = &errors[0].0
else {
panic!("Expected a 'trait method not in scope' error");
};
assert_eq!(ident.to_string(), "foo");
assert_eq!(trait_name, "private_mod::Foo");
}

#[test]
fn calls_trait_method_if_it_is_in_scope() {
let src = r#"
Expand Down Expand Up @@ -1166,3 +1203,36 @@ fn warns_if_trait_is_not_in_scope_for_primitive_method_call_and_there_is_only_on
assert_eq!(ident.to_string(), "foo");
assert_eq!(trait_name, "private_mod::Foo");
}

#[test]
fn warns_if_trait_is_not_in_scope_for_generic_function_call_and_there_is_only_one_trait_method() {
let src = r#"
fn main() {
let x: i32 = 1;
let _ = x.foo();
}
mod private_mod {
pub trait Foo<T> {
fn foo(self) -> i32;
}
impl<T> Foo<T> for T {
fn foo(self) -> i32 {
42
}
}
}
"#;
let errors = get_program_errors(src);
assert_eq!(errors.len(), 1);

let CompilationError::ResolverError(ResolverError::PathResolutionError(
PathResolutionError::TraitMethodNotInScope { ident, trait_name },
)) = &errors[0].0
else {
panic!("Expected a 'trait method not in scope' error");
};
assert_eq!(ident.to_string(), "foo");
assert_eq!(trait_name, "private_mod::Foo");
}
Loading

0 comments on commit 584cc92

Please sign in to comment.