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

varargs[typed] Doesn't accept nnkClosedSymChoice node but typed does #19446

Closed
ynfle opened this issue Jan 24, 2022 · 13 comments · Fixed by #24525
Closed

varargs[typed] Doesn't accept nnkClosedSymChoice node but typed does #19446

ynfle opened this issue Jan 24, 2022 · 13 comments · Fixed by #24525

Comments

@ynfle
Copy link
Contributor

ynfle commented Jan 24, 2022

varargs[typed] as macro parameter type doesn't accept a NimNode of kind nnkClosedSymChoice but just a parameter type of typed does.

Example

import macros

macro typedVarargs(x: varargs[typed]) =
  echo x[0].kind
  
macro typedSingle(x: typed) =
  echo x.kind
  
typedSingle(len)
typedVarargs(len)

Current Output

nnkSym
/usercode/in.nim(10, 13) Error: type mismatch: got <proc (x: HSlice[len.U, len.V]): int{.inline, noSideEffect.} | proc (x: seq[T]): int{.noSideEffect.} | proc (x: string): int{.noSideEffect, gcsafe, locks: 0.} | proc (x: TOpenArray: openArray or varargs): int{.noSideEffect.} | proc (x: set[T]): int{.noSideEffect.} | proc (x: typedesc[array] or array): int{.noSideEffect.} | proc (x: cstring): int{.noSideEffect, gcsafe, locks: 0.} | proc (w: WideCString): int{.noSideEffect, gcsafe, locks: 0.} | proc (n: NimNode): int{.noSideEffect.}>
but expected one of:
macro typedVarargs(x: varargs[typed])
  first type mismatch at position: 1
  required type for x: varargs[typed]
  but expression 'len' is of type: None

expression: typedVarargs(len)

Expected Output

nnkSym
nnkClosedSymChoice

Additional Information

Seems to exist at least back from 1.0.0
Testing on nightly

$ nim -v
Nim Compiler Version 1.6.2 [MacOSX: amd64]
Compiled at 2021-12-20
Copyright (c) 2006-2021 by Andreas Rumpf

active boot switches: -d:release
@Vindaar
Copy link
Contributor

Vindaar commented Jan 24, 2022

Related and/or duplicate of #13913 ?

@ynfle
Copy link
Contributor Author

ynfle commented Jan 24, 2022

Related and/or duplicate of #13913 ?

It is, but the issue there isn't the multiple procs, it's that one of them is overloaded.

Any idea where to look to fix this?

@beef331
Copy link
Collaborator

beef331 commented Jan 24, 2022

Where you need to look is sigmatch.typerel

