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

Overload resolution broken with subclassing + or #16321

Open
planetis-m opened this issue Dec 11, 2020 · 6 comments
Open

Overload resolution broken with subclassing + or #16321

planetis-m opened this issue Dec 11, 2020 · 6 comments

Comments

@planetis-m
Copy link
Contributor

planetis-m commented Dec 11, 2020

In the code below, overload for Option is not used instead the generic object overload takes its place.

Example

import options, streams

proc storeBin*(s: Stream; x: bool) =
  write(s, $x)

proc storeBin*[T](s: Stream; o: Option[T]) =
  let isSome = isSome(o)
  storeBin(s, isSome)
  if isSome:
    storeBin(s, get(o))

proc storeBin*[T: object|tuple](s: Stream; o: T) =
  write(s, "bug")

var s = newStringStream()
s.storeBin(none(bool))
#s.storeBin(some(true)) #same
echo s.data

Current Output

bug

Expected Output

false

Additional Information

$ nim -v
Nim Compiler Version 1.5.1 [Linux: amd64]
Compiled at 2020-12-09
Copyright (c) 2006-2020 by Andreas Rumpf

git hash: 9ce2f87a0a02d85e943b2b181bcae66fbd644f11
@planetis-m
Copy link
Contributor Author

planetis-m commented Dec 11, 2020

Another test:

type
  Foo*[K] = object
    item: K

template empty*[K](t: typedesc[K]): K = high(t)
proc initFoo*[K](): Foo[K] =
  result = Foo[K](item: empty(K))

type
  Bar = distinct uint16

proc `==`(a, b: Bar): bool {.borrow.}
template empty*(t: typedesc[Bar]): Bar = Bar(0)

var x = initFoo[Bar]()
assert x.item == Bar(0)

@timotheecour
Copy link
Member

timotheecour commented Dec 11, 2020

always try to minimize please ;-)
If it's a compiler bug, ideally it should be minimzed to not rely on other imports (or at least only hand-crafted imports).

Here's a minimized repro without imports, and here's an updated diagnostic:

overload resolution wrongly favors tyOr when some argument matches via subclassing.

when true:
  type Stream = object of RootObj
  type Stream2 = object of Stream
  type Foo[T] = object
  proc storeBin[T](s: Stream; o: Foo[T]) = discard
  proc storeBin[T: float|object](s: Stream; o: T) = doAssert false
  # var s = Stream() # ok
  var s = Stream2() # bug
  s.storeBin(Foo[int]())

links

sigmatch.ninm: see isSubtype, subtypeMatches, isGeneric

@planetis-m
Copy link
Contributor Author

Thanks! I tried my best but you helped to make it even smaller.

@timotheecour timotheecour changed the title Overloads getting silently ignored Overload resolution broken with subclassing + or Dec 11, 2020
@planetis-m
Copy link
Contributor Author

I don't know if you want me to submit different issues, but second test case is not related to subclassing as far as I can tell :)

@timotheecour
Copy link
Member

ya, it may be a different bug then.

Maybe try to take a stab at a PR? See sigmatch.nim, it sounds not too complicated (YMMV)

@planetis-m
Copy link
Contributor Author

sure will try tomorrow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants