From 40c730909500acfc62fcdec4ca697b85fc720389 Mon Sep 17 00:00:00 2001 From: Jason Date: Mon, 22 Nov 2021 18:13:59 -0700 Subject: [PATCH] Nonref object inheritance now hints and converts to base in varargs and on assignment array handling and more elaborate tests Better error message for implict object converstion --- compiler/ccgexprs.nim | 11 ++-- compiler/lineinfos.nim | 2 + compiler/semexprs.nim | 7 ++- compiler/transf.nim | 7 +++ tests/objects/t4318.nim | 12 ----- tests/objects/tobj_asgn_dont_slice.nim | 24 --------- tests/typerel/t4799_1.nim | 21 -------- tests/typerel/t4799_2.nim | 21 -------- tests/typerel/t4799_3.nim | 21 -------- tests/types/tinheritance_arrays.nim | 53 ++++++++++++++++++ tests/types/tinheritance_conversion.nim | 71 +++++++++++++++++++++++++ 11 files changed, 146 insertions(+), 104 deletions(-) delete mode 100644 tests/objects/t4318.nim delete mode 100644 tests/objects/tobj_asgn_dont_slice.nim delete mode 100644 tests/typerel/t4799_1.nim delete mode 100644 tests/typerel/t4799_2.nim delete mode 100644 tests/typerel/t4799_3.nim create mode 100644 tests/types/tinheritance_arrays.nim create mode 100644 tests/types/tinheritance_conversion.nim diff --git a/compiler/ccgexprs.nim b/compiler/ccgexprs.nim index fc09fefc35e..284c9007836 100644 --- a/compiler/ccgexprs.nim +++ b/compiler/ccgexprs.nim @@ -262,7 +262,9 @@ proc genGenericAsgn(p: BProc, dest, src: TLoc, flags: TAssignmentFlags) = # tfShallow flag for the built-in string type too! So we check only # here for this flag, where it is reasonably safe to do so # (for objects, etc.): - if optSeqDestructors in p.config.globalOptions: + if optSeqDestructors in p.config.globalOptions or src.lode.kind in {nkObjUpConv, nkObjDownConv}: + # If it's an up/down conv it's an non-ref -> non-ref inheritance which is a copy or assignment. + # As such the assignment is direct, semantic analysis should handle any errors statically. linefmt(p, cpsStmts, "$1 = $2;$n", [rdLoc(dest), rdLoc(src)]) @@ -2604,11 +2606,12 @@ proc upConv(p: BProc, n: PNode, d: var TLoc) = else: genTypeInfoV1(p.module, dest, n.info) if nilCheck != nil: + # We only need to do a conversion check if it's a ref object. + # Since with non refs either a copy is done or a ptr to the element is passed, + # there is nothing dynamic with them and the compiler knows the error happens at semantic analysis. linefmt(p, cpsStmts, "if ($1 && !#isObj($2, $3)){ #raiseObjectConversionError(); $4}$n", [nilCheck, r, checkFor, raiseInstr(p)]) - else: - linefmt(p, cpsStmts, "if (!#isObj($1, $2)){ #raiseObjectConversionError(); $3}$n", - [r, checkFor, raiseInstr(p)]) + if n[0].typ.kind != tyObject: if n.isLValue: putIntoDest(p, d, n, diff --git a/compiler/lineinfos.nim b/compiler/lineinfos.nim index 8bd5a089099..c70c0b72acc 100644 --- a/compiler/lineinfos.nim +++ b/compiler/lineinfos.nim @@ -93,6 +93,7 @@ type hintUser = "User", hintUserRaw = "UserRaw", hintExtendedContext = "ExtendedContext", hintMsgOrigin = "MsgOrigin", # since 1.3.5 hintDeclaredLoc = "DeclaredLoc", # since 1.5.1 + hintImplicitObjConv = "ImplicitObjConv" const MsgKindToStr*: array[TMsgKind, string] = [ @@ -202,6 +203,7 @@ const hintExtendedContext: "$1", hintMsgOrigin: "$1", hintDeclaredLoc: "$1", + hintImplicitObjConv: "Implicit conversion: Receiver '$2' will not receive fields of sub-type '$1' [$3]" ] const diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim index 2c5d2adc242..86d1454be63 100644 --- a/compiler/semexprs.nim +++ b/compiler/semexprs.nim @@ -633,7 +633,12 @@ proc semArrayConstr(c: PContext, n: PNode, flags: TExprFlags): PNode = let xx = semExprWithType(c, x, {}) result.add xx - typ = commonType(c, typ, xx.typ) + if {xx.typ.kind, typ.kind} == {tyObject} and typ != xx.typ: + # Check if both are objects before getting common type, + # this prevents `[Derived(), Parent(), Derived()]` from working for non ref objects. + result[i] = typeMismatch(c.config, x.info, typ, xx.typ, x) + else: + typ = commonType(c, typ, xx.typ) #n[i] = semExprWithType(c, x, {}) #result.add fitNode(c, typ, n[i]) inc(lastIndex) diff --git a/compiler/transf.nim b/compiler/transf.nim index a7869dd5d95..69997a5715f 100644 --- a/compiler/transf.nim +++ b/compiler/transf.nim @@ -569,10 +569,17 @@ proc transformConv(c: PTransf, n: PNode): PNode = result = transformSons(c, n) of tyObject: var diff = inheritanceDiff(dest, source) + template convHint = + if n.kind == nkHiddenSubConv and n.typ.kind notin abstractVarRange: + # Creates hint on converstion to parent type where information is lost, + # presently hints on `proc(thing)` where thing converts to non var base. + rawMessage(c.graph.config, hintImplicitObjConv, [typeToString(source), typeToString(dest), toFileLineCol(c.graph.config, n[1].info)]) if diff < 0: + convHint() result = newTransNode(nkObjUpConv, n, 1) result[0] = transform(c, n[1]) elif diff > 0 and diff != high(int): + convHint() result = newTransNode(nkObjDownConv, n, 1) result[0] = transform(c, n[1]) else: diff --git a/tests/objects/t4318.nim b/tests/objects/t4318.nim deleted file mode 100644 index 34ff722f5ce..00000000000 --- a/tests/objects/t4318.nim +++ /dev/null @@ -1,12 +0,0 @@ -type - A = object of RootObj - B = object of A - -method identify(a:A) {.base.} = echo "A" -method identify(b:B) = echo "B" - -var b: B - -doAssertRaises(ObjectAssignmentDefect): - var a: A = b - discard a diff --git a/tests/objects/tobj_asgn_dont_slice.nim b/tests/objects/tobj_asgn_dont_slice.nim deleted file mode 100644 index 2e36b65a3cd..00000000000 --- a/tests/objects/tobj_asgn_dont_slice.nim +++ /dev/null @@ -1,24 +0,0 @@ -discard """ - outputsub: '''ObjectAssignmentDefect''' - exitcode: "1" -""" - -# bug #7637 -type - Fruit = object of RootObj - name*: string - Apple = object of Fruit - Pear = object of Fruit - -method eat(f: Fruit) {.base.} = - raise newException(Exception, "PURE VIRTUAL CALL") - -method eat(f: Apple) = - echo "fruity" - -method eat(f: Pear) = - echo "juicy" - -let basket = [Apple(name:"a"), Pear(name:"b")] - -eat(basket[0]) diff --git a/tests/typerel/t4799_1.nim b/tests/typerel/t4799_1.nim deleted file mode 100644 index 74012190b84..00000000000 --- a/tests/typerel/t4799_1.nim +++ /dev/null @@ -1,21 +0,0 @@ -discard """ - targets: "c cpp" - outputsub: '''ObjectAssignmentDefect''' - exitcode: "1" -""" - -type - Vehicle[T] = object of RootObj - tire: T - Car[T] = object of Vehicle[T] - Bike[T] = object of Vehicle[T] - -proc testVehicle[T](x: varargs[Vehicle[T]]): string = - result = "" - for c in x: - result.add $c.tire - -var v = Vehicle[int](tire: 3) -var c = Car[int](tire: 4) -var b = Bike[int](tire: 2) -echo testVehicle b, c, v diff --git a/tests/typerel/t4799_2.nim b/tests/typerel/t4799_2.nim deleted file mode 100644 index f97b8962233..00000000000 --- a/tests/typerel/t4799_2.nim +++ /dev/null @@ -1,21 +0,0 @@ -discard """ - targets: "c cpp" - outputsub: '''ObjectAssignmentDefect''' - exitcode: "1" -""" - -type - Vehicle[T] = object of RootObj - tire: T - Car[T] = object of Vehicle[T] - Bike[T] = object of Vehicle[T] - -proc testVehicle[T](x: varargs[Vehicle[T]]): string = - result = "" - for c in x: - result.add $c.tire - -var v = Vehicle[int](tire: 3) -var c = Car[int](tire: 4) -var b = Bike[int](tire: 2) -echo testVehicle([b, c, v]) \ No newline at end of file diff --git a/tests/typerel/t4799_3.nim b/tests/typerel/t4799_3.nim deleted file mode 100644 index 6102b69cc63..00000000000 --- a/tests/typerel/t4799_3.nim +++ /dev/null @@ -1,21 +0,0 @@ -discard """ - targets: "c cpp" - outputsub: '''ObjectAssignmentDefect''' - exitcode: "1" -""" - -type - Vehicle = object of RootObj - tire: int - Car = object of Vehicle - Bike = object of Vehicle - -proc testVehicle(x: varargs[Vehicle]): string = - result = "" - for c in x: - result.add $c.tire - -var v = Vehicle(tire: 3) -var c = Car(tire: 4) -var b = Bike(tire: 2) -echo testVehicle([b, c, v]) \ No newline at end of file diff --git a/tests/types/tinheritance_arrays.nim b/tests/types/tinheritance_arrays.nim new file mode 100644 index 00000000000..4c2704fb310 --- /dev/null +++ b/tests/types/tinheritance_arrays.nim @@ -0,0 +1,53 @@ +discard """ + cmd: "nim check --hints:off $file" + action: reject + nimout: ''' +tinheritance_arrays.nim(20, 18) Error: type mismatch: got but expected 'A = object' +tinheritance_arrays.nim(20, 23) Error: type mismatch: got but expected 'A = object' +tinheritance_arrays.nim(21, 18) Error: type mismatch: got but expected 'C = object' +tinheritance_arrays.nim(21, 23) Error: type mismatch: got but expected 'C = object' +tinheritance_arrays.nim(22, 23) Error: type mismatch: got but expected 'B = object' +''' +""" + + +block: # Value test + type + A = object of RootObj + B = object of A + C = object of A + + discard [A(), B(), C()] + discard [C(), B(), A()] + discard [B(), B(), A()] + discard [B(), B(), B()] + discard [A(), A(), A()] + +block: # ref test + type + A = ref object of RootObj + B = ref object of A + C = ref object of A + + discard [A(), B(), C()] + discard [C(), B(), A()] + discard [B(), B(), A()] + discard [B(), B(), B()] + discard [A(), A(), A()] + +block: # ptr test + type + A = object of RootObj + B = object of A + C = object of A + + template make(t: typedesc): ptr t = + let res = createU(t) + res[] = t() + res + + discard [make A, make B, make C] + discard [make C, make B, make A] + discard [make B, make B, make A] + discard [make B, make B, make B] + discard [make A, make A, make A] diff --git a/tests/types/tinheritance_conversion.nim b/tests/types/tinheritance_conversion.nim new file mode 100644 index 00000000000..a0b4b01ddee --- /dev/null +++ b/tests/types/tinheritance_conversion.nim @@ -0,0 +1,71 @@ +discard """ + cmd: "nim c --hint[Conf]:off --verbosity:0 $file" + nimout: ''' +Hint: Implicit conversion: Receiver 'Base' will not receive fields of sub-type 'Derived' [tinheritance_conversion.nim(30, 15)] [ImplicitObjConv] +Hint: Implicit conversion: Receiver 'Base' will not receive fields of sub-type 'Derived' [tinheritance_conversion.nim(30, 34)] [ImplicitObjConv] +Hint: Implicit conversion: Receiver 'Base' will not receive fields of sub-type 'Derived2' [tinheritance_conversion.nim(38, 3)] [ImplicitObjConv] + +''' +""" + + +type + Base {.inheritable.} = object + field: int + + Derived = object of Base + field2: int + field3: int + Derived2 = object of Base + +block: # Value tests + proc test(args: varargs[Base]) = + for x in args: + assert x.field == 0 + + proc test2(base: var Base) = base.field = 400 + proc test3(base: Base) = discard + var a: Derived = Derived(Base()) + a = Derived(Base(Derived2())) + test(Derived(), Base(), Derived()) + a.field2 = 300 + test2(a) + assert a.field == 400 + assert a.field2 == 300 + var b = Derived2(field: 800) + b.test2() + assert b.field == 400 + b.test3() + + + +block: # Ref tests + type + Base = ref object of RootObj + field: int + Derived = ref object of Base + field2: int + Derived2 = ref object of Base + + var a: Base = Derived() + assert Derived(a) is Derived + doAssertRaises(ObjectConversionDefect): discard Derived2(a)[] + doAssertRaises(ObjectConversionDefect): discard Base(Derived2()).Derived + assert Base(Derived()) is Base + assert Derived2(Base(Derived2())) is Derived2 + assert Derived(Base(Derived())) is Derived + +block: # Pointer tests + template make(t: typedesc): ptr t = + let res = createU(t) + res[] = t() + res + var a: ptr Base = make(Derived) + assert (ptr Derived)(a) is (ptr Derived) + doAssertRaises(ObjectConversionDefect): discard (ptr Derived2)(a)[] + doAssertRaises(ObjectConversionDefect): + var a = make(Derived2) + discard (ptr Derived)((ptr Base)(a)) + assert (ptr Base)(make(Derived)) is (ptr Base) + assert (ptr Derived2)((ptr Base)(make(Derived2))) is (ptr Derived2) + assert (ptr Derived)((ptr Base)(make(Derived))) is (ptr Derived)