From fef4dd94e9d3c3d2e3b23e8d8bf22b18bed5a905 Mon Sep 17 00:00:00 2001 From: shove Date: Wed, 26 Oct 2022 14:39:23 +0800 Subject: [PATCH] ast, checker, cgen: enable `unsafe { nil }` with reference to interface fields in structs (fix #16198) (#16199) --- vlib/v/ast/table.v | 6 ++++-- vlib/v/checker/check_types.v | 8 +++++--- vlib/v/checker/checker.v | 11 ++++++++--- vlib/v/checker/comptime.v | 2 +- vlib/v/checker/fn.v | 10 ++++++---- vlib/v/checker/infix.v | 2 +- vlib/v/checker/struct.v | 5 +++-- vlib/v/checker/tests/pointer_ops.out | 14 ++++++++++++++ vlib/v/eval/expr.v | 4 ++-- vlib/v/gen/c/auto_str_methods.v | 2 +- vlib/v/gen/c/cgen.v | 8 +++++--- vlib/v/gen/c/struct.v | 2 +- vlib/v/gen/js/auto_str_methods.v | 2 +- vlib/v/tests/interface_fields_test.v | 29 ++++++++++++++++++++++++++++ 14 files changed, 81 insertions(+), 24 deletions(-) diff --git a/vlib/v/ast/table.v b/vlib/v/ast/table.v index 4f7bd3d9e5..716c943525 100644 --- a/vlib/v/ast/table.v +++ b/vlib/v/ast/table.v @@ -1530,7 +1530,7 @@ pub fn (t Table) does_type_implement_interface(typ Type, inter_typ Type) bool { } // verify fields for ifield in inter_sym.info.fields { - if ifield.typ == voidptr_type { + if ifield.typ == voidptr_type || ifield.typ == nil_type { // Allow `voidptr` fields in interfaces for now. (for example // to enable .db check in vweb) if t.struct_has_field(sym, ifield.name) { @@ -1549,7 +1549,9 @@ pub fn (t Table) does_type_implement_interface(typ Type, inter_typ Type) bool { } return false } - inter_sym.info.types << typ + if typ != voidptr_type && typ != nil_type && !inter_sym.info.types.contains(typ) { + inter_sym.info.types << typ + } if !inter_sym.info.types.contains(voidptr_type) { inter_sym.info.types << voidptr_type } diff --git a/vlib/v/checker/check_types.v b/vlib/v/checker/check_types.v index a869fc700a..dde4ef6885 100644 --- a/vlib/v/checker/check_types.v +++ b/vlib/v/checker/check_types.v @@ -19,7 +19,7 @@ pub fn (mut c Checker) check_types(got ast.Type, expected ast.Type) bool { if expected == ast.byteptr_type { return true } - if expected == ast.voidptr_type { + if expected == ast.voidptr_type || expected == ast.nil_type { return true } if (expected == ast.bool_type && (got.is_any_kind_of_pointer() || got.is_int())) @@ -112,7 +112,8 @@ pub fn (mut c Checker) check_types(got ast.Type, expected ast.Type) bool { if exp_idx == got_idx { return true } - if exp_idx == ast.voidptr_type_idx || exp_idx == ast.byteptr_type_idx + if exp_idx == ast.voidptr_type_idx || exp_idx == ast.nil_type_idx + || exp_idx == ast.byteptr_type_idx || (expected.is_ptr() && expected.deref().idx() == ast.u8_type_idx) { if got.is_ptr() || got.is_pointer() { return true @@ -125,7 +126,8 @@ pub fn (mut c Checker) check_types(got ast.Type, expected ast.Type) bool { return true } } - if got_idx == ast.voidptr_type_idx || got_idx == ast.byteptr_type_idx + if got_idx == ast.voidptr_type_idx || got_idx == ast.nil_type_idx + || got_idx == ast.byteptr_type_idx || (got_idx == ast.u8_type_idx && got.is_ptr()) { if expected.is_ptr() || expected.is_pointer() { return true diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index e10b3667d7..e8923e7d10 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -850,7 +850,7 @@ fn (mut c Checker) type_implements(typ ast.Type, interface_type ast.Type, pos to inter_sym.methods } // voidptr is an escape hatch, it should be allowed to be passed - if utyp != ast.voidptr_type { + if utyp != ast.voidptr_type && utyp != ast.nil_type { // Verify methods for imethod in imethods { method := c.table.find_method_with_embeds(typ_sym, imethod.name) or { @@ -900,7 +900,7 @@ fn (mut c Checker) type_implements(typ ast.Type, interface_type ast.Type, pos to continue } // voidptr is an escape hatch, it should be allowed to be passed - if utyp != ast.voidptr_type { + if utyp != ast.voidptr_type && utyp != ast.nil_type { // >> Hack to allow old style custom error implementations // TODO: remove once deprecation period for `IError` methods has ended if inter_sym.idx == ast.error_type_idx @@ -913,7 +913,12 @@ fn (mut c Checker) type_implements(typ ast.Type, interface_type ast.Type, pos to } } } - inter_sym.info.types << utyp + if utyp != ast.voidptr_type && utyp != ast.nil_type && !inter_sym.info.types.contains(utyp) { + inter_sym.info.types << utyp + } + if !inter_sym.info.types.contains(ast.voidptr_type) { + inter_sym.info.types << ast.voidptr_type + } } return true } diff --git a/vlib/v/checker/comptime.v b/vlib/v/checker/comptime.v index a14e8cec75..5da06bf4ac 100644 --- a/vlib/v/checker/comptime.v +++ b/vlib/v/checker/comptime.v @@ -224,7 +224,7 @@ fn (mut c Checker) eval_comptime_const_expr(expr ast.Expr, nlevel int) ?ast.Comp if expr.typ == ast.f64_type { return cast_expr_value.f64() or { return none } } - if expr.typ == ast.voidptr_type { + if expr.typ == ast.voidptr_type || expr.typ == ast.nil_type { ptrvalue := cast_expr_value.voidptr() or { return none } return ast.ComptTimeConstValue(ptrvalue) } diff --git a/vlib/v/checker/fn.v b/vlib/v/checker/fn.v index 635dc96b33..24a3c62990 100644 --- a/vlib/v/checker/fn.v +++ b/vlib/v/checker/fn.v @@ -1031,8 +1031,9 @@ pub fn (mut c Checker) fn_call(mut node ast.CallExpr, mut continue_check &bool) // it can lead to codegen errors (except for 'magic' functions like `json.encode` that, // the compiler has special codegen support for), so it should be opt in, that is it // shoould require an explicit voidptr(x) cast (and probably unsafe{} ?) . - if call_arg.typ != param.typ - && (param.typ == ast.voidptr_type || final_param_sym.idx == ast.voidptr_type_idx) + if call_arg.typ != param.typ && (param.typ == ast.voidptr_type + || final_param_sym.idx == ast.voidptr_type_idx + || param.typ == ast.nil_type || final_param_sym.idx == ast.nil_type_idx) && !call_arg.typ.is_any_kind_of_pointer() && func.language == .v && !call_arg.expr.is_lvalue() && func.name != 'json.encode' && !c.pref.translated && !c.file.is_translated { @@ -1110,7 +1111,8 @@ pub fn (mut c Checker) fn_call(mut node ast.CallExpr, mut continue_check &bool) continue } // Allow voidptrs for everything - if param_type == ast.voidptr_type_idx || arg_typ == ast.voidptr_type_idx { + if param_type == ast.voidptr_type_idx || param_type == ast.nil_type_idx + || arg_typ == ast.voidptr_type_idx || arg_typ == ast.nil_type_idx { continue } if param_type.is_any_kind_of_pointer() && arg_typ.is_any_kind_of_pointer() { @@ -1151,7 +1153,7 @@ pub fn (mut c Checker) fn_call(mut node ast.CallExpr, mut continue_check &bool) // Warn about automatic (de)referencing, which will be removed soon. if func.language != .c && !c.inside_unsafe && arg_typ.nr_muls() != param.typ.nr_muls() && !(call_arg.is_mut && param.is_mut) && !(!call_arg.is_mut && !param.is_mut) - && param.typ !in [ast.byteptr_type, ast.charptr_type, ast.voidptr_type] { + && param.typ !in [ast.byteptr_type, ast.charptr_type, ast.voidptr_type, ast.nil_type] { c.warn('automatic referencing/dereferencing is deprecated and will be removed soon (got: $arg_typ.nr_muls() references, expected: $param.typ.nr_muls() references)', call_arg.pos) } diff --git a/vlib/v/checker/infix.v b/vlib/v/checker/infix.v index cd12b1a2e7..74ca8b814c 100644 --- a/vlib/v/checker/infix.v +++ b/vlib/v/checker/infix.v @@ -58,7 +58,7 @@ pub fn (mut c Checker) infix_expr(mut node ast.InfixExpr) ast.Type { if !c.inside_unsafe && !node.left.is_auto_deref_var() && !node.right.is_auto_deref_var() { c.warn('pointer arithmetic is only allowed in `unsafe` blocks', left_right_pos) } - if left_type == ast.voidptr_type && !c.pref.translated { + if (left_type == ast.voidptr_type || left_type == ast.nil_type) && !c.pref.translated { c.error('`$node.op` cannot be used with `voidptr`', left_pos) } } diff --git a/vlib/v/checker/struct.v b/vlib/v/checker/struct.v index 394d055f7c..40cf9f222c 100644 --- a/vlib/v/checker/struct.v +++ b/vlib/v/checker/struct.v @@ -91,9 +91,10 @@ pub fn (mut c Checker) struct_decl(mut node ast.StructDecl) { c.check_expr_opt_call(field.default_expr, default_expr_type) } struct_sym.info.fields[i].default_expr_typ = default_expr_type + interface_implemented := sym.kind == .interface_ + && c.type_implements(default_expr_type, field.typ, field.pos) c.check_expected(default_expr_type, field.typ) or { - if sym.kind == .interface_ - && c.type_implements(default_expr_type, field.typ, field.pos) { + if sym.kind == .interface_ && interface_implemented { if !default_expr_type.is_ptr() && !default_expr_type.is_pointer() && !c.inside_unsafe { if c.table.sym(default_expr_type).kind != .interface_ { diff --git a/vlib/v/checker/tests/pointer_ops.out b/vlib/v/checker/tests/pointer_ops.out index c5f6900093..fa2d2abb02 100644 --- a/vlib/v/checker/tests/pointer_ops.out +++ b/vlib/v/checker/tests/pointer_ops.out @@ -54,6 +54,13 @@ vlib/v/checker/tests/pointer_ops.vv:15:3: error: invalid operator `%` to `&Foo` | ~~~~~~~ 16 | } 17 | } +vlib/v/checker/tests/pointer_ops.vv:22:7: error: `+` cannot be used with `voidptr` + 20 | unsafe { + 21 | mut p := nil + 22 | _ = p + 1 + | ^ + 23 | p++ + 24 | p += 3 vlib/v/checker/tests/pointer_ops.vv:23:4: error: invalid operation: ++ (non-numeric type `voidptr`) 21 | mut p := nil 22 | _ = p + 1 @@ -68,6 +75,13 @@ vlib/v/checker/tests/pointer_ops.vv:24:3: error: operator `+=` not defined on le | ^ 25 | _ = p - 1 26 | p-- +vlib/v/checker/tests/pointer_ops.vv:25:7: error: `-` cannot be used with `voidptr` + 23 | p++ + 24 | p += 3 + 25 | _ = p - 1 + | ^ + 26 | p-- + 27 | p -= 3 vlib/v/checker/tests/pointer_ops.vv:26:4: error: invalid operation: -- (non-numeric type `voidptr`) 24 | p += 3 25 | _ = p - 1 diff --git a/vlib/v/eval/expr.v b/vlib/v/eval/expr.v index 6827fa1668..73ae3b05d4 100644 --- a/vlib/v/eval/expr.v +++ b/vlib/v/eval/expr.v @@ -366,7 +366,7 @@ pub fn (mut e Eval) expr(expr ast.Expr, expecting ast.Type) Object { e.error('unknown cast: ${e.table.sym(expr.expr_type).str()} to ${e.table.sym(expr.typ).str()}') } } - } else if expr.typ == ast.voidptr_type_idx { + } else if expr.typ == ast.voidptr_type_idx || expr.typ == ast.nil_type_idx { match y { char, Int { unsafe { @@ -540,7 +540,7 @@ pub fn (mut e Eval) expr(expr ast.Expr, expecting ast.Type) Object { fn (e Eval) type_to_size(typ ast.Type) u64 { match typ { - ast.voidptr_type_idx, ast.byteptr_type_idx, ast.charptr_type_idx { + ast.voidptr_type_idx, ast.nil_type_idx, ast.byteptr_type_idx, ast.charptr_type_idx { return u64(if e.pref.m64 { 64 } else { diff --git a/vlib/v/gen/c/auto_str_methods.v b/vlib/v/gen/c/auto_str_methods.v index e87d26e466..f10f9adc15 100644 --- a/vlib/v/gen/c/auto_str_methods.v +++ b/vlib/v/gen/c/auto_str_methods.v @@ -766,7 +766,7 @@ fn (g &Gen) type_to_fmt(typ ast.Type) StrIntpType { if typ == ast.char_type_idx { return .si_c } - if typ in ast.voidptr_types || typ in ast.byteptr_types { + if typ in ast.voidptr_types || typ == ast.nil_type || typ in ast.byteptr_types { return .si_p } if typ in ast.charptr_types { diff --git a/vlib/v/gen/c/cgen.v b/vlib/v/gen/c/cgen.v index b6e728fa65..c76fac3655 100644 --- a/vlib/v/gen/c/cgen.v +++ b/vlib/v/gen/c/cgen.v @@ -2406,6 +2406,7 @@ fn (mut g Gen) expr_with_cast(expr ast.Expr, got_type_raw ast.Type, expected_typ } // Generic dereferencing logic neither_void := ast.voidptr_type !in [got_type, expected_type] + && ast.nil_type !in [got_type, expected_type] if expected_type.has_flag(.shared_f) && !got_type_raw.has_flag(.shared_f) && !expected_type.has_flag(.optional) && !expected_type.has_flag(.result) { shared_styp := exp_styp[0..exp_styp.len - 1] // `shared` implies ptr, so eat one `*` @@ -5956,7 +5957,7 @@ fn (mut g Gen) interface_table() string { } else { // the field is embedded in another struct cast_struct.write_string('\t\t.$cname = ($field_styp*)((char*)x') - if st == ast.voidptr_type { + if st == ast.voidptr_type || st == ast.nil_type { cast_struct.write_string('/*.... ast.voidptr_type */') } else { if st_sym.kind == .struct_ { @@ -6004,7 +6005,7 @@ static inline __shared__$interface_name ${shared_fn_name}(__shared__$cctype* x) if g.pref.build_mode != .build_module { methods_struct.writeln('\t{') } - if st == ast.voidptr_type { + if st == ast.voidptr_type || st == ast.nil_type { for mname, _ in methodidx { if g.pref.build_mode != .build_module { methods_struct.writeln('\t\t._method_${c_name(mname)} = (void*) 0,') @@ -6116,7 +6117,8 @@ static inline __shared__$interface_name ${shared_fn_name}(__shared__$cctype* x) // .speak = Cat_speak_Interface_Animal_method_wrapper method_call += iwpostfix } - if g.pref.build_mode != .build_module && st != ast.voidptr_type { + if g.pref.build_mode != .build_module && st != ast.voidptr_type + && st != ast.nil_type { methods_struct.writeln('\t\t._method_${c_name(method.name)} = (void*) $method_call,') } } diff --git a/vlib/v/gen/c/struct.v b/vlib/v/gen/c/struct.v index d7998596c1..505183e061 100644 --- a/vlib/v/gen/c/struct.v +++ b/vlib/v/gen/c/struct.v @@ -436,7 +436,7 @@ fn (mut g Gen) struct_init_field(sfield ast.StructInitField, language ast.Langua } g.write('}') } else { - if sfield.typ != ast.nil_type + if sfield.typ != ast.voidptr_type && sfield.typ != ast.nil_type && (sfield.expected_type.is_ptr() && !sfield.expected_type.has_flag(.shared_f)) && !(sfield.typ.is_ptr() || sfield.typ.is_pointer()) && !sfield.typ.is_number() { g.write('/* autoref */&') diff --git a/vlib/v/gen/js/auto_str_methods.v b/vlib/v/gen/js/auto_str_methods.v index 7ea7078fbc..ef551c96d0 100644 --- a/vlib/v/gen/js/auto_str_methods.v +++ b/vlib/v/gen/js/auto_str_methods.v @@ -651,7 +651,7 @@ fn (g &JsGen) type_to_fmt(typ ast.Type) StrIntpType { if typ == ast.char_type_idx { return .si_c } - if typ in ast.voidptr_types || typ in ast.byteptr_types { + if typ in ast.voidptr_types || typ == ast.nil_type || typ in ast.byteptr_types { return .si_p } if typ in ast.charptr_types { diff --git a/vlib/v/tests/interface_fields_test.v b/vlib/v/tests/interface_fields_test.v index c863ed55b1..f58bb83970 100644 --- a/vlib/v/tests/interface_fields_test.v +++ b/vlib/v/tests/interface_fields_test.v @@ -72,3 +72,32 @@ fn test_interface_fn_pointer_fields() { nf := NofunInterface(Nofun{my_fn}) assert nf.foo(123) == 246 } + +// For issue: 16198 errors when the reference interface type field of the struct is nil(init, assign...) +interface Speaker { + speak() string +} + +struct Wolf {} + +fn (w Wolf) speak() string { + return 'woof' +} + +struct Foo { +mut: + speaker &Speaker = unsafe { nil } +} + +fn test_set_nil_to_ref_interface_type_fields() { + mut foo := Foo{ + speaker: unsafe { nil } + } + assert true + + foo.speaker = unsafe { nil } + assert true + + foo.speaker = &Wolf{} + assert foo.speaker.speak() == 'woof' +}