From c88cbd8f2850a4b77860acc0e58fe65fc024edae Mon Sep 17 00:00:00 2001 From: KtorZ Date: Tue, 18 Mar 2025 09:05:08 +0100 Subject: [PATCH] rework trace label evaluation strategy Namely: 1. Fully evaluate & type-check the label, irrespective of the trace level. So that labels using other variables do not generate "unused identifier" warnings when compiling with different trace mode (and so that the success of a build doesn't depend on the trace level). This was already done for trace arguments, but not for labels, somehow. 2. Move the requirement for compact trace label being String from errors down to warnings; following point (1), we shouldn't fail compilation for different trace levels. It seems more reasonable to simply raise a warning. Signed-off-by: KtorZ --- CHANGELOG.md | 1 + crates/aiken-lang/src/tests/check.rs | 4 +- crates/aiken-lang/src/tipo/error.rs | 10 ++++ crates/aiken-lang/src/tipo/expr.rs | 73 ++++++++++++++-------------- 4 files changed, 50 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4fcba714..561c3364 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ - **aiken-lang**: Change default placeholder for `trace` to `Void` instead of `todo`. @KtorZ - **aiken-lang**: Disallow (parse error) dangling colon `:` in traces. See [#1113](https://github.com/aiken-lang/aiken/issues/1113). @KtorZ - **aiken-lang**: Fix `aiken blueprint apply` wrongly overriding all validators handlers names & ABI to the mint's one. See [#1099](https://github.com/aiken-lang/aiken/issues/1099). @KtorZ +- **aiken-lang**: Always type-check trace label irrespective of the trace level, to avoid unnecessary warnings in compact or silent mode. See [#1122](https://github.com/aiken-lang/aiken/issues/1122). @KtorZ ### Fixed diff --git a/crates/aiken-lang/src/tests/check.rs b/crates/aiken-lang/src/tests/check.rs index 07d39e94..08aec14e 100644 --- a/crates/aiken-lang/src/tests/check.rs +++ b/crates/aiken-lang/src/tests/check.rs @@ -1321,8 +1321,8 @@ fn trace_non_string_label_compact() { "#; assert!(matches!( - check_with_verbosity(parse(source_code), TraceLevel::Compact), - Err((_, Error::CouldNotUnify { .. })) + &check_with_verbosity(parse(source_code), TraceLevel::Compact), + Ok((warnings, _)) if warnings == &[Warning::CompactTraceLabelIsNotstring { location: Span::create(40, 7) }], )) } diff --git a/crates/aiken-lang/src/tipo/error.rs b/crates/aiken-lang/src/tipo/error.rs index 2a438f4a..c5bbbbad 100644 --- a/crates/aiken-lang/src/tipo/error.rs +++ b/crates/aiken-lang/src/tipo/error.rs @@ -1865,6 +1865,15 @@ pub enum Warning { location: Span, suggestion: UntypedPattern, }, + + #[error("I noticed a (compact) dynamic trace label which is not a string")] + #[diagnostic(help("Compiling with a compact trace-level, you are probably expecting compact traces although here, the entire label will need to be serialise *at runtime* which will add a significant overhead.\n\nAs a reminder, trace arguments are fully ignored in compact tracing. Hence, you probably want to put a cute little label here and move the current trace as argument!"))] + #[diagnostic(code("trace::label_is_not_string"))] + #[diagnostic(url("https://aiken-lang.org/language-tour/troubleshooting#traces"))] + CompactTraceLabelIsNotstring { + #[label("compact trace label is not String")] + location: Span, + }, } impl ExtraData for Warning { @@ -1884,6 +1893,7 @@ impl ExtraData for Warning { | Warning::UnusedVariable { .. } | Warning::DiscardedLetAssignment { .. } | Warning::ValidatorInLibraryModule { .. } + | Warning::CompactTraceLabelIsNotstring { .. } | Warning::UseWhenInstead { .. } => None, Warning::Utf8ByteArrayIsValidHexString { value, .. } => Some(value.clone()), Warning::UnusedImportedModule { location, .. } => { diff --git a/crates/aiken-lang/src/tipo/expr.rs b/crates/aiken-lang/src/tipo/expr.rs index 43fe0dd6..a3c2e6d0 100644 --- a/crates/aiken-lang/src/tipo/expr.rs +++ b/crates/aiken-lang/src/tipo/expr.rs @@ -2506,6 +2506,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> { #[allow(clippy::result_large_err)] fn infer_trace_arg(&mut self, arg: UntypedExpr) -> Result { + let location = arg.location(); let typed_arg = self.infer(arg)?; match self.unify( Type::string(), @@ -2514,6 +2515,12 @@ impl<'a, 'b> ExprTyper<'a, 'b> { false, ) { Err(_) => { + if matches!(self.tracing.trace_level(false), TraceLevel::Compact) { + self.environment + .warnings + .push(Warning::CompactTraceLabelIsNotstring { location }); + } + self.unify(Type::data(), typed_arg.tipo(), typed_arg.location(), true)?; Ok(diagnose_expr(typed_arg)) } @@ -2546,44 +2553,38 @@ impl<'a, 'b> ExprTyper<'a, 'b> { }) } + let label = self.infer_trace_arg(label)?; + + let text = if typed_arguments.is_empty() { + label.clone() + } else { + let delimiter = |ix| TypedExpr::String { + location: Span::empty(), + tipo: Type::string(), + value: if ix == 0 { ": " } else { ", " }.to_string(), + }; + typed_arguments + .into_iter() + .enumerate() + .fold(label.clone(), |text, (ix, arg)| { + append_string_expr(append_string_expr(text, delimiter(ix)), arg) + }) + }; + match self.tracing.trace_level(false) { TraceLevel::Silent => Ok(then), - TraceLevel::Compact => { - let text = self.infer(label)?; - self.unify(Type::string(), text.tipo(), text.location(), false)?; - Ok(TypedExpr::Trace { - location, - tipo, - then: Box::new(then), - text: Box::new(text), - }) - } - TraceLevel::Verbose => { - let label = self.infer_trace_arg(label)?; - - let text = if typed_arguments.is_empty() { - label - } else { - let delimiter = |ix| TypedExpr::String { - location: Span::empty(), - tipo: Type::string(), - value: if ix == 0 { ": " } else { ", " }.to_string(), - }; - typed_arguments - .into_iter() - .enumerate() - .fold(label, |text, (ix, arg)| { - append_string_expr(append_string_expr(text, delimiter(ix)), arg) - }) - }; - - Ok(TypedExpr::Trace { - location, - tipo, - then: Box::new(then), - text: Box::new(text), - }) - } + TraceLevel::Compact => Ok(TypedExpr::Trace { + location, + tipo, + then: Box::new(then), + text: Box::new(label), + }), + TraceLevel::Verbose => Ok(TypedExpr::Trace { + location, + tipo, + then: Box::new(then), + text: Box::new(text), + }), } }