Use named fields for match frame members #848
Conversation
zherczeg
force-pushed
the
match_stack
branch
2 times, most recently
from
2ddd780 to
ea954db
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| mb->true_end_subject = mb->end_subject + P->temp_size; | ||
| Feptr = P->temp_sptr[1]; | ||
| F->fields.op_assert_scs.saved_end_subject = mb->end_subject; | ||
| mb->end_subject = P->fields.op_assert_scs.saved_end_subject; |
Check warning
Code scanning / clang
Dereference of null pointer Warning
There was a problem hiding this comment.
We can just ignore this. Clang's static analyzer is usually very helpful, but it has some spammy false positives in this file, due to the goto-based architecture. It thinks it's possible for there to be NULLs, because it can't follow all the conditions across the gotos.
NWilson
left a comment
NWilson
left a comment
There was a problem hiding this comment.
I like it! Better names is good.
| #define Lmin F->fields.char_repeat.min | ||
| #define Lmax F->fields.char_repeat.max | ||
| #define Lc F->fields.char_repeat.c | ||
| #define Loc F->fields.char_repeat.oc |
There was a problem hiding this comment.
Do you know why some of the defines are "Lsomething" and some are "Fsomething"? Is it just inconsistent naming?
There was a problem hiding this comment.
I suspect L is local, and F is frame. So L-s may or many not present in a frame, but F-s are always present and have the same meaning. Of course only @PhilipHazel knows the exact meaning. :)
There was a problem hiding this comment.
@zherczeg is correct. The naming is definitely deliberate. There is this comment in the code:
/* Define short names for general fields in the current backtrack frame, which
is always pointed to by the F variable. Occasional references to fields in
other frames are written out explicitly. There are also some fields in the
current frame whose names start with "temp" that are used for short-term,
localised backtracking memory. These are #defined with Lxxx names at the point
of use and undefined afterwards. */
There was a problem hiding this comment.
I have updated the comments.
| mb->true_end_subject = mb->end_subject + P->temp_size; | ||
| Feptr = P->temp_sptr[1]; | ||
| F->fields.op_assert_scs.saved_end_subject = mb->end_subject; | ||
| mb->end_subject = P->fields.op_assert_scs.saved_end_subject; |
There was a problem hiding this comment.
We can just ignore this. Clang's static analyzer is usually very helpful, but it has some spammy false positives in this file, due to the goto-based architecture. It thinks it's possible for there to be NULLs, because it can't follow all the conditions across the gotos.
zherczeg
force-pushed
the
match_stack
branch
from
ea954db to
d341ade
Compare
3 participants
I hoped the memory can be reduced this way, but no luck so far. The size before
eptris 80 bytes before and after the patch, since char_repeat uses all members. However, due to using a struct type, we might be able to reduce the size of some members in the future.