From d46a89b90d789aed5b3c236d541537fcbce658ab Mon Sep 17 00:00:00 2001 From: joe-conigliaro Date: Thu, 23 Jul 2020 02:10:31 +1000 Subject: [PATCH] parser/checker/fmt: optimize scope lookups by storing object with ident & add if expr smartcast support to vfmt (#5935) --- vlib/v/ast/ast.v | 3 +++ vlib/v/checker/checker.v | 26 ++++++++++++++++---------- vlib/v/fmt/fmt.v | 18 ++++++++++++++++++ vlib/v/parser/assign.v | 24 +++++++++--------------- vlib/v/parser/lock.v | 4 +++- 5 files changed, 49 insertions(+), 26 deletions(-) diff --git a/vlib/v/ast/ast.v b/vlib/v/ast/ast.v index 3866ecd901..e0a7667c18 100644 --- a/vlib/v/ast/ast.v +++ b/vlib/v/ast/ast.v @@ -360,6 +360,8 @@ pub mut: typ table.Type } +// TODO: (joe) remove completely, use ident.obj +// instead which points to the scope object pub struct IdentVar { pub mut: typ table.Type @@ -387,6 +389,7 @@ pub: tok_kind token.Kind pos token.Position pub mut: + obj ScopeObject mod string name string kind IdentKind diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index 72816185a5..35c413abd5 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -679,8 +679,7 @@ fn (mut c Checker) fail_if_immutable(expr ast.Expr) (string, token.Position) { return '', pos } ast.Ident { - scope := c.file.scope.innermost(expr.pos.pos) - if v := scope.find_var(expr.name) { + if expr.obj is ast.Var as v { if !v.is_mut && !v.typ.is_ptr() { c.error('`$expr.name` is immutable, declare it with `mut` to make it mutable', expr.pos) @@ -1628,7 +1627,6 @@ pub fn (mut c Checker) assign_stmt(mut assign_stmt ast.AssignStmt) { if is_decl { c.check_valid_snake_case(left.name, 'variable name', left.pos) } - mut scope := c.file.scope.innermost(assign_stmt.pos.pos) mut ident_var_info := left.var_info() if ident_var_info.share == .shared_t { left_type = left_type.set_flag(.shared_f) @@ -1639,7 +1637,13 @@ pub fn (mut c Checker) assign_stmt(mut assign_stmt ast.AssignStmt) { assign_stmt.left_types[i] = left_type ident_var_info.typ = left_type left.info = ident_var_info - scope.update_var_type(left.name, left_type) + if left_type != 0 { + if left.obj is ast.Var as v { + v.typ = left_type + } else if left.obj is ast.GlobalDecl as v { + v.typ = left_type + } + } } } ast.PrefixExpr { @@ -2419,13 +2423,14 @@ pub fn (mut c Checker) ident(mut ident ast.Ident) table.Type { } else if ident.kind == .unresolved { // first use start_scope := c.file.scope.innermost(ident.pos.pos) - if obj := start_scope.find(ident.name) { - match obj { + if obj1 := start_scope.find(ident.name) { + match obj1 as obj { ast.GlobalDecl { ident.kind = .global ident.info = ast.IdentVar{ typ: obj.typ } + ident.obj = obj1 return obj.typ } ast.Var { @@ -2460,6 +2465,7 @@ pub fn (mut c Checker) ident(mut ident ast.Ident) table.Type { // } // } else { obj.typ = typ + ident.obj = obj1 // unwrap optional (`println(x)`) if is_optional { return typ.clear_flag(.optional) @@ -2474,8 +2480,8 @@ pub fn (mut c Checker) ident(mut ident ast.Ident) table.Type { if !name.contains('.') && ident.mod != 'builtin' { name = '${ident.mod}.$ident.name' } - if obj := c.file.global_scope.find(name) { - match obj { + if obj1 := c.file.global_scope.find(name) { + match obj1 as obj { ast.ConstField { mut typ := obj.typ if typ == 0 { @@ -2487,6 +2493,7 @@ pub fn (mut c Checker) ident(mut ident ast.Ident) table.Type { typ: typ } obj.typ = typ + ident.obj = obj1 return typ } else {} @@ -2745,10 +2752,9 @@ fn (mut c Checker) match_exprs(mut node ast.MatchExpr, type_sym table.TypeSymbol } pub fn (mut c Checker) lock_expr(mut node ast.LockExpr) table.Type { - scope := c.file.scope.innermost(node.pos.pos) for id in node.lockeds { c.ident(mut id) - if v := scope.find_var(id.name) { + if id.obj is ast.Var as v { if v.typ.share() != .shared_t { c.error('`$id.name` must be declared `shared` to be locked', id.pos) } diff --git a/vlib/v/fmt/fmt.v b/vlib/v/fmt/fmt.v index 6d1da0bbc7..a1fd894e3e 100644 --- a/vlib/v/fmt/fmt.v +++ b/vlib/v/fmt/fmt.v @@ -1286,10 +1286,25 @@ pub fn (mut f Fmt) if_expr(it ast.IfExpr) { (it.is_expr || f.is_assign) f.single_line_if = single_line for i, branch in it.branches { + // NOTE: taken from checker in if_expr(). used for smartcast + mut is_variable := false + if branch.cond is ast.InfixExpr { + infix := branch.cond as ast.InfixExpr + if infix.op == .key_is && + (infix.left is ast.Ident || infix.left is ast.SelectorExpr) && + infix.right is ast.Type { + right_expr := infix.right as ast.Type + is_variable = if infix.left is ast.Ident { (infix.left as ast.Ident).kind == + .variable } else { true } + } + } if i == 0 { f.comments(branch.comments, {}) f.write('if ') f.expr(branch.cond) + if is_variable && branch.left_as_name.len > 0 { + f.write(' as $branch.left_as_name') + } f.write(' {') } else if i < it.branches.len - 1 || !it.has_else { if branch.comments.len > 0 { @@ -1300,6 +1315,9 @@ pub fn (mut f Fmt) if_expr(it ast.IfExpr) { } f.write('else if ') f.expr(branch.cond) + if is_variable && branch.left_as_name.len > 0 { + f.write(' as $branch.left_as_name') + } f.write(' {') } else if i == it.branches.len - 1 && it.has_else { if branch.comments.len > 0 { diff --git a/vlib/v/parser/assign.v b/vlib/v/parser/assign.v index 41ba99e444..3dbec0bdc8 100644 --- a/vlib/v/parser/assign.v +++ b/vlib/v/parser/assign.v @@ -116,22 +116,16 @@ fn (mut p Parser) partial_assign_stmt(left []ast.Expr, left_comments []ast.Comme if lx.info is ast.IdentVar { share = (lx.info as ast.IdentVar).share } - if left.len == right.len { - p.scope.register(lx.name, ast.Var{ - name: lx.name - expr: right[i] - share: share - is_mut: lx.is_mut || p.inside_for - pos: lx.pos - }) - } else { - p.scope.register(lx.name, ast.Var{ - name: lx.name - share: share - is_mut: lx.is_mut || p.inside_for - pos: lx.pos - }) + mut v := ast.Var{ + name: lx.name + expr: if left.len == right.len { right[i] } else { ast.Expr{} } + share: share + is_mut: lx.is_mut || p.inside_for + pos: lx.pos } + obj := ast.ScopeObject(v) + lx.obj = obj + p.scope.register(lx.name, obj) } } ast.IndexExpr { diff --git a/vlib/v/parser/lock.v b/vlib/v/parser/lock.v index cb34c9d753..834f9a03c8 100644 --- a/vlib/v/parser/lock.v +++ b/vlib/v/parser/lock.v @@ -11,8 +11,10 @@ fn (mut p Parser) lock_expr() ast.LockExpr { for p.tok.kind == .name { lockeds << ast.Ident{ language: table.Language.v - kind: .variable + // kind is set in checker once ident is processed + // kind: .variable pos: p.tok.position() + mod: p.mod name: p.tok.lit is_mut: true info: ast.IdentVar{}