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

[compiler] delete old binds implementation #14495

Merged
merged 1 commit into from
May 1, 2024

Conversation

patrick-schultz
Copy link
Collaborator

@patrick-schultz patrick-schultz commented Apr 22, 2024

Follows up on #14475 by deleting the old implementation of Bindings in favor of the new one, which has now been checked by CI to not change the behavior.

Copy link
Collaborator Author

patrick-schultz commented Apr 22, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @patrick-schultz and the rest of your teammates on Graphite Graphite

@patrick-schultz patrick-schultz force-pushed the ps-04-16-use_declarative_encoding_in_Binds branch from 8a060f9 to f1a3628 Compare April 23, 2024 16:50
@patrick-schultz patrick-schultz force-pushed the ps-04-18-delete_old_binds_implementation branch from e5dbfd8 to 0e3fa29 Compare April 23, 2024 16:50
@patrick-schultz patrick-schultz changed the base branch from ps-04-16-use_declarative_encoding_in_Binds to main April 23, 2024 19:53
@patrick-schultz patrick-schultz changed the base branch from main to ps-04-16-use_declarative_encoding_in_Binds April 24, 2024 13:53
@patrick-schultz patrick-schultz marked this pull request as ready for review April 24, 2024 14:13
@patrick-schultz patrick-schultz changed the title delete old binds implementation [compiler] delete old binds implementation Apr 24, 2024
Copy link
Collaborator

@ehigham ehigham left a comment

Choose a reason for hiding this comment

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

:shipit:

@patrick-schultz patrick-schultz force-pushed the ps-04-16-use_declarative_encoding_in_Binds branch from f1a3628 to 816819e Compare April 26, 2024 20:23
@patrick-schultz patrick-schultz force-pushed the ps-04-18-delete_old_binds_implementation branch from 0e3fa29 to f99f199 Compare April 26, 2024 20:23
@patrick-schultz patrick-schultz force-pushed the ps-04-16-use_declarative_encoding_in_Binds branch from 816819e to bf95f99 Compare May 1, 2024 17:17
@patrick-schultz patrick-schultz force-pushed the ps-04-18-delete_old_binds_implementation branch from f99f199 to d3760b9 Compare May 1, 2024 17:18
Base automatically changed from ps-04-16-use_declarative_encoding_in_Binds to main May 1, 2024 18:20
hail-ci-robot pushed a commit that referenced this pull request May 1, 2024
As a step towards coralling the specification of the binding structure
of the IR into one place, this rewrites `Bindings` to use only a single
method of the `GenericBindingEnv` interface, `newBlock`, which therefore
captures all possibilities of how a node can modify its parent's
environment in a child. Later work refactors this to return an object
encoding this modification, instead of returning a modified environment,
which allows the caller complete flexibility in how to maintain an
environment appropriately for their use case.

This PR leaves in the old `Bindings` implementation, with an assertion
that they agree. The PR stacked above this, #14495, deletes the old
implementation. This way CI asserts that this refactoring hasn't changed
any behavior.
@patrick-schultz patrick-schultz force-pushed the ps-04-18-delete_old_binds_implementation branch from d3760b9 to 04f9a52 Compare May 1, 2024 18:20
Copy link
Collaborator Author

Merge activity

  • May 1, 2:21 PM EDT: Graphite rebased this pull request after merging its parent, because this pull request is set to merge when ready.

@hail-ci-robot hail-ci-robot merged commit 1e85f9e into main May 1, 2024
2 checks passed
@hail-ci-robot hail-ci-robot deleted the ps-04-18-delete_old_binds_implementation branch May 1, 2024 19:13
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.

None yet

3 participants