checker: warn when using `goto` outside of `unsafe` (#8741)

pull/8768/head
Nick Treleaven 2021-02-15 13:48:24 +00:00 committed by GitHub
parent 6781f732f4
commit 629d43caf5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 39 additions and 32 deletions

View File

@ -3910,21 +3910,25 @@ fn C.DefWindowProc(hwnd int, msg int, lparam int, wparam int)
V allows unconditionally jumping to a label with `goto`. The label name must be contained V allows unconditionally jumping to a label with `goto`. The label name must be contained
within the same function as the `goto` statement. A program may `goto` a label outside within the same function as the `goto` statement. A program may `goto` a label outside
or deeper than the current scope, but it must not skip a variable initialization. or deeper than the current scope. `goto` allows jumping past variable initialization or
jumping back to code that accesses memory that has already been freed, so it requires
`unsafe`.
```v ignore ```v ignore
if x { if x {
// ... // ...
if y { if y {
unsafe {
goto my_label goto my_label
} }
}
// ... // ...
} }
my_label: my_label:
``` ```
`goto` should be avoided when `for` can be used instead. In particular, `goto` should be avoided, particularly when `for` can be used instead.
[labelled break](#labelled-break--continue) can be used to break out of [Labelled break/continue](#labelled-break--continue) can be used to break out of
a nested loop. a nested loop, and those do not risk violating memory-safety.
# Appendices # Appendices

View File

@ -362,8 +362,7 @@ pub fn (fs FlagParser) usage() string {
if positive_min_arg || positive_max_arg || no_arguments { if positive_min_arg || positive_max_arg || no_arguments {
if no_arguments { if no_arguments {
use += 'This application does not expect any arguments\n\n' use += 'This application does not expect any arguments\n\n'
goto end_of_arguments_handling } else {
}
mut s := []string{} mut s := []string{}
if positive_min_arg { if positive_min_arg {
s << 'at least $fs.min_free_args' s << 'at least $fs.min_free_args'
@ -377,7 +376,7 @@ pub fn (fs FlagParser) usage() string {
sargs := s.join(' and ') sargs := s.join(' and ')
use += 'The arguments should be $sargs in number.\n\n' use += 'The arguments should be $sargs in number.\n\n'
} }
end_of_arguments_handling: }
if fs.flags.len > 0 { if fs.flags.len > 0 {
use += 'Options:\n' use += 'Options:\n'
for f in fs.flags { for f in fs.flags {

View File

@ -3178,6 +3178,10 @@ fn (mut c Checker) stmt(node ast.Stmt) {
} }
ast.GotoLabel {} ast.GotoLabel {}
ast.GotoStmt { ast.GotoStmt {
if !c.inside_unsafe {
c.warn('`goto` requires `unsafe` (consider using labelled break/continue)',
node.pos)
}
if node.name !in c.cur_fn.label_names { if node.name !in c.cur_fn.label_names {
c.error('unknown label `$node.name`', node.pos) c.error('unknown label `$node.name`', node.pos)
} }

View File

@ -45,5 +45,5 @@ vlib/v/checker/tests/goto_label.vv:25:7: error: unknown label `undefined`
24 | goto g1 // back 24 | goto g1 // back
25 | goto undefined 25 | goto undefined
| ~~~~~~~~~ | ~~~~~~~~~
26 | } 26 | }}
27 | 27 |

View File

@ -1,4 +1,4 @@
fn f() { fn f() {unsafe {
goto f1 // forward goto f1 // forward
goto f2 goto f2
f1: f1:
@ -14,19 +14,20 @@ fn f() {
goto a1 goto a1
goto f1 // back goto f1 // back
goto f2 goto f2
} }}
fn g() { fn g() {unsafe {
goto g1 // forward goto g1 // forward
g1: g1:
goto f1 goto f1
goto a1 goto a1
goto g1 // back goto g1 // back
goto undefined goto undefined
} }}
// implicit main // implicit main
goto m1 unsafe {
m1: goto m1
goto m1 m1:
goto m1
}

View File

@ -9,9 +9,9 @@ fn (mut p Parser) lock_expr() ast.LockExpr {
mut pos := p.tok.position() mut pos := p.tok.position()
mut lockeds := []ast.Ident{} mut lockeds := []ast.Ident{}
mut is_rlocked := []bool{} mut is_rlocked := []bool{}
for { outer: for {
if p.tok.kind == .lcbr { if p.tok.kind == .lcbr {
goto start_stmts break
} }
is_rlock := p.tok.kind == .key_rlock is_rlock := p.tok.kind == .key_rlock
if !is_rlock && p.tok.kind != .key_lock { if !is_rlock && p.tok.kind != .key_lock {
@ -31,7 +31,7 @@ fn (mut p Parser) lock_expr() ast.LockExpr {
is_rlocked << is_rlock is_rlocked << is_rlock
p.next() p.next()
if p.tok.kind == .lcbr { if p.tok.kind == .lcbr {
goto start_stmts break outer
} }
if p.tok.kind == .semicolon { if p.tok.kind == .semicolon {
p.next() p.next()
@ -40,7 +40,6 @@ fn (mut p Parser) lock_expr() ast.LockExpr {
p.check(.comma) p.check(.comma)
} }
} }
start_stmts:
stmts := p.parse_block() stmts := p.parse_block()
pos.update_last_line(p.prev_tok.line_nr) pos.update_last_line(p.prev_tok.line_nr)
return ast.LockExpr{ return ast.LockExpr{