builtin: x.vstring() instead of string(x) (#6102)

vlib/builtin/string.v

@@ -120,6 +120,23 @@ pub fn tos_lit(s charptr) string {
 	}
 }
 
+// byteptr.vstring() - converts a C style string to a V string. NB: the string data is reused, NOT copied.
+[unsafe]
+pub fn (bp byteptr) vstring() string {
+	return string{
+		str: bp
+		len: unsafe {C.strlen(bp)}
+	}	
+}
+
+// byteptr.vstring_with_len() - converts a C style string to a V string. NB: the string data is reused, NOT copied.
+[unsafe]
+pub fn (bp byteptr) vstring_with_len(len int) string {
+	return string{
+		str: bp
+		len: len
+	}	
+}
 
 // string.clone_static returns an independent copy of a given array
 // It should be used only in -autofree generated code.
@@ -1429,9 +1446,10 @@ pub fn (s string) repeat(count int) string {
 		}
 	}
 	unsafe {
-		ret[s.len * count] = 0
+		new_len := s.len * count    
+		ret[new_len] = 0
+		return ret.vstring_with_len(new_len)
 	}
-	return string(ret)
 }
 
 pub fn (s string) fields() []string {
@@ -1463,7 +1481,7 @@ pub fn (s string) filter(func fn(b byte) bool) string {
 	}
 	unsafe {
 		buf[new_len] = 0
-		return string(buf, new_len)
+		return buf.vstring_with_len(new_len)
 	}
 }
 
@@ -1524,6 +1542,6 @@ pub fn (s string) strip_margin_custom(del byte) string {
 	}
 	unsafe {
 		ret[count] = 0
+		return ret.vstring_with_len(count)
 	}
-	return string(ret)
 }

vlib/builtin/string_test.v

@@ -502,10 +502,10 @@ fn test_bytes_to_string() {
 		buf[3] = `l`
 		buf[4] = `o`
 	}
-	assert string(buf) == 'hello'
+	assert unsafe { buf.vstring() } == 'hello'
-	assert string(buf, 2) == 'he'
+	assert unsafe { buf.vstring_with_len(2) } == 'he'
 	bytes := [`h`, `e`, `l`, `l`, `o`]
