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

Refactor and simplify code to not use Data.Has #903

Merged
merged 2 commits into from
Jan 12, 2023
Merged

Refactor and simplify code to not use Data.Has #903

merged 2 commits into from
Jan 12, 2023

Conversation

arcz
Copy link
Member

@arcz arcz commented Jan 11, 2023

We've been using Data.Has extensively and it's been a significant pattern in our code. While it allows for more abstract code to be written, it's not a commonly used library and it comes at a cost of substantial mental load as it was often hard to understand what the code is doing.

This PR removes all Data.Has from our code and replaces it with more common patterns, in the hope to improve the project's maintenance and development.

I also enabled the OverloadedRecordDot extension that comes with GHC 9.2 that somewhat fixes the records so they can be accessed like in "normal" languages 😄.

@@ -40,7 +40,7 @@ mutator Deletion = deleteRandList
selectAndMutate :: MonadRandom m
=> ([Tx] -> m [Tx]) -> Corpus -> m [Tx]
selectAndMutate f ctxs = do
rtxs <- weighted $ map (\(i, txs) -> (txs, fromInteger i)) $ DS.toDescList ctxs
rtxs <- weighted $ map (\(i, txs) -> (txs, fromIntegral i)) $ DS.toDescList ctxs
Copy link
Member

Choose a reason for hiding this comment

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

Why this was changed?

Copy link
Member Author

Choose a reason for hiding this comment

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

I changed Integer to Int in the Corpus type type Corpus = Set (Int, [Tx]) as we don't need an unbounded Integer there.

Copy link
Member

Choose a reason for hiding this comment

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

Great

return a

-- | Transform an EVM action from HEVM to our MonadState VM
fromEVM :: MonadState VM m => EVM a -> m a
Copy link
Member

Choose a reason for hiding this comment

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

Should we move this to a different file?

Copy link
Member Author

Choose a reason for hiding this comment

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

what about moving it to Echidna.Types?

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good.

@arcz arcz merged commit 8dc4d40 into master Jan 12, 2023
@arcz arcz deleted the less-has branch January 12, 2023 22:56
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.

2 participants