Skip to content

Commit

Permalink
allow replacing captured syms in macro calls in generics (#23091)
Browse files Browse the repository at this point in the history
fixes #22605, separated from #22744

This marks symbol captures in macro calls in generic contexts as
`nfOpenSym`, which means if there is a new symbol in the local
instantiatied body during instantiation time, this symbol replaces the
captured symbol. We have to be careful not to consider symbols outside
of the instantiation body during instantiation, because this will leak
symbols from the instantiation context scope rather than the original
declaration scope. This is done by checking if the local context owner
(maybe should be the symbol of the proc currently getting instantiated
instead? not sure how to get this) is the same as or a parent owner of
the owner of the replacement candidate symbol.

This solution is distinct from the symchoice mechanisms which we
originally assumed had to be related, if this assumption was wrong it
would explain why this solution took so long to arrive at.

(cherry picked from commit 9416595)
  • Loading branch information
metagn authored and narimiran committed Mar 7, 2024
1 parent 0de70dc commit a3e5f4f
Show file tree
Hide file tree
Showing 5 changed files with 121 additions and 4 deletions.
4 changes: 3 additions & 1 deletion compiler/ast.nim
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,7 @@ type
nfFirstWrite # this node is a first write
nfHasComment # node has a comment
nfSkipFieldChecking # node skips field visable checking
nfOpenSym # node is a captured sym but can be overriden by local symbols

TNodeFlags* = set[TNodeFlag]
TTypeFlag* = enum # keep below 32 for efficiency reasons (now: 47)
Expand Down Expand Up @@ -1087,7 +1088,8 @@ const
nfIsRef, nfIsPtr, nfPreventCg, nfLL,
nfFromTemplate, nfDefaultRefsParam,
nfExecuteOnReload, nfLastRead,
nfFirstWrite, nfSkipFieldChecking}
nfFirstWrite, nfSkipFieldChecking,
nfOpenSym}
namePos* = 0
patternPos* = 1 # empty except for term rewriting macros
genericParamsPos* = 2
Expand Down
2 changes: 1 addition & 1 deletion compiler/ic/ic.nim
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ proc addModuleRef(n: PNode; ir: var PackedTree; c: var PackedEncoder; m: var Pac
## add a remote symbol reference to the tree
let info = n.info.toPackedInfo(c, m)
ir.nodes.add PackedNode(kind: nkModuleRef, operand: 3.int32, # spans 3 nodes in total
typeId: storeTypeLater(n.typ, c, m), info: info)
typeId: storeTypeLater(n.typ, c, m), info: info, flags: n.flags)
ir.nodes.add PackedNode(kind: nkInt32Lit, info: info,
operand: toLitId(n.sym.itemId.module.FileIndex, c, m).int32)
ir.nodes.add PackedNode(kind: nkInt32Lit, info: info,
Expand Down
20 changes: 18 additions & 2 deletions compiler/semexprs.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1016,7 +1016,10 @@ proc afterCallActions(c: PContext; n, orig: PNode, flags: TExprFlags; expectedTy
of skMacro: result = semMacroExpr(c, result, orig, callee, flags, expectedType)
of skTemplate: result = semTemplateExpr(c, result, callee, flags, expectedType)
else:
semFinishOperands(c, result)
if callee.magic notin {mArrGet, mArrPut, mNBindSym}:
# calls to `[]` can be explicit generic instantiations,
# don't sem every operand now, leave it to semmagic
semFinishOperands(c, result)
activate(c, result)
fixAbstractType(c, result)
analyseIfAddressTakenInCall(c, result)
Expand Down Expand Up @@ -3015,9 +3018,22 @@ proc semExpr(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType: PType
if result.kind == nkSym:
result = semSym(c, result, result.sym, flags)
of nkSym:
let s = n.sym
if nfOpenSym in n.flags:
let id = newIdentNode(s.name, n.info)
c.isAmbiguous = false
let s2 = qualifiedLookUp(c, id, {})
if s2 != nil and s2 != s and not c.isAmbiguous:
# only consider symbols defined under current proc:
var o = s2.owner
while o != nil:
if o == c.p.owner:
result = semExpr(c, id, flags, expectedType)
return
o = o.owner
# because of the changed symbol binding, this does not mean that we
# don't have to check the symbol for semantics here again!
result = semSym(c, n, n.sym, flags)
result = semSym(c, n, s, flags)
of nkEmpty, nkNone, nkCommentStmt, nkType:
discard
of nkNilLit:
Expand Down
13 changes: 13 additions & 0 deletions compiler/semgnrc.nim
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym,
result.transitionSonsKind(nkClosedSymChoice)
else:
result = symChoice(c, n, s, scOpen)
if {withinMixin, withinConcept} * flags == {withinMixin} and result.kind == nkSym:
result.flags.incl nfOpenSym
result.typ = nil
case s.kind
of skUnknown:
# Introduced in this pass! Leave it as an identifier.
Expand Down Expand Up @@ -95,6 +98,9 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym,
result = n
else:
result = newSymNodeTypeDesc(s, c.idgen, n.info)
if {withinMixin, withinConcept} * flags == {withinMixin}:
result.flags.incl nfOpenSym
result.typ = nil
onUse(n.info, s)
of skParam:
result = n
Expand All @@ -103,11 +109,17 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym,
if (s.typ != nil) and
(s.typ.flags * {tfGenericTypeParam, tfImplicitTypeParam} == {}):
result = newSymNodeTypeDesc(s, c.idgen, n.info)
if {withinMixin, withinConcept} * flags == {withinMixin}:
result.flags.incl nfOpenSym
result.typ = nil
else:
result = n
onUse(n.info, s)
else:
result = newSymNode(s, n.info)
if {withinMixin, withinConcept} * flags == {withinMixin}:
result.flags.incl nfOpenSym
result.typ = nil
onUse(n.info, s)

proc lookup(c: PContext, n: PNode, flags: TSemGenericFlags,
Expand Down Expand Up @@ -147,6 +159,7 @@ proc fuzzyLookup(c: PContext, n: PNode, flags: TSemGenericFlags,

var s = qualifiedLookUp(c, n, luf)
if s != nil:
isMacro = s.kind in {skTemplate, skMacro}
result = semGenericStmtSymbol(c, n, s, ctx, flags)
else:
n[0] = semGenericStmt(c, n[0], flags, ctx)
Expand Down
86 changes: 86 additions & 0 deletions tests/generics/tmacroinjectedsym.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
block: # issue #22605, normal call syntax
const error = "bad"

template valueOr(self: int, def: untyped): untyped =
case false
of true: ""
of false:
template error: untyped {.used, inject.} = "good"
def

proc g(T: type): string =
let x = valueOr 123:
return $error

"ok"

doAssert g(int) == "good"

block: # issue #22605, method call syntax
const error = "bad"

template valueOr(self: int, def: untyped): untyped =
case false
of true: ""
of false:
template error: untyped {.used, inject.} = "good"
def

proc g(T: type): string =
let x = 123.valueOr:
return $error

"ok"

doAssert g(int) == "good"

block: # issue #22605, original complex example
type Xxx = enum
error
value

type
Result[T, E] = object
when T is void:
when E is void:
oResultPrivate*: bool
else:
case oResultPrivate*: bool
of false:
eResultPrivate*: E
of true:
discard
else:
when E is void:
case oResultPrivate*: bool
of false:
discard
of true:
vResultPrivate*: T
else:
case oResultPrivate*: bool
of false:
eResultPrivate*: E
of true:
vResultPrivate*: T

template valueOr[T: not void, E](self: Result[T, E], def: untyped): untyped =
let s = (self) # TODO avoid copy
case s.oResultPrivate
of true:
s.vResultPrivate
of false:
when E isnot void:
template error: untyped {.used, inject.} = s.eResultPrivate
def

proc f(): Result[int, cstring] =
Result[int, cstring](oResultPrivate: false, eResultPrivate: "f")

proc g(T: type): string =
let x = f().valueOr:
return $error

"ok"

doAssert g(int) == "f"

0 comments on commit a3e5f4f

Please sign in to comment.