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

sem: re-sem all call arguments in macro output #1192

Draft
wants to merge 20 commits into
base: devel
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
f58ce0a
sigmatch: re-sem operands without the `nfSem` flag
zerbina Feb 16, 2024
55862cd
debugging: display node flags in tracing output
zerbina Feb 16, 2024
a1ba6d1
semcall: restore borrow target search
zerbina Feb 16, 2024
0bd9a29
semexprs: better bracket rewrite reversion
zerbina Feb 16, 2024
9f60702
semexprs: fix `asBracketExpr` edge case
zerbina Feb 16, 2024
8c15fd5
semexprs: prevent re-sem of hidden derefs
zerbina Feb 16, 2024
19fc011
sigmatch: prevent re-typing of hidden nodes
zerbina Feb 16, 2024
9b1ec67
semexprs: make set constructor analysis roundtrip
zerbina Feb 16, 2024
fe48c71
semexprs: support retyping for `varargs` arguments
zerbina Feb 16, 2024
2d32c66
semexprs: support re-sem'ing of literal AST
zerbina Feb 16, 2024
1d349f2
semexprs: don't perform unnecessary tree copy
zerbina Feb 16, 2024
b528897
sigmatch: mark created `tyStatic` with `tfHasStatic`
zerbina Feb 16, 2024
fe4733f
sigmatch: don't update empty container types in-place
zerbina Feb 16, 2024
cecae1c
sigmatch: prevent `static` arguments from losing their type
zerbina Feb 16, 2024
00ecc17
semexprs: don't ignore hidden nodes
zerbina Feb 16, 2024
fb55c41
semexprs: work around sequence constructions
zerbina Feb 16, 2024
fc21905
sigmatch: don't re-sem default arguments
zerbina Feb 16, 2024
155ba45
sigmatch: mark constant-evaluated expression with `nfSem`
zerbina Feb 16, 2024
2fa828b
semcall: don't re-sem default values (2)
zerbina Feb 16, 2024
3cbc842
Merge branch 'devel' into sem-resem-operands
zerbina Aug 8, 2024
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
7 changes: 1 addition & 6 deletions compiler/mir/proto_mir.nim
Original file line number Diff line number Diff line change
Expand Up @@ -800,12 +800,7 @@ proc exprToPmir(c: TranslateCtx, result: var seq[ProtoItem], n: PNode, sink: boo
recurse(selectWhenBranch(n, c.pickVm), sink)
of nkStmtListExpr:
recurse(n.lastSon, sink)
# HACK: inherit the type from the child node. This prevents incorrectly
# typed array constructions (a sem bug) from appearing to work
# correctly (see
# tests/lang_exprs/tempty_typed_expressions_issues.nim)
result.add ProtoItem(orig: n, typ: result[^1].typ, kind: pirStmtList)
#node pirStmtList
node pirStmtList
of nkPragmaBlock:
# the pragma is uninteresting here, just skip it
recurse(n.lastSon, sink)
Expand Down
11 changes: 9 additions & 2 deletions compiler/sem/semcall.nim
Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,10 @@ proc semResolvedCall(c: PContext, x: TCandidate,
instGenericConvertersSons(c, result, x)
result[0] = newSymNode(finalCallee, getCallLineInfo(result[0]))
result.typ = finalCallee.typ[0]
result = updateDefaultParams(c.config, result)
if gp.isGenericParams:
# default parameters only need to be updated for instantiated generic
# routines. For normal routines they're already correct
result = updateDefaultParams(c.config, result)

proc semOverloadedCall(c: PContext, n, nOrig: PNode,
filter: TSymKinds, flags: TExprFlags): PNode {.nosinks.} =
Expand Down Expand Up @@ -748,7 +751,11 @@ proc searchForBorrowProc(c: PContext, startScope: PScope, fn: PSym): PSym =
x.addSonSkipIntLit(t.baseOfDistinct(c.graph, c.idgen), c.idgen)
else:
x = t.baseOfDistinct(c.graph, c.idgen)
call.add(newNodeIT(nkEmpty, fn.info, x))
let arg = newNodeIT(nkEmpty, fn.info, x)
# mark the node as analysed so that operand analysis doesn't analyze it
# again
arg.flags.incl nfSem
call.add(arg)
if hasDistinct:
let filter = if fn.kind in {skProc, skFunc}: {skProc, skFunc} else: {fn.kind}
var resolved = semOverloadedCall(c, call, call, filter, {})
Expand Down
101 changes: 77 additions & 24 deletions compiler/sem/semexprs.nim
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ proc semOperand(c: PContext, n: PNode, flags: TExprFlags = {}): PNode =
result = c.config.newError(n, PAstDiag(kind: adSemProcHasNoConcreteType))
elif result.typ.kind in {tyVar, tyLent}:
result = newDeref(result)
result.flags.incl nfSem
elif {efWantStmt, efAllowStmt} * flags != {}:
result.typ = newTypeS(tyVoid, c)
else:
Expand Down Expand Up @@ -93,6 +94,7 @@ proc semExprWithType(c: PContext, n: PNode, flags: TExprFlags = {}): PNode =

elif result.typ.kind in {tyVar, tyLent}:
result = newDeref(result)
result.flags.incl nfSem

proc semExprNoDeref(c: PContext, n: PNode, flags: TExprFlags = {}): PNode =
result = semExprCheck(c, n, flags)
Expand Down Expand Up @@ -1840,7 +1842,8 @@ proc dotTransformation(c: PContext, n: PNode): PNode =
PAstDiag(kind: adSemExpectedIdentifierWithExprContext,
expr: n))

result.add copyTree(n[0])
# the operand was already sem'ed immediately prior. No need to copy it
result.add n[0]

if result[0].kind == nkError: # handle the potential ident error
result = c.config.wrapError(result)
Expand Down Expand Up @@ -2807,6 +2810,8 @@ proc setMs(n: PNode, s: PSym): PNode =
n[0] = newSymNode(s)
n[0].info = n.info

proc asBracketExpr(c: PContext; n: PNode): PNode

proc semMagic(c: PContext, n: PNode, s: PSym, flags: TExprFlags): PNode =
# this is a hotspot in the compiler!
result = n
Expand Down Expand Up @@ -2891,6 +2896,14 @@ proc semMagic(c: PContext, n: PNode, s: PSym, flags: TExprFlags): PNode =
of mSizeOf:
markUsed(c, n.info, s)
result = semSizeOf(c, setMs(n, s))
of mArrGet:
# this could be a rewritten subscript operation
let b = asBracketExpr(c, n)
if b.isNil:
# it's not
result = semDirectOp(c, n, flags)
else:
result = semArrayAccess(c, b, flags)
else:
result = semDirectOp(c, n, flags)

Expand Down Expand Up @@ -3407,47 +3420,61 @@ proc semExport(c: PContext, n: PNode): PNode =

s = nextOverloadIter(o, c, a)

func containsArrGet(n: PNode): bool =
## Analyses `n` for whether it is or could be the ``mArrGet`` magic.
case n.kind
of nkSymChoices:
for it in n.items:
if it.kind == nkSym and it.sym.magic == mArrGet:
result = true
break
of nkSym:
# can happen in rare cases
result = n.sym.magic == mArrGet
else:
result = false

proc shouldBeBracketExpr(n: PNode): bool =
assert n.kind in nkCallKinds
let a = n[0]
if a.kind in nkCallKinds:
let b = a[0]
if b.kind in nkSymChoices:
for i in 0..<b.len:
if b[i].kind == nkSym and b[i].sym.magic == mArrGet:
result = true
break
elif b.kind == nkSym and b.sym.magic == mArrGet:
# can happen in rare cases
result = true

if result:
if containsArrGet(a[0]):
let be = newNodeI(nkBracketExpr, n.info)
for i in 1..<a.len:
be.add(a[i])
n[0] = be

proc asBracketExpr(c: PContext; n: PNode): PNode =
proc isGeneric(c: PContext; n: PNode): bool =
if n.kind in {nkIdent, nkAccQuoted}:
# XXX: this guesswork is meant to figure out whether a ``[](x, y)``
# expression *could* have been a ``x[y]`` expression where `x` is a
# generic procedure
case n.kind
of nkIdent, nkAccQuoted:
let s = qualifiedLookUp(c, n, {})
if s.isError:
# XXX: move to propagating nkError, skError, and tyError
localReport(c.config, s.ast)
result = false
else:
result = s != nil and isGenericRoutineStrict(s)
of nkSym:
result = isGenericRoutineStrict(n.sym)
of nkSymChoices:
for it in n.items:
if isGenericRoutineStrict(it.sym):
result = true
break
else:
result = false

assert n.kind in nkCallKinds
if n.len > 1 and isGeneric(c, n[1]):
let b = n[0]
if b.kind in nkSymChoices:
for i in 0..<b.len:
if b[i].kind == nkSym and b[i].sym.magic == mArrGet:
result = newNodeI(nkBracketExpr, n.info)
for i in 1..<n.len: result.add(n[i])
return result
if containsArrGet(b):
result = newNodeI(nkBracketExpr, n.info)
for i in 1..<n.len: result.add(n[i])
return result
return nil

proc hoistParamsUsedInDefault(c: PContext, call, letSection, defExpr: var PNode) =
Expand Down Expand Up @@ -3770,7 +3797,15 @@ proc semExpr(c: PContext, n: PNode, flags: TExprFlags = {}): PNode =
of nkTupleConstr:
result = semTupleConstr(c, n, flags)
of nkCurly: result = semSetConstr(c, n)
of nkBracket: result = semArrayConstr(c, n, flags)
of nkBracket:
if n.typ == nil or (tfVarargs notin n.typ.flags and
n.typ.skipTypes(abstractInst).kind != tySequence):
result = semArrayConstr(c, n, flags)
else:
# HACK: there's not enough information here to re-type an already
# typed varargs container, so it gets passed on for
# ``sigmatch`` to take care of re-typing it
result = n
of nkObjConstr: result = semObjConstr(c, n, flags)
of nkLambdaKinds:
result = semProcAnnotation(c, n)
Expand All @@ -3792,17 +3827,29 @@ proc semExpr(c: PContext, n: PNode, flags: TExprFlags = {}): PNode =
checkSonsLen(n, 1, c.config)
result[0] = semAddrArg(c, n[0])
result.typ = makePtrType(c, result[0].typ)
of nkHiddenAddr, nkHiddenDeref:
of nkHiddenAddr:
checkSonsLen(n, 1, c.config)
result = semExpr(c, n[0], flags)
of nkHiddenDeref:
checkSonsLen(n, 1, c.config)
n[0] = semExpr(c, n[0], flags)
# the type of the deref can be synthesized, so try to keep it
let operand = semExpr(c, n[0], flags)
case operand.typ.kind
of tyVar, tyLent:
result = newTreeIT(nkHiddenDeref, n.info, operand.typ.base): operand
else:
# an error or something else. Don't complain and pass it on, the
# receiver will figure out what to do with it
result = operand
of nkCast: result = c.config.extract(semCast(c, n))
of nkIfExpr, nkIfStmt: result = semIf(c, n, flags)
of nkConv:
checkSonsLen(n, 2, c.config)
result = semConv(c, n)
of nkHiddenStdConv, nkHiddenSubConv, nkHiddenCallConv:
checkSonsLen(n, 2, c.config)
considerGenSyms(c, n)
# discard the hidden conversion. It's restored again (or not) if necessary
result = semExpr(c, n[1], flags)
of nkStringToCString, nkCStringToString, nkObjDownConv, nkObjUpConv:
checkSonsLen(n, 1, c.config)
considerGenSyms(c, n)
Expand All @@ -3811,7 +3858,9 @@ proc semExpr(c: PContext, n: PNode, flags: TExprFlags = {}): PNode =
considerGenSyms(c, n)
of nkCheckedFieldExpr:
checkMinSonsLen(n, 2, c.config)
considerGenSyms(c, n)
# discard the check; analysis of the field expression will restore it if
# necessary
result = semExpr(c, n[0], flags)
of nkTableConstr:
result = semTableConstr(c, n)
of nkClosedSymChoice, nkOpenSymChoice:
Expand Down Expand Up @@ -3897,6 +3946,10 @@ proc semExpr(c: PContext, n: PNode, flags: TExprFlags = {}): PNode =
c.p.localBindStmts.add n
else:
localReport(c.config, n, reportSem rsemInvalidBindContext)
of nkNimNodeLit:
checkSonsLen(n, 1, c.config)
# the content is raw AST, so there's nothing to validate
result = copyNodeWithKids(n)
of nkError:
discard "ignore errors for now"
else:
Expand Down
69 changes: 63 additions & 6 deletions compiler/sem/sigmatch.nim
Original file line number Diff line number Diff line change
Expand Up @@ -2043,6 +2043,7 @@ proc implicitConv(kind: TNodeKind, f: PType, arg: PNode, m: TCandidate,

result.add c.graph.emptyNode
result.add arg
result.flags.incl nfSem

proc isLValue(c: PContext; n: PNode): bool {.inline.} =
case isAssignable(nil, n)
Expand Down Expand Up @@ -2111,9 +2112,11 @@ proc userConvMatch(c: PContext, m: var TCandidate, f, a: PType,
newTreeIT(nkHiddenAddr, arg.info, conv.typ[1], copyTree(arg))
else:
copyTree(arg))
result.flags.incl nfSem

if dest.kind in {tyVar, tyLent}:
result = newDeref(result)
result.flags.incl nfSem

inc(m.convMatches)

Expand Down Expand Up @@ -2278,10 +2281,13 @@ proc paramTypesMatchAux(m: var TCandidate, f, a: PType,
# 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)
evaluated.flags.incl nfSem
typ.sons = @[evaluated.typ]
typ.n = evaluated
typ.flags.incl tfHasStatic
arg = copyTree(arg) # fix #12864
arg.typ = typ
arg.flags.incl nfSem
a = typ
else:
if m.callee.kind == tyGenericBody:
Expand Down Expand Up @@ -2411,11 +2417,9 @@ proc paramTypesMatchAux(m: var TCandidate, f, a: PType,
if arg.typ.isNil():
result = arg
elif skipTypes(arg.typ, abstractVar-{tyTypeDesc}).kind == tyTuple or
m.inheritancePenalty > oldInheritancePenalty:
m.inheritancePenalty > oldInheritancePenalty or
arg.typ.isEmptyContainer:
result = implicitConv(nkHiddenSubConv, f, arg, m, c)
elif arg.typ.isEmptyContainer:
result = arg.copyTree
result.typ = getInstantiatedType(c, arg, m, f)
else:
result = arg
of isBothMetaConvertible:
Expand Down Expand Up @@ -2629,7 +2633,7 @@ proc prepareOperand(c: PContext; formal: PType; a, aOrig: PNode): PNode =
if formal.kind == tyUntyped:
assert formal.len != 1
result = aOrig
elif a.typ.isNil:
elif nfSem notin a.flags:
# XXX This is unsound! 'formal' can differ from overloaded routine to
# overloaded routine!
Comment on lines 2637 to 2638
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Soft suggestion, but you can remove this if you'd like.

I don't think this is valid anymore, because of untyped vs non-untyped argument isolation, the soon to be included nfSem flag guards, and the removal of the tyIterable hack a while back.

result = c.semOperand(c, a, {efAllowStmt})
Expand All @@ -2641,9 +2645,10 @@ proc prepareOperand(c: PContext; formal: PType; a, aOrig: PNode): PNode =
result.typ.kind in {tyVar, tyLent} and
c.matchedConcept.isNil():
result = newDeref(result)
result.flags.incl nfSem

proc prepareOperand(c: PContext; a: PNode): PNode =
if a.typ.isNil:
if nfSem notin a.flags:
result = c.semOperand(c, a)
else:
result = a
Expand Down Expand Up @@ -2687,6 +2692,43 @@ proc incrIndexType(t: PType) =
assert t.kind == tyArray
inc t[0].n[1].intVal

proc matchContainer(m: var TCandidate, formal: PType, n: PNode): PNode =
## Takes an already typed varargs container and re-analyses and matches
## each item against the formal type, producing an AST of the shape the
## usual (untyped -> typed) matching would have produced.
assert formal.kind == tyVarargs
var hasError = false

result = shallowCopy(n)
result.typ = arrayConstr(m.c, n.info)
result.typ.flags.incl tfVarargs

for i, it in n.pairs:
let
it = prepareOperand(m.c, it)
arg = paramTypesMatchAux(m, formal, it.typ, it)

if arg.isNil:
# it's a legacy error that was already reported
result[i] = it
elif arg.isError:
result[i] = arg
hasError = true
else:
result[i] = arg

incrIndexType(result.typ)

if hasError:
result = m.c.config.wrapError(result)
else:
if result.len > 0:
# update the array's type:
result.typ[1] = result[0].typ
propagateToOwner(result.typ, result.typ[1])
# a conversion is required to go from array -> varargs
result = implicitConv(nkHiddenStdConv, formal, result, m, m.c)

template isVarargsUntyped(x): untyped =
x.kind == tyVarargs and x[0].kind == tyUntyped

Expand Down Expand Up @@ -2958,6 +3000,10 @@ proc matchesAux(c: PContext, n, nOrig: PNode, m: var TCandidate, marker: var Int

if f != formalLen - 1: # not the last formal param
container = nil # xxx: is this more vararg stuff?
elif formal.typ.kind == tyVarargs:
# it's a varargs to varargs match (only possible during re-typing)
operand = matchContainer(m, formal.typ, arg)
setSon(m.call, formal.position + 1, operand)
else:
setSon(m.call, formal.position + 1, arg)

Expand Down Expand Up @@ -3099,6 +3145,13 @@ proc matchesAux(c: PContext, n, nOrig: PNode, m: var TCandidate, marker: var Int
# pick the formal from the end, so a regular param can follow a
# varargs: 'foo(x: int, y: varargs[typed], blk: untyped): typed'
f = max(f, formalLen - n.len + a + 1)
elif formal.typ.kind == tyVarargs and arg.kind == nkBracket and
container.isNil:
# a pre-existing container matches against a varargs type (only
# possible during re-typing)
operand = matchContainer(m, formal.typ, arg)
setSon(m.call, formal.position + 1, operand)
inc f
elif formal.typ.kind != tyVarargs or container.isNil(): # regular arg
setSon(m.call, formal.position + 1, arg)
inc f
Expand Down Expand Up @@ -3347,6 +3400,10 @@ proc matches*(c: PContext, n, nOrig: PNode, m: var TCandidate) =
if defaultValue.isError:
# xxx: change this to propagate
c.config.localReport(defaultValue)
else:
# the expression is already typed and valid; don't analyze it again,
# which might lose the type
defaultValue.flags.incl nfSem

if nfDefaultRefsParam in formal.ast.flags:
m.call.flags.incl nfDefaultRefsParam
Expand Down
3 changes: 2 additions & 1 deletion compiler/utils/astrepr.nim
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,8 @@ var
trfShowSymKind,
trfShowNodeErrors,
trfShowNodeIds,
trfShowNodeLineInfo
trfShowNodeLineInfo,
trfShowNodeFlags
}
base
## default tree repr config for compiler tracing, meant to be compact as
Expand Down
Loading