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

fix(sem): issues with method-call syntax in templates #1298

Merged
merged 14 commits into from
May 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 7 additions & 3 deletions compiler/ast/renderer.nim
Original file line number Diff line number Diff line change
Expand Up @@ -958,9 +958,13 @@ proc infixArgument(g: var TSrcGen, n: PNode, i: int) =
if needsParenthesis:
put(g, tkParRi, ")")

proc isCustomLit(n: PNode): bool =
proc isCustomLit(n: PNode, g: TSrcGen): bool =
if n.len == 2 and n[0].kind == nkRStrLit:
let ident = n[1].getPIdent
let ident =
if n[1].kind in nkSymChoices:
getPIdent(n[1][0])
else:
getPIdent(n[1])
result = ident != nil and ident.s.startsWith('\'')

proc gsub(g: var TSrcGen, n: PNode, c: TContext, fromStmtList = false) =
Expand Down Expand Up @@ -1187,7 +1191,7 @@ proc gsub(g: var TSrcGen, n: PNode, c: TContext, fromStmtList = false) =
gcomma(g, n, c)
put(g, tkBracketRi, "]")
of nkDotExpr:
if isCustomLit(n):
if isCustomLit(n, g):
put(g, tkCustomLit, n[0].strVal)
gsub(g, n, 1)
else:
Expand Down
1 change: 1 addition & 0 deletions compiler/sem/sem.nim
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import
std/[
strutils,
hashes,
math,
strtabs,
intsets,
Expand Down
14 changes: 12 additions & 2 deletions compiler/sem/semexprs.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1716,6 +1716,16 @@ proc tryReadingTypeField(c: PContext, n: PNode, i: PIdent, ty: PType): PNode =
else:
result = tryReadingGenericParam(c, n, i, ty)

proc originalName(cache: IdentCache, ident: PIdent): PIdent =
## Returns the identifier stripped off of the '`gensym' suffix, if any.
let i = find(ident.s, '`')
if i != -1:
# if there's a backtick in the name, the name must come from a gensym'ed
# symbol. Strip the '`gensym' suffix
cache.getIdent(ident.s.cstring, i, hashIgnoreStyle(ident.s, 0, i - 1))
else:
ident

proc builtinFieldAccess(c: PContext, n: PNode, flags: TExprFlags): PNode =
## returns nil if it's not a built-in field access
checkSonsLen(n, 2, c.config)
Expand All @@ -1742,7 +1752,7 @@ proc builtinFieldAccess(c: PContext, n: PNode, flags: TExprFlags): PNode =

n[0] = semExprWithType(c, n[0], flags)
var
i = legacyConsiderQuotedIdent(c, n[1], n)
i = originalName(c.cache, legacyConsiderQuotedIdent(c, n[1], n))
ty = n[0].typ
f: PSym = nil

Expand Down Expand Up @@ -2067,7 +2077,7 @@ proc semArrayAccess(c: PContext, n: PNode, flags: TExprFlags): PNode =
result = semExpr(c, result, flags)

proc propertyWriteAccess(c: PContext, n, a: PNode): PNode =
var id = legacyConsiderQuotedIdent(c, a[1],a)
var id = originalName(c.cache, legacyConsiderQuotedIdent(c, a[1],a))
var setterId = newIdentNode(getIdent(c.cache, id.s & '='), a[1].info)
# a[0] is already checked for semantics, that does ``builtinFieldAccess``
# this is ugly. XXX Semantic checking should use the ``nfSem`` flag for
Expand Down
158 changes: 95 additions & 63 deletions compiler/sem/semtempl.nim
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ type
scClosed, scOpen, scForceOpen

proc symChoice(c: PContext, n: PNode, s: PSym, r: TSymChoiceRule;
isField = false): PNode =
noGenSyms = false): PNode =
var
a: PSym
o: TOverloadIter
Expand All @@ -112,11 +112,11 @@ proc symChoice(c: PContext, n: PNode, s: PSym, r: TSymChoiceRule;
# XXX this makes more sense but breaks bootstrapping for now:
# (s.kind notin routineKinds or s.magic != mNone):
# for instance 'nextTry' is both in tables.nim and astalgo.nim ...
if not(isField and sfGenSym in s.flags):
if noGenSyms and sfGenSym in s.flags:
result = n
else:
result = newSymNode(s, info)
markUsed(c, info, s)
else:
result = n
else:
# semantic checking requires a type; `fitNode` deals with it
# appropriately
Expand All @@ -125,7 +125,7 @@ proc symChoice(c: PContext, n: PNode, s: PSym, r: TSymChoiceRule;
result = newNodeIT(kind, info, newTypeS(tyNone, c))
a = initOverloadIter(o, c, n)
while a != nil:
if a.kind != skModule and not(isField and sfGenSym in s.flags):
if a.kind != skModule and not(noGenSyms and sfGenSym in a.flags):
incl(a.flags, sfUsed)
markOwnerModuleAsUsed(c, a)
result.add newSymNode(a, info)
Expand Down Expand Up @@ -215,7 +215,6 @@ type
owner: PSym
cursorInBody: bool # only for nimsuggest
scopeN: int
noGenSym: int
inTemplateHeader: int

proc getIdentNode(c: var TemplCtx, n: PNode): PNode =
Expand Down Expand Up @@ -251,10 +250,13 @@ proc getIdentNode(c: var TemplCtx, n: PNode): PNode =
expectedKinds: {nkPostfix, nkPragmaExpr, nkIdent,
nkAccQuoted}))

