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.

Sign up for GitHub

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

Jump to bottom

Add PIsData and PlutusType instances for PMap and PValue #161

Merged
merged 3 commits into from
Jan 21, 2022
Merged

Add PIsData and PlutusType instances for PMap and PValue #161

merged 3 commits into from
Jan 21, 2022

Conversation

TotallyNotChase
Copy link
Collaborator

Here it is, it works!

We're still missing PConstant/PLift instance for PMap and PValue - I can't figure out the error but I have documented it. With that said, I'd argue a PConstant/PLift instance isn't as important right now.

@TotallyNotChase TotallyNotChase mentioned this pull request Jan 21, 2022
Merged
@TotallyNotChase
Copy link
Collaborator Author

actually I think I can implement the remaining instances manually. Don't merge this yet

@L-as
Copy link
Member

L-as commented Jan 21, 2022

PLift a is necessary for PBuiltinList a.

t1lde
t1lde reviewed Jan 21, 2022
View reviewed changes
Copy link
Collaborator

@t1lde t1lde 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 👍 ,
If further compromise is needed just to get lifting for Value working, then it itself could just be monomorphically special-cased.

@TotallyNotChase
Copy link
Collaborator Author

Never mind, the PConstant and PLift instances must follow later. Faced some issues. This PR can be merged on its own.

If further compromise is needed just to get lifting for Value working, then it itself could just be monomorphically special-cased.

So, the instance for Value comes naturally and automatically from the deriver if PMap has the correct instances. I have tested this. It's just the implementation for PMap that I'm stuck on right now. Getting endless missing constraints.....

L-as
L-as reviewed Jan 21, 2022
View reviewed changes
Plutarch/Api/V1.hs
@@ -572,10 +571,15 @@ instance PIsDataRepr PDCert where

---------- AssocMap

newtype PMap (a :: PType) (b :: PType) (s :: S)
= PMap (Term s (PBuiltinMap a b))
newtype PMap (k :: PType) (v :: PType) (s :: S) = PMap (Term s (PBuiltinMap k v))
Copy link
Member

Choose a reason for hiding this comment

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

@t1lde why did you make a newtype for this BTW?

L-as
L-as approved these changes Jan 21, 2022
View reviewed changes
Copy link
Member

@L-as L-as left a comment

Choose a reason for hiding this comment

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

LGTM

@L-as L-as merged commit 62fa59d into staging Jan 21, 2022
@L-as L-as deleted the pmap-instances branch January 21, 2022 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@t1lde t1lde t1lde left review comments

@L-as L-as L-as approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@TotallyNotChase @L-as @t1lde