Adds non allocating alternatives to ZON parse functions#22916
Conversation
|
Why not just change the only functions to have an
and
Aside from I haven't even made a PR to zig yet, so feel free to disregard if this doesn't make sense to you. |
That's a good suggestion, I like that it's consistent w/ the rest of the standard library and that the "default" is non allocating as you mentioned. Right now it's a little weird because all of these functions technically allocate since they all allocate an AST. I'm considering writing a parser that doesn't generate an intermediate AST, if I do that then the suggested naming scheme will become unambiguous and since removing the allocator arg is already a breaking change anyway so I could rename them at that point. Alternatively, I could adopt these names now in anticipation of that, but it'd be a bit weird in the interim/if plans change. (If I do write a parser that skips the AST, I'll make sure the use case in #22973 is still supported.) |
|
@MasonRemaley I think the naming suggestion above is a good one -- at least, better than what this PR currently does (not your fault; as always, naming is a hard problem!). Mind making that change? I'll probably be happy to merge this then. |
6868862 to
de20452
Compare
|
Done! |
Head branch was pushed to by a user without write access
de20452 to
2bc4938
Compare
|
Should be mergeable after a rebase to resolve the conflict, I think? |
|
Whoops, I totally forgot this wasn't merged. I'll go ahead and do the rebase in a minute. |
2bc4938 to
a9b2dd5
Compare
|
Rebased but I still wanna tidy up the tests a bit. |
|
Done, should be ready to merge once CI passes! |
Adds the following functions to
std.zon.parse:fromSliceFlatfromZoirFlatfromZoirNodeFlatThese are identical to the existing functions with "flat" stripped from the name, but they assert at compile time that the result type doesn't contain a pointer and therefore doesn't need to be freed. As such, the latter two functions do not require an allocator as an argument. (
fromSlicestill does as it needs to allocate theAstandZoir.)I'm open to alternative suggestions for the names. I think "flat" as in "flat in memory" is pretty reasonable, though it could be misinterpreted as to do with nesting. I would have simply named them
noAlloc, but this is confusing sincefromSliceFlatdoes in fact create (temporary) allocations.Replaces #22835, CC @NicoElbers (thanks for the reminder that I wanted to do this!)