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

compiler should give ambiguity errors in case of multiple compatible matches #8568

Closed
timotheecour opened this issue Aug 8, 2018 · 12 comments

Comments

Projects
None yet
5 participants
@timotheecour
Copy link
Contributor

commented Aug 8, 2018

see all entries marked as BUG:

#[ 
reduced from D20180808T025705
 ]#

type
  A0 = object
  A = object
  B = object
  C[T] = object
  C2[T] = distinct C[T]

  D[T] = object
  E[T] = object

proc `$`(a: B|A0):string = "foo0"
proc `$`(a: A|A0):string = "foo1"
proc `$`(a: A|B):string = "foo2"
proc `$`(a: A):string = "foo3"
proc `$`(a: B): string = "foo4"

proc `$`(a: C|C2[int]):string = "foo5"
proc `$`(a: C|C2):string = "foo6"
proc `$`(a: C):string = "foo7"
proc `$`(a: C2):string = "foo8"
proc `$`(a: C[int]):string = "foo9"

proc `$`(a: D|E):string = "foo10"
proc `$`(a: D):string = "foo11"
proc `$`(a: E):string = "foo12"

proc test()=
  #[
foo3
foo4
foo6
foo6
foo10
foo10
   ]#
  echo A() # foo3 ; is that in Nim manual? looks like it prefers matching to A instead of A|B ;  maybe a BUG
  echo B() # foo4

  echo C[int]() # foo6 BUG=>foo7 and especially foo9 match more strongly ; should probably give compile error due to ambiguity

  var c2:C2[int] # foo6 => BUG: foo5 or foo8 match more strongly (more specialized); should probably give compile error due to ambiguity
  echo c2

  echo D[int]() #foo10; BUG: should give compile error: ambiguous defnition
  echo E[int]() # ditto

test()
@andreaferretti

This comment has been minimized.

Copy link
Collaborator

commented Aug 8, 2018

Here is the algorithm in excruciating detail.

About echo A() giving foo3 - this is in the manual: the first type of match considered is "exact match"

@timotheecour

This comment has been minimized.

Copy link
Contributor Author

commented Aug 8, 2018

About echo A() giving foo3 - this is in the manual: the first type of match considered is "exact match"

actually the manual is not clear here for A() is of type A:
"proc $(a: A)" => exact match (ok)
"proc $(a: A|B):string" => is that an exact match or a generic match?

Exact match: a and f are of the same type.
Generic match: f is a generic type and a matches, for instance a is int and f is a generic (constrained) parameter type (like in [T] or [T: int|char].

that depends on definition of "the same type" and "generic":

  • is that defined by "a is A|B" ? then it's same type (so there is ambiguity, and should give compile error)
  • is "proc $(a: A|B):string" considered generic ? there is no bracketed expression here [T] as mentioned in doc like in [T] or [T: int|char]

As for the other points marked as BUG, do you agree they are bugs? we can debate what correct answer should be for these (ambiguity error or a different match), but selected match doesn't make sense

@andreaferretti

This comment has been minimized.

Copy link
Collaborator

commented Aug 8, 2018

proc $(a: A|B) is generic, because A|B is not a concrete type. It will need to be specialized when it is called, to either A or B - depending on the call site.

I agree all other points are bugs.

@timotheecour

This comment has been minimized.

Copy link
Contributor Author

commented Aug 8, 2018

ok; then docs in "Generic match: f is a generic type and a matches, for instance a is int and f is a generic (constrained) parameter type (like in [T] or [T: int|char]." definitely need to be clarified and add an example like "A|B"

@timotheecour

This comment has been minimized.

Copy link
Contributor Author

commented Aug 8, 2018

yikes, that implies that vesion1 and vesion2 below have different semantics [1], and that version 2 should be preferred if we wanna avoid hijacking even though it's more convenient and "doesn't look" generic (even though it is)

# version 1
proc foo(x:A|B) = bar(x)

# version 2
proc foo(x:A) = bar(x)
proc foo(x:B) = bar(x)

[1] with yet another caveat: version 1 is hijackable (and not version 2), except if A is itself a generic...

@andreaferretti

This comment has been minimized.

Copy link
Collaborator

commented Aug 8, 2018

Yes- at least, this is my understanding. The difference is that in version 2 you are actually writing two specializations yourself. I find that thinking in terms of concrete types vs things that have to be specialized is the simplest point of view for understading most of Nim.

(I am sure @Araq or @zah can correct if I got something wrong here)

@Araq

This comment has been minimized.

Copy link
Member

commented Aug 8, 2018

@andreaferretti Your remarks are all correct.

@Araq Araq added the Showstopper label Aug 8, 2018

@pqflx3

This comment has been minimized.

Copy link
Contributor

commented Aug 9, 2018

I think this is another case, written in slightly different terms:

proc foo(a: int): string = "foo1 "
proc foo(a: static[int]): string = "foo2 "
proc foo(a: var int): string = "foo3 "
proc foo[T: int](a: T): string = "foo4 "
#proc foo[T](a: T): string = "foo5 " # enable | disable this proc
    
const tmp1 = 1
let tmp2 = 1
var tmp3 = 1

echo foo(1),      foo(tmp1),      foo(tmp2),      foo(tmp3)
echo foo[int](1), foo[int](tmp1), foo[int](tmp2), foo[int](tmp3)

#[
Results (disabled):
foo2 foo2 foo1 foo3 
foo4 foo4 foo4 foo4   

Results (enabled):
foo2 foo2 foo1 foo3 
foo1 foo1 foo1 foo3 
]#

I'm not sure why specifying the generic in foo[int] works iff either foo[T] or foo[T:int] are defined, but not both.

@timotheecour

This comment has been minimized.

Copy link
Contributor Author

commented Aug 22, 2018

/cc @LemonBoy in #8700 you say:

These types are generic

Are they? I expected a tyAnd/tyOr() not to be considered as generic.

(also reflected in PR title: Don't consider tyAnd/tyNot/tyOr/tyAnything as generic)

this contradicts @andreaferretti 's comment above

proc $(a: A|B) is generic, because A|B is not a concrete type. It will need to be specialized when it is called, to either A or B - depending on the call site.

so... what's the truth?

Note: the answer to this affects how echo A() should be handled in my above example (the other ones are bugs)

@zah

This comment has been minimized.

Copy link
Member

commented Aug 22, 2018

A|B is a generic type. It makes sense only in contexts where generic types are allowed and it will trigger all consequences of using a generic type (separate compilation of each instantiation of a proc, etc).

@timotheecour

This comment has been minimized.

Copy link
Contributor Author

commented Aug 22, 2018

makes sense. So I guess:

type T = T1|T2
proc foo(a:T)=discard

#is equivalent to:
proc foo[U: T1 | T2](a:U)=discard

?

  • so was there any inconsistency with #8700 ? (Don't consider tyAnd/tyNot/tyOr/tyAnything as generic) ?
@Araq

This comment has been minimized.

Copy link
Member

commented Aug 28, 2018

Must not block the release, high prio instead of showstopper.

@Araq Araq added High Priority and removed Showstopper labels Aug 28, 2018

@Araq Araq changed the title overload resolution buggy, leading to symbol hijacking => compiler should give ambiguity errors in case of multiple compatible matches compiler should give ambiguity errors in case of multiple compatible matches Feb 23, 2019

Araq added a commit that referenced this issue May 22, 2019

@Araq Araq closed this in fd16875 May 22, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.