Skip to content

Commit

Permalink
Clippy
Browse files Browse the repository at this point in the history
  • Loading branch information
jfecher committed Feb 21, 2025
2 parents b3f7887 + 8e68ce6 commit 1f0244d
Show file tree
Hide file tree
Showing 11 changed files with 288 additions and 31 deletions.
77 changes: 58 additions & 19 deletions compiler/noirc_frontend/src/elaborator/enums.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,8 @@ impl Elaborator<'_> {

let rows = vecmap(rules, |(pattern, branch)| {
self.push_scope();
let pattern = self.expression_to_pattern(pattern, &expected_pattern_type);
let pattern =
self.expression_to_pattern(pattern, &expected_pattern_type, &mut Vec::new());
let columns = vec![Column::new(variable_to_match, pattern)];

let guard = None;
Expand All @@ -295,7 +296,12 @@ impl Elaborator<'_> {
}

/// Convert an expression into a Pattern, defining any variables within.
fn expression_to_pattern(&mut self, expression: Expression, expected_type: &Type) -> Pattern {
fn expression_to_pattern(
&mut self,
expression: Expression,
expected_type: &Type,
variables_defined: &mut Vec<Ident>,
) -> Pattern {
let expr_location = expression.type_location();
let unify_with_expected_type = |this: &mut Self, actual| {
this.unify(actual, expected_type, expr_location.file, || {
Expand Down Expand Up @@ -335,13 +341,25 @@ impl Elaborator<'_> {
Vec::new(),
expected_type,
location,
variables_defined,
),
Err(_) if path_len == 1 => {
// Define the variable
let kind = DefinitionKind::Local(None);
// TODO: `allow_shadowing` is false while I'm too lazy to add a check that we
// don't define the same name multiple times in one pattern.
let id = self.add_variable_decl(last_ident, false, false, true, kind).id;

if let Some(existing) =
variables_defined.iter().find(|elem| *elem == &last_ident)
{
let error = ResolverError::VariableAlreadyDefinedInPattern {
existing: existing.clone(),
new_span: last_ident.span(),
};
self.push_err(error, self.file);
} else {
variables_defined.push(last_ident.clone());
}

let id = self.add_variable_decl(last_ident, false, true, true, kind).id;
self.interner.push_definition_type(id, expected_type.clone());
Pattern::Binding(id)
}
Expand All @@ -354,32 +372,39 @@ impl Elaborator<'_> {
}
}
}
ExpressionKind::Call(call) => {
self.expression_to_constructor(*call.func, call.arguments, expected_type)
ExpressionKind::Call(call) => self.expression_to_constructor(
*call.func,
call.arguments,
expected_type,
variables_defined,
),
ExpressionKind::Constructor(constructor) => {
self.constructor_to_pattern(*constructor, variables_defined)
}
ExpressionKind::Constructor(constructor) => self.constructor_to_pattern(*constructor),
ExpressionKind::Tuple(fields) => {
let field_types = vecmap(0..fields.len(), |_| self.interner.next_type_variable());
let actual = Type::Tuple(field_types.clone());
unify_with_expected_type(self, &actual);

let fields = vecmap(fields.into_iter().enumerate(), |(i, field)| {
let expected = field_types.get(i).unwrap_or(&Type::Error);
self.expression_to_pattern(field, expected)
self.expression_to_pattern(field, expected, variables_defined)
});

Pattern::Constructor(Constructor::Tuple(field_types.clone()), fields)
}

ExpressionKind::Parenthesized(expr) => self.expression_to_pattern(*expr, expected_type),
ExpressionKind::Parenthesized(expr) => {
self.expression_to_pattern(*expr, expected_type, variables_defined)
}
ExpressionKind::Interned(id) => {
let kind = self.interner.get_expression_kind(id);
let expr = Expression::new(kind.clone(), expression.location);
self.expression_to_pattern(expr, expected_type)
self.expression_to_pattern(expr, expected_type, variables_defined)
}
ExpressionKind::InternedStatement(id) => {
if let StatementKind::Expression(expr) = self.interner.get_statement_kind(id) {
self.expression_to_pattern(expr.clone(), expected_type)
self.expression_to_pattern(expr.clone(), expected_type, variables_defined)
} else {
panic!("Invalid expr kind {expression}")
}
Expand Down Expand Up @@ -410,12 +435,16 @@ impl Elaborator<'_> {
}
}

fn constructor_to_pattern(&mut self, constructor: ConstructorExpression) -> Pattern {
fn constructor_to_pattern(
&mut self,
constructor: ConstructorExpression,
variables_defined: &mut Vec<Ident>,
) -> Pattern {
let location = constructor.typ.location;
let typ = match constructor.struct_type {
Some(id) => {
let typ = self.interner.get_type(id);
let generics = typ.borrow().instantiate(&mut self.interner);
let generics = typ.borrow().instantiate(self.interner);
Type::DataType(typ, generics)
}
None => self.resolve_type(constructor.typ),
Expand Down Expand Up @@ -443,7 +472,9 @@ impl Elaborator<'_> {
};

let (field_name, expected_field_type) = expected_field_types.swap_remove(field_index);
fields.insert(field_name, self.expression_to_pattern(field, &expected_field_type));
let pattern =
self.expression_to_pattern(field, &expected_field_type, variables_defined);
fields.insert(field_name, pattern);
}

if !expected_field_types.is_empty() {
Expand All @@ -463,6 +494,7 @@ impl Elaborator<'_> {
name: Expression,
args: Vec<Expression>,
expected_type: &Type,
variables_defined: &mut Vec<Ident>,
) -> Pattern {
match name.kind {
ExpressionKind::Variable(path) => {
Expand All @@ -474,6 +506,7 @@ impl Elaborator<'_> {
args,
expected_type,
location,
variables_defined,
),
Err(error) => {
self.push_err(error, location.file);
Expand All @@ -483,16 +516,21 @@ impl Elaborator<'_> {
}
}
ExpressionKind::Parenthesized(expr) => {
self.expression_to_constructor(*expr, args, expected_type)
self.expression_to_constructor(*expr, args, expected_type, variables_defined)
}
ExpressionKind::Interned(id) => {
let kind = self.interner.get_expression_kind(id);
let expr = Expression::new(kind.clone(), name.location);
self.expression_to_constructor(expr, args, expected_type)
self.expression_to_constructor(expr, args, expected_type, variables_defined)
}
ExpressionKind::InternedStatement(id) => {
if let StatementKind::Expression(expr) = self.interner.get_statement_kind(id) {
self.expression_to_constructor(expr.clone(), args, expected_type)
self.expression_to_constructor(
expr.clone(),
args,
expected_type,
variables_defined,
)
} else {
panic!("Invalid expr kind {name}")
}
Expand All @@ -507,6 +545,7 @@ impl Elaborator<'_> {
args: Vec<Expression>,
expected_type: &Type,
location: Location,
variables_defined: &mut Vec<Ident>,
) -> Pattern {
let span = location.span;

Expand Down Expand Up @@ -565,7 +604,7 @@ impl Elaborator<'_> {

let args = args.into_iter().zip(expected_arg_types);
let args = vecmap(args, |(arg, expected_arg_type)| {
self.expression_to_pattern(arg, &expected_arg_type)
self.expression_to_pattern(arg, &expected_arg_type, variables_defined)
});
let constructor = Constructor::Variant(actual_type, variant_index);
Pattern::Constructor(constructor, args)
Expand Down
10 changes: 9 additions & 1 deletion compiler/noirc_frontend/src/hir/resolution/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,8 @@ pub enum ResolverError {
LoopNotYetSupported { span: Span },
#[error("Expected a trait but found {found}")]
ExpectedTrait { found: String, span: Span },
#[error("Variable '{existing}' was already defined in the same match pattern")]
VariableAlreadyDefinedInPattern { existing: Ident, new_span: Span },
}

impl ResolverError {
Expand Down Expand Up @@ -691,8 +693,14 @@ impl<'a> From<&'a ResolverError> for Diagnostic {
format!("Expected a trait, found {found}"),
String::new(),
*span)

}
ResolverError::VariableAlreadyDefinedInPattern { existing, new_span } => {
let message = format!("Variable `{existing}` was already defined in the same match pattern");
let secondary = format!("`{existing}` redefined here");
let mut error = Diagnostic::simple_error(message, secondary, *new_span);
error.add_secondary(format!("`{existing}` was previously defined here"), existing.span());
error
},
}
}
}
19 changes: 19 additions & 0 deletions compiler/noirc_frontend/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4475,3 +4475,22 @@ fn errors_on_unspecified_unstable_match() {

assert!(matches!(error.reason(), Some(ParserErrorReason::ExperimentalFeature(_))));
}

#[test]
fn errors_on_repeated_match_variables_in_pattern() {
let src = r#"
fn main() {
match (1, 2) {
(_x, _x) => (),
}
}
"#;

let errors = get_program_errors(src);
assert_eq!(errors.len(), 1);

assert!(matches!(
&errors[0].0,
CompilationError::ResolverError(ResolverError::VariableAlreadyDefinedInPattern { .. })
));
}
6 changes: 6 additions & 0 deletions test_programs/execution_success/regression_7195/Nargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "regression_7195"
type = "bin"
authors = [""]

[dependencies]
2 changes: 2 additions & 0 deletions test_programs/execution_success/regression_7195/Prover.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
x = "1"
y = "2"
21 changes: 21 additions & 0 deletions test_programs/execution_success/regression_7195/src/main.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
fn bar(y: Field) {
assert(y != 0);
}

fn foo(x: Field) {
// Safety: test
let y = unsafe { baz(x) };
bar(y);
}

unconstrained fn baz(x: Field) -> Field {
x
}

fn main(x: Field, y: pub Field) {
// Safety: test
let x = unsafe { baz(x) };
foo(x);
foo(y);
assert(x != y);
}
3 changes: 2 additions & 1 deletion tooling/debugger/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ pub fn run_repl_session<B: BlackBoxFunctionSolver<FieldElement>>(
solver: &B,
program: CompiledProgram,
initial_witness: WitnessMap<FieldElement>,
raw_source_printing: bool,
) -> Result<Option<WitnessStack<FieldElement>>, NargoError<FieldElement>> {
repl::run(solver, program, initial_witness)
repl::run(solver, program, initial_witness, raw_source_printing)
}

pub fn run_dap_loop<R: Read, W: Write, B: BlackBoxFunctionSolver<FieldElement>>(
Expand Down
16 changes: 14 additions & 2 deletions tooling/debugger/src/repl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ pub struct ReplDebugger<'a, B: BlackBoxFunctionSolver<FieldElement>> {

// Brillig functions referenced from the ACIR circuits above
unconstrained_functions: &'a [BrilligBytecode<FieldElement>],

// whether to print the source without highlighting, pretty-printing,
// or line numbers
raw_source_printing: bool,
}

impl<'a, B: BlackBoxFunctionSolver<FieldElement>> ReplDebugger<'a, B> {
Expand All @@ -41,6 +45,7 @@ impl<'a, B: BlackBoxFunctionSolver<FieldElement>> ReplDebugger<'a, B> {
debug_artifact: &'a DebugArtifact,
initial_witness: WitnessMap<FieldElement>,
unconstrained_functions: &'a [BrilligBytecode<FieldElement>],
raw_source_printing: bool,
) -> Self {
let foreign_call_executor = Box::new(DefaultDebugForeignCallExecutor::from_artifact(
PrintOutput::Stdout,
Expand Down Expand Up @@ -68,6 +73,7 @@ impl<'a, B: BlackBoxFunctionSolver<FieldElement>> ReplDebugger<'a, B> {
initial_witness,
last_result,
unconstrained_functions,
raw_source_printing,
}
}

Expand Down Expand Up @@ -97,7 +103,11 @@ impl<'a, B: BlackBoxFunctionSolver<FieldElement>> ReplDebugger<'a, B> {
}
}
let locations = self.context.get_source_location_for_debug_location(&location);
print_source_code_location(self.debug_artifact, &locations);
print_source_code_location(
self.debug_artifact,
&locations,
self.raw_source_printing,
);
}
}
}
Expand Down Expand Up @@ -125,7 +135,7 @@ impl<'a, B: BlackBoxFunctionSolver<FieldElement>> ReplDebugger<'a, B> {
}
}
let locations = self.context.get_source_location_for_debug_location(debug_location);
print_source_code_location(self.debug_artifact, &locations);
print_source_code_location(self.debug_artifact, &locations, self.raw_source_printing);
}

pub fn show_current_call_stack(&self) {
Expand Down Expand Up @@ -427,6 +437,7 @@ pub fn run<B: BlackBoxFunctionSolver<FieldElement>>(
blackbox_solver: &B,
program: CompiledProgram,
initial_witness: WitnessMap<FieldElement>,
raw_source_printing: bool,
) -> Result<Option<WitnessStack<FieldElement>>, NargoError<FieldElement>> {
let circuits = &program.program.functions;
let debug_artifact =
Expand All @@ -438,6 +449,7 @@ pub fn run<B: BlackBoxFunctionSolver<FieldElement>>(
debug_artifact,
initial_witness,
unconstrained_functions,
raw_source_printing,
));
let ref_context = &context;

Expand Down
Loading

0 comments on commit 1f0244d

Please sign in to comment.