From 6525f21712ab1ed5f97fa62a029aa102fc022f60 Mon Sep 17 00:00:00 2001 From: KtorZ Date: Wed, 15 Feb 2023 21:53:11 +0100 Subject: [PATCH] Remove 'Todo' from the AST & AIR Todo is fundamentally just a trace and an error. The only reason we kept it as a separate element in the AST is for the formatter to work out whether it should format something back to a todo or something else. However, this introduces redundancy in the code internally and makes the AIR more complicated than it needs to be. Both todo and errors can actually be represented as trace + errors, and we only need to record their preferred shape when parsing so that we can format them back to what's expected. --- crates/aiken-lang/src/air.rs | 8 -- crates/aiken-lang/src/ast.rs | 7 +- crates/aiken-lang/src/builder.rs | 9 -- crates/aiken-lang/src/expr.rs | 35 +++--- crates/aiken-lang/src/format.rs | 3 - crates/aiken-lang/src/parser.rs | 56 ++++----- crates/aiken-lang/src/tests/parser.rs | 162 +++++++++++++++++++++----- crates/aiken-lang/src/tipo/error.rs | 3 +- crates/aiken-lang/src/tipo/expr.rs | 33 ++---- crates/aiken-lang/src/uplc.rs | 34 ------ 10 files changed, 179 insertions(+), 171 deletions(-) diff --git a/crates/aiken-lang/src/air.rs b/crates/aiken-lang/src/air.rs index 39909907..1bd2add4 100644 --- a/crates/aiken-lang/src/air.rs +++ b/crates/aiken-lang/src/air.rs @@ -235,12 +235,6 @@ pub enum Air { }, // Misc. - Todo { - scope: Vec, - label: Option, - tipo: Arc, - }, - ErrorTerm { scope: Vec, tipo: Arc, @@ -290,7 +284,6 @@ impl Air { | Air::ListExpose { scope, .. } | Air::TupleAccessor { scope, .. } | Air::TupleIndex { scope, .. } - | Air::Todo { scope, .. } | Air::ErrorTerm { scope, .. } | Air::Trace { scope, .. } => scope.clone(), } @@ -373,7 +366,6 @@ impl Air { | Air::ListExpose { tipo, .. } | Air::TupleAccessor { tipo, .. } | Air::TupleIndex { tipo, .. } - | Air::Todo { tipo, .. } | Air::ErrorTerm { tipo, .. } | Air::Trace { tipo, .. } => Some(tipo.clone()), diff --git a/crates/aiken-lang/src/ast.rs b/crates/aiken-lang/src/ast.rs index b137c7f2..2f28b034 100644 --- a/crates/aiken-lang/src/ast.rs +++ b/crates/aiken-lang/src/ast.rs @@ -995,9 +995,10 @@ pub struct RecordUpdateSpread { } #[derive(Debug, Clone, PartialEq, Eq)] -pub enum TodoKind { - Keyword, - EmptyFunction, +pub enum TraceKind { + Trace, + Todo, + Error, } #[derive(Copy, Clone, PartialEq, Eq)] diff --git a/crates/aiken-lang/src/builder.rs b/crates/aiken-lang/src/builder.rs index f204e91f..063c623f 100644 --- a/crates/aiken-lang/src/builder.rs +++ b/crates/aiken-lang/src/builder.rs @@ -1678,15 +1678,6 @@ pub fn monomorphize( needs_variant = true; } } - Air::Todo { scope, label, tipo } => { - if tipo.is_generic() { - let mut tipo = tipo.clone(); - find_generics_to_replace(&mut tipo, &generic_types); - - new_air[index] = Air::Todo { scope, tipo, label }; - needs_variant = true; - } - } Air::ErrorTerm { scope, tipo } => { if tipo.is_generic() { let mut tipo = tipo.clone(); diff --git a/crates/aiken-lang/src/expr.rs b/crates/aiken-lang/src/expr.rs index 9afb0803..9bf00b3c 100644 --- a/crates/aiken-lang/src/expr.rs +++ b/crates/aiken-lang/src/expr.rs @@ -5,8 +5,7 @@ use vec1::Vec1; use crate::{ ast::{ Annotation, Arg, AssignmentKind, BinOp, CallArg, Clause, DefinitionLocation, IfBranch, - Pattern, RecordUpdateSpread, Span, TodoKind, TypedRecordUpdateArg, UnOp, - UntypedRecordUpdateArg, + Pattern, RecordUpdateSpread, Span, TypedRecordUpdateArg, UnOp, UntypedRecordUpdateArg, }, builtins::void, tipo::{ModuleValueConstructor, PatternConstructor, Type, ValueConstructor}, @@ -143,12 +142,6 @@ pub enum TypedExpr { tuple: Box, }, - Todo { - location: Span, - label: Option, - tipo: Arc, - }, - ErrorTerm { location: Span, tipo: Arc, @@ -176,7 +169,6 @@ impl TypedExpr { Self::Trace { then, .. } => then.tipo(), Self::Fn { tipo, .. } | Self::Int { tipo, .. } - | Self::Todo { tipo, .. } | Self::ErrorTerm { tipo, .. } | Self::When { tipo, .. } | Self::List { tipo, .. } @@ -222,7 +214,6 @@ impl TypedExpr { | TypedExpr::List { .. } | TypedExpr::Call { .. } | TypedExpr::When { .. } - | TypedExpr::Todo { .. } | TypedExpr::ErrorTerm { .. } | TypedExpr::BinOp { .. } | TypedExpr::Tuple { .. } @@ -261,7 +252,6 @@ impl TypedExpr { | Self::Int { location, .. } | Self::Var { location, .. } | Self::Trace { location, .. } - | Self::Todo { location, .. } | Self::ErrorTerm { location, .. } | Self::When { location, .. } | Self::Call { location, .. } @@ -297,7 +287,6 @@ impl TypedExpr { | Self::Int { location, .. } | Self::Trace { location, .. } | Self::Var { location, .. } - | Self::Todo { location, .. } | Self::ErrorTerm { location, .. } | Self::When { location, .. } | Self::Call { location, .. } @@ -420,12 +409,6 @@ pub enum UntypedExpr { tuple: Box, }, - Todo { - kind: TodoKind, - location: Span, - label: Option, - }, - ErrorTerm { location: Span, }, @@ -444,7 +427,22 @@ pub enum UntypedExpr { }, } +pub const DEFAULT_TODO_STR: &str = "aiken::todo"; + +pub const DEFAULT_ERROR_STR: &str = "aiken::error"; + impl UntypedExpr { + pub fn todo(location: Span, reason: Option) -> Self { + UntypedExpr::Trace { + location, + then: Box::new(UntypedExpr::ErrorTerm { location }), + text: Box::new(reason.unwrap_or_else(|| UntypedExpr::String { + location, + value: DEFAULT_TODO_STR.to_string(), + })), + } + } + pub fn append_in_sequence(self, next: Self) -> Self { let location = Span { start: self.location().start, @@ -500,7 +498,6 @@ impl UntypedExpr { Self::Fn { location, .. } | Self::Var { location, .. } | Self::Int { location, .. } - | Self::Todo { location, .. } | Self::ErrorTerm { location, .. } | Self::When { location, .. } | Self::Call { location, .. } diff --git a/crates/aiken-lang/src/format.rs b/crates/aiken-lang/src/format.rs index 12e42e40..edc131d4 100644 --- a/crates/aiken-lang/src/format.rs +++ b/crates/aiken-lang/src/format.rs @@ -654,9 +654,6 @@ impl<'comments> Formatter<'comments> { final_else, .. } => self.if_expr(branches, final_else), - UntypedExpr::Todo { label: None, .. } => "todo".to_doc(), - - UntypedExpr::Todo { label: Some(l), .. } => docvec!["todo(\"", l, "\")"], UntypedExpr::PipeLine { expressions, .. } => self.pipeline(expressions), diff --git a/crates/aiken-lang/src/parser.rs b/crates/aiken-lang/src/parser.rs index 84816880..46dcc6c9 100644 --- a/crates/aiken-lang/src/parser.rs +++ b/crates/aiken-lang/src/parser.rs @@ -7,7 +7,7 @@ pub mod lexer; pub mod token; use crate::{ - ast::{self, BinOp, Span, TodoKind, UnOp, UntypedDefinition, CAPTURE_VARIABLE}, + ast::{self, BinOp, Span, UnOp, UntypedDefinition, CAPTURE_VARIABLE}, expr, }; @@ -254,11 +254,7 @@ pub fn fn_parser() -> impl Parser impl Parser impl Parser impl Parser( - r: Recursive<'a, Token, expr::UntypedExpr, ParseError>, +pub fn todo_parser( + r: Recursive<'_, Token, expr::UntypedExpr, ParseError>, keyword: Token, - default_value: &'a str, -) -> impl Parser + 'a { - just(keyword) +) -> impl Parser + '_ { + just(keyword.clone()) .ignore_then(expr_parser(r.clone()).or_not()) - .then(r.clone().or_not()) - .map_with_span(|(text, then_), span| match then_ { - None => expr::UntypedExpr::Trace { + .map_with_span(move |text, span| expr::UntypedExpr::Trace { + location: span, + then: Box::new(expr::UntypedExpr::ErrorTerm { location: span }), + text: Box::new(text.unwrap_or_else(|| expr::UntypedExpr::String { location: span, - then: Box::new(expr::UntypedExpr::ErrorTerm { location: span }), - text: Box::new(text.unwrap_or_else(|| expr::UntypedExpr::String { - location: span, - value: default_value.to_string(), - })), - }, - Some(e) => expr::UntypedExpr::Trace { - location: span, - then: Box::new( - expr::UntypedExpr::ErrorTerm { location: span }.append_in_sequence(e), - ), - text: Box::new(text.unwrap_or_else(|| expr::UntypedExpr::String { - location: span, - value: default_value.to_string(), - })), - }, + value: match keyword { + Token::ErrorTerm => expr::DEFAULT_ERROR_STR.to_string(), + Token::Todo => expr::DEFAULT_TODO_STR.to_string(), + _ => unreachable!(), + }, + })), }) } @@ -946,6 +929,7 @@ pub fn expr_parser( just(Token::RightParen), ), just(Token::ErrorTerm).rewind().ignore_then(seq_r.clone()), + just(Token::Todo).rewind().ignore_then(seq_r.clone()), )); let anon_fn_parser = just(Token::Fn) diff --git a/crates/aiken-lang/src/tests/parser.rs b/crates/aiken-lang/src/tests/parser.rs index 322ccb73..15712962 100644 --- a/crates/aiken-lang/src/tests/parser.rs +++ b/crates/aiken-lang/src/tests/parser.rs @@ -308,10 +308,15 @@ fn empty_function() { code, vec![ast::UntypedDefinition::Fn(Function { arguments: vec![], - body: expr::UntypedExpr::Todo { - kind: ast::TodoKind::EmptyFunction, + body: expr::UntypedExpr::Trace { location: Span::new((), 0..15), - label: None, + text: Box::new(expr::UntypedExpr::String { + value: "aiken::todo".to_string(), + location: Span::new((), 0..15), + }), + then: Box::new(expr::UntypedExpr::ErrorTerm { + location: Span::new((), 0..15), + }), }, doc: None, location: Span::new((), 0..12), @@ -1781,10 +1786,15 @@ fn function_def() { vec![ast::UntypedDefinition::Fn(Function { doc: None, arguments: vec![], - body: expr::UntypedExpr::Todo { - kind: ast::TodoKind::EmptyFunction, + body: expr::UntypedExpr::Trace { location: Span::new((), 0..11), - label: None, + text: Box::new(expr::UntypedExpr::String { + value: "aiken::todo".to_string(), + location: Span::new((), 0..11), + }), + then: Box::new(expr::UntypedExpr::ErrorTerm { + location: Span::new((), 0..11), + }), }, location: Span::new((), 0..8), name: "foo".to_string(), @@ -2710,7 +2720,6 @@ fn parse_keyword_error() { let code = indoc! {r#" fn foo() { error "not implemented" - Void } fn bar() { @@ -2726,18 +2735,9 @@ fn parse_keyword_error() { ast::Definition::Fn(Function { arguments: vec![], body: expr::UntypedExpr::Trace { - location: Span::new((), 13..43), - then: Box::new(expr::UntypedExpr::Sequence { - location: Span::new((), 13..43), - expressions: vec![ - expr::UntypedExpr::ErrorTerm { - location: Span::new((), 13..43), - }, - expr::UntypedExpr::Var { - location: Span::new((), 39..43), - name: "Void".to_string(), - }, - ], + location: Span::new((), 13..36), + then: Box::new(expr::UntypedExpr::ErrorTerm { + location: Span::new((), 13..36), }), text: Box::new(expr::UntypedExpr::String { location: Span::new((), 19..36), @@ -2750,22 +2750,22 @@ fn parse_keyword_error() { public: false, return_annotation: None, return_type: (), - end_position: 44, + end_position: 37, }), ast::Definition::Fn(Function { arguments: vec![], body: expr::UntypedExpr::When { - location: Span::new((), 60..116), + location: Span::new((), 53..109), subjects: vec![expr::UntypedExpr::Var { - location: Span::new((), 65..66), + location: Span::new((), 58..59), name: "x".to_string(), }], clauses: vec![ ast::Clause { - location: Span::new((), 78..95), + location: Span::new((), 71..88), pattern: vec![ast::Pattern::Constructor { is_record: false, - location: Span::new((), 78..87), + location: Span::new((), 71..80), name: "Something".to_string(), arguments: vec![], module: None, @@ -2776,25 +2776,25 @@ fn parse_keyword_error() { alternative_patterns: vec![], guard: None, then: expr::UntypedExpr::Var { - location: Span::new((), 91..95), + location: Span::new((), 84..88), name: "Void".to_string(), }, }, ast::Clause { - location: Span::new((), 102..112), + location: Span::new((), 95..105), pattern: vec![ast::Pattern::Discard { name: "_".to_string(), - location: Span::new((), 102..103), + location: Span::new((), 95..96), }], alternative_patterns: vec![], guard: None, then: expr::UntypedExpr::Trace { - location: Span::new((), 107..112), + location: Span::new((), 100..105), then: Box::new(expr::UntypedExpr::ErrorTerm { - location: Span::new((), 107..112), + location: Span::new((), 100..105), }), text: Box::new(expr::UntypedExpr::String { - location: Span::new((), 107..112), + location: Span::new((), 100..105), value: "aiken::error".to_string(), }), }, @@ -2802,12 +2802,110 @@ fn parse_keyword_error() { ], }, doc: None, - location: Span::new((), 47..55), + location: Span::new((), 40..48), name: "bar".to_string(), public: false, return_annotation: None, return_type: (), - end_position: 117, + end_position: 110, + }), + ], + ) +} + +#[test] +fn parse_keyword_todo() { + let code = indoc! {r#" + fn foo() { + todo "not implemented" + } + + fn bar() { + when x is { + Something -> Void + _ -> todo + } + } + "#}; + assert_definitions( + code, + vec![ + ast::Definition::Fn(Function { + arguments: vec![], + body: expr::UntypedExpr::Trace { + location: Span::new((), 13..35), + then: Box::new(expr::UntypedExpr::ErrorTerm { + location: Span::new((), 13..35), + }), + text: Box::new(expr::UntypedExpr::String { + location: Span::new((), 18..35), + value: "not implemented".to_string(), + }), + }, + doc: None, + location: Span::new((), 0..8), + name: "foo".to_string(), + public: false, + return_annotation: None, + return_type: (), + end_position: 36, + }), + ast::Definition::Fn(Function { + arguments: vec![], + body: expr::UntypedExpr::When { + location: Span::new((), 52..107), + subjects: vec![expr::UntypedExpr::Var { + location: Span::new((), 57..58), + name: "x".to_string(), + }], + clauses: vec![ + ast::Clause { + location: Span::new((), 70..87), + pattern: vec![ast::Pattern::Constructor { + is_record: false, + location: Span::new((), 70..79), + name: "Something".to_string(), + arguments: vec![], + module: None, + constructor: (), + with_spread: false, + tipo: (), + }], + alternative_patterns: vec![], + guard: None, + then: expr::UntypedExpr::Var { + location: Span::new((), 83..87), + name: "Void".to_string(), + }, + }, + ast::Clause { + location: Span::new((), 94..103), + pattern: vec![ast::Pattern::Discard { + name: "_".to_string(), + location: Span::new((), 94..95), + }], + alternative_patterns: vec![], + guard: None, + then: expr::UntypedExpr::Trace { + location: Span::new((), 99..103), + then: Box::new(expr::UntypedExpr::ErrorTerm { + location: Span::new((), 99..103), + }), + text: Box::new(expr::UntypedExpr::String { + location: Span::new((), 99..103), + value: "aiken::todo".to_string(), + }), + }, + }, + ], + }, + doc: None, + location: Span::new((), 39..47), + name: "bar".to_string(), + public: false, + return_annotation: None, + return_type: (), + end_position: 108, }), ], ) diff --git a/crates/aiken-lang/src/tipo/error.rs b/crates/aiken-lang/src/tipo/error.rs index 5fa694a7..fdae1c3e 100644 --- a/crates/aiken-lang/src/tipo/error.rs +++ b/crates/aiken-lang/src/tipo/error.rs @@ -1,6 +1,6 @@ use super::Type; use crate::{ - ast::{Annotation, BinOp, CallArg, Span, TodoKind, UntypedPattern}, + ast::{Annotation, BinOp, CallArg, Span, UntypedPattern}, expr::{self, UntypedExpr}, format::Formatter, levenshtein, @@ -1118,7 +1118,6 @@ pub enum Warning { #[diagnostic(help("You probably want to replace that one with real code... eventually."))] #[diagnostic(code("todo"))] Todo { - kind: TodoKind, #[label] location: Span, tipo: Arc, diff --git a/crates/aiken-lang/src/tipo/expr.rs b/crates/aiken-lang/src/tipo/expr.rs index 70b2e739..6a906579 100644 --- a/crates/aiken-lang/src/tipo/expr.rs +++ b/crates/aiken-lang/src/tipo/expr.rs @@ -5,7 +5,7 @@ use vec1::Vec1; use crate::{ ast::{ Annotation, Arg, ArgName, AssignmentKind, BinOp, CallArg, Clause, ClauseGuard, Constant, - RecordUpdateSpread, Span, TodoKind, TypedArg, TypedCallArg, TypedClause, TypedClauseGuard, + RecordUpdateSpread, Span, TypedArg, TypedCallArg, TypedClause, TypedClauseGuard, TypedConstant, TypedIfBranch, TypedMultiPattern, TypedRecordUpdateArg, UnOp, UntypedArg, UntypedClause, UntypedClauseGuard, UntypedConstant, UntypedIfBranch, UntypedMultiPattern, UntypedPattern, UntypedRecordUpdateArg, @@ -221,7 +221,6 @@ impl<'a, 'b> ExprTyper<'a, 'b> { | UntypedExpr::RecordUpdate { .. } | UntypedExpr::Sequence { .. } | UntypedExpr::String { .. } - | UntypedExpr::Todo { .. } | UntypedExpr::Tuple { .. } | UntypedExpr::TupleIndex { .. } | UntypedExpr::UnOp { .. } @@ -249,13 +248,6 @@ impl<'a, 'b> ExprTyper<'a, 'b> { /// returning an error. pub fn infer(&mut self, expr: UntypedExpr) -> Result { match expr { - UntypedExpr::Todo { - location, - label, - kind, - .. - } => Ok(self.infer_todo(location, kind, label)), - UntypedExpr::ErrorTerm { location } => Ok(self.infer_error_term(location)), UntypedExpr::Var { location, name, .. } => self.infer_var(name, location), @@ -1855,22 +1847,6 @@ impl<'a, 'b> ExprTyper<'a, 'b> { }) } - fn infer_todo(&mut self, location: Span, kind: TodoKind, label: Option) -> TypedExpr { - let tipo = self.new_unbound_var(); - - self.environment.warnings.push(Warning::Todo { - kind, - location, - tipo: tipo.clone(), - }); - - TypedExpr::Todo { - location, - label, - tipo, - } - } - fn infer_error_term(&mut self, location: Span) -> TypedExpr { let tipo = self.new_unbound_var(); @@ -1889,6 +1865,13 @@ impl<'a, 'b> ExprTyper<'a, 'b> { let then = self.infer(then)?; let tipo = then.tipo(); + // TODO: reinstate once we can distinguish traces + // + // self.environment.warnings.push(Warning::Todo { + // location, + // tipo: tipo.clone(), + // }) + Ok(TypedExpr::Trace { location, tipo, diff --git a/crates/aiken-lang/src/uplc.rs b/crates/aiken-lang/src/uplc.rs index ab2dfc00..1d8fd79c 100644 --- a/crates/aiken-lang/src/uplc.rs +++ b/crates/aiken-lang/src/uplc.rs @@ -587,13 +587,6 @@ impl<'a> CodeGenerator<'a> { constants_ir(literal, ir_stack, scope); } }, - TypedExpr::Todo { label, tipo, .. } => { - ir_stack.push(Air::Todo { - scope, - label: label.clone(), - tipo: tipo.clone(), - }); - } TypedExpr::RecordUpdate { spread, args, tipo, .. } => { @@ -3494,16 +3487,6 @@ impl<'a> CodeGenerator<'a> { tuple_index, }; } - Air::Todo { tipo, scope, label } => { - let mut replaced_type = tipo.clone(); - replace_opaque_type(&mut replaced_type, self.data_types.clone()); - - ir_stack[index] = Air::Todo { - scope, - label, - tipo: replaced_type, - }; - } Air::ErrorTerm { tipo, scope } => { let mut replaced_type = tipo.clone(); replace_opaque_type(&mut replaced_type, self.data_types.clone()); @@ -5286,23 +5269,6 @@ impl<'a> CodeGenerator<'a> { arg_stack.push(term); } } - Air::Todo { label, .. } => { - let term = apply_wrap( - apply_wrap( - Term::Builtin(DefaultFunction::Trace).force_wrap(), - Term::Constant( - UplcConstant::String( - label.unwrap_or_else(|| "aiken::todo".to_string()), - ) - .into(), - ), - ), - Term::Delay(Term::Error.into()), - ) - .force_wrap(); - - arg_stack.push(term); - } Air::RecordUpdate { highest_index, indices,