func isTemplParam(c: TemplCtx, s: PSym): bool {.inline.} =
## True if `s` is a parameter symbol of the current template.
s.kind == skParam and s.owner == c.owner and sfTemplateParam in s.flags

func isTemplParam(c: TemplCtx, n: PNode): bool {.inline.} =
## True if `n` is a parameter symbol of the current template.
n.kind == nkSym and n.sym.kind == skParam and n.sym.owner == c.owner and
sfTemplateParam in n.sym.flags
n.kind == nkSym and isTemplParam(c, n.sym)

func definitionTemplParam(c: TemplCtx, n: PNode): bool {.inline.} =
## True if `n` is an `untyped` parameter symbol of the current template.
Expand Down Expand Up @@ -418,19 +420,13 @@ proc semTemplSymbol(c: PContext, n: PNode, s: PSym; isField: bool): PNode =
n # Introduced in this pass! Leave it as an identifier.
of OverloadableSyms-{skEnumField}:
result = symChoice(c, n, s, scOpen, isField)
of skGenericParam:
if isField and sfGenSym in s.flags: result = n
else: result = newSymNodeTypeDesc(s, c.idgen, n.info)
of skType, skGenericParam:
result = newSymNodeTypeDesc(s, c.idgen, n.info)
of skParam:
result = n
of skType:
if isField and sfGenSym in s.flags: result = n
else: result = newSymNodeTypeDesc(s, c.idgen, n.info)
else:
if s.kind == skEnumField and overloadableEnums in c.features:
result = symChoice(c, n, s, scOpen, isField)
elif isField and sfGenSym in s.flags:
result = n
else:
result = newSymNode(s, n.info)
# Issue #12832
Expand All @@ -440,6 +436,22 @@ proc semTemplSymbol(c: PContext, n: PNode, s: PSym; isField: bool): PNode =
if not isField and {optStyleHint, optStyleError} * c.config.globalOptions != {}:
styleCheckUse(c.config, n.info, s)

