Skip to content

Commit

Permalink
fix fields with names containing backticks being inaccessible (#1389)
Browse files Browse the repository at this point in the history
## Summary

Fix a regression where field names with backticks in them couldn't be
accessed with a field access.

Fixes #1379.

## Details

* identifier nodes resulting from gensyms are now tagged with a new
  node flag (`nfWasGensym`)
* `originalName` only strips the `gensym` suffix from identifiers
  marked with the flag
* the node flag is persistent, so that it stays on the node across tree
  copies

Looking for the full "`gensym" suffix wouldn't work, because it would
also trigger the stripping for user-created names containing the
suffix.
  • Loading branch information
zerbina committed Jul 27, 2024
1 parent d36b9a3 commit f0c722a
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 11 deletions.
3 changes: 2 additions & 1 deletion compiler/ast/ast_query.nim
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ const
PtrLikeKinds*: TTypeKinds = {tyPointer, tyPtr} # for VM

PersistentNodeFlags*: TNodeFlags = {nfDotSetter, nfDotField, nfLL,
nfFromTemplate, nfDefaultRefsParam}
nfFromTemplate, nfDefaultRefsParam,
nfWasGensym}

namePos* = 0 ## Name of the type/proc-like node
patternPos* = 1 ## empty except for term rewriting macros
Expand Down
2 changes: 2 additions & 0 deletions compiler/ast/ast_types.nim
Original file line number Diff line number Diff line change
Expand Up @@ -612,6 +612,8 @@ type
nfDefaultRefsParam ## a default param value references another parameter
## the flag is applied to proc default values and to calls
nfHasComment ## node has a comment
nfWasGensym ## the identifier node was a gensym prior to template
## evaluation

TNodeFlags* = set[TNodeFlag]
TTypeFlag* = enum ## keep below 32 for efficiency reasons (now: 43)
Expand Down
1 change: 1 addition & 0 deletions compiler/sem/evaltempl.nim
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ proc evalTemplateAux(templ, actual: PNode, c: var TemplCtx, result: PNode) =
if sfGenSym in s.flags:
result.add newIdentNode(getIdent(c.ic, x.name.s & "`gensym" & $c.instID),
if c.instLines: actual.info else: templ.info)
result.flags.incl nfWasGensym
else:
result.add newSymNode(x, if c.instLines: actual.info else: templ.info)
else:
Expand Down
19 changes: 10 additions & 9 deletions compiler/sem/semexprs.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1711,13 +1711,14 @@ 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))
proc originalName(c: PContext, n, orig: PNode): PIdent =
## Returns the identifier stripped off of the '`gensym' suffix, if
## it originated from a processed gensym symbol node.
let ident = legacyConsiderQuotedIdent(c, n, orig)
if nfWasGensym in n.flags:
let i = rfind(ident.s, '`')
# strip the suffix
c.cache.getIdent(ident.s.cstring, i, hashIgnoreStyle(ident.s, 0, i - 1))
else:
ident

Expand Down Expand Up @@ -1747,7 +1748,7 @@ proc builtinFieldAccess(c: PContext, n: PNode, flags: TExprFlags): PNode =

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

Expand Down Expand Up @@ -2072,7 +2073,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 = originalName(c.cache, legacyConsiderQuotedIdent(c, a[1],a))
var id = originalName(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
18 changes: 17 additions & 1 deletion tests/lang_callable/macros/tmacros_issues.nim
Original file line number Diff line number Diff line change
Expand Up @@ -538,4 +538,20 @@ block orginal_parameter_types:
doAssert x.intVal == 1
doAssert y.intVal == 2

m(1, 2)
m(1, 2)

block field_names_with_backticks:
# field names containing backticks (only possible with macro-generated
# code) couldn't be accessed with dot expressions
macro test() =
let name = ident"a`b"

result = quote do:
type Object = object
`name`: int

# test accessing the field with both constructors and a dot expression
let o = Object(`name`: 1)
doAssert o.`name` == 1

test()

0 comments on commit f0c722a

Please sign in to comment.