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.

Sign up for GitHub

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

Jump to bottom

Can't modify distinct addresses with conversions in VM #24097

Closed
metagn opened this issue Sep 11, 2024 · 0 comments · Fixed by #24124
Closed

Can't modify distinct addresses with conversions in VM #24097

metagn opened this issue Sep 11, 2024 · 0 comments · Fixed by #24124
Labels
Distinct Types VM see also `const` label

Comments

@metagn
Copy link
Collaborator

metagn commented Sep 11, 2024

Description

type Foo = distinct int

proc foo(x: var Foo) =
  int(x) += 1

static:
  var x = Foo(1)
  int(x) = int(x) + 1
  echo x.int # 1
  int(x) += 1
  echo x.int # 1
  foo(x)
  echo x.int # 1

Nim Version

793cee4

Current Output

1
1
1

Expected Output

2
3
4

Known Workarounds

No response

Additional Information

No response
@metagn metagn added VM see also `const` label Distinct Types labels Sep 11, 2024
metagn added a commit to metagn/Nim that referenced this issue Sep 17, 2024
@metagn
make distinct conversions addressable in VM
a6b6269 
fixes nim-lang#24097
@metagn metagn mentioned this issue Sep 17, 2024
Merged
@Araq Araq closed this as completed in 1fbb67f Sep 17, 2024
narimiran pushed a commit that referenced this issue Dec 20, 2024
@metagn @narimiran
make distinct conversions addressable in VM (#24124)
95fa7f0 
fixes #24097

For `nkConv` addresses where the conversion is between 2 types that are
equal between backends, treat assignments the same as assignments to the
argument of the conversion. In the VM this seems to be in `genAsgn` and
`genAsgnPatch`, as evidenced by the special logic for `nkDerefExpr` etc.

This doesn't handle ranges after #24037 because `sameBackendType` is
used and not `sameBackendTypeIgnoreRange`. This is so this is
backportable without #24037 and another PR can be opened that implements
it for ranges and adds tests as well. We can also merge
`sameBackendTypeIgnoreRange` with `sameBackendType` since it doesn't
seem like anything that uses it would be affected (only cycle checks and
the VM), but then we still have to add tests.

(cherry picked from commit 1fbb67f)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Distinct Types VM see also `const` label
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

make distinct conversions addressable in VM metagn/Nim
1 participant
@metagn