-	assert string(bytes, 5) == 'hello'
+	assert bytes.bytestr() == 'hello'
 }
 
 fn test_count() {

vlib/clipboard/clipboard_darwin.c.v

@@ -70,7 +70,7 @@ fn (mut cb Clipboard) get_text() string {
 	#}
 
 	#utf8_clip = [ns_clip UTF8String];
-	return string(utf8_clip)
+	return unsafe { utf8_clip.vstring() }
 }
 
 pub fn new_primary() &Clipboard {

vlib/clipboard/clipboard_linux.c.v

@@ -309,7 +309,7 @@ fn (mut cb Clipboard) start_listener(){
 						C.XDeleteProperty(event.xselection.display, event.xselection.requestor, event.xselection.property)
 						if cb.is_supported_target(prop.actual_type) {
 							cb.got_text = true
-							cb.text = string(prop.data) //TODO: return byteptr to support other mimetypes
+							cb.text = byteptr(prop.data).vstring() //TODO: return byteptr to support other mimetypes
 						}
 						cb.mutex.unlock()
 					}

vlib/darwin/darwin.v

@@ -44,7 +44,7 @@ pub fn resource_path() string {
 	if conv_result == 0 {
 		panic('CFURLGetFileSystemRepresentation failed')
 	}
-	result := string(buffer)
+	result := unsafe { buffer.vstring() }
 	C.CFRelease(resource_dir_url)
 	return result
 }

vlib/mysql/mysql.v

@@ -116,7 +116,7 @@ pub fn (conn Connection) escape_string(s string) string {
     quote := byte(39) // single quote
 
     C.mysql_real_escape_string_quote(conn.conn, to, s.str, len, quote)
-    return string(to)
+    return unsafe { to.vstring() }
 }
 
 // set_option is used to set extra connect options and affect behavior for a connection.
@@ -175,12 +175,12 @@ pub fn (conn Connection) info() string {
 
 // get_host_info returns a string describing the connection.
 pub fn (conn Connection) get_host_info() string {
-	return string(C.mysql_get_host_info(conn.conn))
+	return unsafe { C.mysql_get_host_info(conn.conn).vstring() }
 }
 
 // get_server_info returns the server version number as a string.
 pub fn (conn Connection) get_server_info() string {
-	return string(C.mysql_get_server_info(conn.conn))
+	return unsafe { C.mysql_get_server_info(conn.conn).vstring() }
 }
 
 // get_server_version returns the server version number as an integer.
@@ -192,7 +192,7 @@ pub fn (conn Connection) get_server_version() u64 {
 
 // get_client_info returns client version information as a string.
 pub fn get_client_info() string {
-	return string(C.mysql_get_client_info())
+	return unsafe { C.mysql_get_client_info().vstring() }
 }
 
 // get_client_version returns client version information as an integer.

vlib/mysql/result.v

@@ -34,7 +34,7 @@ pub fn (r Result) rows() []Row {
 			if rr[i] == 0 {
 				row.vals << ''
 			} else {
-				row.vals << string(&byte(rr[i]))
+				row.vals << mystring( byteptr(rr[i]) )
 			}
 		}
 		rows << row
@@ -64,12 +64,12 @@ pub fn (r Result) fields() []Field {
 	orig_fields := C.mysql_fetch_fields(r.result)
 	for i in 0..nr_cols {
 		fields << Field{
-			name: string(orig_fields[i].name)
+			name: mystring(orig_fields[i].name)
-			org_name: string(orig_fields[i].org_name)
+			org_name: mystring(orig_fields[i].org_name)
-			table: string(orig_fields[i].table)
+			table: mystring(orig_fields[i].table)
-			org_table: string(orig_fields[i].org_table)
+			org_table: mystring(orig_fields[i].org_table)
-			db: string(orig_fields[i].db)
+			db: mystring(orig_fields[i].db)
-			catalog: string(orig_fields[i].catalog)
+			catalog: mystring(orig_fields[i].catalog)
 			def: resolve_nil_str(orig_fields[i].def)
 			length: orig_fields.length
 			max_length: orig_fields.max_length

vlib/mysql/utils.v

@@ -2,7 +2,7 @@ module mysql
 
 // get_error_msg returns error message from MySQL instance.
 fn get_error_msg(conn &C.MYSQL) string {
-	return string(C.mysql_error(conn))
+	return unsafe { C.mysql_error(conn).vstring() }
 }
 
 // get_errno returns error number from MySQL instance.
@@ -13,5 +13,12 @@ fn get_errno(conn &C.MYSQL) int {
 // resolve_nil_str returns empty string if passed value is a nil pointer.
 fn resolve_nil_str(ptr byteptr) string {
 	if isnil(ptr) { return '' }
-	return string(ptr)
+	return unsafe { ptr.vstring() }
+}
+
+[inline]
+fn mystring(b byteptr) string {
+	unsafe {
+		return b.vstring()
+	}
 }

vlib/net/ftp/ftp.v

@@ -156,7 +156,7 @@ pub fn (ftp FTP) pwd() string {
 		return ''
 	}
 	_, data := ftp.read()
-	spl := data.split('"')
+	spl := data.split('"') // "
 	if spl.len >= 2 {
 		return spl[1]
 	}
@@ -236,7 +236,7 @@ pub fn (ftp FTP) dir() ?[]string {
 	}
 	dtp.close()
 	mut dir := []string{}
-	sdir := string(byteptr(list_dir.data))
+	sdir := list_dir.bytestr()
 	for lfile in sdir.split('\n') {
 		if lfile.len > 1 {
 			spl := lfile.split(' ')

vlib/net/http/cookie.v

@@ -275,7 +275,7 @@ fn sanitize(valid fn(byte) bool, v string) string {
 		break
 	}
 	if ok {
-		return v
+		return v.clone()
 	}
 	// TODO: Use `filter` instead of this nonesense
 	buf := v.bytes()
@@ -285,7 +285,7 @@ fn sanitize(valid fn(byte) bool, v string) string {
 			bytes.delete(i)
 		}
 	}
-	return string(bytes)
+	return bytes.bytestr()
 }
 
 fn sanitize_cookie_name(name string) string {

vlib/net/urllib/urllib.v

@@ -272,7 +272,7 @@ fn escape(s string, mode EncodingMode) string {
 				t[i] = `+`
 			}
 		}
-		return string(t,t.len)
+		return t.bytestr()
 	}
 	upperhex := '0123456789ABCDEF'
 	mut j := 0
@@ -293,7 +293,7 @@ fn escape(s string, mode EncodingMode) string {
 			j++
 		}
 	}
-	return string(t,t.len)
+	return t.bytestr()
 }
 
 // A URL represents a parsed URL (technically, a URI reference).
@@ -367,7 +367,7 @@ fn (u &Userinfo) empty() bool {
 
 // string returns the encoded userinfo information in the standard form
 // of 'username[:password]'.
-fn (u &Userinfo) string() string {
+fn (u &Userinfo) str() string {
 	if u.empty() {
 		return ''
 	}
@@ -771,7 +771,7 @@ pub fn (u URL) str() string {
 				buf.write('//')
 			}
 			if !u.user.empty() {
-				buf.write(u.user.string())
+				buf.write(u.user.str())
 				buf.write('@')
 			}
 			if u.host != '' {

vlib/net/websocket/utils.v

@@ -32,12 +32,8 @@ fn create_key_challenge_response(seckey string) string {
 	guid := '258EAFA5-E914-47DA-95CA-C5AB0DC85B11'
 	sha1buf := seckey + guid
 	hash := sha1.sum(sha1buf.bytes())
-	hashstr := string(byteptr(hash.data))
+	hashstr := hash.bytestr()
 	b64 := base64.encode(hashstr)
-	unsafe {
-		sha1buf.free()
-		hash.free()
-	}
 	return b64
 }
 

vlib/net/websocket/ws.v

@@ -294,7 +294,7 @@ pub fn (mut ws Client) write(payload byteptr, payload_len int, code OPCode) int
 	}
 	bytes_written = ws.write_to_server(fbdata, frame_len)
 	if bytes_written == -1 {
-		err := string(byteptr(C.strerror(C.errno)))
+		err := unsafe { byteptr(C.strerror(C.errno)).vstring() }
 		ws.log.error('write: there was an error writing data: $err')
 		ws.send_error_event('Error writing data')
 		goto free_data
@@ -344,7 +344,7 @@ pub fn (mut ws Client) read() int {
 				return -1
 			}
 			-1 {
-				err := string(byteptr(C.strerror(C.errno)))
+				err := unsafe { byteptr(C.strerror(C.errno)).vstring() }
 				ws.log.error('read: error reading frame. $err')
 				ws.send_error_event('error reading frame')
 				goto free_data
@@ -564,7 +564,7 @@ pub fn (mut ws Client) read() int {
 			code = (int(unsafe {data[header_len]}) << 8) + int(unsafe {data[header_len + 1]})
 			header_len += 2
 			payload_len -= 2
-			reason = unsafe {string(&data[header_len])}
+			reason = unsafe { byteptr(&data[header_len]).vstring() }
 			ws.log.info('Closing with reason: $reason & code: $code')
 			if reason.len > 1 && !utf8.validate(reason.str, reason.len) {
 				ws.log.error('malformed utf8 payload')

vlib/os/os.v

@@ -39,7 +39,7 @@ pub fn read_file(path string) ?string {
 		mut str := malloc(fsize + 1)
 		C.fread(str, fsize, 1, fp)
 		str[fsize] = 0
-		return string(str,fsize)
+		return str.vstring_with_len(fsize)
 	}
 }
 
@@ -673,7 +673,7 @@ pub fn get_raw_line() string {
 				}
 				offset++
 			}
-			return string(buf, offset)
+			return buf.vstring_with_len(offset)
 		}
 	} $else {
 		max := size_t(0)
@@ -858,7 +858,7 @@ pub fn executable() string {
 			eprintln('os.executable() failed at reading /proc/self/exe to get exe path')
 			return executable_fallback()
 		}
-		return string(result)
+		return unsafe { result.vstring() }
 	}
 	$if windows {
 		max := 512
@@ -896,7 +896,7 @@ pub fn executable() string {
 			eprintln('os.executable() failed at calling proc_pidpath with pid: $pid . proc_pidpath returned $ret ')
 			return executable_fallback()
 		}
-		return string(result)
+		return unsafe { result.vstring() }
 	}
 	$if freebsd {
 		mut result := vcalloc(max_path_len)
@@ -905,7 +905,7 @@ pub fn executable() string {
 		unsafe {
 			C.sysctl(mib.data, 4, result, &size, 0, 0)
 		}
-		return string(result)
+		return unsafe { result.vstring() }
 	}
 	// "Sadly there is no way to get the full path of the executed file in OpenBSD."
 	$if openbsd {}
@@ -918,7 +918,7 @@ pub fn executable() string {
 			eprintln('os.executable() failed at reading /proc/curproc/exe to get exe path')
 			return executable_fallback()
 		}
-		return string(result,count)
+		return result.vstring_with_len(count)
 	}
 	$if dragonfly {
 		mut result := vcalloc(max_path_len)
@@ -927,7 +927,7 @@ pub fn executable() string {
 			eprintln('os.executable() failed at reading /proc/curproc/file to get exe path')
 			return executable_fallback()
 		}
-		return string(result,count)
+		return unsafe { result.vstring_with_len(count) }
 	}
 	return executable_fallback()
 }
@@ -1056,7 +1056,7 @@ pub fn getwd() string {
 		if C.getcwd(charptr(buf), 512) == 0 {
 			return ''
 		}
-		return string(buf)
+		return unsafe { buf.vstring() }
 	}
 }
 
@@ -1079,7 +1079,7 @@ pub fn real_path(fpath string) string {
 			return fpath
 		}
 	}
-	return string(fullpath)
+	return unsafe { fullpath.vstring() }
 }
 
 // is_abs_path returns true if `path` is absolute.

vlib/os/os_nix.c.v

@@ -51,8 +51,10 @@ fn init_os_args(argc int, argv &&byte) []string {
 	// mut args := []string(make(0, argc, sizeof(string)))
 	// mut args := []string{len:argc}
 	for i in 0 .. argc {
-		// args [i] = string(argv[i])
+		// args [i] = argv[i].vstring()
-		args << unsafe {string(argv[i])}
+		unsafe {
+			args << byteptr(argv[i]).vstring()
+		}
 	}
 	return args
 }

vlib/pg/pg.v

@@ -45,7 +45,7 @@ pub fn connect(config Config) ?DB {
 	println("status=$status")
 	if status != C.CONNECTION_OK {
 		error_msg := C.PQerrorMessage(conn)
-		return error ('Connection to a PG database failed: ' + string(error_msg))
+		return error ('Connection to a PG database failed: ' + unsafe { error_msg.vstring() } )
 	}
 	return DB {conn: conn}
 }
@@ -58,7 +58,8 @@ fn res_to_rows(res voidptr) []Row {
 		mut row := Row{}
 		for j in 0..nr_cols {
 			val := C.PQgetvalue(res, i, j)
-			row.vals << string(val)
+			sval := unsafe { val.vstring() }
+			row.vals << sval
 		}
 		rows << row
 	}
@@ -100,7 +101,7 @@ pub fn (db DB) q_strings(query string) []Row {
 
 pub fn (db DB) exec(query string) []Row {
 	res := C.PQexec(db.conn, query.str)
-	e := string(C.PQerrorMessage(db.conn))
+	e := unsafe { C.PQerrorMessage(db.conn).vstring() }
 	if e != '' {
 		println('pg exec error:')
 		println(e)
@@ -118,7 +119,7 @@ fn rows_first_or_empty(rows []Row) ?Row {
 
 pub fn (db DB) exec_one(query string) ?Row {
 	res := C.PQexec(db.conn, query.str)
-	e := string(C.PQerrorMessage(db.conn))
+	e := unsafe { C.PQerrorMessage(db.conn).vstring() }
 	if e != '' {
 		return error('pg exec error: "$e"')
 	}
@@ -156,7 +157,7 @@ pub fn (db DB) exec_param(query string, param string) []Row {
 }
 
 fn (db DB) handle_error_or_result(res voidptr, elabel string) []Row {
-	e := string(C.PQerrorMessage(db.conn))
+	e := unsafe { C.PQerrorMessage(db.conn).vstring() }
 	if e != '' {
 		println('pg $elabel error:')
 		println(e)

vlib/rand/rand.v

@@ -141,7 +141,7 @@ pub fn string(len int) string {
 			buf[i] = chars[intn(chars.len)]
 		}
 	}
-	return string(buf, len)
+	return unsafe { buf.vstring_with_len(len) }
 }
 
 // rand.uuid_v4 generate a completely random UUID (v4)
@@ -181,7 +181,7 @@ pub fn uuid_v4() string {
 		buf[14] = `4`
 		buf[buflen] = 0
 	}
-	return string(buf, buflen)
+	return unsafe { buf.vstring_with_len(buflen) }
 }
 
 const(
@@ -232,5 +232,5 @@ pub fn ulid_at_millisecond(unix_time_milli u64) string {
 	unsafe{
 		buf[26] = 0
 	}
-	return string(buf, buflen)
+	return unsafe { buf.vstring_with_len(buflen) }
 }

vlib/strings/builder.js.v

@@ -38,7 +38,7 @@ pub fn (mut b Builder) writeln(s string) {
 }
 
 pub fn (b Builder) str() string {
-	return string(b.buf, b.len)
+	return unsafe { byteptr(b.buf.data).vstring_with_len(b.len) }
 }
 
 pub fn (mut b Builder) cut(n int) {

vlib/strings/builder.v

@@ -116,7 +116,7 @@ pub fn (mut b Builder) str() string {
 			'If you want to reuse a builder, call b.free() first.')
 	}
 	b.buf << `\0`
-	s := string(b.buf,b.len)
+	s := tos(b.buf.data, b.len)
 	bis := b.initial_size
 	//free(b.buf.data)
 	b.buf = []byte{cap: bis}

vlib/strings/strings.c.v

@@ -10,7 +10,7 @@ pub fn repeat(c byte, n int) string {
 		C.memset( bytes, c, n )
 		bytes[n] = `0`
 	}
-	return string( bytes, n )
+	return unsafe { bytes.vstring_with_len(n) }
 }
 
 // strings.repeat_string - gives you `n` repetitions of the substring `s`
@@ -34,5 +34,5 @@ pub fn repeat_string(s string, n int) string {
 	unsafe {
 		bytes[blen] = `0`
 	}
-	return string( bytes, blen )
+	return unsafe { bytes.vstring_with_len(blen) }
 }

vlib/strings/strings.js.v

@@ -4,9 +4,8 @@ pub fn repeat(c byte, n int) string {
 	if n <= 0 {
 		return ''
 	}
-	mut arr := [c].repeat(n + 1)
+	arr := [c].repeat(n)
-	arr[n] = `\0`
+	return arr.bytestr()
-	return string(arr,n)
 }
 
 pub fn repeat_string(s string, n int) string {

vlib/v/ast/ast.v

@@ -769,6 +769,14 @@ pub:
 	has_low  bool
 }
 
+// NB: &string(x) gets parsed as ast.PrefixExpr{ right: ast.CastExpr{...} }
+// TODO: that is very likely a parsing bug. It should get parsed as just
+// ast.CastExpr{...}, where .typname is '&string' instead.
+// The current situation leads to special cases in vfmt and cgen
+// (see prefix_expr_cast_expr in fmt.v, and .is_amp in cgen.v)
+// .in_prexpr is also needed because of that, because the checker needs to
+// show warnings about the deprecated C->V conversions `string(x)` and
+// `string(x,y)`, while skipping the real pointer casts like `&string(x)`.
 pub struct CastExpr {
 pub:
 	expr      Expr // `buf` in `string(buf, n)`
@@ -779,6 +787,7 @@ pub mut:
 	typname   string
 	expr_type table.Type // `byteptr`
 	has_arg   bool
+	in_prexpr bool // is the parent node an ast.PrefixExpr
 }
 
 pub struct AssertStmt {

vlib/v/checker/checker.v

@@ -2265,6 +2265,16 @@ pub fn (mut c Checker) expr(node ast.Expr) table.Type {
 			node.expr_type = c.expr(node.expr)
 			from_type_sym := c.table.get_type_symbol(node.expr_type)
 			to_type_sym := c.table.get_type_symbol(node.typ)
+			expr_is_ptr := node.expr_type.is_ptr() || node.expr_type.idx() in table.pointer_type_idxs
+			if expr_is_ptr && to_type_sym.kind == .string && !node.in_prexpr {
+				if node.has_arg {
+					c.warn('to convert a C string buffer pointer to a V string, please use x.vstring_with_len(len) instead of string(x,len)',
+						node.pos)
+				} else {
+					c.warn('to convert a C string buffer pointer to a V string, please use x.vstring() instead of string(x)',
+						node.pos)
+				}
+			}
 			if node.expr_type == table.byte_type && to_type_sym.kind == .string {
 				c.error('can not cast type `byte` to string, use `${node.expr.str()}.str()` instead.',
 					node.pos)

vlib/v/checker/tests/warnings_for_string_c2v_calls.out

@@ -0,0 +1,14 @@
+vlib/v/checker/tests/warnings_for_string_c2v_calls.v:8:14: error: to convert a C string buffer pointer to a V string, please use x.vstring() instead of string(x)
+    6 |         p[2] = `z`
+    7 |     }
+    8 |     x := string(p)
+      |                 ^
+    9 |     y := string(p, 10)
+   10 |     eprintln('x: $x | y: $y')
+vlib/v/checker/tests/warnings_for_string_c2v_calls.v:9:14: error: to convert a C string buffer pointer to a V string, please use x.vstring_with_len(len) instead of string(x,len)
+    7 |     }
+    8 |     x := string(p)
+    9 |     y := string(p, 10)
+      |                 ^
+   10 |     eprintln('x: $x | y: $y')
+   11 |     eprintln('x.len: $x.len | y.len: $y.len')

vlib/v/checker/tests/warnings_for_string_c2v_calls.vv

@@ -0,0 +1,12 @@
+fn main() {
+	mut p := vcalloc(20)
+	unsafe {
+		p[0] = `A`
+		p[1] = `B`
+		p[2] = `z`
+	}
+	x := string(p)
+	y := string(p, 10)
+	eprintln('x: $x | y: $y')
+	eprintln('x.len: $x.len | y.len: $y.len')
+}

vlib/v/fmt/tests/to_string_2_forms_keep.vv

@@ -5,7 +5,7 @@ fn abc() string {
 		fullpath[1] = `b`
 		fullpath[2] = `c`
 		fullpath[3] = 0
-		return string(fullpath)
+		return fullpath.vstring()
 	}
 	return ''
 }
@@ -17,7 +17,7 @@ fn def() string {
 		fullpath[1] = `b`
 		fullpath[2] = `c`
 		fullpath[3] = 0
-		return string(fullpath, 3)
+		return fullpath.vstring_with_len(3)
 	}
 	return ''
 }

vlib/v/parser/pratt.v

@@ -309,8 +309,11 @@ fn (mut p Parser) prefix_expr() ast.PrefixExpr {
 	// p.warn('unsafe')
 	// }
 	p.next()
-	right := if op == .minus { p.expr(token.Precedence.call) } else { p.expr(token.Precedence.prefix) }
+	mut right := if op == .minus { p.expr(token.Precedence.call) } else { p.expr(token.Precedence.prefix) }
 	p.is_amp = false
+	if mut right is ast.CastExpr {
+		right.in_prexpr = true
+	}
 	return ast.PrefixExpr{
 		op: op
 		right: right

vlib/v/scanner/scanner.v

@@ -323,7 +323,7 @@ fn filter_num_sep(txt byteptr, start, end int) string {
 			}
 		}
 		b[i1] = 0 // C string compatibility
-		return string(b)
+		return b.vstring_with_len(i1)
 	}
 }
 

vlib/v/tests/cstrings_test.v

@@ -10,7 +10,7 @@ fn test_cstring() {
 
 fn test_cstring_with_zeros() {
 	rawbytes := c'\x00username\x00password'
-	s := string(rawbytes, 18)
+	s := unsafe { rawbytes.vstring_with_len(18) }
 	h := s.bytes().hex()
 	assert h == '00757365726e616d650070617373776f7264'
 }

vlib/vweb/vweb.v

@@ -583,7 +583,7 @@ fn readall(conn net.Socket) string {
 	for {
 		n := C.recv(conn.sockfd, buf, 1024, 0)
 		m := conn.crecv(buf, 1024)
-		message += string( byteptr(buf), m )
+		message += unsafe { byteptr(buf).vstring_with_len(m) }
 		if message.len > max_http_post_size { break }
 		if n == m { break }
 	}