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

Nim object variant issue #1286

Closed
dom96 opened this issue Jun 18, 2014 · 8 comments

Comments

Projects
None yet
7 participants
@dom96
Copy link
Member

commented Jun 18, 2014

https://github.com/dom96/jester/blob/master/jester.nim#L24

i.e.:

type
  TJester = object
    isHttp*: bool
    case isAsync: bool
    of true:
      asyncHTTP: PAsyncHTTPServer
      asyncSCGI: PAsyncScgiState
    of false:
      dummyA, dummyB: pointer # workaround a Nimrod API issue
      s: TServer
      scgiServer: TScgiState
@def-

This comment has been minimized.

Copy link
Member

commented Jun 21, 2014

Might be the same issue as #1275

@dom96 dom96 added this to the 0.9.6 milestone Aug 14, 2014

@dom96 dom96 removed this from the 0.9.6 milestone Aug 15, 2014

@yglukhov

This comment has been minimized.

Copy link
Member

commented Jan 24, 2017

Its kinda unclear what this issue is about. @dom96, is it still actual?

@dom96

This comment has been minimized.

Copy link
Member Author

commented Jan 25, 2017

Not sure. @Araq will know. It's regarding this line:

dummyA, dummyB: pointer # workaround a Nimrod API issue

@Araq

This comment has been minimized.

Copy link
Member

commented Jan 25, 2017

It's in fact one top priority in my todo.txt for version 1 to fix this language problem. It's definitely still an issue.

@Araq Araq changed the title Nimrod object variant issue Nim object variant issue Apr 4, 2018

@andreaferretti

This comment has been minimized.

Copy link
Collaborator

commented Jun 28, 2018

I still do not understand why these two dummy pointers are needed, @dom96 can you write a line of explanation?

@dom96 dom96 added this to the v1 milestone Jul 3, 2018

@dom96

This comment has been minimized.

Copy link
Member Author

commented Jul 3, 2018

Araq can explain it better than I can.

@Araq

This comment has been minimized.

Copy link
Member

commented Jul 4, 2018

The transition from false to true in the object variant doesn't trigger a system.reset which is why these two dummy pointers are required in order to not break memory safety.

Araq added a commit that referenced this issue May 26, 2019

@Araq Araq closed this in 49e686a May 27, 2019

@genotrance

This comment has been minimized.

Copy link
Contributor

commented May 31, 2019

This change is a little last minute and breaks nimble in multiple places. I am creating a PR to fix but it might break several others. Also, it doesn't help that it fails at runtime rather than compile time so you never know until a certain code path is exercised.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.