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

Macro: bool is converted to int inside quote do #7375

Closed
mratsim opened this issue Mar 20, 2018 · 9 comments · Fixed by #17426 or #21433
Closed

Macro: bool is converted to int inside quote do #7375

mratsim opened this issue Mar 20, 2018 · 9 comments · Fixed by #17426 or #21433

Comments

@mratsim
Copy link
Collaborator

mratsim commented Mar 20, 2018

When creating a boolean in a macro and passing it to other macro, it is transformed to a static[int]

import macros

macro fails(b: static[bool]): untyped =
  echo b
  result = newStmtList()

macro works(b: static[int]): untyped =
  echo b
  result = newStmtList()

macro foo(): untyped =

  var b = false

  ## Fails
  result = quote do:
    fails(`b`)

  ## Works
  # result = quote do:
  #   works(`b`)

foo()
Error: type mismatch: got <static[int literal(0)](0)>
but expected one of:
macro fails(b: static[bool]): untyped

expression: fails(0)

I would expect a conversion to static[bool] instead of static[int]

originally posted in #10719

It seems that passing true/false literals within macros gets them converted to integer literals internally, which further results in errors if you try to use them as booleans. This worked as expected in 0.18.0.

Example

import macros

macro someMacro*(): typed =
  template tmpl(boolean) =
    when boolean:
      echo "it's true!"
  result = getAst(tmpl(true))

someMacro()

Current Output

test.nim(9, 10) template/generic instantiation of `someMacro` from here
test.nim(7, 23) Error: type mismatch: got <int literal(1)> but expected 'bool'
@mratsim
Copy link
Collaborator Author

mratsim commented Aug 24, 2018

Couple of updates.

  • Enums are also affected
  • This is not only when passing macro to macro but passing from compile-time to runtime:
    Test case:
import macros


macro foo(): untyped =
  result = quote do: `littleEndian`


doAssert littleEndian == foo() # Error: type mismatch: got <Endianness, int literal(0)>
  • Following 361a2d8 all codes that depended on compile-time comparison result to use when will have type mismatch:

Test case:

import macros

macro eqSym(x, y: untyped): untyped =
  let eq = $x == $y # Unfortunately eqIdent compares to string.
  result = quote do: `eq`

var r, a, b: int

template fma(result: var int, a, b: int, op: untyped) =
  # fused multiple-add
  when eqSym(op, `+=`):
    echo "+="
  else:
    echo "+"

fma(r, a, b, `+=`)

Impacted lib: https://github.com/status-im/nim-stint/blob/bd734b6845342feff426a849240f478020e6ee85/stint/private/uint_mul.nim#L83-L109

Workarounds:

Instead of

result = quote do: `eq`
  1. either use result = newLit eq

  2. or use

result = quote do: Endianness(`eq`)

@Araq
Copy link
Member

Araq commented Aug 31, 2018

Turns out the regression was unrelated to this bug.

@timotheecour
Copy link
Member

I just hit this bug coming from another case;
unfortunately the workaround (result = newLit eq) doesn't work so well in my case where the expression inside quote do can be complex (and turning it into AST via newCall etc, is undesirable)

#[
KEY bool converted to int in quote do
https://github.com/nim-lang/Nim/issues/7375
]#
import macros
macro foo(): untyped=
  let isReq = true
  result = quote do:
    echo `isReq`
    # or, complexExpression(`isReq`)
foo()

@timotheecour timotheecour changed the title Macro: bool is converted to static[int] when passed to other macros Macro: bool is converted to int inside quote do Dec 5, 2018
@bluenote10
Copy link
Contributor

bluenote10 commented Dec 30, 2018

The issue is not specific to quote, it looks like getAst is also affected:

import macros

template test(boolArg: bool) =
  static:
    echo "type is: ", type(boolArg)
  let x: bool = boolArg # compile error here, because boolArg became an int

macro testWrapped1(boolArg: bool): untyped =
  # forwarding boolArg directly works
  result = getAst(test(boolArg))

macro testWrapped2(boolArg: bool): untyped =
  # forwarding boolArg via a local variable also works
  let b = boolArg
  result = getAst(test(b))

