flatbuffers icon indicating copy to clipboard operation
flatbuffers copied to clipboard

[Nim] Nim implementation

Open danlapid opened this issue 2 years ago • 11 comments

So this is a mostly working implementation for flat buffers in nim. Based on the existing languages (mostly - swift, python, lua) and on https://github.com/RecruitMain707/NimFlatbuffers Relevant issue: #5855

This is incomplete, I would like to add:

  1. more tests
  2. one file support
  3. object api
  4. Documentation

It's actually my first PR so I am opening it even before it's finished just to make sure what I'm doing is appropriate and okay with everyone. If I'm missing anything please tell me. Thanks!

danlapid avatar Jun 22 '22 14:06 danlapid

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

google-cla[bot] avatar Jun 22 '22 14:06 google-cla[bot]

Wow, thanks for the huge effort!

CasperN avatar Jun 23 '22 19:06 CasperN

So I’m almost finished with object api After that I’ll transfer everything to bfbs during which I will clean up the whole code, remove comments and weird code bits. I will ping you once I’m done so as not to continue wasting your time meanwhile :) @ @CasperN

danlapid avatar Jun 24 '22 14:06 danlapid

@CasperN So, I've come up a little short, it seems there's no support for circular dependencies in the language Examples like:

union Any { M1: Monster, M2: Monster, M3: Monster }

table Monster {
  any:Any;
}

When implementing the ObjectAPI then AnyT would refer to MonsterT and MonsterT would refer to AnyT

A possible solution I've found would be to force onefile If the code is declared in a single file then I can declare all types under single "type" statement which works for odd reasons:

type
  Any* {.pure.} = enum
    NONE = 0.uint8,
    M1 = 1.uint8,
    M2 = 2.uint8,
    M3 = 3.uint8,

  AnyT* {.union.} = ref object
    asM1*: MonsterT
    asM2*: MonsterT
    asM3*: MonsterT

  MonsterT* = ref object
    anyType*: Any
    any*: Option[AnyT]

  Monster* = object of FlatObj

Problem is, then we'll have to implement namespaces as underscores making MyGame.Example.Monster -> MyGame_Example_Monster for every usage of it. Otherwise we'll possibly have circular dependencies between the "one file"s of every namespace

I'd love to know your opinion. Thanks

danlapid avatar Jun 25 '22 08:06 danlapid

Alternatively I could implement "namespaces" in one file with backticks thus the on file will look like:

type
  `Namespace1.Any`* {.pure.} = enum
    NONE = 0.uint8,
    M1 = 1.uint8,
    M2 = 2.uint8,
    M3 = 3.uint8,

  `Namespace1.AnyT`* {.union.} = ref object
    asM1*: `Namespace2.MonsterT`
    asM2*: `Namespace2.MonsterT`
    asM3*: `Namespace2.MonsterT`


  `Namespace2.MonsterT`* = ref object
    anyType*: `Namespace1.Any`
    any*: Option[`Namespace1.AnyT`]

  `Namespace2.Monster`* = object of FlatObj

And the usage will look like:

import RecursiveImports_generated
var fbb = newBuilder(0)
fbb.MonsterStart()
let root = fbb.MonsterEnd()
fbb.Finish(root)

var defaults: `Namespace2.Monster`
defaults.GetRootAs(fbb.FinishedBytes(), 0)
var defaultst: `Namespace2.MonsterT`
defaultst.InitFromBuf(fbb.FinishedBytes(), 0)

Again, not sure what to think as to what's better Just to clarify, all of this is relevant only to the object_based_api which causes all these recursive imports We could allow the multiple files when there's no object_based_api and only force one file for object_based_api

danlapid avatar Jun 25 '22 13:06 danlapid

Hm, I am not a nim expert so I cant say what is most ergonomic for users of the language.

Does nim support tricks like defining everything in one file then importing it into a namespace hierarchy that reexports everything?

It's interesting that only the object API forces these recursive imports, the normal API definitely needs to know the types of all the fields....... maybe its a distinction between returning a type vs having a type as field?

CasperN avatar Jun 30 '22 14:06 CasperN

@danlapid Just checking in on this work. Would love to have it.

dbaileychess avatar Aug 06 '22 05:08 dbaileychess

I haven’t had time to work on it lately As is it works great either without object api or with object api but no recursive references. Obviously I would love to get it to work flawlessly always but considering nim doesn’t really support type forward declaration nor recursive imports I currently don’t have a good idea on how to get this done. The leading option would be a single file with all of the implementation and then re-exporting directory tree but it still requires a bit more work I need to finish on another branch.

danlapid avatar Aug 06 '22 05:08 danlapid

I would prefer having nim supported with those caveats, no need to get things perfect in the first iteration

CasperN avatar Aug 08 '22 14:08 CasperN

I am looking forward to this, but will want to postpone any merge until after 2.0.7 is release.

dbaileychess avatar Aug 15 '22 16:08 dbaileychess

@danlapid I hate to be this person, but i think I only want to accept new implementations if they follow the BFBS code generation. See how this is done with the Lua codegenerator.

This probably will make your code easier to write, but since you already spent a lot of time writing idl_nim_gen.cc it may take a while to port over.

dbaileychess avatar Sep 12 '22 17:09 dbaileychess