Skip to content

Commit

Permalink
don't use previous bindings of auto for routine return types (#23207)
Browse files Browse the repository at this point in the history
fixes #23200, fixes #18866

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.

(cherry picked from commit f46f26e)
  • Loading branch information
metagn authored and narimiran committed Apr 27, 2024
1 parent 9e38e75 commit 4e5680b
Show file tree
Hide file tree
Showing 10 changed files with 91 additions and 53 deletions.
3 changes: 2 additions & 1 deletion compiler/semtypes.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1422,7 +1422,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, nextTypeId c.idgen, r.owner)
r.flags.incl tfRetType
elif r.kind == tyStatic:
# type allowed should forbid this type
discard
Expand Down
7 changes: 6 additions & 1 deletion compiler/semtypinst.nim
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,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
Expand Down Expand Up @@ -558,7 +561,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

Expand Down
82 changes: 41 additions & 41 deletions compiler/sigmatch.nim
Original file line number Diff line number Diff line change
Expand Up @@ -2129,6 +2129,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
Expand Down Expand Up @@ -2192,28 +2193,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:
Expand All @@ -2240,23 +2246,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:
Expand All @@ -2270,8 +2268,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:
Expand Down
2 changes: 1 addition & 1 deletion tests/concepts/tconcepts_issues.nim
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,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:
Expand Down
2 changes: 1 addition & 1 deletion tests/errmsgs/t16654.nim
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
discard """
cmd: "nim check $options $file"
errormsg: "type mismatch: got <int> but expected 'float'"
errormsg: "type mismatch: got <int literal(1), proc (r: GenericParam): auto>"
"""

when true: # bug #16654
Expand Down
2 changes: 1 addition & 1 deletion tests/misc/t12869.nim
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
discard """
errormsg: "type mismatch: got <bool> but expected 'int'"
errormsg: "type mismatch: got <openArray[int], proc (x: GenericParam, y: GenericParam): auto, SortOrder>"
line: 12
"""

Expand Down
36 changes: 36 additions & 0 deletions tests/proc/tinferlambdareturn.nim
Original file line number Diff line number Diff line change
@@ -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)
3 changes: 2 additions & 1 deletion tests/stdlib/tsugar.nim
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,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()
Expand Down
2 changes: 1 addition & 1 deletion tests/types/t15836.nim
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
discard """
errormsg: "type mismatch: got <string> but expected 'int'"
errormsg: "type mismatch: got <int literal(1), proc (a: GenericParam): auto>"
line: 11
"""

Expand Down
5 changes: 0 additions & 5 deletions tests/types/t15836_2.nim
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@

discard """
action: "compile"
disabled: true
"""
import std/sugar

type Tensor[T] = object
Expand Down

0 comments on commit 4e5680b

Please sign in to comment.