Skip to content

Commit

Permalink
transformer: refactor + apply transform to (hopefully) all nodes (#13216
Browse files Browse the repository at this point in the history
)
  • Loading branch information
timbasel committed Jan 20, 2022
1 parent d67be63 commit 14b33ba
Show file tree
Hide file tree
Showing 8 changed files with 797 additions and 659 deletions.
145 changes: 75 additions & 70 deletions vlib/v/ast/ast.v

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions vlib/v/checker/checker.v
Original file line number Diff line number Diff line change
Expand Up @@ -1821,7 +1821,7 @@ fn (mut c Checker) stmt(node ast.Stmt) {
node.typ = c.expr(node.expr)
c.expected_type = ast.void_type
mut or_typ := ast.void_type
match node.expr {
match mut node.expr {
ast.IndexExpr {
if node.expr.or_expr.kind != .absent {
node.is_expr = true
Expand All @@ -1837,7 +1837,7 @@ fn (mut c Checker) stmt(node ast.Stmt) {
else {}
}
if !c.pref.is_repl && (c.stmt_level == 1 || (c.stmt_level > 1 && !c.is_last_stmt)) {
if node.expr is ast.InfixExpr {
if mut node.expr is ast.InfixExpr {
if node.expr.op == .left_shift {
left_sym := c.table.final_sym(node.expr.left_type)
if left_sym.kind != .array {
Expand Down Expand Up @@ -2422,7 +2422,7 @@ pub fn (mut c Checker) expr(node ast.Expr) ast.Type {
c.error('expected `string` instead of `$expr_sym.name` (e.g. `field.name`)',
node.field_expr.position())
}
if node.field_expr is ast.SelectorExpr {
if mut node.field_expr is ast.SelectorExpr {
left_pos := node.field_expr.expr.position()
if c.comptime_fields_type.len == 0 {
c.error('compile time field access can only be used when iterating over `T.fields`',
Expand Down Expand Up @@ -3025,7 +3025,7 @@ pub fn (mut c Checker) ident(mut node ast.Ident) ast.Type {
c.inside_const = false
c.mod = old_c_mod

if obj.expr is ast.CallExpr {
if mut obj.expr is ast.CallExpr {
if obj.expr.or_block.kind != .absent {
typ = typ.clear_flag(.optional)
}
Expand Down
4 changes: 2 additions & 2 deletions vlib/v/checker/for.v
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,10 @@ fn (mut c Checker) for_stmt(mut node ast.ForStmt) {
if !node.is_inf && typ.idx() != ast.bool_type_idx && !c.pref.translated {
c.error('non-bool used as for condition', node.pos)
}
if node.cond is ast.InfixExpr {
if mut node.cond is ast.InfixExpr {
infix := node.cond
if infix.op == .key_is {
if infix.left in [ast.Ident, ast.SelectorExpr] && infix.right is ast.TypeNode {
if infix.right is ast.TypeNode && infix.left in [ast.Ident, ast.SelectorExpr] {
is_variable := if mut infix.left is ast.Ident {
infix.left.kind == .variable
} else {
Expand Down
4 changes: 2 additions & 2 deletions vlib/v/checker/if.v
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ pub fn (mut c Checker) if_expr(mut node ast.IfExpr) ast.Type {
if node.is_comptime { // Skip checking if needed
// smartcast field type on comptime if
mut comptime_field_name := ''
if branch.cond is ast.InfixExpr {
if mut branch.cond is ast.InfixExpr {
if branch.cond.op == .key_is {
if branch.cond.right !is ast.TypeNode {
c.error('invalid `\$if` condition: expected a type', branch.cond.right.position())
Expand Down Expand Up @@ -106,7 +106,7 @@ pub fn (mut c Checker) if_expr(mut node ast.IfExpr) ast.Type {
}
} else if c.pref.output_cross_c {
mut is_freestanding_block := false
if branch.cond is ast.Ident {
if mut branch.cond is ast.Ident {
if branch.cond.name == 'freestanding' {
is_freestanding_block = true
}
Expand Down
2 changes: 1 addition & 1 deletion vlib/v/checker/match.v
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import strings
pub fn (mut c Checker) match_expr(mut node ast.MatchExpr) ast.Type {
node.is_expr = c.expected_type != ast.void_type
node.expected_type = c.expected_type
if node.cond is ast.ParExpr && !c.pref.translated {
if mut node.cond is ast.ParExpr && !c.pref.translated {
c.error('unnecessary `()` in `match` condition, use `match expr {` instead of `match (expr) {`.',
node.cond.pos)
}
Expand Down
8 changes: 4 additions & 4 deletions vlib/v/parser/parse_type.v
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@ pub fn (mut p Parser) parse_array_type(expecting token.Kind) ast.Type {
}
ast.Ident {
mut show_non_const_error := false
if const_field := p.table.global_scope.find_const('${p.mod}.$size_expr.name') {
if const_field.expr is ast.IntegerLiteral {
if mut const_field := p.table.global_scope.find_const('${p.mod}.$size_expr.name') {
if mut const_field.expr is ast.IntegerLiteral {
fixed_size = const_field.expr.val.int()
} else {
if const_field.expr is ast.InfixExpr {
if mut const_field.expr is ast.InfixExpr {
mut t := transformer.new_transformer(p.pref)
folded_expr := t.infix_expr(const_field.expr)
folded_expr := t.infix_expr(mut const_field.expr)

if folded_expr is ast.IntegerLiteral {
fixed_size = folded_expr.val.int()
Expand Down
123 changes: 123 additions & 0 deletions vlib/v/transformer/index_state.v
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
module transformer

struct KeyVal {
key string
value int
}

[if debug_bounds_checking ?]
fn debug_bounds_checking(str string) {
println(str)
}

// IndexState is used to track the index analysis performed when parsing the code
// `IndexExpr` nodes are annotated with `is_direct`, indicating that the array index can be safely directly accessed.

// The c_gen code check will handle this annotation and perform this direct memory access. The following cases are considered valid for this optimisation:
// 1. the array size is known and has a `len` larger than the index requested
// 2. the array was previously accessed with a higher value which would have reported the issue already
// 3. the array was created from a range expression a := range[10..13] and the offset'ed indexes are safe

// Current limitations:
// * any function using break/continue or goto/label stopped from being optimised as soon as the relevant AST nodes are found as the code can not be ensured to be sequential
// * `enum` and `const` indexes are not optimised (they could probably be looked up)
// * for loops with multiple var in their init and/or inc are not analysed
// * mut array are not analysed as their size can be reduced, but self-assignment in a single line

pub struct IndexState {
mut:
// max_index has the biggest array index accessed for then named array
// so if a[2] was set or read, it will be 2
// A new array with no .len will recorded as -1 (accessing a[0] would be invalid)
// the value -2 is used to indicate that the array should not be analysed
// this is used for a mut array
max_index map[string]int
// We need to snapshot when entering `if` and `for` blocks and restore on exit
// as the statements may not be run. This is managed by indent() & unindent().
saved_disabled []bool
saved_key_vals [][]KeyVal
pub mut:
// on encountering goto/break/continue statements we stop any analysis
// for the current function (as the code is not linear anymore)
disabled bool
level int
}

// we are remembering the last array accessed and checking if the value is safe
// the node is updated with this information which can then be used by the code generators
fn (mut i IndexState) safe_access(key string, new int) bool {
$if no_bounds_checking {
return false
}
if i.disabled {
return false
}
old := i.max_index[key] or {
debug_bounds_checking('$i.level ${key}.len = $new')
i.max_index[key] = new
return false
}
if new > old {
if old < -1 {
debug_bounds_checking('$i.level $key[$new] unsafe (mut array)')
return false
}
debug_bounds_checking('$i.level $key[$new] unsafe (index was $old)')
i.max_index[key] = new
return false
}
debug_bounds_checking('$i.level $key[$new] safe (index is $old)')
return true
}

// safe_offset returns for a previvous array what was the highest
// offset we ever accessed for that identifier
fn (mut i IndexState) safe_offset(key string) int {
$if no_bounds_checking {
return -2
}
if i.disabled {
return -2
}
return i.max_index[key] or { -1 }
}

// indent is used for when encountering new code blocks (if, for and functions)
// The code analysis needs to take into consideration blocks of code which
// may not run at runtime (if/for) and therefore even if a new maximum for an
// index access is found on an if branch it can not be used within the parent
// code. The same is true with for blocks. indent() snapshot the current state,
// to allow restoration with unindent()
// Also within a function, analysis must be `disabled` when goto or break are
// encountered as the code flow is then not lineear, and only restart when a
// new function analysis is started.
[if !no_bounds_checking]
fn (mut i IndexState) indent(is_function bool) {
mut kvs := []KeyVal{cap: i.max_index.len}
for k, v in i.max_index {
kvs << KeyVal{k, v}
}
i.saved_disabled << i.disabled
i.saved_key_vals << kvs
if is_function {
i.disabled = false
}
i.level += 1
}

// restoring the data as it was before the if/for/unsafe block
[if !no_bounds_checking]
fn (mut i IndexState) unindent() {
i.level -= 1
mut keys := []string{cap: i.max_index.len}
for k, _ in i.max_index {
keys << k
}
for k in keys {
i.max_index.delete(k)
}
for saved in i.saved_key_vals.pop() {
i.max_index[saved.key] = saved.value
}
i.disabled = i.saved_disabled.pop()
}
Loading

0 comments on commit 14b33ba

Please sign in to comment.