nim-lang / Nim

Nim is a statically typed compiled systems programming language. It combines successful concepts from mature languages like Python, Ada and Modula. Its design focuses on efficiency, expressiveness, and elegance (in that order of priority).
https://nim-lang.org
Other
16.63k stars 1.47k forks source link

Method dispatch silently breaks when sequtils.map in use #4320

Open mcclure opened 8 years ago

mcclure commented 8 years ago

Following is with Nim built from git:07d7d35d995262830

Consider this code:

from sequtils import map

type
    A = ref object of RootObj
    B = ref object of A

method returnself(a:A) : A {.base.} = echo "A"; return a

method returnself(b:B) : A = echo "B"; return b

var bees: seq[A] = @[]
add(bees, new B)
add(bees, new B)

discard map(bees, returnself)

discard map(bees, proc (a:A) : A = return a.returnself)

Observed behavior: This program prints

A
A
B
B

There are no warnings or errors, even on verbosity:2. If instead of importing map from sequtils I copy the body of sequtils.map directly into this program, I get the same behavior.

Expected behavior: One of the following should be true:

  1. The "multi-methods" section of the manual represents methods as just procs with multiple dispatch. No caveats are mentioned. Therefore, I should be able to pass a method anywhere I can pass a proc, and it should not change its behavior. Passing a method, and passing a proc which calls the method, should never produce different results. The above program should print "B\nB\nB\nB".
  2. If for some reason this is not possible, there should be an error or warning at the site where I pass a method (such as returnself) into a place where methods cannot be correctly accepted (such as map). Moreover, the manual should clearly describe under "multi-methods" under which circumstances methods will break in this fashion.
Araq commented 8 years ago

There is no reason (1.) cannot work here. :-)

mcclure commented 8 years ago

Cool.

By the way, I came up with a simpler test case where sequtils is not required.

proc dothis[T](data: T, op: proc (x: T) {.closure.}) =
    op(data)

type
    A = ref object of RootObj
    B = ref object of A

method identify(a:A) {.base.} = echo "A"

method identify(b:B) = echo "B"

var b:B; b.new
var a:A = b

dothis(a, identify)

dothis(a, proc(x:A) = x.identify)

Expected: B\nB Observed: A\nB Replacing ".closure." with ".nimcall." or removing the "{.closure.}" annotation altogether produces the same results.

stale[bot] commented 4 years ago

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.