Conversation
tenderlove
left a comment
tenderlove
left a comment
There was a problem hiding this comment.
I think this looks good. The arena implementation seems fine, I left a few minor commit suggestions but I don't see any actual problems.
@jhawthorn do you mind taking a look at this (if you have time)? Otherwise I'm going to merge it.
| * #endif | ||
| * ``` | ||
| */ | ||
|
|
| } | ||
| case PM_TOKEN_QUESTION_MARK: { | ||
| context_push(parser, PM_CONTEXT_TERNARY); | ||
|
|
| .warn_mismatched_indentation = true | ||
| }; | ||
|
|
||
| // TODO: find a better starting size |
There was a problem hiding this comment.
👍
4k is probably a fine starting size, maybe we can add some logging later and figure out a better start.
|
|
||
| *stack = current->prev; | ||
| xfree(current); | ||
| // xfree(current); |
|
@tenderlove make sure you don't merge this before we get this working in ruby/ruby using the new API, otherwise this is going to break a bunch of stuff. I've been holding off on merging this because I want to make sure we can do some benchmarking on both speed and memory from within CRuby before we go this route, since it's a such an invasive change. |
|
I'll start testing this in ruby/ruby and see what changes are necessary. In the meantime I'm going to label this PR as "blocked" just to indicate that it should not be merged until it is fully tested over there. |
4 participants
Rebased from #2913 into main