proc templBindSym(c: TemplCtx, s: PSym, n: PNode, isField: bool): PNode =
if contains(c.toBind, s.id):
result = symChoice(c.c, n, s, scClosed, isField)
elif contains(c.toMixin, s.name.id):
result = symChoice(c.c, n, s, scForceOpen, isField)
elif s.owner == c.owner and sfGenSym in s.flags:
# XXX: this is a tremendous hack. Symbol choices cannot contain
# template-introduced gensyms, since gensym'ed symbols are turned
# into identifiers during template evaluation. To at least support
# basic usage of gensyms, they're bound directly. As a consequence,
# overloads part of the definition scope won't be considered
incl(s.flags, sfUsed)
result = newSymNode(s, n.info)
else:
result = semTemplSymbol(c.c, n, s, isField)

proc semRoutineInTemplName(c: var TemplCtx, n: PNode): PNode =
## Analyses the `namePos` in a routine-like occurring in a template body,
## producing a checked name node or an `nkError`.
Expand Down Expand Up @@ -626,21 +638,12 @@ proc semTemplBody(c: var TemplCtx, n: PNode): PNode =
discard # result is already set to n
elif s.isError:
result = s.ast
elif isTemplParam(c, s):
incl(s.flags, sfUsed)
result = newSymNode(s, n.info)
else:
if s.owner == c.owner and s.kind == skParam and sfTemplateParam in s.flags:
incl(s.flags, sfUsed)
result = newSymNode(s, n.info)
elif contains(c.toBind, s.id):
result = symChoice(c.c, n, s, scClosed, c.noGenSym > 0)
elif contains(c.toMixin, s.name.id):
result = symChoice(c.c, n, s, scForceOpen, c.noGenSym > 0)
elif s.owner == c.owner and sfGenSym in s.flags and c.noGenSym == 0:
# template tmp[T](x: var seq[T]) =
# var yz: T
incl(s.flags, sfUsed)
result = newSymNode(s, n.info)
else:
result = semTemplSymbol(c.c, n, s, c.noGenSym > 0)
result = templBindSym(c, s, n, isField=false)

of nkBind:
result = semTemplBody(c, n[0])
of nkBindStmt:
Expand Down Expand Up @@ -906,47 +909,76 @@ proc semTemplBody(c: var TemplCtx, n: PNode): PNode =
# so we use the generic code for nkDotExpr too
let s = qualifiedLookUp(c.c, n, {})

if s.isNil:
discard
elif s.isError:
result = s.ast
else:
# do not symchoice a quoted template parameter (bug #2390):
if s.owner == c.owner and s.kind == skParam and
n.kind == nkAccQuoted and n.len == 1:
if s != nil:
if isTemplParam(c, s):
incl(s.flags, sfUsed)
return newSymNode(s, n.info)
result = newSymNode(s, n.info)
elif s.isError:
result = s.ast
elif contains(c.toBind, s.id):
return symChoice(c.c, n, s, scClosed, c.noGenSym > 0)
result = symChoice(c.c, n, s, scClosed)
elif contains(c.toMixin, s.name.id):
return symChoice(c.c, n, s, scForceOpen, c.noGenSym > 0)
result = symChoice(c.c, n, s, scForceOpen)
else:
return symChoice(c.c, n, s, scOpen, c.noGenSym > 0)

case n.kind
of nkDotExpr:
result = n

result[0] = semTemplBody(c, n[0])

inc c.noGenSym
result[1] = semTemplBody(c, n[1])
dec c.noGenSym
# FIXME: ``semTemplSymbol`` needs to be used here to ensure correct
# typing for type symbols
result = symChoice(c.c, n, s, scOpen)

if nkError in {result[0].kind, result[1].kind}:
result = c.c.config.wrapError(result)
of nkAccQuoted:
result = semTemplBodySons(c, n)
else:
unreachable("should never have gotten here")
of nkExprColonExpr, nkExprEqExpr:
if n.len == 2:
inc c.noGenSym
elif n.kind == nkDotExpr:
# a normal dot expression
result[0] = semTemplBody(c, n[0])
dec c.noGenSym
result[1] = semTemplBody(c, n[1])
var
iter: TOverloadIter
s = initOverloadIter(iter, c.c, n[1])

