Skip to content

Commit

Permalink
sem: fix type inference for static parameters (#1433)
Browse files Browse the repository at this point in the history
## Summary

* fix compiler crash when passing empty aggregate values to `static`
  parameters
* fix `static T` parameter values not being of type `T`, when the
  argument is not directly of type `T` but requires a conversion
* fix converters not being considered for arguments to `static`
  parameters

## Details

When matching an expression against a formal parameter that:
* is a `static` parameter
* contains a `static` type somewhere (e.g., `int or static[float]`)

the expression was compile-time evaluated first, and on success,
assigned a `tyStatic` type. If the `tyStatic` argument type matched the
formal `tyStatic` type, the argument type was bound to the formal
`tyStatic` as-is.

In case an implicit conversion is necessary, the conversion was applied
only *after* the evaluated `tyStatic` was bound to the parameter type,
leading to:
* the actual value inlined at usages of the `static` parameter having
  the wrong type
* type inference for empty aggregate types not working

### Solution

If the formal type is a `static T`, full argument matching (including
implicit conversion and converter handling) for between the argument
and `T` is performed first, and only in case of success is the
expression compile-time evaluated, and the resulting `tyStatic` type
bound to the parameter type.

Post-match fitting has to take place prior to const evaluation (in
order to correctly type empty aggregates), so a new procedure
`tryEvalStaticArgument` is introduced that handles the static
arguments.

The new `static` handling renders the macro/template `static` special-
casing obsolete; replacing the `static` argument with the evaluation
result is now done in `evalTemplateArgs`.

### Typed AST

For calls to routines with `static` arguments, the typed argument
expression now stays in the AST as-is, instead of being replaced with
the evaluated value.
  • Loading branch information
zerbina authored Aug 27, 2024
1 parent 1295287 commit 17f96a1
Show file tree
Hide file tree
Showing 7 changed files with 106 additions and 36 deletions.
6 changes: 5 additions & 1 deletion compiler/sem/evaltempl.nim
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,11 @@ proc evalTemplateArgs*(n: PNode, s: PSym; conf: ConfigRef; fromHlo: bool): PNode
for e in walkErrors(conf, n[i]):
conf.localReport(e)

result.add n[i]
if n[i].typ != nil and n[i].typ.kind == tyStatic and n[i].typ.n != nil:
# replace static parameter arguments with the value expression
result.add n[i].typ.n
else:
result.add n[i]

# handle parameters with default values, which were
# not supplied by the user
Expand Down
28 changes: 27 additions & 1 deletion compiler/sem/sem.nim
Original file line number Diff line number Diff line change
Expand Up @@ -694,6 +694,32 @@ proc semRealConstExpr(c: PContext, n: PNode): PNode =
if result.kind != nkError:
result = evalConstExpr(c, result)

proc tryEvalStaticArgument(c: PContext, n: PNode): PNode =
## Tries to evaluate an expression passed to a static parameter, while
## overload resolution is still in progress. Returns nil if not successful.
var n = n
# `n` comes from sigmatch after a match and thus needs post-match fitting
if n.kind in {nkHiddenStdConv, nkHiddenSubConv, nkHiddenCallConv}:
# post-match fitting expects AST it can freely modify, which is guaranteed
# to be true for `n`, so copy the tree first
n = fitNodePostMatch(c, copyTree(n))
else:
n = copyNodeWithKids(n)

# prevent re-semming of the expression, which would throw away the
# types again:
n.flags.incl nfSem

let e = tryConstExpr(c, n)
if e != nil:
let typ = newTypeS(tyStatic, c)
typ.sons = @[e.typ]
typ.n = e
result =
if e == n: copyNodeWithKids(n)
else: n
result.typ = typ

when not defined(nimHasSinkInference):
{.pragma: nosinks.}

Expand Down Expand Up @@ -919,7 +945,7 @@ proc myOpen(graph: ModuleGraph; module: PSym;
c.semConstExpr = semConstExpr
c.semExpr = semExpr
c.semTryExpr = tryExpr
c.semTryConstExpr = tryConstExpr
c.tryEvalStaticArgument = tryEvalStaticArgument
c.computeRequiresInit = computeRequiresInit
c.semOperand = semOperand
c.semConstBoolExpr = semConstBoolExpr
Expand Down
4 changes: 2 additions & 2 deletions compiler/sem/semdata.nim
Original file line number Diff line number Diff line change
Expand Up @@ -604,9 +604,9 @@ type
semTryExpr*: proc (c: PContext, n: PNode, flags: TExprFlags = {}): PNode {.nimcall.}
## read to break cyclic dependencies, init in sem during module open and
## read in sigmatch
semTryConstExpr*: proc (c: PContext, n: PNode): PNode {.nimcall.}
tryEvalStaticArgument*: proc (c: PContext, n: PNode): PNode {.nimcall.}
## read to break cyclic dependencies, init in sem during module open and
## read in semcall and sigmatch
## read in sigmatch
computeRequiresInit*: proc (c: PContext, t: PType): bool {.nimcall.}
## read to break cyclic dependencies, init in sem during module open and
## read in semtypinst
Expand Down
7 changes: 7 additions & 0 deletions compiler/sem/semexprs.nim
Original file line number Diff line number Diff line change
Expand Up @@ -3772,6 +3772,13 @@ proc semExpr(c: PContext, n: PNode, flags: TExprFlags = {}): PNode =
of nkCurly: result = semSetConstr(c, n)
of nkBracket: result = semArrayConstr(c, n, flags)
of nkObjConstr: result = semObjConstr(c, n, flags)
of nkClosure:
# only possible when constants / static parameters were inlined
checkSonsLen(n, 2, c.config)
# closures that capture something should not be able to reach here
internalAssert(c.config, n[1].kind == nkNilLit, n.info)
# make sure the result is correctly typed (i.e., with a closure type)
result = fitNode(c, n.typ, semExpr(c, n[0], flags), n.info)
of nkLambdaKinds:
result = semProcAnnotation(c, n)
if result == nil:
Expand Down
64 changes: 32 additions & 32 deletions compiler/sem/sigmatch.nim
Original file line number Diff line number Diff line change
Expand Up @@ -2272,24 +2272,45 @@ proc paramTypesMatchAux(m: var TCandidate, f, a: PType,
tfGenericTypeParam notin a.flags:
result = newNodeIT(nkType, arg.info, makeTypeFromExpr(c, arg))
return
else:
var evaluated = c.semTryConstExpr(c, arg)
elif f.kind != tyStatic or f.base.kind == tyNone:
# try to evaluate the expression up-front
let evaluated = c.tryEvalStaticArgument(c, arg)
if evaluated != nil:
# Don't build the type in-place because `evaluated` and `arg` may point
# to the same object and we'd end up creating recursive types (#9255)
let typ = newTypeS(tyStatic, c)
typ.sons = @[evaluated.typ]
typ.n = evaluated
arg = copyTree(arg) # fix #12864
arg.typ = typ
a = typ
arg = evaluated
a = arg.typ
else:
if m.callee.kind == tyGenericBody:
if f.kind == tyStatic and typeRel(m, f.base, a) != isNone:
result = makeStaticExpr(m.c, arg)
result.typ.flags.incl tfUnresolved
result.typ.n = arg
return
else:
# for proper conversion handling, the inner type must be matched against
# first
var callee: PSym = nil
# HACK: macros and templates use special parameter matching behaviour
# that disables implicit conversions. To get around that, the
# calleeSym is temporary set to nil
swap(callee, m.calleeSym)
result = paramTypesMatchAux(m, f.base, a, argSemantized)
swap(callee, m.calleeSym)

# evaluate the expression *after* implicit conversions were introduced
if result != nil:
result = c.tryEvalStaticArgument(c, result)
if result != nil:
assert result.typ.kind == tyStatic
# XXX: the below partially duplicates the tyStatic handling from
# typeRel
let prev = PType(idTableGet(m.bindings, f))
if prev != nil:
if not exprStructuralEquivalent(prev.n, result.typ.n):
result = nil # no match
else:
put(m, f, result.typ)

return

let oldInheritancePenalty = m.inheritancePenalty
var r = typeRel(m, f, a)
Expand All @@ -2308,29 +2329,8 @@ proc paramTypesMatchAux(m: var TCandidate, f, a: PType,
incMatches(m, r)
result =
case f.kind
of tyTyped, tyTypeDesc:
of tyTyped, tyTypeDesc, tyStatic:
arg
of tyStatic:
let n =
if arg.typ.n.isNil: # no value on the type
argSemantized
else: # value on the type
arg.typ.n

# XXX: the implicit conversion handling is duplicated from the non-
# template/non-macro path. Template and macro arguments shouldn't
# be special-cased like this
case r
of isEqual: n
of isGeneric:
if n.typ.isEmptyContainer:
implicitConv(nkHiddenStdConv, f[0], n, m, c)
else:
n
of isSubtype:
implicitConv(nkHiddenSubConv, f[0], n, m, c)
else:
implicitConv(nkHiddenStdConv, f[0], n, m, c)
else:
argSemantized
return
Expand Down
12 changes: 12 additions & 0 deletions tests/lang_callable/converter/tconverter_for_static_param.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
discard """
description: '''
Ensure that converters are considered for arguments to static parameters
"
"""

converter toInt(x: float): int = int(x)

proc test(x: static int) =
doAssert x == 1

test(1.5) # wouldn't work without a converter
21 changes: 21 additions & 0 deletions tests/lang_exprs/tempty_typed_expressions.nim
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,24 @@ doAssert get2((discard; [])) == 0
doAssert get2((discard; @[])) == 0

discard get3((discard; @[]))

# -----------------
# static parameters

proc get_static(x: static pointer): pointer = x
proc get_static(x: static array[0, int]): array[0, int] = x
proc get_static(x: static seq[int]): seq[int] = x
proc get_static(x: static set[char]): set[char] = x

# simple case: empty-container typed expression is passed directly
discard get_static(nil)
discard get_static([])
discard get_static(@[])
discard get_static({})

# more complex case: statement-list expressions
discard get_static((discard; nil))
# XXX: not working yet
#discard get_static((discard; []))
discard get_static((discard; @[]))
discard get_static((discard; {}))

0 comments on commit 17f96a1

Please sign in to comment.