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

Adding a genSym-ed field with the same name as a pre-existing field causes redefinition error in an object definition #17851

Open
alaviss opened this issue Apr 25, 2021 · 0 comments
Labels
CPS bugs/pulls related to the cps project Macros

Comments

@alaviss
Copy link
Collaborator

alaviss commented Apr 25, 2021

Example

import std/macros

macro typeMacro(td: untyped): untyped =
  expectKind td, nnkTypeDef
  expectKind td[2], nnkObjectTy
  td[2][2].add newIdentDefs(genSym(nskField, "foo"), ident"string", newEmptyNode())
  result = td
  echo repr result

type
  O {.typeMacro.} = object
    foo: int

Current Output

O {..} = object
  foo: int
  foo_369098762: string

test.nim(6, 35) Error: attempt to redefine: 'foo'

Expected Output

Compiles successfully.

Possible Solution

The record field redefinition check within the compiler matches against the node name:

Nim/compiler/semtypes.nim

Lines 805 to 806 in 9e6f2d7

if containsOrIncl(check, f.name.id):
localError(c.config, info, "attempt to redefine: '" & f.name.s & "'")

This doesn't work with genSym-ed symbols.

Additional Information

  • Found while developing cps.
  • From what I can infer from the code, this issue would extend to proc types and tuples.
$ nim -v
Nim Compiler Version 1.5.1 [Linux: amd64]
Compiled at 2021-04-25
Copyright (c) 2006-2021 by Andreas Rumpf

git hash: 22e06ee95aeb32929a5b912096dfaa662165c162
active boot switches: -d:release -d:nimUseLinenoise -d:nativeStackTrace
@alaviss alaviss added the Macros label Apr 25, 2021
@alaviss alaviss changed the title Adding a genSym-ed field to a typedef with the same name as a pre-existing field causes redefinition error Adding a genSym-ed field with the same name as a pre-existing field causes redefinition error in an object definition Apr 25, 2021
alaviss added a commit to alaviss/cps that referenced this issue Apr 26, 2021
This is a WIP commit.

Experiments shows that we only have to desym due to issues in
environment definition and access.

Upstream bug: nim-lang/Nim#17851

This commit disable blanket desymification and only perform them
on environment as a workaround for the mentioned bug, which fixes
the foreign symbol tests.
alaviss added a commit to alaviss/cps that referenced this issue Apr 27, 2021
This is a WIP commit.

Experiments shows that we only have to desym due to issues in
environment definition and access.

Upstream bug: nim-lang/Nim#17851

This commit disable blanket desymification and only perform them
on environment as a workaround for the mentioned bug, which fixes
the foreign symbol tests.
alaviss added a commit to alaviss/cps that referenced this issue Apr 28, 2021
A proper workaround for nim-lang/Nim#17851,
which lets us remove the desym injections into cps/environment.
disruptek pushed a commit to nim-works/cps that referenced this issue Apr 28, 2021
* cps, cps/environment: only desym environment access

This is a WIP commit.

Experiments shows that we only have to desym due to issues in
environment definition and access.

Upstream bug: nim-lang/Nim#17851

This commit disable blanket desymification and only perform them
on environment as a workaround for the mentioned bug, which fixes
the foreign symbol tests.

* cps: disable blanket desym with comments

* cps: nicer getCallSym interface

* cps/environment: don't desym Env type

The better fix in #72 removed the need for this 🎉.

* cps/[environment, spec]: added genField for generating object fields

A proper workaround for nim-lang/Nim#17851,
which lets us remove the desym injections into cps/environment.

* cps: cater getCallSym to reviews

* cps/spec: English is hard
@alaviss alaviss added the CPS bugs/pulls related to the cps project label May 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CPS bugs/pulls related to the cps project Macros
Projects
None yet
Development

No branches or pull requests

1 participant