block resolve:
# only routines and types are eligible for the right-hand side, look
# for such a symbol:
while s != nil:
if s.isError:
localReport(c.c.config, s.ast)
elif isTemplParam(c, s):
# template parameters are bound eagerly
incl(s.flags, sfUsed)
result[1] = newSymNode(s, n[1].info)
break resolve
elif s.kind in routineKinds + {skType, skGenericParam}:
break # found a symbol that fits
s = nextOverloadIter(iter, c.c, n[1])

if s != nil:
var field = templBindSym(c, s, n[1], isField=true)
if field.kind == nkSym and sfGenSym notin field.sym.flags and
field.sym.kind in OverloadableSyms:
# ``semexprs.dotTransformation`` ignores single symbols, so we
# need to wrap the symbol in a sym-choice, to preserve the bound
# symbol
field = newTreeIT(nkOpenSymChoice, n[1].info,
newTypeS(tyNone, c.c), field)
# XXX: should be a closed symbol choice, like it works for
# generics, but code relies on the symbol being open...
# XXX: ``dotTransformation`` should not ignore symbols in the
# first place

result[1] = field

hasError = nkError in {result[0].kind, result[1].kind}:
else:
# a quoted identifier or identifier construction
result = semTemplBodySons(c, n)

of nkExprColonExpr, nkExprEqExpr:
let s = qualifiedLookUp(c.c, n[0], {})
# template parameters can be substituted into the name position of a
# ``a: b`` or ``a = b`` construct
if s != nil and isTemplParam(c, s):
result[0] = newSymNode(s, n[0].info)
elif n[0].kind == nkAccQuoted and n[0].len > 1:
# make sure to also process identifier constructions
result[0] = semTemplBody(c, n[1])

result[1] = semTemplBody(c, n[1])
hasError = nkError in {result[0].kind, result[1].kind}
of nkTableConstr:
# also transform the keys (bug #12595)
for i in 0..<n.len:
Expand Down
35 changes: 0 additions & 35 deletions doc/manual.rst
Original file line number Diff line number Diff line change
Expand Up @@ -5731,41 +5731,6 @@ no semantics outside of a template definition and cannot be abstracted over:
To get rid of hygiene in templates, one can use the `dirty`:idx: pragma for
a template. `inject` and `gensym` have no effect in `dirty` templates.

`gensym`'ed symbols cannot be used as `field` in the `x.field` syntax.
Nor can they be used in the `ObjectConstruction(field: value)`
and `namedParameterCall(field = value)` syntactic constructs.

The reason for this is that code like

.. code-block:: nim
:test: "nim c $1"

type
T = object
f: int

template tmp(x: T) =
let f = 34
echo x.f, T(f: 4)


should work as expected.

However, this means that the method call syntax is not available for
`gensym`'ed symbols:

.. code-block:: nim
:test: "nim c $1"
:status: 1

template tmp(x) =
type
T {.gensym.} = int

echo x.T # invalid: instead use: 'echo T(x)'.

tmp(12)



Limitations of the method call syntax
Expand Down
5 changes: 5 additions & 0 deletions tests/lang_callable/template/mmethod_call_symbol_binding.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
proc p(x: int): int = x

template templ*(x: untyped): untyped =
# `p` is a symbol that's not overloaded
x.p()
26 changes: 26 additions & 0 deletions tests/lang_callable/template/tclosed_symbol_with_method_call.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
discard """
description: '''
Ensure that non-overloaded symbols used as the method in the method-call
syntax are closed symbols
'''
action: reject
knownIssue: '''
Non-overloaded symbols used as the callee with the method call syntax
are always open
'''
"""

proc p(x: int) =
discard

template test(x: string) =
# `p` is not overloaded at this point, and it's thus a closed symbol
x.p()

# the overload with which the call would work:
proc p(x: string) =
discard

# the symbol of the callee is bound early and is closed, so the correct
# overload cannot be picked, meaning that an error ensues
test("")
Loading
Loading