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

Compile time failure for chained comparison expressions #587

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jasonpaulos
Copy link
Contributor

@tzaffi pointed out that the code Int(1) != Int(2) == Int(3) is valid Python, yet it produces an unexpected result.

The Python docs explain that a chained comparison of the form a op b op c should result in a op b and b op c. Yet for the above example, we actually get Int(2) == Int(3) (and this may rely on undefined behavior).

The core problem is that Python uses the native and operator to combine chained comparisons. Since this operator cannot be override, we can't produce correct code for chained comparisons in PyTeal.

This PR contains a potential solution to the issue by forbidding chained comparisons. It makes Expr.__bool__ raise an error, which means using the native and operator on two PyTeal expressions is no longer possible.

To get this out of draft status, the following are needed:

  • Unit tests to ensure the forbidden behavior actually produces the right error
  • Thorough investigation of the codebase to ensure that all of our expression boolean coercion has been changed. I only found and changed what our unit tests cover, which may not be everything.
  • Consideration for whether this changes breaks expected behavior for our consumers. If we commonly coerced expressions to booleans, perhaps our users do as well.

Copy link
Contributor

@tzaffi tzaffi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good. You've suggested some tasks which seem sensible. In particular, adding a unit test for the following broken examples seems prudent:

        # PyTEAL bugs - the following don't get parsed as expected!

        simple_surprising = pt.Int(1) != pt.Int(2) == pt.Int(3)

        complex_surprising = (
            pt.Int(1) + pt.Int(2) - pt.Int(3) * pt.Int(4) / pt.Int(5) % pt.Int(6)
            < pt.Int(7)
            > pt.Int(8)
            <= pt.Int(9) ** pt.Int(10)
            != pt.Int(11)
            == pt.Int(12)
        )

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.

2 participants