From 94f7e9683fb5c9f643b7e4af367a3a6457d5ad7f Mon Sep 17 00:00:00 2001 From: Ryan McConnell Date: Fri, 15 Dec 2023 06:48:34 +0000 Subject: [PATCH] Param match relax (#23033) #23032 --------- Co-authored-by: Nikolay Nikolov Co-authored-by: Pylgos <43234674+Pylgos@users.noreply.github.com> Co-authored-by: Andreas Rumpf Co-authored-by: ringabout <43030857+ringabout@users.noreply.github.com> Co-authored-by: Jason Beetham --- compiler/ast.nim | 6 +- compiler/lookups.nim | 12 ++ compiler/sigmatch.nim | 137 +++++++++++++---------- lib/pure/sugar.nim | 5 +- testament/important_packages.nim | 2 +- tests/lookups/issue_23032/deep_scope.nim | 2 + tests/lookups/t23032.nim | 13 +++ tests/macros/t23032_1.nim | 19 ++++ tests/macros/t23032_2.nim | 20 ++++ tests/macros/tgetimpl.nim | 7 +- 10 files changed, 154 insertions(+), 69 deletions(-) create mode 100644 tests/lookups/issue_23032/deep_scope.nim create mode 100644 tests/lookups/t23032.nim create mode 100644 tests/macros/t23032_1.nim create mode 100644 tests/macros/t23032_2.nim diff --git a/compiler/ast.nim b/compiler/ast.nim index c880cb6517..aa12c6421f 100644 --- a/compiler/ast.nim +++ b/compiler/ast.nim @@ -2033,13 +2033,15 @@ proc skipGenericOwner*(s: PSym): PSym = ## Generic instantiations are owned by their originating generic ## symbol. This proc skips such owners and goes straight to the owner ## of the generic itself (the module or the enclosing proc). - result = if s.kind in skProcKinds and sfFromGeneric in s.flags and s.owner.kind != skModule: + result = if s.kind == skModule: + s + elif s.kind in skProcKinds and sfFromGeneric in s.flags and s.owner.kind != skModule: s.owner.owner else: s.owner proc originatingModule*(s: PSym): PSym = - result = s.owner + result = s while result.kind != skModule: result = result.owner proc isRoutine*(s: PSym): bool {.inline.} = diff --git a/compiler/lookups.nim b/compiler/lookups.nim index 1a60de7e53..cfefe764ba 100644 --- a/compiler/lookups.nim +++ b/compiler/lookups.nim @@ -256,6 +256,18 @@ proc searchInScopesFilterBy*(c: PContext, s: PIdent, filter: TSymKinds): seq[PSy if s.kind in filter: result.add s +proc cmpScopes*(ctx: PContext, s: PSym): int = + # Do not return a negative number + if s.originatingModule == ctx.module: + result = 2 + var owner = s + while true: + owner = owner.skipGenericOwner + if owner.kind == skModule: break + inc result + else: + result = 1 + proc isAmbiguous*(c: PContext, s: PIdent, filter: TSymKinds, sym: var PSym): bool = result = false block outer: diff --git a/compiler/sigmatch.nim b/compiler/sigmatch.nim index 8e4dba0904..ceb3f5a516 100644 --- a/compiler/sigmatch.nim +++ b/compiler/sigmatch.nim @@ -135,15 +135,7 @@ proc initCandidate*(ctx: PContext, callee: PSym, result = initCandidateAux(ctx, callee.typ) result.calleeSym = callee if callee.kind in skProcKinds and calleeScope == -1: - if callee.originatingModule == ctx.module: - result.calleeScope = 2 - var owner = callee - while true: - owner = owner.skipGenericOwner - if owner.kind == skModule: break - inc result.calleeScope - else: - result.calleeScope = 1 + result.calleeScope = cmpScopes(ctx, callee) else: result.calleeScope = calleeScope result.diagnostics = @[] # if diagnosticsEnabled: @[] else: nil @@ -297,7 +289,7 @@ proc writeMatches*(c: TCandidate) = echo " conv matches: ", c.convMatches echo " inheritance: ", c.inheritancePenalty -proc cmpCandidates*(a, b: TCandidate): int = +proc cmpCandidates*(a, b: TCandidate, isFormal=true): int = result = a.exactMatches - b.exactMatches if result != 0: return result = a.genericMatches - b.genericMatches @@ -311,13 +303,14 @@ proc cmpCandidates*(a, b: TCandidate): int = # the other way round because of other semantics: result = b.inheritancePenalty - a.inheritancePenalty if result != 0: return - # check for generic subclass relation - result = checkGeneric(a, b) + if isFormal: + # check for generic subclass relation + result = checkGeneric(a, b) + if result != 0: return + # prefer more specialized generic over more general generic: + result = complexDisambiguation(a.callee, b.callee) if result != 0: return - # prefer more specialized generic over more general generic: - result = complexDisambiguation(a.callee, b.callee) # only as a last resort, consider scoping: - if result != 0: return result = a.calleeScope - b.calleeScope proc argTypeToString(arg: PNode; prefer: TPreferedDesc): string = @@ -2353,56 +2346,76 @@ proc paramTypesMatch*(m: var TCandidate, f, a: PType, if arg == nil or arg.kind notin nkSymChoices: result = paramTypesMatchAux(m, f, a, arg, argOrig) else: - # CAUTION: The order depends on the used hashing scheme. Thus it is - # incorrect to simply use the first fitting match. However, to implement - # this correctly is inefficient. We have to copy `m` here to be able to - # roll back the side effects of the unification algorithm. - let c = m.c - var - x = newCandidate(c, m.callee) - y = newCandidate(c, m.callee) - z = newCandidate(c, m.callee) - x.calleeSym = m.calleeSym - y.calleeSym = m.calleeSym - z.calleeSym = m.calleeSym + let matchSet = {skProc, skFunc, skMethod, skConverter,skIterator, skMacro, + skTemplate, skEnumField} + var best = -1 - for i in 0.. bestScope: 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 - - if x.state == csEmpty: - result = nil - elif y.state == csMatch and cmpCandidates(x, y) == 0: - if x.state != csMatch: - internalError(m.c.graph.config, arg.info, "x.state is not csMatch") - # ambiguous: more than one symbol fits! - # See tsymchoice_for_expr as an example. 'f.kind == tyUntyped' should match - # anyway: - if f.kind in {tyUntyped, tyTyped}: result = arg - else: result = nil + bestScope = thisScope + counts = 0 + elif thisScope == bestScope: + inc counts + if best == -1: + result = nil + elif counts > 0: + best = -1 else: + # CAUTION: The order depends on the used hashing scheme. Thus it is + # incorrect to simply use the first fitting match. However, to implement + # this correctly is inefficient. We have to copy `m` here to be able to + # roll back the side effects of the unification algorithm. + let c = m.c + var + x = newCandidate(c, m.callee) + y = newCandidate(c, m.callee) + z = newCandidate(c, m.callee) + x.calleeSym = m.calleeSym + y.calleeSym = m.calleeSym + z.calleeSym = m.calleeSym + + for i in 0.. -1 and result != nil: # only one valid interpretation found: markUsed(m.c, arg.info, arg[best].sym) onUse(arg.info, arg[best].sym) diff --git a/lib/pure/sugar.nim b/lib/pure/sugar.nim index cfa04a8376..7833ed0633 100644 --- a/lib/pure/sugar.nim +++ b/lib/pure/sugar.nim @@ -345,9 +345,10 @@ proc collectImpl(init, body: NimNode): NimNode {.since: (1, 1).} = let res = genSym(nskVar, "collectResult") var bracketExpr: NimNode if init != nil: - expectKind init, {nnkCall, nnkIdent, nnkSym} + expectKind init, {nnkCall, nnkIdent, nnkSym, nnkClosedSymChoice, nnkOpenSymChoice} bracketExpr = newTree(nnkBracketExpr, - if init.kind == nnkCall: freshIdentNodes(init[0]) else: freshIdentNodes(init)) + if init.kind in {nnkCall, nnkClosedSymChoice, nnkOpenSymChoice}: + freshIdentNodes(init[0]) else: freshIdentNodes(init)) else: bracketExpr = newTree(nnkBracketExpr) let (resBody, keyType, valueType) = trans(body, res, bracketExpr) diff --git a/testament/important_packages.nim b/testament/important_packages.nim index c2b2476e70..d3d3f06437 100644 --- a/testament/important_packages.nim +++ b/testament/important_packages.nim @@ -60,7 +60,7 @@ pkg "compactdict" pkg "comprehension", "nimble test", "https://github.com/alehander92/comprehension" pkg "cowstrings" pkg "criterion", allowFailure = true # needs testing binary -pkg "datamancer" +pkg "datamancer", url="https://github.com/Graveflo/Datamancer.git" pkg "dashing", "nim c tests/functional.nim" pkg "delaunay" pkg "docopt" diff --git a/tests/lookups/issue_23032/deep_scope.nim b/tests/lookups/issue_23032/deep_scope.nim new file mode 100644 index 0000000000..3e25809a70 --- /dev/null +++ b/tests/lookups/issue_23032/deep_scope.nim @@ -0,0 +1,2 @@ +type A*[T] = object +proc foo*(a: A[int]): bool = false diff --git a/tests/lookups/t23032.nim b/tests/lookups/t23032.nim new file mode 100644 index 0000000000..144abcb059 --- /dev/null +++ b/tests/lookups/t23032.nim @@ -0,0 +1,13 @@ +discard """ +action: "run" +outputsub: "proc (a: A[system.float]): bool{.noSideEffect, gcsafe.}" +""" + +import issue_23032/deep_scope + +proc foo(a: A[float]):bool = true + +let p: proc = foo +echo p.typeof +doAssert p(A[float]()) == true +doAssert compiles(doAssert p(A[int]()) == true) == false diff --git a/tests/macros/t23032_1.nim b/tests/macros/t23032_1.nim new file mode 100644 index 0000000000..4e1707414a --- /dev/null +++ b/tests/macros/t23032_1.nim @@ -0,0 +1,19 @@ +import std/macros + +type A[T, H] = object + +proc `%*`(a: A): bool = true +proc `%*`[T](a: A[int, T]): bool = false + +macro collapse(s: untyped) = + result = newStmtList() + result.add quote do: + doAssert(`s`(A[float, int]()) == true) + +macro startHere(n: untyped): untyped = + result = newStmtList() + let s = n[0] + result.add quote do: + `s`.collapse() + +startHere(`a` %* `b`) diff --git a/tests/macros/t23032_2.nim b/tests/macros/t23032_2.nim new file mode 100644 index 0000000000..cb8e772e70 --- /dev/null +++ b/tests/macros/t23032_2.nim @@ -0,0 +1,20 @@ +discard """ + action: "reject" + errormsg: "ambiguous identifier '%*'" +""" +import std/macros + +type A[T, H] = object + +proc `%*`[T](a: A) = discard +proc `%*`[T](a: A[int, T]) = discard + +macro collapse(s: typed) = discard + +macro startHere(n: untyped): untyped = + result = newStmtList() + let s = n[0] + result.add quote do: + collapse(`s`.typeof()) + +startHere(`a` %* `b`) diff --git a/tests/macros/tgetimpl.nim b/tests/macros/tgetimpl.nim index 66722a2344..3989576729 100644 --- a/tests/macros/tgetimpl.nim +++ b/tests/macros/tgetimpl.nim @@ -44,8 +44,11 @@ static: doAssert checkOwner(poo, 2) == "nskProc" doAssert checkOwner(poo, 3) == "nskModule" doAssert isSameOwner(foo, poo) - doAssert isSameOwner(foo, echo) == false - doAssert isSameOwner(poo, len) == false + proc wrappedScope() = + proc dummyproc() = discard + doAssert isSameOwner(foo, dummyproc) == false + doAssert isSameOwner(poo, dummyproc) == false + wrappedScope() #---------------------------------------------------------------