Backported fix for string.find recording - #10
Conversation
This is a squashed changeset of the following commits from tarantool/luajit repo: * 587532e * 98b1cfc Relates to LuaJIT gh-505
|
Oh, by the way, @igormunkin, could you please provide a link to the CI results for your patch? I cannot find any. |
| * Added MOVTVPRI optimization, which extends MOVTV for recording-time nil, false and true values | ||
| * Extended information about contributors | ||
| * Excluded movtv and movtvpri from -O4 | ||
| * Backported fix for string.find recording from tarantool/luajit repo |
There was a problem hiding this comment.
The fact that this is a backport is mentioned in a commit. Here, it would be more useful to describe what about string.find recording was fixed.
There was a problem hiding this comment.
IMHO, it's a bit excessive condering a conversation within the related LuaJIT issue.
There was a problem hiding this comment.
The line in ChangeLog doesn't point to an issue, though
There was a problem hiding this comment.
Commit message does. The issue can be obtained by blaming exactly this ChangeLog line.
There was a problem hiding this comment.
Then it's enough to write "Fix string.find fold machinery"
There was a problem hiding this comment.
No, it's a backporting of the specified commits. Yes, they are adjusted considering uJIT specific, but the original commits are from tarantool/luajit repo. The similar we have here. See no sense in the further conversation. Your move.
There was a problem hiding this comment.
The commit hash is not specified in ChangeLog, which is why I commented on that. So, for message to be helpful, it should either have a commit hash in it, or shortly describe what was fixed.
|
|
||
| $tester->run('find-fold-bug.lua', args => '-p-') | ||
| ->exit_ok | ||
| ->stdout_has_no(qr/TRACE.+?abort.?/) |
There was a problem hiding this comment.
Was the trace abort the only observable behaviour of this bug? I think there needs to be a check that fold transformations are not buggy (std_out_has_no(/* before fix output */))
There was a problem hiding this comment.
I guess it is enough. Previous implementation of string.find recording breaks the sematics while folding. Thereby we need to check whether a trace is assembled and the result is valid (see the assert in the corresponding lua chunk).
There was a problem hiding this comment.
Was there a trace abort before the change?
There was a problem hiding this comment.
There were no aborts before (please see the original issue and commits for more info).
@igelhaus, here you are: travis.
You can find them near green tick with "All check have passed" within details. |
|
@EinsamHauer, thanks a lot. |
4 participants
This is a squashed changeset of the following commits from
tarantool/luajit repo:
Relates to LuaJIT gh-505