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

early param count check for overloads #21673

Closed
wants to merge 9 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions changelogs/changelog_2_0_0.md
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,25 @@
replacement of the `nnkFormalParams` node as well as having child nodes
unlike other type class AST.

- Overloaded routine calls will now eagerly check if the parameter counts are
compatible with each overload. This means `untyped` parameters will not be
type checked in some cases where they previously were, i.e.:

```nim
template foo(x: int, body: untyped) =
let value {.inject.} = x
body
template foo(body: untyped) = foo(123, body)

# previously did not compile, now compiles:
foo:
echo value # 123
```

To fix cases where code depended on this old behavior, change the relevant
`untyped` parameters to `typed`. The switch `--legacy:noEagerParamCountMatch`
is also provided to use the old behavior for the time being.

## Standard library additions and changes

[//]: # "Changes:"
Expand Down
5 changes: 5 additions & 0 deletions compiler/ast.nim
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,8 @@ type
tfInheritable, # is the object inheritable?
tfHasOwned, # type contains an 'owned' type and must be moved
tfEnumHasHoles, # enum cannot be mapped into a range
# for objects becomes tfObjHasKids
# for routines becomes tfHasVarargsParam
tfShallow, # type can be shallow copied on assignment
tfThread, # proc type is marked as ``thread``; alias for ``gcsafe``
tfFromGeneric, # type is an instantiation of a generic; this is needed
Expand Down Expand Up @@ -629,6 +631,7 @@ const
tfUnion* = tfNoSideEffect
tfGcSafe* = tfThread
tfObjHasKids* = tfEnumHasHoles
tfHasVarargsParam* = tfEnumHasHoles ## routine type has varargs parameter
tfReturnsNew* = tfInheritable
skError* = skUnknown

Expand Down Expand Up @@ -917,6 +920,8 @@ type
# for modules, an unique index corresponding
# to the module's fileIdx
# for variables a slot index for the evaluator
# for routines the minimum required parameters
# (could be part of the type but no space there)
offset*: int # offset of record field
loc*: TLoc
annex*: PLib # additional fields (seldom used, so we use a
Expand Down
4 changes: 4 additions & 0 deletions compiler/options.nim
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,10 @@ type
laxEffects
## Lax effects system prior to Nim 2.0.
verboseTypeMismatch
noEagerParamCountMatch
## Routine calls will not eagerly fail a match on incompatible
## parameter counts. This causes some `untyped` parameters to become
## typed.

SymbolFilesOption* = enum
disabledSf, writeOnlySf, readOnlySf, v2Sf, stressTest
Expand Down
13 changes: 13 additions & 0 deletions compiler/semtypes.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1281,6 +1281,7 @@ proc semProcTypeNode(c: PContext, n, genericParams: PNode,
result = newProcType(c, n.info, prev)
var check = initIntSet()
var counter = 0
var requiredCount = 0 # minimum required parameters

for i in 1..<n.len:
var a = n[i]
Expand All @@ -1306,6 +1307,8 @@ proc semProcTypeNode(c: PContext, n, genericParams: PNode,
if kind in {skProc, skFunc} and (typ.kind == tyTyped or typ.kind == tyUntyped):
if not isMagic(getCurrOwner(c)):
localError(c.config, a[^2].info, "'" & typ.sym.name.s & "' is only allowed in templates and macros or magic procs")
if typ.kind == tyVarargs:
result.flags.incl tfHasVarargsParam


if hasDefault:
Expand Down Expand Up @@ -1358,6 +1361,9 @@ proc semProcTypeNode(c: PContext, n, genericParams: PNode,
elif typ.kind == tyStatic:
def = semConstExpr(c, def)
def = fitNode(c, typ, def, def.info)
else: # no default value
if not (hasType and typ.kind in {tyVarargs, tyVoid}):
requiredCount += a.len - 2

if not hasType and not hasDefault:
if isType: localError(c.config, a.info, "':' expected")
Expand Down Expand Up @@ -1399,6 +1405,13 @@ proc semProcTypeNode(c: PContext, n, genericParams: PNode,
onDef(a[j].info, arg)
a[j] = newSymNode(arg)

if not isType:
# routine symbol for which the params are being checked
let s = result.owner
if s != nil:
# set the position field to the minimum number of required parameters
s.position = requiredCount

var r: PType
if n[0].kind != nkEmpty:
r = semTypeNode(c, n[0], nil)
Expand Down
31 changes: 29 additions & 2 deletions compiler/sigmatch.nim
Original file line number Diff line number Diff line change
Expand Up @@ -2399,8 +2399,9 @@ proc findFirstArgBlock(m: var TCandidate, n: PNode): int =

proc matchesAux(c: PContext, n, nOrig: PNode, m: var TCandidate, marker: var IntSet) =

template noMatch() =
c.mergeShadowScope #merge so that we don't have to resem for later overloads
template noMatch(paramScopeOpen = true) =
when paramScopeOpen:
c.mergeShadowScope #merge so that we don't have to resem for later overloads
m.state = csNoMatch
m.firstMismatch.arg = a
m.firstMismatch.formal = formal
Expand Down Expand Up @@ -2438,6 +2439,32 @@ proc matchesAux(c: PContext, n, nOrig: PNode, m: var TCandidate, marker: var Int
formal = if formalLen > 1: m.callee.n[1].sym else: nil # current routine parameter
container: PNode = nil # constructed container
let firstArgBlock = findFirstArgBlock(m, n)

# early check for parameter count (nimsuggest allows partial calls)
let givenCount = n.len - 1
let expectedCount = formalLen - f
if givenCount != expectedCount and
noEagerParamCountMatch notin c.config.legacyFeatures and
(when defined(nimsuggest): c.config.ideCmd notin {ideSug, ideCon} else: true):
# routine symbols precalculate minimum argument count in `position` field
# if there is no routine symbol, don't bother calculating
if m.callee.kind == tyProc and m.calleeSym != nil:
# if this fails just add it as an `and`:
assert m.calleeSym.kind in skProcKinds
# if this is unset, it's 0 by default, which matches everything anyway:
let minCount = m.calleeSym.position
if givenCount < minCount:
m.firstMismatch.kind = kMissingParam
a = givenCount + 1
formal = m.callee.n[a].sym
noMatch(paramScopeOpen = false)
# no max param count for varargs
if {tfVarargs, tfHasVarargsParam} * m.callee.flags == {}:
if givenCount > expectedCount:
m.firstMismatch.kind = kExtraArg
a = expectedCount + 1
noMatch(paramScopeOpen = false)

while a < n.len:
c.openShadowScope

Expand Down
4 changes: 3 additions & 1 deletion compiler/vm.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1157,7 +1157,9 @@ proc rawExecute(c: PCtx, start: int, tos: PStackFrame): TFullReg =
let nc = rcReg.node
if nb.kind != nc.kind: discard
elif (nb == nc) or (nb.kind == nkNilLit): ret = true # intentional
elif nb.kind in {nkSym, nkTupleConstr, nkClosure} and nb.typ != nil and nb.typ.kind == tyProc and sameConstant(nb, nc):
elif nb.typ != nil and nb.typ.kind == tyProc and
nb.kind in {nkSym, nkTupleConstr, nkClosure} and
sameConstant(nb, nc):
ret = true
# this also takes care of procvar's, represented as nkTupleConstr, e.g. (nil, nil)
elif nb.kind == nkIntLit and nc.kind == nkIntLit and nb.intVal == nc.intVal: # TODO: nkPtrLit
Expand Down
4 changes: 2 additions & 2 deletions compiler/vmgen.nim
Original file line number Diff line number Diff line change
Expand Up @@ -461,8 +461,8 @@ proc sameConstant*(a, b: PNode): bool =
# if a.floatVal == 0.0: result = cast[uint64](a.floatVal) == cast[uint64](b.floatVal)
# else: result = a.floatVal == b.floatVal
of nkStrLit..nkTripleStrLit: result = a.strVal == b.strVal
of nkType, nkNilLit: result = a.typ == b.typ
of nkEmpty: result = true
of nkType: result = a.typ == b.typ
of nkEmpty, nkNilLit: result = true
else:
if a.len == b.len:
for i in 0..<a.len:
Expand Down
2 changes: 1 addition & 1 deletion testament/important_packages.nim
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ var packages*: seq[NimblePackage]
proc pkg(name: string; cmd = "nimble test"; url = "", useHead = true, allowFailure = false) =
packages.add NimblePackage(name: name, cmd: cmd, url: url, useHead: useHead, allowFailure: allowFailure)

pkg "alea"
pkg "alea", "nim c --debuginfo --path:. --run --define:reportConceptFailures --legacy:noEagerParamCountMatch tests/test.nim" # pending https://github.com/andreaferretti/alea/pull/9
pkg "argparse"
pkg "arraymancer", "nim c tests/tests_cpu.nim"
pkg "ast_pattern_matching", "nim c -r tests/test1.nim"
Expand Down
46 changes: 17 additions & 29 deletions tests/concepts/t3330.nim
Original file line number Diff line number Diff line change
Expand Up @@ -2,38 +2,33 @@ discard """
matrix: "--mm:refc"
errormsg: "type mismatch: got <Bar[system.int]>"
nimout: '''
t3330.nim(70, 4) Error: type mismatch: got <Bar[system.int]>
t3330.nim(58, 4) Error: type mismatch: got <Bar[system.int]>
but expected one of:
proc test(foo: Foo[int])
first type mismatch at position: 1
required type for foo: Foo[int]
but expression 'bar' is of type: Bar[system.int]
t3330.nim(55, 8) Hint: Non-matching candidates for add(k, string, T)
t3330.nim(43, 8) Hint: Non-matching candidates for add(k, string, T)
proc add(x: var string; y: char)
first type mismatch at position: 1
required type for x: var string
but expression 'k' is of type: Alias
first type mismatch at position: 3
extra argument given
proc add(x: var string; y: cstring)
first type mismatch at position: 1
required type for x: var string
but expression 'k' is of type: Alias
first type mismatch at position: 3
extra argument given
proc add(x: var string; y: string)
first type mismatch at position: 1
required type for x: var string
but expression 'k' is of type: Alias
first type mismatch at position: 3
extra argument given
proc add[T](x: var seq[T]; y: openArray[T])
first type mismatch at position: 1
required type for x: var seq[T]
but expression 'k' is of type: Alias
first type mismatch at position: 3
extra argument given
proc add[T](x: var seq[T]; y: sink T)
first type mismatch at position: 1
required type for x: var seq[T]
but expression 'k' is of type: Alias
first type mismatch at position: 3
extra argument given

t3330.nim(55, 8) template/generic instantiation of `add` from here
t3330.nim(62, 6) Foo: 'bar.value' cannot be assigned to
t3330.nim(55, 8) template/generic instantiation of `add` from here
t3330.nim(63, 6) Foo: 'bar.x' cannot be assigned to
t3330.nim(43, 8) template/generic instantiation of `add` from here
t3330.nim(50, 6) Foo: 'bar.value' cannot be assigned to
t3330.nim(43, 8) template/generic instantiation of `add` from here
t3330.nim(51, 6) Foo: 'bar.x' cannot be assigned to

expression: test(bar)'''
"""
Expand All @@ -42,14 +37,7 @@ expression: test(bar)'''










## line 60
## line 40
type
Foo[T] = concept k
add(k, string, T)
Expand Down
14 changes: 7 additions & 7 deletions tests/concepts/texplain.nim
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
discard """
cmd: "nim c --verbosity:0 --colors:off $file"
cmd: "nim c --verbosity:0 --colors:off --showAllMismatches:on $file"
nimout: '''
texplain.nim(162, 10) Hint: Non-matching candidates for e(y)
proc e(i: int): int
Expand Down Expand Up @@ -54,9 +54,8 @@ proc r(o: RegularConcept): int
texplain.nim(173, 9) template/generic instantiation of `assert` from here
texplain.nim(132, 5) RegularConcept: concept predicate failed
proc r[T](a: SomeNumber; b: T; c: auto)
first type mismatch at position: 1
required type for a: SomeNumber
but expression 'n' is of type: NonMatchingType
first type mismatch at position: 2
missing parameter: b

expression: r(n)
texplain.nim(174, 20) Hint: Non-matching candidates for r(y)
Expand All @@ -65,9 +64,8 @@ proc r(i: string): int
required type for i: string
but expression 'y' is of type: MatchingType
proc r[T](a: SomeNumber; b: T; c: auto)
first type mismatch at position: 1
required type for a: SomeNumber
but expression 'y' is of type: MatchingType
first type mismatch at position: 2
missing parameter: b

texplain.nim(182, 2) Error: type mismatch: got <MatchingType>
but expected one of:
Expand Down Expand Up @@ -118,6 +116,8 @@ expression: f(y)'''








Expand Down
16 changes: 8 additions & 8 deletions tests/errmsgs/tsigmatch.nim
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,8 @@ proc f(a: A)
first type mismatch at position: 2
extra argument given
proc f(b: B)
first type mismatch at position: 1
required type for b: B
but expression 'A()' is of type: A
first type mismatch at position: 2
extra argument given

expression: f(A(), "extra")
tsigmatch.nim(125, 6) Error: type mismatch: got <(string, proc (){.gcsafe.})>
Expand Down Expand Up @@ -43,16 +42,15 @@ expression: takesFuncs([proc (x: int) {.gcsafe.} = echo [x]])
tsigmatch.nim(149, 4) Error: type mismatch: got <int literal(10), a0: int literal(5), string>
but expected one of:
proc f(a0: uint8; b: string)
first type mismatch at position: 2
named param already provided: a0
first type mismatch at position: 3
extra argument given

expression: f(10, a0 = 5, "")
tsigmatch.nim(156, 4) Error: type mismatch: got <string, string, string, string, string, float64, string>
but expected one of:
proc f(a1: int)
first type mismatch at position: 1
required type for a1: int
but expression '"asdf"' is of type: string
first type mismatch at position: 2
extra argument given
proc f(a1: string; a2: varargs[string]; a3: float; a4: var string)
first type mismatch at position: 7
required type for a4: var string
Expand Down Expand Up @@ -97,6 +95,8 @@ see also: tests/errmsgs/tdeclaredlocs.nim





## line 100
when true:
# bug #11061 Type mismatch error "first type mismatch at" points to wrong argument/position
Expand Down
3 changes: 3 additions & 0 deletions tests/template/moverloadeduntypedparam.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
template fun2*(a: bool, body: untyped): untyped = discard
template fun2*(a: int, body: untyped): untyped = discard
template fun2*(body: untyped): untyped = discard
Loading