Skip to content

Conversation

@yav
Copy link
Member

@yav yav commented Nov 24, 2025

This PR wraps the vectors used to implement Crucible vector values with a newtype, which should allow us to support more representations in the future (e.g., vectors backed by SMT arrays, which would be useful for uninterpreted functions returning vectors)

@yav yav marked this pull request as draft November 24, 2025 23:34
Comment on lines +70 to +72
-- | We expect this to have one instance: for `RegValue'`.
-- We don't use that type directly to avoid recursive module dependencies.
class IsExprBuilder sym => IsRegValue sym f | f -> sym where
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you always expect this class to have exactly one instance? If so, my preference would be to move the definition of RegValue' to another module to avoid recursive module dependencies, rather than complicating all of the type signatures in this module with yet another type class.

Comment on lines +80 to +87
class MonadIO m => VecMonad sym m where

-- | We use this for partial operations (e.g., indexing a vector out of bounds).
vecAbort :: IsSymBackend sym bak => bak -> SimErrorReason -> m a

-- | A pre-condition on the current operation. Assertions we generate
-- are conditional on this.
vecPre :: IsExprBuilder sym => sym -> m (Pred sym)
Copy link
Contributor

Choose a reason for hiding this comment

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

For symmetry with the VecSize class, it might be worth factoring out the IsExprBuilder sym constraint into a superclass, i.e.,

Suggested change
class MonadIO m => VecMonad sym m where
-- | We use this for partial operations (e.g., indexing a vector out of bounds).
vecAbort :: IsSymBackend sym bak => bak -> SimErrorReason -> m a
-- | A pre-condition on the current operation. Assertions we generate
-- are conditional on this.
vecPre :: IsExprBuilder sym => sym -> m (Pred sym)
class (IsExprBuilder sym, MonadIO m) => VecMonad sym m where
-- | We use this for partial operations (e.g., indexing a vector out of bounds).
vecAbort :: IsSymBackend sym bak => bak -> SimErrorReason -> m a
-- | A pre-condition on the current operation. Assertions we generate
-- are conditional on this.
vecPre :: sym -> m (Pred sym)

Comment on lines +145 to +146
data VecVal (f :: CrucibleType -> Type) (tp :: CrucibleType) where
VecVal :: V.Vector (f tp) -> VecVal f tp
Copy link
Contributor

Choose a reason for hiding this comment

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

The name of this module is "VecValue", but this data type is named "VecVal". For consistency, it would be nice to use the same name throughout. My personal preference would be "VecValue", as it more closely aligns with the names of other types in Crucible (e.g., RegValue and AtomValue).

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.

3 participants