ziggy-pydust
ziggy-pydust copied to clipboard
Restore `zig build` for 0.13
@jburgy WoW! Nice comptime tricks in the src/discovery.zig. I am curious whether the deeply nested function calls at comptime may require bumping up the branch quota with @setEvalBranchQuota especially when binding large zig modules to python.
@gatesn @lwwmanning @delta003 @robert3005 as the members of Spiral listed on this repository, would one of you consider reviewing this PR?
Will make sure to have a look before EOW, likely during the weekend
@robert3005 I deleted commented out code or confirmed your understanding and closed the corresponding conversations. I left explanations without closing the remaining ones so they stand out, please take another look. Here are my replies to your overall comment:
Thanks a lot for the pr. I went through the type discovery and registration as that is the only part that actually changes where the rest is just api adjustments for that change. I think it's overly verbose for the user to explicitly pass the parent type every time. I wonder if we can change signature of the definition methods like
py.class, etc. to include the parent type.
I tried several variations but this is the only one that worked. Other attempts either suffered "circular dependency" or "closure capture of comptime var" errors.
For all the builtin types I'm not sure we need to pass root, i.e. does
PyStringrequire handle to the current state? It's a bit unfortunate that we must return new types now instead of rewriting existing types but that's zigs design choice. It would be cool if you could mutate types in comptime
Passing root to builtin types is required by the many calls to tramp.Trampoline in src/conversions.zig and src/pytypes.zig. I agree that it makes for an awkward API but that's the only permutation I could find.