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

feedback request for refactoring of macros.hasCustomPragma #46

Closed
krux02 opened this issue Nov 1, 2019 · 6 comments
Closed

feedback request for refactoring of macros.hasCustomPragma #46

krux02 opened this issue Nov 1, 2019 · 6 comments
Assignees
Labels
bug Something isn't working help wanted Extra attention is needed imporant

Comments

@krux02
Copy link

krux02 commented Nov 1, 2019

I am currently refactoring macros.hasCustomPragma. Here is the PR, but when I try to compile tsqlite with that refactoring enabled I get the following error. Can you tell me what could have gone wrong?

tests/tsqlite.nim(49, 7) template/generic instantiation of `suite` from here
tests/tsqlite.nim(79, 8) template/generic instantiation of `test` from here
tests/tsqlite.nim(80, 5) template/generic instantiation of `withDb` from here
tests/tsqlite.nim(86, 39) Error: type mismatch: got <typeof(nil)>
but expected one of: 
proc dbValue(v: DbValue): DbValue
  first type mismatch at position: 1
  required type for v: DbValue
  but expression 'nil' is of type: typeof(nil)
proc dbValue(v: int | int32 | int64 | uint): DbValue
  first type mismatch at position: 1
  required type for v: int or int32 or int64 or uint
  but expression 'nil' is of type: typeof(nil)
proc dbValue(v: float): DbValue
  first type mismatch at position: 1
  required type for v: float
  but expression 'nil' is of type: typeof(nil)
proc dbValue(v: string): DbValue
  first type mismatch at position: 1
  required type for v: string
  but expression 'nil' is of type: typeof(nil)
proc dbValue(v: DbBlob): DbValue
  first type mismatch at position: 1
  required type for v: DbBlob
  but expression 'nil' is of type: typeof(nil)
proc dbValue(v: DbNull): DbValue
  first type mismatch at position: 1
  required type for v: DbNull
  but expression 'nil' is of type: typeof(nil)
proc dbValue(v: DateTime): DbValue
  first type mismatch at position: 1
  required type for v: DateTime
  but expression 'nil' is of type: typeof(nil)
proc dbValue[T](v: Option[T]): DbValue
  first type mismatch at position: 1
  required type for v: Option[dbValue.T]
  but expression 'nil' is of type: typeof(nil)
proc dbValue(v: bool): DbValue
  first type mismatch at position: 1
  required type for v: bool
  but expression 'nil' is of type: typeof(nil)
@moigagoo moigagoo self-assigned this Nov 2, 2019
@moigagoo moigagoo added bug Something isn't working help wanted Extra attention is needed imporant labels Nov 2, 2019
@moigagoo
Copy link
Owner

moigagoo commented Nov 2, 2019

Hello, Arne! Thanks for coming by.

Apparently, this has nothing to do with Norm's own code. In the trace, I can see this proc declaration:

proc dbValue(v: DbNull): DbValue
  first type mismatch at position: 1
  required type for v: DbNull
  but expression 'nil' is of type: typeof(nil)

However, it looks differently in ndb:

proc dbValue*(v: DbNull|type(nil)): DbValue =
  ## Wrap NULL value.
  ## Caveat: ``dbValue(nil)`` doesn't compile on Nim 0.19.x, see
  ## https://github.com/nim-lang/Nim/pull/9231.
  DbValue(kind: dvkNull)

As you can see, nil should be handled by this proc.

However, this PR nim-lang/Nim#9231 is referenced in the comment stating dbValue(nil) shouldn't compile in Nim 0.19. The referenced PR is already merged though, so the issue should be eliminated. Also, I swear it does compile in Nim 0.19 and 1.0 otherwise this test wouldn't have passed.

So, I have two hypotheses:

  1. Handling of type(nil) has been changed in Nim in a way that cancels the mentioned PR. Maybe, typeof(nil) must be used now?
  2. Your refactoring somehow affected handling of type(nil).

In both cases, it's dbValue(nil) in ndb that is broken, not Norm itself.

Probably should ask @xzfc's input too.

@xzfc
Copy link
Contributor

xzfc commented Nov 2, 2019

@krux02 It seems you have an outdated version of ndb in your nimble cache. Probably because nimble doesn't update @#head versions? Possible fix: rm -r ~/.nimble; nimble install 'ndb@#head'.


@moigagoo

Also, I swear it does compile in Nim 0.19 and 1.0 otherwise this test wouldn't have passed.

Maybe you mean 0.20 and 1.0? The first revision of Norm that mentions dbValue nil is 6911b37 and it was tested on Nim 0.20.0.

@moigagoo
Copy link
Owner

moigagoo commented Nov 2, 2019

@xzfc sure, my bad, I meant 0.20. Thanks for the correction.

@moigagoo
Copy link
Owner

moigagoo commented Nov 3, 2019

@krux02 could you please try running the tests now, after #47 is merged?

@krux02
Copy link
Author

krux02 commented Nov 3, 2019

I will do tomorrow, thanks for the reply.

@krux02
Copy link
Author

krux02 commented Nov 12, 2019

Thanks for the update. Tests are passing now.

@krux02 krux02 closed this as completed Nov 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed imporant
Projects
None yet
Development

No branches or pull requests

3 participants