Giter Site home page Giter Site logo

Comments (3)

travisstaloch avatar travisstaloch commented on July 28, 2024 1

I like the idea and would favor using lower camel case names in generated zig code. However, all of the other implementations I looked at used title camel case including go, cpp and python if i remember correctly. So I chose to follow them because its a little easier to compare generated code.

I'm not sure why this is a thing. I can see why the go impl does this as it follows the go style guide. But the cpp and python impls aren't following their style guides.

Maybe we could add a build or config option for changing the function casing? I would be glad to accept a PR for this. But I'd like to default to title case for now, at least until a day when this project is more trusted (known to work well) and comparing generated code shouldn't be necessary.

from flatbufferz.

clickingbuttons avatar clickingbuttons commented on July 28, 2024 1

Hey @travisstaloch , would you accept a PR refactoring src/codegen.zig?

Serious issues

  1. --no-gen-object-api still imports object API ...T types and Unpack functions
  2. Property name collisions with unions can lead to generated code calling non-existent functions. For example:
union Type { Null, Int }
table Field { type: Type }

generates a non-existent call to rcv.Type_Type() instead of the correct rcv.TypeType():

pub fn UnpackTo(rcv: Field, t: *FieldT, __pack_opts: fb.common.PackOptions) !void {
    if (rcv.Type_()) |_tab| {
        t.type = try TypeT.Unpack(rcv.Type_Type(), _tab, __pack_opts);
    }
}
  1. Object API optionals should be optional_field: ?T instead of optional_field: ?*T to prevent unnecessary allocation.
  2. Add error printing on invalid codegen (useful for when Zig breaks codegenned syntax and for codegen devs)

Style issues

  1. Remove _s from variable names. Zig purposefully disallows private struct members. Also, constants like __file_indent are already namespaced to the source file. Parameters like __builder can simply be renamed to builder.
  2. Consistently use self: Self instead of inconsitently rcv: {type} and self: {type}
  3. Add option for an index file with all generated types since there's a file generated per table/struct/union which can add up quickly.
  4. As for the casing, I think a reason for Title over snake_case is to prevent parameter name shadowing like:
pub fn rcv(rcv: Field) bool {
    ...
}

This can of course be solved by renaming the parameter to rcv_ or something similar but adds another layer of complexity.

Sorry to hijack this issue, let me know if there's a better way to contact you. I'm happy to make all these changes.

from flatbufferz.

travisstaloch avatar travisstaloch commented on July 28, 2024

Hey @travisstaloch , would you accept a PR refactoring src/codegen.zig?

Sorry so long. Looked at this earler today and forgot to respond.

These all sound like legitimate problems to me and I would gladly accpt a PR. 👍

I've created #3 with the text from above. Lets discuss there

from flatbufferz.

Related Issues (15)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.