Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong info in docs on OP_TEST and OP_TESTSET #184

Closed
Kristopher38 opened this issue Apr 5, 2020 · 5 comments
Closed

Wrong info in docs on OP_TEST and OP_TESTSET #184

Kristopher38 opened this issue Apr 5, 2020 · 5 comments

Comments

@Kristopher38
Copy link

This issue is regarding Lua bytecode reference docs. In the section on OP_TEST and OP_TESTSET there is a line that states:

For TESTSET, register R(B) is coerced into a boolean and compared to the boolean field C. If R(B) matches C, the next instruction is skipped, otherwise R(B) is assigned to R(A) and the VM continues with the next instruction.

And a pseudocode:

TEST        A C     if not (R(A) <=> C) then pc++
TESTSET     A B C   if (R(B) <=> C) then R(A) := R(B) else pc++

But closely reading the Lua source for version 5.3.5, where the handling of OP_TEST and OP_TESTSET happens, we can conclude that the info in the docs is the wrong way around. Let me explain that with OP_TESTSET (the same applies to OP_TEST as well, just without R(A) = R(B) and the argument slots are different as stated in the docs). In the lvm.c the code for handling OP_TESTSET is as follows:

if (GETARG_C(i) ? l_isfalse(rb) : !l_isfalse(rb))
  ci->u.l.savedpc++;
else {
  setobjs2s(L, ra, rb);
  donextjump(ci);
}

Now let me rewrite that in pseudocode (value in R(B) is coerced into a Lua boolean):

if C == false then
  if R(B) == true then
    skip next instruction
  else -- R(B) == false
    R(A) = R(B)
  end
else -- C == true
  if R(B) == true then
    R(A) = R(B)
  else -- R(B) == false
    skip next instruction
  end
end

Which essentially means:

if C is different than R(B) then
  skip next instruction
else -- C is the same as R(B)
  R(A) = R(B)
end

But the docs clearly state the opposite:

If R(B) matches C, the next instruction is skipped, otherwise R(B) is assigned to R(A)

Paragraphs on OP_TEST and OP_TESTSET should be corrected to not be misleading.

@dibyendumajumdar
Copy link
Owner

Thank you @Kristopher38 appreciate the feedback. I will check and correct.

@dibyendumajumdar
Copy link
Owner

Hi @Kristopher38, strange as the comment in lopcodes.h must be wrong then?
You may be right about this, I need to go through some examples to verify.

@dibyendumajumdar
Copy link
Owner

Hi @Kristopher38 Please would you review https://github.com/dibyendumajumdar/ravi/blob/master/readthedocs/lua_bytecode_reference.rst? I am still working through the various examples of bytecode

@Kristopher38
Copy link
Author

Hi, I've read through the commits and they look correct to me. The updated bytecode explanations will especially help at getting a better grasp of what exactly is the execution flow of those opcodes.

Also thanks for creating this reference manual and keeping it up to date :)

dibyendumajumdar added a commit that referenced this issue Apr 18, 2020
@dibyendumajumdar
Copy link
Owner

@Kristopher38 Thank you for reporting the issue and also for reviewing the changes. Really appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants