nimdbx icon indicating copy to clipboard operation
nimdbx copied to clipboard

Stop using notnil until it's more stable

Open aleclarson opened this issue 3 years ago • 5 comments

I found your thread: https://forum.nim-lang.org/t/6301#38854

I'm trying to use getCollectionOrNil but I can't prove to the compiler that the collection is non-nil by using raise within a if col == nil statement (even if I nest it in an else block).

proc getCollection*(query: OpleQuery, name: string): Collection not nil =
  let col = query.database.openCollectionOrNil name
  if col == nil:
    raise newException(Defect, "bad ref")
  else: col

aleclarson avatar Jul 09 '21 02:07 aleclarson

Oh wait, I went full noob there. Just need to remove the not nil in my own proc 😅

aleclarson avatar Jul 09 '21 02:07 aleclarson

Interestingly, I get a cannot prove "x" is not nil error when I remove not nil from my proc...

proc hasDocument*(docRef: OpleRef) {.query.} =
  let col = query.getCollection(docRef.collection)
  let doc = col.with(query.snapshot).get docRef.id
  #         ^ cannot prove 'col' is not nil
  return \doc.exists

aleclarson avatar Jul 09 '21 02:07 aleclarson

Okay, here's my workaround:

proc getCollection*(query: OpleQuery, name: string): Collection not nil =
  var col = query.database.openCollectionOrNil name
  if col == nil:
    raise newException(Defect, "bad ref")
  # HACK: this proves the result is non-nil
  if col == nil: query.database.openCollection name
  else: col

The reason I'm not just using openCollection is that I have to throw a custom exception. :)

aleclarson avatar Jul 09 '21 02:07 aleclarson

Reopening, as it would be nice if with did not require proof of not nil for the snapshot and collection arguments.

aleclarson avatar Jul 09 '21 03:07 aleclarson

The flakiness of not nil is a primary reason I’m not using Nim much anymore, TBH. It’s an important feature for me, but (as shown here) it’s too frustrating to use in real code.

I had a (sort of heated) exchange with Araq on the Nim forum a few months ago in which he basically said that the control flow analysis behind the “cannot prove…” error is incomplete — it doesn’t consider return statements — but he has no plans to fix that because he doesn’t think people should be using return. 🤯

snej avatar Apr 08 '23 19:04 snej