macro testWrapped3(boolArg: bool): untyped =
  # but using a literal `true` as a local variable will be converted to int
  let b = true
  result = getAst(test(b))

test(true) # ok
testWrapped1(true) # ok
testWrapped2(true) # ok
testWrapped3(true) # error

I would say it is an regression, because the godot bindings were relying on getAst with local bools. The work-around with wrapping them in newLit also works with getAst.

bluenote10 added a commit to bluenote10/godot-nim that referenced this issue Dec 30, 2018
@nc-x
Copy link
Contributor

nc-x commented Jan 2, 2019

The issue here is that -

Nim/lib/system.nim

Lines 42 to 44 in a1e268e

type # we need to start a new type section here, so that ``0`` can have a type
bool* {.magic: Bool.} = enum ## built-in boolean type
false = 0, true = 1

compiler converts enum fields to integers in the semantic phase -

Nim/compiler/semfold.nim

Lines 551 to 552 in a1e268e

of skEnumField:
result = newIntNodeT(s.position, n, g)

So if at compile time - the boolean value gets converted to integer value, then at runtime there is no boolean value, only integer value. (Internally in the compiler there is no notion of a bool, .intVal is checked for being 0 or 1, similarly VM does not know anything about bool)

@krux02
Copy link
Contributor

krux02 commented Jan 21, 2019

I would like to reference my new RFC here, because it would be affected:
https://github.com/nim-lang/Nim/issues/10412
Effectively the pattern to use newLit would be enforced.

@krux02
Copy link
Contributor

krux02 commented Feb 21, 2019

I did some investigations. So first of all, enum values are atcually represented as integer literals, but with a type that says that they are actually enum values. So there is nothing wrong with that. Except that the type is lost in translation.

For getAst a macro call is generated here:

c.genCall(arg, dest)

The argument is passed as an immediate int here:

c.gABx(n, opcLdImmInt, dest, n.intVal.int)

The type field is not serialized.

In the vm the register in translated back into a node here:

let node = regs[rb+i].regToNode

This node is an integer literal node without any type information (nil).
Then in semConstExpr→semExprWithType→semExpr the missing type is set to be actually of type int (tyInt).

let e = forceBool(c, semConstExpr(c, it.sons[0]))

This crashes then in forceBool, because an integer is not a boolean.

@JohnAD
Copy link
Contributor

JohnAD commented Aug 18, 2019

For when this is later fixed; here is another bit of sample code to test against. Very likely the same bug involving a template called from macro:

https://forum.nim-lang.org/t/4995

@krux02
Copy link
Contributor

krux02 commented Aug 18, 2019

@JohanaD just make sure all arguments to the template you want to call with getAst arg of type NimNode.

macro joe(body: untyped): untyped =
  result = newStmtList()
  let t = newLit(true)
  let temp = getAst test(t)
  result.add(temp)

endragor pushed a commit to pragmagic/godot-nim that referenced this issue Sep 15, 2019
workaround for nim/macro/typedesc issue

work-around for nim-lang/Nim#7375

fixed more isNil usages

fixed new name of setStackBottom
@ringabout ringabout reopened this Feb 26, 2023
@Araq Araq closed this as completed in 9948fed Mar 2, 2023
capocasa pushed a commit to capocasa/Nim that referenced this issue Mar 31, 2023
…nim-lang#9607; rework quote do; `getAst` uses type info to annotate the type of quoted variables; no more type erasures for quoted variables (nim-lang#21433)

* fixes nim-lang#21326; getAst uses type info to annotateType quoted variables

* simplify logics; sem types first

* fixes important packages

* add testcases

* tiny
bung87 pushed a commit to bung87/Nim that referenced this issue Jul 29, 2023
…nim-lang#9607; rework quote do; `getAst` uses type info to annotate the type of quoted variables; no more type erasures for quoted variables (nim-lang#21433)

* fixes nim-lang#21326; getAst uses type info to annotateType quoted variables

* simplify logics; sem types first

* fixes important packages

* add testcases

* tiny
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment