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

Boolean operators should return true for successful evaluation #144

Merged
merged 2 commits into from
Mar 18, 2021

Conversation

jefft0
Copy link
Collaborator

@jefft0 jefft0 commented Mar 17, 2021

A guard can be a boolean expression. As an example, here is a simple guard using the boolean operator for equals:

(= false false)

The value of this boolean expression is true, so the guard succeeds. But now consider:

(= (> 1 2) false)

The first expression, (> 1 2) evaluates to false. So equals should check false equals false, and the equals expression result should be true as before. But this is not the current behavior. The bug is in the code for all boolean operators.

To show the cause of the bug, first consider the arithmetic operator for logarithm, which works correctly.

bool log(const Context &context, uint16 &index)

Note that the operator returns a boolean value which is true if the expression was evaluated successfully or false for a problem such as arguments of the wrong type. You can see that log returns false if the argument isn't a float. The actual result of the expression is placed in the context object.

Now consider the "greater than" operator used above. It successfully evaluates the expression result r and places the result in the context object. Because it was successfully evaluated it should return true. But instead, it returns r, which is this case is false. The guard evaluator sees that an expression failed to be evaluated, so the guard fails, even though it should succeed.

This pull request has two commits. The first commit updates the guard evaluator for programs and HLP (models and composite states) to independently check the result of the expression. If the result type is boolean, and the value is false, then the guard fails. (We have to check the type of the expression result because models and composite states can have assignment guards which are not boolean.)

The second commit updates the boolean operators to return true for successful evaluation, even if the expression value is false. Now, The code for the "greater than" operator evaluates (> 1 2) and returns true (for successful evaluation). The expression value of false is stored in the context object. Then the code for the "equals" operator evaluates (= (> 1 2) false). It gets the value of false from the context object of the first sub expression, compares it to false and stores the expression result of true in the context, as expected.

jefft0 added 2 commits March 17, 2021 14:30
…separately check for boolean guard result. See pull request #144.
@jefft0 jefft0 added the bug Something isn't working label Mar 17, 2021
@jefft0 jefft0 merged commit 7c3fdea into master Mar 18, 2021
@jefft0 jefft0 deleted the evaluate-boolean-guard branch March 18, 2021 17:02
jefft0 added a commit that referenced this pull request Mar 24, 2021
…separately check for boolean guard result. See pull request #144.
jefft0 added a commit that referenced this pull request Mar 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant