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

Remove polymorphism from As_prover.t, hoist Typ.t #854

Merged
merged 16 commits into from
Dec 16, 2024

Conversation

mrmr1993
Copy link
Member

@mrmr1993 mrmr1993 commented Dec 16, 2024

This PR bundles a few changes. This can be broken into smaller pieces by commit if needed.

The goal of this PR is to remove the generalisation over Field.t from Checked.t and As_prover.t, allowing us to hoist Typ.t from the global implementation in Types. The result is a unique Typ.t type for each instantiation of snarky, allowing us to generate it from any checked backend (and in particular from a rust backend).

To achieve this, we

  • create a new helper functor Types.Make_types
  • remove the free field argument from As_prover.t
  • replace uses of the concretised types with those from the shared Types module signature
  • remove some unused or now-unnecessary type and functor machinery.

The recommended way to review this PR is to 'ignore whitespace' in the GitHub UI (or equivalently using git diff --ignore-all-space), to ignore the indentation changes resulting from functorisation.

module Handle = struct
let value (t : ('var, 'value) Handle.t) : ('value, 'field) t =
fun _ -> Option.value_exn t.value
type nonrec ('a, 'f) t = ('a, 'f) Types.Provider.t
Copy link
Member

Choose a reason for hiding this comment

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

It was confusing first that the type changes. But ok :).

@@ -1,46 +1,5 @@
open Core_kernel

module Data_spec0 = struct
Copy link
Member

Choose a reason for hiding this comment

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

Yay.

@@ -58,8 +58,6 @@ module Typ = struct
('var, 'value, 'aux, 'field, 'checked) typ'
-> ('var, 'value, 'field, 'checked) typ
end

include T
Copy link
Member

Choose a reason for hiding this comment

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

Yay.

@@ -15,7 +15,41 @@ module Simple_types = struct
end

module Typ = struct
include Types.Typ.T
Copy link
Member

Choose a reason for hiding this comment

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

Yay.


val run : 'a t -> field Run_state.t -> field Run_state.t * 'a
end

module Unextend
Copy link
Member

Choose a reason for hiding this comment

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

Yay.

Copy link
Member

@dannywillems dannywillems left a comment

Choose a reason for hiding this comment

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

Reviewed commit by commit. Makes sense so far. Curious to see the final result.

@dannywillems dannywillems merged commit 9d434d8 into master Dec 16, 2024
3 checks passed
@mrmr1993 mrmr1993 deleted the feature/lift-typ branch December 16, 2024 17:26
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