Skip to content

FpySequencer push FW_SERIALIZE_TRUE/FALSE for boolean ops#4712

Open
zimri-leisher wants to merge 3 commits intonasa:develfrom
zimri-leisher:4050-fpy-bool-val
Open

FpySequencer push FW_SERIALIZE_TRUE/FALSE for boolean ops#4712
zimri-leisher wants to merge 3 commits intonasa:develfrom
zimri-leisher:4050-fpy-bool-val

Conversation

@zimri-leisher
Copy link
Collaborator

…dx tlm to debug

Related Issue(s) #4050
Has Unit Tests (y/n) y
Documentation Included (y/n) y
Generative AI was used in this contribution (y/n) y

Change Description

  • Instead of pushing 1 or 0 to the stack, operators that push a boolean value to the stack now push the FW_SERIALIZE_TRUE_VALUE or FW_SERIALIZE_FALSE_VALUE
  • Add a Debug_NextStatementIndex telemetry point so that you can always see what statement is next without looking at events while debugging a sequence

Rationale

This change fixes a bug where (1 == 1) == True evaluates to False, because the lhs pushes 1, and the rhs pushes FW_SERIALIZE_TRUE_VALUE. They would be compared with MEMCMP, which sees two diff binary values.

AI Usage (see policy)

AI wrote all the code and tests.

F64 lhs = this->m_runtime.stack.pop<F64>();
// eq is true if they are equal and neither is nan
this->m_runtime.stack.push(static_cast<U8>((lhs == rhs) ? 1 : 0));
this->m_runtime.stack.push(static_cast<U8>((lhs == rhs) ? static_cast<U8>(FW_SERIALIZE_TRUE_VALUE)

Check notice

Code scanning / CodeQL

Equality test on floating-point values Note

Equality checks on floating point values can yield unexpected results.
F64 lhs = this->m_runtime.stack.pop<F64>();
// ne is true if they are not equal or either is nan
this->m_runtime.stack.push(static_cast<U8>((lhs != rhs) ? 1 : 0));
this->m_runtime.stack.push(static_cast<U8>((lhs != rhs) ? static_cast<U8>(FW_SERIALIZE_TRUE_VALUE)

Check notice

Code scanning / CodeQL

Equality test on floating-point values Note

Equality checks on floating point values can yield unexpected results.
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

Successfully merging this pull request may close these issues.

1 participant