From 0060804d1aed48bba280388803d206ff11429550 Mon Sep 17 00:00:00 2001 From: KtorZ Date: Tue, 1 Oct 2024 13:17:00 +0200 Subject: [PATCH] Fix redundant warning when destructuring validator params This is not a "proper" fix as it simply get rid of the warning altogether (whether you use or not the destructured values). The reason for removing the warning entirely is because (1) it's simpler, but more so (2) there's no impact on the final code produced _anyway_. Redundant let bindings are already removed by the compiler; and while it's an implicit behaviour that requires a proper warning when it's coming from a user-defined assignment; here the redundant assignment is introduced by the compiler to begin with as another implicit behavior! So we have an implicit behaviour triggering a warning on another implicit behaviour. Truth is, there's no impact in having those parameters destructured and unused. So since users are already not aware that this results in an implicit let assignment being inserted in place for them; there's no need for the warning at all. --- CHANGELOG.md | 1 + crates/aiken-lang/src/ast.rs | 32 +++++++++++++- crates/aiken-lang/src/tests/check.rs | 52 +++++++++++++++++++++++ crates/aiken-lang/src/tipo/environment.rs | 2 + crates/aiken-lang/src/tipo/infer.rs | 32 +++++++++++--- 5 files changed, 111 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b382f868..7295e9db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ ### Changed - **aiken-lang**: Fix compiler crash on trace + expect as last expression of a clause. See #1029. @KtorZ +- **aiken-lang**: Fix redundant warning on introduced identifiers when destructuring validator params. @KtorZ - **uplc**: Fix (again :grimacing:) cost-models for PlutusV1 & PlutusV2. @MicroProofs ### Removed diff --git a/crates/aiken-lang/src/ast.rs b/crates/aiken-lang/src/ast.rs index b74f4d04..3a0555dd 100644 --- a/crates/aiken-lang/src/ast.rs +++ b/crates/aiken-lang/src/ast.rs @@ -1047,7 +1047,7 @@ impl UntypedArg { // NOTE: We use ordinal here not only because it's cute, but because // such a name cannot be parsed to begin with and thus, will not clash // with any user-defined name. - let name = format!("{}_arg", Ordinal::(ix).suffix()); + let name = format!("{}{}_arg", ix + 1, Ordinal::(ix + 1).suffix()); ArgName::Named { label: name.clone(), name, @@ -1770,6 +1770,36 @@ impl UntypedPattern { is_record: false, } } + + pub fn collect_identifiers(&self, collect: &mut F) + where + F: FnMut((String, Span)), + { + match self { + Pattern::Var { name, location } => { + collect((name.to_string(), *location)); + } + Pattern::List { elements, .. } => { + elements.iter().for_each(|e| e.collect_identifiers(collect)); + } + Pattern::Pair { fst, snd, .. } => { + fst.collect_identifiers(collect); + snd.collect_identifiers(collect); + } + Pattern::Tuple { elems, .. } => { + elems.iter().for_each(|e| e.collect_identifiers(collect)); + } + Pattern::Constructor { arguments, .. } => { + arguments + .iter() + .for_each(|arg| arg.value.collect_identifiers(collect)); + } + Pattern::Int { .. } + | Pattern::ByteArray { .. } + | Pattern::Discard { .. } + | Pattern::Assign { .. } => {} + } + } } impl TypedPattern { diff --git a/crates/aiken-lang/src/tests/check.rs b/crates/aiken-lang/src/tests/check.rs index d51ae517..ed4ae19c 100644 --- a/crates/aiken-lang/src/tests/check.rs +++ b/crates/aiken-lang/src/tests/check.rs @@ -3342,3 +3342,55 @@ fn dangling_trace_let_in_trace() { Err((_, Error::LastExpressionIsAssignment { .. })) )) } + +#[test] +fn destructuring_validator_params_tuple() { + let source_code = r#" + validator foo((x, y): (Int, Int)) { + mint(_redeemer, _policy_id, _self) { + x + y > 42 + } + + else(_) { + fail + } + } + "#; + + let result = check_validator(parse(source_code)); + assert!(result.is_ok()); + + let (warnings, _) = result.unwrap(); + assert!( + matches!(&warnings[..], &[]), + "should be empty: {warnings:#?}" + ); +} + +#[test] +fn destructuring_validator_params_record() { + let source_code = r#" + pub type Foo { + Foo(Int, Int) + } + + validator foo(Foo(x, y): Foo) { + mint(_redeemer, _policy_id, _self) { + x + y > 42 + } + + else(_) { + fail + } + } + "#; + + let result = check_validator(parse(source_code)); + assert!(result.is_ok()); + + let (warnings, _) = result.unwrap(); + assert!( + matches!(&warnings[..], &[]), + "should be empty: {warnings:#?}" + ); +} diff --git a/crates/aiken-lang/src/tipo/environment.rs b/crates/aiken-lang/src/tipo/environment.rs index a9b4c3a6..9e9f4c92 100644 --- a/crates/aiken-lang/src/tipo/environment.rs +++ b/crates/aiken-lang/src/tipo/environment.rs @@ -39,6 +39,7 @@ pub struct Environment<'a> { pub entity_usages: Vec>, pub id_gen: IdGenerator, pub importable_modules: &'a HashMap, + pub validator_params: HashSet<(String, Span)>, /// Modules that have been imported by the current module, along with the /// location of the import statement where they were imported. @@ -792,6 +793,7 @@ impl<'a> Environment<'a> { annotations: HashMap::new(), warnings, entity_usages: vec![HashMap::new()], + validator_params: HashSet::new(), target_env, } } diff --git a/crates/aiken-lang/src/tipo/infer.rs b/crates/aiken-lang/src/tipo/infer.rs index 5aa2d40f..d69ef03e 100644 --- a/crates/aiken-lang/src/tipo/infer.rs +++ b/crates/aiken-lang/src/tipo/infer.rs @@ -7,10 +7,10 @@ use super::{ }; use crate::{ ast::{ - Annotation, ArgName, ArgVia, DataType, Definition, Function, ModuleConstant, ModuleKind, - RecordConstructor, RecordConstructorArg, Tracing, TypeAlias, TypedArg, TypedDefinition, - TypedModule, TypedValidator, UntypedArg, UntypedDefinition, UntypedModule, UntypedPattern, - UntypedValidator, Use, Validator, + Annotation, ArgBy, ArgName, ArgVia, DataType, Definition, Function, ModuleConstant, + ModuleKind, RecordConstructor, RecordConstructorArg, Tracing, TypeAlias, TypedArg, + TypedDefinition, TypedModule, TypedValidator, UntypedArg, UntypedDefinition, UntypedModule, + UntypedPattern, UntypedValidator, Use, Validator, }, expr::{TypedExpr, UntypedAssignmentKind}, tipo::{expr::infer_function, Span, Type, TypeVar}, @@ -100,6 +100,14 @@ impl UntypedModule { .collect(); // Generate warnings for unused items + println!("warnings: {:#?}", environment.warnings); + println!("params: {:#?}", environment.validator_params); + environment.warnings.retain(|warning| match warning { + Warning::UnusedVariable { location, name } => !environment + .validator_params + .contains(&(name.to_string(), *location)), + _ => true, + }); environment.convert_unused_to_warnings(); // Remove private and imported types and values to create the public interface @@ -812,7 +820,11 @@ fn annotate_fuzzer(tipo: &Type, location: &Span) -> Result { } } -fn put_params_in_scope(name: &str, environment: &mut Environment, params: &[UntypedArg]) { +fn put_params_in_scope<'a>( + name: &'_ str, + environment: &'a mut Environment, + params: &'a [UntypedArg], +) { let preregistered_fn = environment .get_variable(name) .expect("Could not find preregistered type for function"); @@ -828,7 +840,7 @@ fn put_params_in_scope(name: &str, environment: &mut Environment, params: &[Unty .zip(args_types[0..params.len()].iter()) .enumerate() { - match &arg.arg_name(ix) { + match arg.arg_name(ix) { ArgName::Named { name, label: _, @@ -842,7 +854,13 @@ fn put_params_in_scope(name: &str, environment: &mut Environment, params: &[Unty t.clone(), ); - environment.init_usage(name.to_string(), EntityKind::Variable, arg.location); + if let ArgBy::ByPattern(ref pattern) = arg.by { + pattern.collect_identifiers(&mut |identifier| { + environment.validator_params.insert(identifier); + }) + } + + environment.init_usage(name, EntityKind::Variable, arg.location); } ArgName::Named { .. } | ArgName::Discarded { .. } => (), };