proc typeRel(c: var TCandidate, f, aOrig: PType,

@ynfle
Copy link
Contributor Author

ynfle commented Jan 25, 2022

So it seems that this is the issue

Nim/compiler/sigmatch.nim

Lines 2208 to 2211 in 7bdfeb7

proc paramTypesMatch*(m: var TCandidate, f, a: PType,
arg, argOrig: PNode): PNode =
if arg == nil or arg.kind notin nkSymChoices:
result = paramTypesMatchAux(m, f, a, arg, argOrig)

When it isn't overloaded symbol, then it delegates to paramTypesMatchAux which if it doesn't get a match (r = isNone), userConvMatch(c, m, f, a, arg) == nil, f.kind == tyVarargs & f.n == nil, it tries to find a match with base(f) which has kind == tyTyped which matches nkClosedSymChoice.

If, however, the symbol is overloaded, it tries every one of the symbols, but doesn't try base(f) if it's varargs.

Not sure if this is the intended behaviour or what the proper way to fix it if it isn't the intended behaviour.

I could technically just add and else statement here

Nim/compiler/sigmatch.nim

Lines 2236 to 2250 in 7bdfeb7

let r = typeRel(z, f, arg[i].typ)
incMatches(z, r, 2)
if r != isNone:
z.state = csMatch
case x.state
of csEmpty, csNoMatch:
x = z
best = i
of csMatch:
let cmp = cmpCandidates(x, z)
if cmp < 0:
best = i
x = z
elif cmp == 0:
y = z # z is as good as x

with something like

elif f.kind == tyVarargs and f.n == nil and userConvMatch(c, m, f, a, arg) == nil:

then something like this

case r

I don't know the possibles cases of the compiler well enough to make an informed change

Any guidance (or help) would be much appreciated

@ynfle
Copy link
Contributor Author

ynfle commented Jan 26, 2022

I guess my questions are:

  • primarily, why doesn't paramTypesMatch look at the subtype when it's varargs (and maybe the other necessary conditions)

    Nim/compiler/sigmatch.nim

    Lines 2177 to 2182 in 7bdfeb7

    if result == nil and f.kind == tyVarargs:
    if f.n != nil:
    # Forward to the varargs converter
    result = localConvMatch(c, m, f, a, arg)
    else:
    r = typeRel(m, base(f), a)
    but paramTypesMatchAux does?
  • when is f.n nil?
    if f.n != nil:
  • What are userConvMatch & localConvMatch and do I need to care about them in this case?
  • When will the tfVarargs in a.flags be true?
    if tfVarargs in a.flags:

@beef331
Copy link
Collaborator

beef331 commented Jan 26, 2022

When will the tfVarargs in a.flags be true?

When using the {.varargs.} pragma for C interop.

The rest I do not know.

@ynfle
Copy link
Contributor Author

ynfle commented Jan 26, 2022

When will the tfVarargs in a.flags be true?

When using the {.varargs.} pragma for C interop.

The rest I do not know.

Makes sense

Although there there is this

container.typ.flags.incl tfVarargs

@ynfle
Copy link
Contributor Author

ynfle commented Jan 27, 2022

More questions:

  • What do x, y, & z represent?

    Nim/compiler/sigmatch.nim

    Lines 2219 to 2221 in 7bdfeb7

    x = newCandidate(c, m.callee)
    y = newCandidate(c, m.callee)
    z = newCandidate(c, m.callee)
    • it seems like:
      • z is the total matches for the current overloaded candidate
      • x is if the best match
  • Why is result set to nil here?
    else: result = nil
    • It probably depends on what y means
    • There for sure is a match, so why is nothing returned?

@beef331
Copy link
Collaborator

beef331 commented Jan 27, 2022

Perhaps you should ask these questions inside the internals realtime chat

ynfle added a commit to ynfle/Nim-1 that referenced this issue Jan 31, 2022
There was a bug that params wouldn't be matched if the param was
overloaded and the candidate type was varargs. This checks the base type
of the `varargs` and then will to an overloaded symbol if in a macros or
template.

Closes nim-lang#19446 & nim-lang#13913
@ynfle
Copy link
Contributor Author

ynfle commented Jan 31, 2022

This isn't an issue with varargs[typed] but rather varargs[T] where the param is an overloaded symbol.

import sequtils

echo @[1, 3, 4, 5].map(len)

@ynfle
Copy link
Contributor Author

ynfle commented Jan 31, 2022

Not sure how to proceed with this one. Would resolving as a nnkClosedSymChoice be helpful?

@ynfle
Copy link
Contributor Author

ynfle commented Jan 31, 2022

Never mind, it seems like SymChoices aren't intended to be passed in to procs in this way (I think)

ynfle added a commit to ynfle/Nim-1 that referenced this issue Jan 31, 2022
There was a bug that params wouldn't be matched if the param was
overloaded and the candidate type was varargs. This checks the base type
of the `varargs` and then will to an overloaded symbol if in a macros or
template.

Fixes nim-lang#19446
Fixes nim-lang#13913
ynfle added a commit to ynfle/Nim-1 that referenced this issue Jan 31, 2022
There was a bug that params wouldn't be matched if the param was
overloaded and the candidate type was varargs. This checks the base type
of the `varargs` and then will to an overloaded symbol if in a macros or
template.

Fixes nim-lang#19446 &
nim-lang#13913
ynfle added a commit to ynfle/Nim-1 that referenced this issue Jan 31, 2022
There was a bug that params wouldn't be matched if the param was
overloaded and the candidate type was varargs. This checks the base type
of the `varargs` and then will to an overloaded symbol if in a macros or
template.

Fixes nim-lang#19446 &
nim-lang#13913
ynfle added a commit to ynfle/Nim-1 that referenced this issue Jan 31, 2022
There was a bug that params wouldn't be matched if the param was
overloaded and the candidate type was varargs. This checks the base type
of the `varargs` and then will to an overloaded symbol if in a macros or
template.

Fixes nim-lang#19446 &
nim-lang#13913
ynfle added a commit to ynfle/Nim-1 that referenced this issue Feb 3, 2022
There was a bug that params wouldn't be matched if the param was
overloaded and the candidate type was varargs. This checks the base type
of the `varargs` and then will to an overloaded symbol if in a macros or
template.

Fixes nim-lang#19446 &
nim-lang#13913
ynfle added a commit to ynfle/Nim-1 that referenced this issue Apr 14, 2022
There was a bug that params wouldn't be matched if the param was
overloaded and the candidate type was varargs. This checks the base type
of the `varargs` and then will to an overloaded symbol if in a macros or
template.

Fixes nim-lang#19446 &
nim-lang#13913
@metagn
Copy link
Collaborator

metagn commented Sep 26, 2024

Works now maybe because of opt-in symchoices?

metagn added a commit to metagn/Nim that referenced this issue Dec 8, 2024
@Araq Araq closed this as completed in aeb3fe9 Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants