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

correct macros.genSym comment doc #20633

Closed
wants to merge 1 commit into from
Closed
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
6 changes: 4 additions & 2 deletions lib/core/macros.nim
Original file line number Diff line number Diff line change
Expand Up @@ -499,8 +499,10 @@ proc bindSym*(ident: string | NimNode, rule: BindSymRule = brClosed): NimNode {.

proc genSym*(kind: NimSymKind = nskLet; ident = ""): NimNode {.
magic: "NGenSym", noSideEffect.}
## Generates a fresh symbol that is guaranteed to be unique. The symbol
## needs to occur in a declaration context.
## Generates a fresh symbol.
## if `ident` is not empty, the generated symbol maybe ambiguous.
Copy link
Member

@Araq Araq Oct 24, 2022

Choose a reason for hiding this comment

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

No, it's really always unique, it's just that the string representation of the code might not reflect this:

    of opcGenSym:
      decodeBC(rkNode)
      let k = regs[rb].intVal
      let name = if regs[rc].node.strVal.len == 0: ":tmp"
                 else: regs[rc].node.strVal
      if k < 0 or k > ord(high(TSymKind)):
        internalError(c.config, c.debug[pc], "request to create symbol of invalid kind")
      var sym = newSym(k.TSymKind, getIdent(c.cache, name), nextSymId c.idgen, c.module.owner, c.debug[pc])
      incl(sym.flags, sfGenSym)
      regs[ra].node = newSymNode(sym)
      regs[ra].node.flags.incl nfIsRef

Copy link
Collaborator Author

@bung87 bung87 Oct 24, 2022

Choose a reason for hiding this comment

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

the problem is the symbol id is unique but the ident id is not. which will cause ambiguous, I've seen serveral issue about macro users submited issues about this.

Copy link
Member

Choose a reason for hiding this comment

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

So users need to understand symbols but your documentation patch makes it worse.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

okay then, someone may have good one in mind.

## if `ident` is empty, generates a fresh symbol that is guaranteed to be unique.
## The symbol needs to occur in a declaration context.

proc callsite*(): NimNode {.magic: "NCallSite", benign, deprecated:
"Deprecated since v0.18.1; use `varargs[untyped]` in the macro prototype instead".}
Expand Down