Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Method dispatch silently breaks on non-ref objects #4318

Closed
mcclure opened this issue Jun 13, 2016 · 2 comments
Closed

Method dispatch silently breaks on non-ref objects #4318

mcclure opened this issue Jun 13, 2016 · 2 comments
Labels

Comments

@mcclure
Copy link

mcclure commented Jun 13, 2016

Following is with Nim built from git:07d7d35d995262830

Consider this code:

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
var a: A = b

identify(b)
identify(a)

Observed behavior: This program prints

B
A

There are no warnings or errors, even on verbosity:2.

It appears that when I say "var a: A = b" I am implicitly converting b to A type and therefore the b identity is lost. If B has fields, it appears those fields are getting thrown away. There is no text indicating this loss of identity in the manual either under http://nim-lang.org/docs/manual.html#types-tuples-and-object-types or http://nim-lang.org/docs/manual.html#multi-methods (the former section says "the default assignment operator for objects copies each component" but it is far from obvious this text implies "...and subclass-related are thrown away"). The behavior appears to be in active conflict with the "convertible relations" section http://nim-lang.org/docs/manual.html#type-relations-convertible-relation which say A and B should be explicitly convertible because B is a subtype but do not say they are implicitly convertible.

Expected behavior:

  1. One or more of the following should be true:
    1. Naively, my expectation is that if I copy b:B to a:A, the value referred to by a should still be a B, act like a B, have all of B's fields etc. This is how ref types work and therefore this is how the examples in the Nim manual work, and this is how my intuition works of objects in an object-oriented language. Therefore the above program should print B\nB. However, given how I understand the Nim memory model to work, I expect this is probably not possible for non-ref objects. (C++ for example would not support this for stack objects.)
    2. If the language's actual behavior cannot match my naive intuition, the language should provide an error or warning when I copy objects between vars in a way which will non-intuitively change their behavior. This would be consistent with other behavior in Nim already. For example, in Nim, I cannot copy a int value to a float var or vice versa without a cast. Copying between vars of non-ref type A and B of A involves a loss of information or at least a change analogous to copying between ints and floats, therefore copying between a:A and b:B where A and B are non-ref objects should require an explicit cast. Again, this behavior (error on direct assignment from B of A to A without cast) seems to be required by the "convertible relation" section of the manual.
    3. Because Nim is usually aggressive about giving warnings when I am doing something that looks like a mistake (variable named l, etc) Nim should give me a warning for the above code that method is useless and I should use proc instead. In general, any pair of methods could just as well be procs unless at least one argument dispatches on a ref object. Any use of method where the special dispatch capabilities of method are not being used therefore is probably accidental on the part of the user and a warning is appropriate.
  2. No matter what, the manual should unambiguously state that assigning an object of non-ref type B to non-ref var A will convert it to an A, and thus polymorphism/"method" are effectively exclusive to ref objects. In addition, the "convertible relation" section should accurately describe under what conditions objects are implicitly converted.
@Araq Araq added the Methods label Jun 13, 2016
@stale
Copy link

stale bot commented Aug 4, 2020

This issue has been automatically marked as stale because it has not had recent activity. If you think it is still a valid issue, write a comment below; otherwise it will be closed. Thank you for your contributions.

@stale stale bot added the stale Staled PR/issues; remove the label after fixing them label Aug 4, 2020
@ringabout ringabout removed the stale Staled PR/issues; remove the label after fixing them label Nov 10, 2020
@ringabout
Copy link
Member

Since Nim v 0.19.0, this will give error:

C:\Users\blue\.choosenim\toolchains\nim-#devel\lib\system\assign.nim(139) genericAssign
C:\Users\blue\.choosenim\toolchains\nim-#devel\lib\system\assign.nim(125) genericAssignAux
C:\Users\blue\.choosenim\toolchains\nim-#devel\lib\system\fatal.nim(49) sysFatal
Error: unhandled exception: invalid object assignment [ObjectAssignmentDefect]
Error: execution of an external program failed: 'D:\QQPCmgr\Desktop\Nim\test.exe '

ringabout added a commit to ringabout/Nim that referenced this issue Nov 10, 2020
narimiran pushed a commit that referenced this issue Nov 16, 2020
* close #4318(add testcase for #4318)

* Update tests/objects/t4318.nim

Co-authored-by: Juan Carlos <juancarlospaco@gmail.com>

Co-authored-by: Juan Carlos <juancarlospaco@gmail.com>
(cherry picked from commit 35f8803)
PMunch pushed a commit to PMunch/Nim that referenced this issue Jan 6, 2021
* close nim-lang#4318(add testcase for nim-lang#4318)

* Update tests/objects/t4318.nim

Co-authored-by: Juan Carlos <juancarlospaco@gmail.com>

Co-authored-by: Juan Carlos <juancarlospaco@gmail.com>
mildred pushed a commit to mildred/Nim that referenced this issue Jan 11, 2021
* close nim-lang#4318(add testcase for nim-lang#4318)

* Update tests/objects/t4318.nim

Co-authored-by: Juan Carlos <juancarlospaco@gmail.com>

Co-authored-by: Juan Carlos <juancarlospaco@gmail.com>
irdassis pushed a commit to irdassis/Nim that referenced this issue Mar 16, 2021
* close nim-lang#4318(add testcase for nim-lang#4318)

* Update tests/objects/t4318.nim

Co-authored-by: Juan Carlos <juancarlospaco@gmail.com>

Co-authored-by: Juan Carlos <juancarlospaco@gmail.com>
ardek66 pushed a commit to ardek66/Nim that referenced this issue Mar 26, 2021
* close nim-lang#4318(add testcase for nim-lang#4318)

* Update tests/objects/t4318.nim

Co-authored-by: Juan Carlos <juancarlospaco@gmail.com>

Co-authored-by: Juan Carlos <juancarlospaco@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants