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

Derive IsPlutusData #56

Merged
merged 12 commits into from
Nov 4, 2024
Merged

Conversation

chfanghr
Copy link
Contributor

@chfanghr chfanghr commented Oct 8, 2024

  • implement derive macro forIsPlutusData.
  • Move PlutusData and IsPlutusData to a crate plutus-data. The reason was that there's not a unified way to reference types and functions in the macro otherwise.
  • Fixed a few bugs in error messages of instances for "primitive" types; improved parsing logic.
  • Replace as many manually written IsPlutusData instances in plutus-ledger-api with derivation as possible.
  • All tests passed, including golden tests, to verify the correctness of the generated instances.

TODOs:

  • Test the derive macro itself, mainly edge cases

@chfanghr chfanghr changed the base branch from main to szg251/improvements October 8, 2024 13:13
@chfanghr chfanghr force-pushed the connor/is_plutus_data_derive branch 3 times, most recently from bf66073 to dddfbb6 Compare October 14, 2024 12:02
@chfanghr chfanghr marked this pull request as ready for review October 14, 2024 12:02
@chfanghr chfanghr requested review from szg251 and bladyjoker October 14, 2024 12:02
@chfanghr chfanghr force-pushed the connor/is_plutus_data_derive branch from dddfbb6 to 2e4e643 Compare October 14, 2024 12:04
@chfanghr chfanghr force-pushed the connor/is_plutus_data_derive branch from 2e4e643 to 5a0c33a Compare October 17, 2024 16:55
@chfanghr chfanghr force-pushed the connor/is_plutus_data_derive branch from 5a0c33a to c7812dd Compare October 17, 2024 17:04
@szg251
Copy link
Collaborator

szg251 commented Oct 21, 2024

Move PlutusData and IsPlutusData to a crate plutus-data. The reason was that there's not a unified way to reference types and functions in the macro otherwise.

Personally, I'd prefer to have a use crate as plutus_ledger_api; in every module in plutus-ledger-api where we're using the macro, instead of having a separate package.

@chfanghr
Copy link
Contributor Author

@szg251 Done. I don't know how to implement the derive feature flag tho.

@chfanghr chfanghr force-pushed the connor/is_plutus_data_derive branch from a4ef319 to 32b00e6 Compare October 21, 2024 12:46

#[derive(Debug, thiserror::Error)]
enum DeriveStrategyError {
#[error("Unknown strategy {0}. Should be one of Newtype, List and Constr.")]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
#[error("Unknown strategy {0}. Should be one of Newtype, List and Constr.")]
#[error("Unknown strategy {0}. Should be one of Newtype, List or Constr.")]

is-plutus-data-derive/src/derive_impl.rs Show resolved Hide resolved
@szg251 szg251 merged commit 62a1ce5 into szg251/improvements Nov 4, 2024
2 checks passed
@szg251 szg251 deleted the connor/is_plutus_data_derive branch November 4, 2024 11:09
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