From f46f26e79aada7195f4d83d8601f0d856520763d Mon Sep 17 00:00:00 2001 From: metagn Date: Wed, 17 Jan 2024 13:59:54 +0300 Subject: [PATCH] don't use previous bindings of `auto` for routine return types (#23207) fixes #23200, fixes #18866 #21065 made it so `auto` proc return types remained as `tyAnything` and not turned to `tyUntyped`. This had the side effect that anything previously bound to `tyAnything` in the proc type match was then bound to the proc return type, which is wrong since we don't know the proc return type even if we know the expected parameter types (`tyUntyped` also [does not care about its previous bindings in `typeRel`](https://github.com/nim-lang/Nim/blob/ab4278d2179639f19967431a7aa1be858046f7a7/compiler/sigmatch.nim#L1059-L1061) maybe for this reason). Now we mark `tyAnything` return types for routines as `tfRetType` [as done for other meta return types](https://github.com/nim-lang/Nim/blob/18b5fb256d4647efa6a64df451d37129d36e96f3/compiler/semtypes.nim#L1451), and ignore bindings to `tyAnything` + `tfRetType` types in `semtypinst`. On top of this, we reset the type relation in `paramTypesMatch` only after creating the instantiation (instead of trusting `isInferred`/`isInferredConvertible` before creating the instantiation), using the same mechanism that `isBothMetaConvertible` uses. This fixes the issues as well as making the disabled t15386_2 test introduced in #21065 work. As seen in the changes for the other tests, the error messages give an obscure `proc (a: GenericParam): auto` now, but it does give the correct error that the overload doesn't match instead of matching the overload pre-emptively and expecting a specific return type. tsugar had to be changed due to #16906, which is the problem where `void` is not inferred in the case where `result` was never touched. --- compiler/semtypes.nim | 3 +- compiler/semtypinst.nim | 7 ++- compiler/sigmatch.nim | 82 ++++++++++++++--------------- tests/concepts/tconcepts_issues.nim | 2 +- tests/errmsgs/t16654.nim | 2 +- tests/misc/t12869.nim | 2 +- tests/proc/tinferlambdareturn.nim | 36 +++++++++++++ tests/stdlib/tsugar.nim | 3 +- tests/types/t15836.nim | 2 +- tests/types/t15836_2.nim | 5 -- 10 files changed, 91 insertions(+), 53 deletions(-) create mode 100644 tests/proc/tinferlambdareturn.nim diff --git a/compiler/semtypes.nim b/compiler/semtypes.nim index fe87c29731..1f671effa1 100644 --- a/compiler/semtypes.nim +++ b/compiler/semtypes.nim @@ -1436,7 +1436,8 @@ proc semProcTypeNode(c: PContext, n, genericParams: PNode, "' is only valid for macros and templates") # 'auto' as a return type does not imply a generic: elif r.kind == tyAnything: - discard + r = copyType(r, c.idgen, r.owner) + r.flags.incl tfRetType elif r.kind == tyStatic: # type allowed should forbid this type discard diff --git a/compiler/semtypinst.nim b/compiler/semtypinst.nim index 1510a3f9c5..d1327a3aee 100644 --- a/compiler/semtypinst.nim +++ b/compiler/semtypinst.nim @@ -315,6 +315,9 @@ proc replaceTypeVarsS(cl: var TReplTypeVars, s: PSym, t: PType): PSym = result.ast = replaceTypeVarsN(cl, s.ast) proc lookupTypeVar(cl: var TReplTypeVars, t: PType): PType = + if tfRetType in t.flags and t.kind == tyAnything: + # don't bind `auto` return type to a previous binding of `auto` + return nil result = cl.typeMap.lookup(t) if result == nil: if cl.allowMetaTypes or tfRetType in t.flags: return @@ -551,7 +554,9 @@ proc replaceTypeVarsTAux(cl: var TReplTypeVars, t: PType): PType = result = t if t == nil: return - if t.kind in {tyStatic, tyGenericParam, tyConcept} + tyTypeClasses: + const lookupMetas = {tyStatic, tyGenericParam, tyConcept} + tyTypeClasses - {tyAnything} + if t.kind in lookupMetas or + (t.kind == tyAnything and tfRetType notin t.flags): let lookup = cl.typeMap.lookup(t) if lookup != nil: return lookup diff --git a/compiler/sigmatch.nim b/compiler/sigmatch.nim index 557d4c4477..9e99353d10 100644 --- a/compiler/sigmatch.nim +++ b/compiler/sigmatch.nim @@ -2120,6 +2120,7 @@ template matchesVoidProc(t: PType): bool = proc paramTypesMatchAux(m: var TCandidate, f, a: PType, argSemantized, argOrig: PNode): PNode = + result = nil var fMaybeStatic = f.skipTypes({tyDistinct}) arg = argSemantized @@ -2182,28 +2183,33 @@ proc paramTypesMatchAux(m: var TCandidate, f, a: PType, else: return argSemantized # argOrig - # If r == isBothMetaConvertible then we rerun typeRel. - # bothMetaCounter is for safety to avoid any infinite loop, - # I don't have any example when it is needed. - # lastBindingsLenth is used to check whether m.bindings remains the same, - # because in that case there is no point in continuing. - var bothMetaCounter = 0 - var lastBindingsLength = -1 - while r == isBothMetaConvertible and - lastBindingsLength != m.bindings.counter and - bothMetaCounter < 100: - lastBindingsLength = m.bindings.counter - inc(bothMetaCounter) - if arg.kind in {nkProcDef, nkFuncDef, nkIteratorDef} + nkLambdaKinds: - result = c.semInferredLambda(c, m.bindings, arg) - elif arg.kind != nkSym: - return nil - else: - let inferred = c.semGenerateInstance(c, arg.sym, m.bindings, arg.info) - result = newSymNode(inferred, arg.info) - inc(m.convMatches) - arg = result - r = typeRel(m, f, arg.typ) + block instantiateGenericRoutine: + # In the case where the matched value is a generic proc, we need to + # fully instantiate it and then rerun typeRel to make sure it matches. + # instantiationCounter is for safety to avoid any infinite loop, + # I don't have any example when it is needed. + # lastBindingCount is used to check whether m.bindings remains the same, + # because in that case there is no point in continuing. + var instantiationCounter = 0 + var lastBindingCount = -1 + while r in {isBothMetaConvertible, isInferred, isInferredConvertible} and + lastBindingCount != m.bindings.counter and + instantiationCounter < 100: + lastBindingCount = m.bindings.counter + inc(instantiationCounter) + if arg.kind in {nkProcDef, nkFuncDef, nkIteratorDef} + nkLambdaKinds: + result = c.semInferredLambda(c, m.bindings, arg) + elif arg.kind != nkSym: + return nil + elif arg.sym.kind in {skMacro, skTemplate}: + return nil + else: + if arg.sym.ast == nil: + return nil + let inferred = c.semGenerateInstance(c, arg.sym, m.bindings, arg.info) + result = newSymNode(inferred, arg.info) + arg = result + r = typeRel(m, f, arg.typ) case r of isConvertible: @@ -2230,23 +2236,15 @@ proc paramTypesMatchAux(m: var TCandidate, f, a: PType, result = arg else: result = implicitConv(nkHiddenStdConv, f, arg, m, c) - of isInferred, isInferredConvertible: - if arg.kind in {nkProcDef, nkFuncDef, nkIteratorDef} + nkLambdaKinds: - result = c.semInferredLambda(c, m.bindings, arg) - elif arg.kind != nkSym: - return nil - elif arg.sym.kind in {skMacro, skTemplate}: - return nil - else: - if arg.sym.ast == nil: - return nil - let inferred = c.semGenerateInstance(c, arg.sym, m.bindings, arg.info) - result = newSymNode(inferred, arg.info) - if r == isInferredConvertible: - inc(m.convMatches) - result = implicitConv(nkHiddenStdConv, f, result, m, c) - else: - inc(m.genericMatches) + of isInferred: + # result should be set in above while loop: + assert result != nil + inc(m.genericMatches) + of isInferredConvertible: + # result should be set in above while loop: + assert result != nil + inc(m.convMatches) + result = implicitConv(nkHiddenStdConv, f, result, m, c) of isGeneric: inc(m.genericMatches) if arg.typ == nil: @@ -2260,8 +2258,10 @@ proc paramTypesMatchAux(m: var TCandidate, f, a: PType, else: result = arg of isBothMetaConvertible: - # This is the result for the 101th time. - result = nil + # result should be set in above while loop: + assert result != nil + inc(m.convMatches) + result = arg of isFromIntLit: # too lazy to introduce another ``*matches`` field, so we conflate # ``isIntConv`` and ``isIntLit`` here: diff --git a/tests/concepts/tconcepts_issues.nim b/tests/concepts/tconcepts_issues.nim index 1d5e415dd2..d6c8674fd8 100644 --- a/tests/concepts/tconcepts_issues.nim +++ b/tests/concepts/tconcepts_issues.nim @@ -510,7 +510,7 @@ proc depthOf*[V](orderType: typedesc[BreadthOrder], tree: AnyTree[V], root, goal if root == goal: return 0 var order = init[LevelNode[V]](orderType) - order.expand(tree, root, (leaf) => (1, leaf)) + order.expand(tree, root, (leaf) => (1.uint, leaf)) while order.hasNext(): let depthNode: LevelNode[V] = order.popNext() if depthNode.node == goal: diff --git a/tests/errmsgs/t16654.nim b/tests/errmsgs/t16654.nim index 749707c069..b2b57619bd 100644 --- a/tests/errmsgs/t16654.nim +++ b/tests/errmsgs/t16654.nim @@ -1,6 +1,6 @@ discard """ cmd: "nim check $options $file" - errormsg: "type mismatch: got but expected 'float'" + errormsg: "type mismatch: got " """ when true: # bug #16654 diff --git a/tests/misc/t12869.nim b/tests/misc/t12869.nim index 731a4e95ec..054e28a038 100644 --- a/tests/misc/t12869.nim +++ b/tests/misc/t12869.nim @@ -1,5 +1,5 @@ discard """ - errormsg: "type mismatch: got but expected 'int'" + errormsg: "type mismatch: got " line: 12 """ diff --git a/tests/proc/tinferlambdareturn.nim b/tests/proc/tinferlambdareturn.nim new file mode 100644 index 0000000000..e9e5928719 --- /dev/null +++ b/tests/proc/tinferlambdareturn.nim @@ -0,0 +1,36 @@ +import std/[sugar, sequtils] + +block: # issue #23200 + proc dosomething(iter: int -> (iterator: int)) = + discard + proc dosomething(iter: int -> seq[int]) = + discard + proc makeSeq(x: int): seq[int] = + @[x] + # Works fine with 1.6.12 and 1.6.14 + dosomething(makeSeq) + # Works with 1.6.12, fails with 1.6.14 + dosomething((y) => makeSeq(y)) + dosomething(proc (y: auto): auto = makeSeq(y)) + proc foo(y: auto): auto = makeSeq(y) + dosomething(foo) + +block: # issue #18866 + proc somefn[T](list: openarray[T], op: proc (v: T): float) = + discard op(list[0]) + + type TimeD = object + year: Natural + month: 1..12 + day: 1..31 + + doAssert not compiles(@[TimeD()].somefn(proc (v: auto): auto = + v + )) + @[TimeD()].somefn(proc (v: auto): auto = + v.year.float + ) + proc foo(v: auto): auto = v + doAssert not compiles(@[TimeD()].somefn(foo)) + proc bar(v: auto): auto = v.year.float + @[TimeD()].somefn(bar) diff --git a/tests/stdlib/tsugar.nim b/tests/stdlib/tsugar.nim index b9cbdd3e37..2ea96cfbbf 100644 --- a/tests/stdlib/tsugar.nim +++ b/tests/stdlib/tsugar.nim @@ -295,7 +295,8 @@ template main() = for i in 0..5: xs.add(i) - xs.apply(d => ys.add(d)) + xs.apply(proc (d: auto) = ys.add(d)) + # ^ can be turned into d => ys.add(d) when we can infer void return type, #16906 doAssert ys == @[0, 1, 2, 3, 4, 5] test() diff --git a/tests/types/t15836.nim b/tests/types/t15836.nim index 9c0c26decf..27d3ad0d0d 100644 --- a/tests/types/t15836.nim +++ b/tests/types/t15836.nim @@ -1,5 +1,5 @@ discard """ - errormsg: "type mismatch: got but expected 'int'" + errormsg: "type mismatch: got " line: 11 """ diff --git a/tests/types/t15836_2.nim b/tests/types/t15836_2.nim index 9afef416a8..6a16e2d222 100644 --- a/tests/types/t15836_2.nim +++ b/tests/types/t15836_2.nim @@ -1,8 +1,3 @@ - -discard """ - action: "compile" - disabled: true -""" import std/sugar type Tensor[T] = object