Skip to content

Conversation

@bettinaheim
Copy link
Collaborator

@bettinaheim bettinaheim commented Oct 22, 2025

... will update this description shortly. Below are some (by now very incomplete) notes.

This PR

  • introduces a proper value stack that makes sure that each node in the AST gets the values associated with it
  • makes the value stack/visit automatically validate that are values were processed and that no node pushes more than one value
  • changes range and enumerate to create a proper stdvec rather than pushing an array and size as two separate values to the value stack
  • changes the attribute visit to not process built-in noise channels; these channels can only be used within calls to apply_noise and are handled there to simplify code and avoid pushing multiple separate values for the same node
  • cc.Arrays are now exclusively used only as the underlying data of vectors and for strings; currently, no node visit will produce and array. If that is changed in the future, a bunch other other node visits will need to be updated to handle arrays appropriately.

Bug fixes:

  • Fixes an issue where the length of some ranges was not computed correctly, which can lead to segfaults
  • Fixes and issue where boolean expressions with multiple operands (a and b and c) were not correctly processed past the first two operands

Other clean up:

  • conversions to bool are handled by a shared helper function
  • cleaning up loop handling; don't attach 'invariant' attribute for loops that may not be invariant and eliminate code duplication in visit_for and visit_while

TODO:

  • enable tests that previously weren't being processed properly
  • write a proper set of tests for boolean expressions
  • it would be nice to isolate logic around qvec, stdvec and arrays into helpers that the visits call to make it easier to change.

Signed-off-by: Bettina Heim <[email protected]>
Signed-off-by: Bettina Heim <[email protected]>
Signed-off-by: Bettina Heim <[email protected]>
Signed-off-by: Bettina Heim <[email protected]>
Signed-off-by: Bettina Heim <[email protected]>
Signed-off-by: Bettina Heim <[email protected]>
Signed-off-by: Bettina Heim <[email protected]>
Signed-off-by: Bettina Heim <[email protected]>
Signed-off-by: Bettina Heim <[email protected]>
Signed-off-by: Bettina Heim <[email protected]>
Signed-off-by: Bettina Heim <[email protected]>
Signed-off-by: Bettina Heim <[email protected]>
Signed-off-by: Bettina Heim <[email protected]>
Signed-off-by: Bettina Heim <[email protected]>
Signed-off-by: Bettina Heim <[email protected]>
Signed-off-by: Bettina Heim <[email protected]>
Signed-off-by: Bettina Heim <[email protected]>
Signed-off-by: Bettina Heim <[email protected]>
Signed-off-by: Bettina Heim <[email protected]>
Signed-off-by: Bettina Heim <[email protected]>
Signed-off-by: Bettina Heim <[email protected]>
if __name__ == "__main__":
loc = os.path.abspath(__file__)
pytest.main([loc, "-rP"])
# ============================================================================ #
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems like the GitHub diff UI is choking here as well.

Signed-off-by: Bettina Heim <[email protected]>
Signed-off-by: Bettina Heim <[email protected]>
@bettinaheim bettinaheim requested a review from bmhowe23 as a code owner November 7, 2025 16:27
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.

4 participants