refactor!: remove generic parameter from proof plans and exprs #339
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
iajoiner
reviewed
Nov 5, 2024
iajoiner
reviewed
Nov 5, 2024
iajoiner
reviewed
Nov 5, 2024
|
I addressed the review, fixed a bug, added tests to i256, and ran cargo fmt. I put all of these changes in one commit: e137206. I figured this will be the easiest way to review. I will rebase and clean up the commits before merging. If you want me to rebase now before you review, let me know. |
iajoiner
reviewed
Nov 6, 2024
iajoiner
approved these changes
Nov 6, 2024
…ant, remove `Scalar` generic parameter from `LiteralValue`. Note: these two changes have to come together.
…arget only) NOTE: in isolation, this commit will not fully pass CI check because it only applies to the lib target, and not tests, examples, or benches. The next two commits are the corresponding changes to these The generic parameter removal includes the `ProofPlan` and `ProofExpr` traits as well as all implementors of them, such as `DynProofPlan` and `DynProofExpr`
… target only) NOTE: in isolation, this commit will not fully pass CI check because it only applies to the lib target, and not tests, examples, or benches. The previous commit next commit are the corresponding changes to these The generic parameter removal includes the `ProofPlan` and `ProofExpr` traits as well as all implementors of them, such as `DynProofPlan` and `DynProofExpr`
…es and benches targets) NOTE: in isolation, this commit will not fully pass CI check because it only applies to the examples and benches target, and not lib or tests. The previous two commits are the corresponding changes to these targets The generic parameter removal includes the `ProofPlan` and `ProofExpr` traits as well as all implementors of them, such as `DynProofPlan` and `DynProofExpr`
JayWhite2357
force-pushed
the
refactor/concrete-proof-plan
branch
from
fe0a0cf
to
56f242d
Compare
|
🎉 This PR is included in version 0.36.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rationale for this change
The
DynProofPlantype does not need to have a generic parameter. It only has one as a relic of past requirements.What changes are included in this PR?
The first four commits remove the generic
Scalarparameter fromLiteralValue. In order to not useScalaras the underlying store for theDecimal75variant, anI256type is introduced. (This type could be extended to become the main 256-bit integer used in the crate, but this is left for later work.)The next 3 commits are the bulk of the PR and remove the generic parameter from the
ProofPlanandProofExprtraits as well as all implementers of them, such asDynProofPlanandDynProofExpr.Are these changes tested?
Yes. This is a refactoring change, and existing test pass.