From 966b95ca4e5032bd5b5832ff629e130f50e7a088 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20D=C3=A4schle?= Date: Fri, 27 Nov 2020 03:08:42 +0100 Subject: [PATCH] parser: move mut in if/match to expr (#6973) --- vlib/v/ast/ast.v | 17 +++++++------ vlib/v/ast/scope.v | 18 -------------- vlib/v/checker/checker.v | 30 +++++++++++------------ vlib/v/fmt/fmt.v | 17 ++++--------- vlib/v/parser/if_match.v | 24 ------------------ vlib/v/parser/parser.v | 14 +++++++++++ vlib/v/parser/tests/unnecessary_mut_2.out | 4 +-- 7 files changed, 45 insertions(+), 79 deletions(-) diff --git a/vlib/v/ast/ast.v b/vlib/v/ast/ast.v index a9e686d523..a3e6f7bf72 100644 --- a/vlib/v/ast/ast.v +++ b/vlib/v/ast/ast.v @@ -106,6 +106,8 @@ pub: pos token.Position expr Expr // expr.field_name field_name string + is_mut bool // is used for the case `if mut ident.selector is MyType {`, it indicates if the root ident is mutable + mut_pos token.Position pub mut: expr_type table.Type // type of `Foo` in `Foo.bar` typ table.Type // type of the entire thing (`Foo.bar`) @@ -447,6 +449,7 @@ pub: language table.Language tok_kind token.Kind pos token.Position + mut_pos token.Position pub mut: obj ScopeObject mod string @@ -529,14 +532,13 @@ pub mut: pub struct IfBranch { pub: - cond Expr - pos token.Position - body_pos token.Position - comments []Comment - is_mut_name bool // `if mut name is` + cond Expr + pos token.Position + body_pos token.Position + comments []Comment pub mut: - stmts []Stmt - smartcast bool // true when cond is `x is SumType`, set in checker.if_expr // no longer needed with union sum types TODO: remove + stmts []Stmt + smartcast bool // true when cond is `x is SumType`, set in checker.if_expr // no longer needed with union sum types TODO: remove } pub struct UnsafeExpr { @@ -562,7 +564,6 @@ pub: cond Expr branches []MatchBranch pos token.Position - is_mut bool // `match mut ast_node {` pub mut: is_expr bool // returns a value return_type table.Type diff --git a/vlib/v/ast/scope.v b/vlib/v/ast/scope.v index 74bc34069a..2fe3e83529 100644 --- a/vlib/v/ast/scope.v +++ b/vlib/v/ast/scope.v @@ -202,21 +202,3 @@ pub fn (sc &Scope) show(depth int, max_depth int) string { pub fn (sc &Scope) str() string { return sc.show(0, 0) } - -// is_selector_root_mutable checks if the root ident is mutable -// Example: -// ``` -// mut x := MyStruct{} -// x.foo.bar.z -// ``` -// Since x is mutable, it returns true. -pub fn (s &Scope) is_selector_root_mutable(t &table.Table, selector_expr SelectorExpr) bool { - if mut selector_expr.expr is SelectorExpr { - return s.is_selector_root_mutable(t, selector_expr.expr) - } else if mut selector_expr.expr is Ident { - if v := s.find_var(selector_expr.expr.name) { - return v.is_mut - } - } - return false -} diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index 9637e0970b..95a8729ead 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -623,6 +623,16 @@ pub fn (mut c Checker) infix_expr(mut infix_expr ast.InfixExpr) table.Type { c.warn('pointer arithmetic is only allowed in `unsafe` blocks', left_pos) } mut return_type := left_type + if infix_expr.op != .key_is { + match mut infix_expr.left { + ast.Ident, ast.SelectorExpr { + if infix_expr.left.is_mut { + c.error('remove unnecessary `mut`', infix_expr.left.mut_pos) + } + } + else {} + } + } // Single side check // Place these branches according to ops' usage frequency to accelerate. // TODO: First branch includes ops where single side check is not needed, or needed but hasn't been implemented. @@ -3451,11 +3461,6 @@ pub fn (mut c Checker) match_expr(mut node ast.MatchExpr) table.Type { // node.expected_type = c.expected_type // } node.return_type = ret_type - if node.is_mut { - // Mark `x` in `match mut x {` as changed, and ensure it's mutable - // TODO2 enable when code is fixed - // c.fail_if_immutable(node.cond) - } return ret_type } @@ -3585,9 +3590,8 @@ fn (mut c Checker) match_exprs(mut node ast.MatchExpr, type_sym table.TypeSymbol if field := scope.find_struct_field(node.cond.expr_type, node.cond.field_name) { sum_type_casts << field.sum_type_casts } - is_root_mut := scope.is_selector_root_mutable(c.table, node.cond) // smartcast either if the value is immutable or if the mut argument is explicitly given - if (!is_root_mut && !is_mut) || node.is_mut { + if !is_mut || node.cond.is_mut { sum_type_casts << expr_type scope.register_struct_field(ast.ScopeStructField{ struct_type: node.cond.expr_type @@ -3608,14 +3612,14 @@ fn (mut c Checker) match_exprs(mut node ast.MatchExpr, type_sym table.TypeSymbol is_already_casted = v.pos.pos == node.cond.pos.pos } // smartcast either if the value is immutable or if the mut argument is explicitly given - if (!is_mut || node.is_mut) && !is_already_casted { + if (!is_mut || node.cond.is_mut) && !is_already_casted { sum_type_casts << expr_type scope.register(node.cond.name, ast.Var{ name: node.cond.name typ: node.cond_type pos: node.cond.pos is_used: true - is_mut: node.is_mut + is_mut: node.cond.is_mut sum_type_casts: sum_type_casts }) } @@ -3846,7 +3850,7 @@ pub fn (mut c Checker) if_expr(mut node ast.IfExpr) table.Type { } if left_sym.kind == .sum_type { // smartcast either if the value is immutable or if the mut argument is explicitly given - if !is_mut || branch.is_mut_name { + if !is_mut || infix.left.is_mut { sum_type_casts << right_expr.typ scope.register(infix.left.name, ast.Var{ name: infix.left.name @@ -3879,12 +3883,8 @@ pub fn (mut c Checker) if_expr(mut node ast.IfExpr) table.Type { infix.left.field_name) { sum_type_casts << field.sum_type_casts } - is_root_mut := scope.is_selector_root_mutable(c.table, - infix.left) // smartcast either if the value is immutable or if the mut argument is explicitly given - if ((!is_root_mut && !is_mut) || - branch.is_mut_name) && - left_sym.kind == .sum_type { + if (!is_mut || infix.left.is_mut) && left_sym.kind == .sum_type { sum_type_casts << right_expr.typ scope.register_struct_field(ast.ScopeStructField{ struct_type: infix.left.expr_type diff --git a/vlib/v/fmt/fmt.v b/vlib/v/fmt/fmt.v index 6c98253471..21a63c916e 100644 --- a/vlib/v/fmt/fmt.v +++ b/vlib/v/fmt/fmt.v @@ -251,9 +251,6 @@ pub fn (mut f Fmt) stmt(node ast.Stmt) { for i, left in node.left { if left is ast.Ident { var_info := left.var_info() - if var_info.is_mut { - f.write(var_info.share.str() + ' ') - } if var_info.is_static { f.write('static ') } @@ -835,10 +832,12 @@ pub fn (mut f Fmt) expr(node ast.Expr) { f.if_expr(node) } ast.Ident { - f.write_language_prefix(node.language) - if true { - } else { + if mut node.info is ast.IdentVar { + if node.info.is_mut { + f.write(node.info.share.str() + ' ') + } } + f.write_language_prefix(node.language) if node.name == 'it' && f.it_name != '' && !f.inside_lambda { // allow `it` in lambdas f.write(f.it_name) } else if node.kind == .blank_ident { @@ -1411,9 +1410,6 @@ pub fn (mut f Fmt) if_expr(it ast.IfExpr) { } if i < it.branches.len - 1 || !it.has_else { f.write('${dollar}if ') - if branch.is_mut_name { - f.write('mut ') - } f.expr(branch.cond) f.write(' ') } @@ -1532,9 +1528,6 @@ pub fn (mut f Fmt) call_expr(node ast.CallExpr) { pub fn (mut f Fmt) match_expr(it ast.MatchExpr) { f.write('match ') - if it.is_mut { - f.write('mut ') - } f.expr(it.cond) if it.cond is ast.Ident { f.it_name = it.cond.name diff --git a/vlib/v/parser/if_match.v b/vlib/v/parser/if_match.v index 447daf54ed..710f7e9f73 100644 --- a/vlib/v/parser/if_match.v +++ b/vlib/v/parser/if_match.v @@ -84,15 +84,6 @@ fn (mut p Parser) if_expr(is_comptime bool) ast.IfExpr { p.error('cannot use `match` with `if` statements') } comments << p.eat_comments() - // `if mut name is T` - mut is_mut_name := false - mut mut_pos := token.Position{} - if p.tok.kind == .key_mut { - is_mut_name = true - mut_pos = p.tok.position() - p.next() - comments << p.eat_comments() - } mut cond := ast.Expr{} mut is_guard := false // `if x := opt() {` @@ -120,15 +111,6 @@ fn (mut p Parser) if_expr(is_comptime bool) ast.IfExpr { cond = p.expr(0) } comments << p.eat_comments() - if mut cond is ast.InfixExpr { - // if sum is T - is_is_cast := cond.op == .key_is - if !is_is_cast && is_mut_name { - p.error_with_pos('remove unnecessary `mut`', mut_pos) - } - } else if is_mut_name { - p.error_with_pos('remove unnecessary `mut`', mut_pos) - } end_pos := p.prev_tok.position() body_pos := p.tok.position() p.inside_if = false @@ -142,7 +124,6 @@ fn (mut p Parser) if_expr(is_comptime bool) ast.IfExpr { pos: start_pos.extend(end_pos) body_pos: body_pos.extend(p.prev_tok.position()) comments: comments - is_mut_name: is_mut_name } comments = p.eat_comments() if is_comptime { @@ -170,11 +151,7 @@ fn (mut p Parser) match_expr() ast.MatchExpr { match_first_pos := p.tok.position() p.inside_match = true p.check(.key_match) - is_mut := p.tok.kind == .key_mut mut is_sum_type := false - if is_mut { - p.next() - } cond := p.expr(0) p.inside_match = false no_lcbr := p.tok.kind != .lcbr @@ -281,7 +258,6 @@ fn (mut p Parser) match_expr() ast.MatchExpr { cond: cond is_sum_type: is_sum_type pos: pos - is_mut: is_mut } } diff --git a/vlib/v/parser/parser.v b/vlib/v/parser/parser.v index bab28b38f1..67ddfaa4f0 100644 --- a/vlib/v/parser/parser.v +++ b/vlib/v/parser/parser.v @@ -914,6 +914,7 @@ pub fn (mut p Parser) parse_ident(language table.Language) ast.Ident { // p.warn('name ') is_shared := p.tok.kind == .key_shared is_atomic := p.tok.kind == .key_atomic + mut_pos := p.tok.position() is_mut := p.tok.kind == .key_mut || is_shared || is_atomic if is_mut { p.next() @@ -951,6 +952,7 @@ pub fn (mut p Parser) parse_ident(language table.Language) ast.Ident { mod: p.mod pos: pos is_mut: is_mut + mut_pos: mut_pos info: ast.IdentVar{ is_mut: is_mut is_static: is_static @@ -1349,10 +1351,22 @@ fn (mut p Parser) dot_expr(left ast.Expr) ast.Expr { } return mcall_expr } + mut is_mut := false + mut mut_pos := token.Position{} + if p.inside_match || p.inside_if_expr { + match left { + ast.Ident, ast.SelectorExpr { + is_mut = left.is_mut + mut_pos = left.mut_pos + } + else {} + } + } sel_expr := ast.SelectorExpr{ expr: left field_name: field_name pos: name_pos + is_mut: is_mut } mut node := ast.Expr{} node = sel_expr diff --git a/vlib/v/parser/tests/unnecessary_mut_2.out b/vlib/v/parser/tests/unnecessary_mut_2.out index 7016e470b9..5f7dfee46b 100644 --- a/vlib/v/parser/tests/unnecessary_mut_2.out +++ b/vlib/v/parser/tests/unnecessary_mut_2.out @@ -1,6 +1,6 @@ -vlib/v/parser/tests/unnecessary_mut_2.vv:2:5: error: remove unnecessary `mut` +vlib/v/parser/tests/unnecessary_mut_2.vv:2:9: error: unexpected token `true` 1 | fn main() { 2 | if mut true { - | ~~~ + | ~~~~ 3 | println(true) 4 | } \ No newline at end of file