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

noRewrite pragma doesn't prevent rewrites #16620

Open
exelotl opened this issue Jan 7, 2021 · 1 comment
Open

noRewrite pragma doesn't prevent rewrites #16620

exelotl opened this issue Jan 7, 2021 · 1 comment

Comments

@exelotl
Copy link
Contributor

exelotl commented Jan 7, 2021

While playing with term rewriting macros, I found that the {.noRewrite.} pragma doesn't behave as expected at all.

Example

# test 1
# ------

proc get(x: int): int = x

template t{get(a)}(a: int): int =
  {.noRewrite.}:
    get(a) + 1

echo get(0)


# test 2
# ------

var x: int

template asgn{a = b}(a: int{lvalue}, b: int) =
  {.noRewrite.}:
    a = b + 1

x = 10
echo x

Current output

Tested from Nim 0.20.0 to 1.4.2:

49
85

Expected output

1
11

Additional information

The output on Nim 0.19.6 is different, test 2 gives the correct result:

98
11
@exelotl
Copy link
Contributor Author

exelotl commented Jan 9, 2021

Workaround:
Depending on the use case, you may be able to use a helper proc defined before the term-rewriting macro, to which the rules of the macro don't apply:

# test 1
# ------

proc get(x: int): int = x

proc getFinal(x: int): int {.inline.} =
  get(x)

template t{get(a)}(a: int): int =
    getFinal(a) + 1

echo get(0)


# test 2
# ------

var x: int

proc asgnFinal[T](a: var T, b: T) {.inline.} =
  a = b

template asgn{a = b}(a: int{lvalue}, b: int) =
  asgnFinal(a, b + 1)

x = 10
echo x

Output:

1
11

metagn added a commit to metagn/Nim that referenced this issue Aug 2, 2022
metagn added a commit to metagn/Nim that referenced this issue Sep 23, 2022
Araq pushed a commit that referenced this issue Sep 28, 2022
* continue #9582 for consts, close #9331, fix #20114

also move extractPragma to ast to pave the way for things like {.strdefine: "abc".} etc

* changelog correctly

* fix jsgen

* update tgetimpl

* fix sighashes

* fix #19766, add comment about postfix

* fix noRewrite LOL

refs #16620

* fix changelog

* fix destructors
capocasa pushed a commit to capocasa/Nim that referenced this issue Mar 31, 2023
* continue nim-lang#9582 for consts, close nim-lang#9331, fix nim-lang#20114

also move extractPragma to ast to pave the way for things like {.strdefine: "abc".} etc

* changelog correctly

* fix jsgen

* update tgetimpl

* fix sighashes

* fix nim-lang#19766, add comment about postfix

* fix noRewrite LOL

refs nim-lang#16620

* fix changelog

* fix destructors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants