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

SetupValue: Use proper TTG-style extension fields #1919

Merged
merged 7 commits into from
Sep 1, 2023
Merged

SetupValue: Use proper TTG-style extension fields #1919

merged 7 commits into from
Sep 1, 2023

Conversation

RyanGlScott
Copy link
Contributor

This removes all of the HasSetup* :: Bool type families in favor of new X* :: Type type families. These largely serve the same purpose of distinguishing which constructors of SetupValue are permissible in each language backend, but they also permit bundling additional, language-specific information into a SetupValue. For instance, the XSetupStruct instance for LLVM evaluates to Bool, representing whether we are dealing with an LLVM packed struct or not, and the XSetupCast instance for LLVM evaluates to an LLVM Type to represent the type to cast to.

One consequence of this design, aside from some SetupValue fields moving around, is that ppSetupValue now must perform case analysis on which language backend is currently in use in order to pretty-print any backend-specific data. This is accomplished with the new SAWExt data type and the corresponding IsExt class.

There is a lot of code that shifts around in this patch, but everything is still functionally equivalent. In subsequent patches, I will introduce additional data into MIR-specific extension fields.

Fixes #1914.

Comment on lines +26 to +27
module SAWScript.Crucible.Common.MethodSpec
( AllocIndex(..)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note to reviewers: this module still exports the same things that it did before this patch. I added an explicit export list here because this module now re-exports some things from SAWScript.Crucible.Common.Setup.Value, which requires using an export list to accomplish.

Comment on lines +28 to +29
module SAWScript.Crucible.JVM.MethodSpecIR
( JIdent
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note to reviewers: this module still exports the same things that it did before this patch. I added an explicit export list here because this module now re-exports some things from SAWScript.Crucible.JVM.Setup.Value, which requires using an export list to accomplish.

ppSetupValue :: Show (CastType ext) => SetupValue ext -> PP.Doc ann
-- Consider using 'resolveSetupValue' and printing the language-specific value
-- (e.g., an 'LLVMVal') with @PP.pretty@ instead.
ppSetupValue :: forall ext ann. IsExt ext => SetupValue ext -> PP.Doc ann
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding this IsExt constraint to ppSetupValue is probably the largest change in this patch, aside from changing the TTG-related type families, as I have to perform case analysis on which extension is being used in order to pretty-print SetupValues properly. This is primarily what motivated moving code around so much in this patch, as I needed to be able to import all of the language backends from SAWScript.Crucible.Common.MethodSpec without creating import cycles.

Comment on lines +120 to +125
-- 'True' if this is an LLVM packed struct, 'False' otherwise.
type instance Setup.XSetupStruct (LLVM _) = Bool
type instance Setup.XSetupArray (LLVM _) = ()
type instance Setup.XSetupElem (LLVM _) = ()
type instance Setup.XSetupField (LLVM _) = ()
type instance Setup.XSetupCast (LLVM _) = L.Type
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These XSetupStruct and XSetupCast instances are the big payoffs in this patch. They no longer evaluate to (), but rather to Bool/L.Type. Moreover, this information is only used in the LLVM backend, which is why we are able to, e.g., delete so many uses of "packed" in the SetupStruct cases for the JVM and MIR backends.

@RyanGlScott RyanGlScott mentioned this pull request Aug 30, 2023
Copy link
Contributor

@bboston7 bboston7 left a comment

Choose a reason for hiding this comment

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

Looks good! Nothing too surprising here.

src/SAWScript/Crucible/Common/Setup/Value.hs Outdated Show resolved Hide resolved
This is purely a refactor. This will be useful in a subsequent commit to avoid
import cycles.
…to *.Setup.Value modules

This is purely a refactor. This will become useful in a subsequent commit in
order to avoid import cycles.
This is purely a refactor. This will become useful in a subsequent commit in
order to avoid import cycles.
This removes all of the `HasSetup* :: Bool` type families in favor of new `X*
:: Type` type families. These largely serve the same purpose of distinguishing
which constructors of `SetupValue` are permissible in each language backend,
but they are permit bundling additional, language-specific information into a
`SetupValue`. For instance, the `XSetupStruct` instance for `LLVM` evaluates to
`Bool`, representing whether we are dealing with an LLVM packed struct or not,
and the `XSetupCast` instance for `LLVM` evaluates to an LLVM `Type` to
represent the type to cast to.

One consequence of this design, aside from some `SetupValue` fields moving
around, is that `ppSetupValue` now must perform case analysis on which language
backend is currently in use in order to pretty-print any backend-specific data.
This is accomplished with the new `SAWExt` data type and the corresponding
`IsExt` class.

There is a lot of code that shifts around in this patch, but everything is
still functionally equivalent. In a subsequent patch, I will introduce
additional data into MIR-specific extension fields.

Fixes #1914.
I've included a pointer to #1929 as well to indicate our intention to support
ghost state on other language backends.
@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 1, 2023
@mergify mergify bot merged commit 62100c0 into master Sep 1, 2023
@mergify mergify bot deleted the T1914 branch September 1, 2023 15:44
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.

Add Trees That Grow-style extension fields to SetupValue
2 participants