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

VM: const case object gets some fields zeroed out at runtime #13081

Closed
timotheecour opened this issue Jan 9, 2020 · 4 comments
Closed

VM: const case object gets some fields zeroed out at runtime #13081

timotheecour opened this issue Jan 9, 2020 · 4 comments
Labels
VM see also `const` label

Comments

@timotheecour
Copy link
Member

timotheecour commented Jan 9, 2020

when a const case object is used at runtime, some fields get re-initialized to 0. But not all, see below

Example

  type Kind = enum
    k0, k1, k2, k3

  type Foo = object
    x0: float
    case kind: Kind
    of k0: discard
    of k1: x1: int
    of k2: x2: string
    of k3: x3: string

  const j1 = Foo(x0: 1.2, kind: k1, x1: 12)
  const j2 = Foo(x0: 1.3, kind: k2, x2: "abc")
  const j3 = Foo(x0: 1.3, kind: k3, x3: "abc2")
  static:
    echo j1
    echo j2
    echo j3
  echo j1
  echo j2
  echo j3

Current Output

(x0: 1.2, kind: k1, x1: 12)
(x0: 1.3, kind: k2, x2: "abc")
(x0: 1.3, kind: k3, x3: "abc2")
Hint: 29857... [SuccessX]
(x0: 1.2, kind: k1, x1: 0)
(x0: 1.3, kind: k2, x2: "")
(x0: 1.3, kind: k3, x3: "abc2")

Expected Output

(x0: 1.2, kind: k1, x1: 12)
(x0: 1.3, kind: k2, x2: "abc")
(x0: 1.3, kind: k3, x3: "abc2")
Hint: 29857... [SuccessX]
(x0: 1.2, kind: k1, x1: 12)
(x0: 1.3, kind: k2, x2: "abc")
(x0: 1.3, kind: k3, x3: "abc2")

workaround

use const x2 = j2.x2, the x2 will get correct value at RT since it's not a case object

Additional Information

Const case objects never were supposed to compile, there is logic in the compiler to detect and reject that but as we can see it's not triggered.

however IMO compiler SHOULD support case objects, instead of outright disabling them. It makes more code usable at CT, even if above mentioned caveats; the workaround mentioned above works; whereas if we outright disabled const case objects, we'd have no easy equivalent of this (not without some contorsions at least)

const j = getFoo() # potentially expensive computation at CT; => only do it once
const x1 = j.x1
const x2 = j.x2

so let's fix it instead of disabling it :)

If there's one thing we may disable until this is fixed, it's the runtime access to a const case object field

@cooldome
Copy link
Member

I believe it is related issue
#13178.

@timotheecour
Copy link
Member Author

@cooldome I just tried your PR, it doesn't fix the above bug (note that this issue isn't using --gc:arc, and your PR mentions arc)

@cooldome
Copy link
Member

cooldome commented Oct 8, 2020

Retested on latest dev and it works for me.
@timotheecour please retest

timotheecour added a commit to timotheecour/Nim that referenced this issue Oct 8, 2020
@timotheecour
Copy link
Member Author

timotheecour commented Oct 8, 2020

  • just retested, the top example now works, the underlying problem is still there as shown in close #13081 #15529:
block: # bug #13081
  type Kind = enum
    k0, k1, k2, k3

  type Foo = object
    x0: float
    case kind: Kind
    of k0: discard
    of k1: x1: int
    of k2: x2: string
    of k3: x3: string

  const j1 = Foo(x0: 1.2, kind: k1, x1: 12)
  const j2 = Foo(x0: 1.3, kind: k2, x2: "abc")
  const j3 = Foo(x0: 1.3, kind: k3, x3: "abc2")
  static:
    doAssert $j1 == "(x0: 1.2, kind: k1, x1: 12)"
    doAssert $j2 == """(x0: 1.3, kind: k2, x2: "abc")"""
    doAssert $j3 == """(x0: 1.3, kind: k3, x3: "abc2")"""
  doAssert $j1 == "(x0: 1.2, kind: k1, x1: 12)"
  doAssert $j2 == """(x0: 1.3, kind: k2, x2: "abc")"""
  doAssert $j3 == """(x0: 1.3, kind: k3, x3: "abc2")"""

  when true:
    # BUG: this doesn't work yet
    # Error: unhandled exception: 'sons' is not accessible using discriminant 'kind' of type 'TNode' [FieldDefect]
    discard j1.x1
    static:
      # ditto
      discard j1.x1

note, this also affects #15528

  • we also need to git bisect to find which commit fixed at least the top example

EDIT: => #15532

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
VM see also `const` label
Projects
None yet
Development

No branches or pull requests

2 participants