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

mir_tuple_value #1934

Merged
merged 2 commits into from
Sep 11, 2023
Merged

mir_tuple_value #1934

merged 2 commits into from
Sep 11, 2023

Conversation

RyanGlScott
Copy link
Contributor

This adds support for a mir_tuple_value function in the MIR backend, which allows constructing tuple SetupValues. This is distinct from the notion of structs in MIR, so there are now separate SetupStruct and SetupTuple constructors for SetupValue to encode this distinction. At the moment, only the MIR backend makes use of SetupTuples, and they are not used at all in the LLVM or JVM backends.

In the SAW remote API, the "tuple" type of setup value previously referred to SetupStructs, but this becomes especially confusing when SAW supports both structs and tuples. There are now separate "struct" and "tuple" setup values to properly tell them apart.

This checks off one box in #1859.

RyanGlScott and others added 2 commits September 11, 2023 10:06
This adds support for a `mir_tuple_value` function in the MIR backend, which
allows constructing tuple `SetupValue`s. This is distinct from the notion of
structs in MIR, so there are now separate `SetupStruct` and `SetupTuple`
constructors for `SetupValue` to encode this distinction. At the moment, only
the MIR backend makes use of `SetupTuple`s, and they are not used at all in the
LLVM or JVM backends.

In the SAW remote API, the `"tuple"` type of `setup value` previously referred
to `SetupStruct`s, but this becomes especially confusing when SAW supports both
structs _and_ tuples. There are now separate `"struct"` and `"tuple"` `setup
value`s to properly tell them apart.

This checks off one box in #1859.
Comment on lines +20 to +22
* The old `"tuple"` `setup value` has been renamed to `"struct"`. This better
reflects its intended purpose of representing struct values. There is now a
new `"tuple"` `setup value` that is only used to represent MIR tuples.
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to confirm - this is not a user-facing change, right? It looks like the Python API was using the name struct/StructVal for the thing that was internally called "tuple", and only the internal name has changed.

I was going to ask whether users who forgot to update their scripts would get a useful error message, but if the Python API is actually unchanged, then it's a moot point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, the Python API remains unchanged. The only way this could break anything is if there are any other libraries out there somewhere that interface with the SAW remote API, but I'm not aware of any such libraries.

@RyanGlScott RyanGlScott added the PR: ready to merge Magic flag for pull requests to ask Mergify to merge given an approval and a successful CI run label Sep 11, 2023
@mergify mergify bot merged commit cca5d53 into master Sep 11, 2023
38 checks passed
@mergify mergify bot deleted the T1859-mir_tuple_value branch September 11, 2023 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: ready to merge Magic flag for pull requests to ask Mergify to merge given an approval and a successful CI run
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants