From 19c226fcf84aff292064a0764ff02c8a852d1421 Mon Sep 17 00:00:00 2001 From: Nick Treleaven Date: Tue, 28 Jul 2020 09:20:52 +0100 Subject: [PATCH] parser: Support `unsafe(expr)` (#5973) --- doc/docs.md | 11 +++++------ vlib/v/ast/ast.v | 1 + vlib/v/checker/checker.v | 9 ++++++++- vlib/v/fmt/fmt.v | 3 +++ vlib/v/parser/parser.v | 23 ++++++++++++++++------- vlib/v/parser/pratt.v | 21 ++++++++++++++++----- vlib/v/tests/unsafe_test.v | 25 +++++-------------------- 7 files changed, 54 insertions(+), 39 deletions(-) diff --git a/doc/docs.md b/doc/docs.md index fd91b9038e..bcec7916fd 100644 --- a/doc/docs.md +++ b/doc/docs.md @@ -1907,10 +1907,10 @@ To mark potentially memory-unsafe operations, enclose them in an `unsafe` block: ```v // allocate 2 uninitialized bytes & return a reference to them -mut p := unsafe { &byte(malloc(2)) } +mut p := unsafe(&byte(malloc(2))) p[0] = `h` // Error: pointer indexing is only allowed in `unsafe` blocks unsafe { - p[0] = `h` + p[0] = `h` // OK p[1] = `i` } p++ // Error: pointer arithmetic is only allowed in `unsafe` blocks @@ -1920,13 +1920,12 @@ unsafe { assert *p == `i` ``` -Best practice is to avoid putting memory-safe expressions inside an `unsafe` block, +Best practice is to avoid putting memory-safe expressions inside an `unsafe` expression/block, so that the reason for using `unsafe` is as clear as possible. Generally any code -you think is memory-safe should not be inside an `unsafe` block, so the compiler -can verify it. +you think is memory-safe should be verified by the compiler. If you suspect your program does violate memory-safety, you have a head start on -finding the cause: look at the `unsafe` blocks (and how they interact with +finding the cause: look for the `unsafe` keyword (and how it affects the surrounding code). * Note: This is work in progress. diff --git a/vlib/v/ast/ast.v b/vlib/v/ast/ast.v index 1881e127f9..40988b96e3 100644 --- a/vlib/v/ast/ast.v +++ b/vlib/v/ast/ast.v @@ -728,6 +728,7 @@ pub: pub struct ParExpr { pub: expr Expr + is_unsafe bool // unsafe(expr) } pub struct GoStmt { diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index a92bf1505b..7d993f51e5 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -2374,7 +2374,14 @@ pub fn (mut c Checker) expr(node ast.Expr) table.Type { return table.void_type } ast.ParExpr { - return c.expr(node.expr) + if !node.is_unsafe { + return c.expr(node.expr) + } + assert !c.inside_unsafe + c.inside_unsafe = true + t := c.expr(node.expr) + c.inside_unsafe = false + return t } ast.RangeExpr { // never happens diff --git a/vlib/v/fmt/fmt.v b/vlib/v/fmt/fmt.v index e166dcaf7a..51623a457b 100644 --- a/vlib/v/fmt/fmt.v +++ b/vlib/v/fmt/fmt.v @@ -902,6 +902,9 @@ pub fn (mut f Fmt) expr(node ast.Expr) { panic('fmt: OrExpr should to linked to CallExpr') } ast.ParExpr { + if node.is_unsafe { + f.write('unsafe') + } f.write('(') f.par_level++ f.expr(node.expr) diff --git a/vlib/v/parser/parser.v b/vlib/v/parser/parser.v index 95f187d5b3..22f2fd55b9 100644 --- a/vlib/v/parser/parser.v +++ b/vlib/v/parser/parser.v @@ -602,13 +602,22 @@ pub fn (mut p Parser) stmt(is_top_level bool) ast.Stmt { } } .key_unsafe { - p.next() - assert !p.inside_unsafe - p.inside_unsafe = true - stmts := p.parse_block() - p.inside_unsafe = false - return ast.UnsafeStmt{ - stmts: stmts + // unsafe { + if p.peek_tok.kind == .lcbr { + p.next() + assert !p.inside_unsafe + p.inside_unsafe = true + stmts := p.parse_block() + p.inside_unsafe = false + return ast.UnsafeStmt{ + stmts: stmts + } + } + // unsafe( + pos := p.tok.position() + return ast.ExprStmt{ + expr: p.expr(0) + pos: pos } } .hash { diff --git a/vlib/v/parser/pratt.v b/vlib/v/parser/pratt.v index 6a9084cabe..5b6ccfd07d 100644 --- a/vlib/v/parser/pratt.v +++ b/vlib/v/parser/pratt.v @@ -93,12 +93,23 @@ pub fn (mut p Parser) expr(precedence int) ast.Expr { pos := p.tok.position() assert !p.inside_unsafe p.inside_unsafe = true - stmts := p.parse_block() - p.inside_unsafe = false - node = ast.UnsafeExpr{ - stmts: stmts - pos: pos + if p.tok.kind == .lpar { + // unsafe( + p.check(.lpar) + node = ast.ParExpr{ + expr: p.expr(0) + is_unsafe: true + } + p.check(.rpar) + } else { + // unsafe { + // old syntax, UnsafeExpr can be removed later + node = ast.UnsafeExpr{ + stmts: p.parse_block() + pos: pos + } } + p.inside_unsafe = false } .key_lock, .key_rlock { node = p.lock_expr() diff --git a/vlib/v/tests/unsafe_test.v b/vlib/v/tests/unsafe_test.v index 82e06e1440..c18506a340 100644 --- a/vlib/v/tests/unsafe_test.v +++ b/vlib/v/tests/unsafe_test.v @@ -3,17 +3,9 @@ fn test_ptr_assign() { mut p := &v[0] unsafe { (*p)++ - } - unsafe { - p++ - } // p now points to v[1] - unsafe { + p++ // p now points to v[1] (*p) += 2 - } - unsafe { - p += 2 - } // p now points to v[3] - unsafe { + p += 2 // p now points to v[3] *p = 31 } assert v[0] == 6 @@ -24,16 +16,9 @@ fn test_ptr_assign() { fn test_ptr_infix() { v := 4 - mut q := unsafe { - &v - 1 - } - - q = unsafe { - q + 3 - } - - _ := q - _ := v + mut q := unsafe(&v - 1) + q = unsafe(q + 3) + assert q == unsafe(&v + 2) } struct S1 {