-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Tracking issue for RFC 2532, "Associated type defaults" #29661
Comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…plorations#168) * Extend Circuit trait to take parameters in config The Circuit trait is extended with the following: ``` pub trait Circuit<F: Field> { /// [...] type Params: Default; fn params(&self) -> Self::Params { Self::Params::default() } fn configure_with_params(meta: &mut ConstraintSystem<F>, params: &Self::Params) -> Self::Config { Self::configure(meta) } fn configure(meta: &mut ConstraintSystem<F>) -> Self::Config; } ``` This allows runtime parametrization of the circuit configuration. The extension to the Circuit trait has been designed to minimize the breaking change: existing circuits only need to define the associated `type Params`. Unfortunately "Associated type defaults" are unstable in Rust, otherwise this would be a non-breaking change. See rust-lang/rust#29661 * Implement circuit params under feature flag * Don't overwrite configure method * Fix doc test
Any ETA on when we might see this in Stable? Next year perhaps? |
This is a tracking issue, a better place to ask this is on Zulip. ETA is impossible to give as there's still some unresolved questions about this feature and how it should be implemented that need to be resolved before someone starts working on it - check the linked issues and offer suggestions if you have any in the meantime. |
* feat: call synthesize in `MockProver` multiple times to behave same as real prover * modify previous commit * Expose mod `permutation` and re-export `permutation::keygen::Assembly` (privacy-scaling-explorations#149) * feat: expose mod ule `permutation` and re-export `permutation::keygen::Assembly` * feat: derive `lone` for `permutation::keygen::Assembly` * feat: bump MSRV for `inferno` * change: Migrate workspace to pasta_curves-0.5 (privacy-scaling-explorations#157) * change: Migrate workspace to pasta_curves-0.5 This ports the majority of the workspace to the `pasta_curves-0.5.0` leaving some tricky edge-cases that we need to handle carefully. Resolves: privacy-scaling-explorations#132 * fix: Complete latest trait bounds to compile halo2proofs * change: Migrate examples & benches to pasta 0.5 * change: Migrate halo2_gadgets to pasta-0.5 * change: Update gadgets outdated code with latest upstream * fix: Sha3 gadget circuit * fix: doc tests * chore: Update merged main * fix: Apply review suggestions * fix previous commit * Extend Circuit trait to take parameters in config (privacy-scaling-explorations#168) * Extend Circuit trait to take parameters in config The Circuit trait is extended with the following: ``` pub trait Circuit<F: Field> { /// [...] type Params: Default; fn params(&self) -> Self::Params { Self::Params::default() } fn configure_with_params(meta: &mut ConstraintSystem<F>, params: &Self::Params) -> Self::Config { Self::configure(meta) } fn configure(meta: &mut ConstraintSystem<F>) -> Self::Config; } ``` This allows runtime parametrization of the circuit configuration. The extension to the Circuit trait has been designed to minimize the breaking change: existing circuits only need to define the associated `type Params`. Unfortunately "Associated type defaults" are unstable in Rust, otherwise this would be a non-breaking change. See rust-lang/rust#29661 * Implement circuit params under feature flag * Don't overwrite configure method * Fix doc test * Allow halo2 constraint names to have non static names (privacy-scaling-explorations#156) * static ref to String type in Gates, Constraints, VirtualCell, Argument * 'lookup'.to_string() * return &str for gate name and constriant_name, also run fmt * Update halo2_gadgets/Cargo.toml Co-authored-by: Han <[email protected]> * upgrade rust-toochain --------- Co-authored-by: Carlos Pérez <[email protected]> Co-authored-by: Han <[email protected]> * Improve halo2 query calls (privacy-scaling-explorations#154) * return expression from cell * add example * selector * recurse Expression to fill in index * minimized changes from the original * backword compatible meta.query_X & challange.expr() * cargo fmt * fixed lookup to pass all tests * Update comments Co-authored-by: Brecht Devos <[email protected]> * Update comments Co-authored-by: Brecht Devos <[email protected]> * Update comments Co-authored-by: Brecht Devos <[email protected]> * Update comments Co-authored-by: Brecht Devos <[email protected]> * Update comments Co-authored-by: Brecht Devos <[email protected]> * Update comments Co-authored-by: Brecht Devos <[email protected]> * update Co-authored-by: Brecht Devos <[email protected]> * add primitives.rs back * remove example2 * backward compatible meta.query_X & Column.cur(), next(), prev(), at(usize) * impl Debug & make side effects only when query.index.is_none() * change impl Debug for Expression instead & revert test in plonk_api * upgrade rust-toolchain * Update halo2_proofs/src/plonk/circuit.rs Co-authored-by: Han <[email protected]> * Update halo2_proofs/src/plonk/circuit.rs Co-authored-by: Han <[email protected]> * ran clippy * Update halo2_proofs/src/plonk/circuit.rs Co-authored-by: Han <[email protected]> --------- Co-authored-by: Brecht Devos <[email protected]> Co-authored-by: Han <[email protected]> * Implement Clone trait for Hash, Absorbing, and Sponge structs (privacy-scaling-explorations#171) * fix: Fix serialization for VerifyingKey (privacy-scaling-explorations#178) Now the value returned when the number of selectors is a multiple of 8 is correct. Resolves: privacy-scaling-explorations#175 * Add more getters to expose internal fields * add a constructor (privacy-scaling-explorations#164) * add a constructor * add more comment * fix as review * remove clone * remove * no need to use new variable * change comment * fix clippy * rename to from_parts * remove n declaration * feat: send sync region (privacy-scaling-explorations#180) * feat: send / sync region * Update layout.rs * update * lol * debug * Update keygen.rs * Update keygen.rs * Update keygen.rs * Update keygen.rs * thread-safe-region feature flag * cleanup * patch dev-graph * patch non-determinism in mapping creation * reduce mem usage for vk and pk * mock proving examples * swap for hashmap for insertion speed * reduce update overhead * replace BTree with Vec * add benchmarks * make the benchmarks massive * patch clippy * simplify lifetimes * patch benches * Update halo2_proofs/src/plonk/permutation/keygen.rs Co-authored-by: Han <[email protected]> * Update halo2_proofs/examples/vector-mul.rs Co-authored-by: Han <[email protected]> * rm benches * order once * patch lints --------- Co-authored-by: Han <[email protected]> * fix previous commit * Fix `parallelize` workload imbalance (privacy-scaling-explorations#186) * fix parallelize workload imbalance * remove the need of unsafe * Updates halo2_curves dependency to released package (privacy-scaling-explorations#190) THe package release ressets the version from those inherited by the legacy halo2curves repo's fork history. The upstream diff is: https://github.com/privacy-scaling-explorations/halo2curves/compare/9f5c50810bbefe779ee5cf1d852b2fe85dc35d5e..9a7f726fa74c8765bc7cdab11519cf285d169ecf * fix: explicitly define mds diff type (privacy-scaling-explorations#196) * fix: explicitly define mds diff type * rm paren * feat: expose `transcript_repr` of `VerifyingKey` and reduce the trait constraint (privacy-scaling-explorations#200) * implement native shuffle argument and api fix: remove nonsense comment strictly check shuffle rows address doc typos move compression into product commitment typo add shuffle errors for `verify_at_rows_par` dedup expression evaluation cargo fmt fix fields in sanity-checks feature * feat: public cells to allow for implementations of custom `Layouter` (privacy-scaling-explorations#192) * feat: public cells * Update mds.rs * Update mds.rs * Update single_pass.rs Co-authored-by: Han <[email protected]> * bump toolchain to resolve errors * fix clippy errors for CI run * rustfmt post clippy * plz let it be the last lint * patch clippy lints in gadgets * clippy lints for sha256 bench * patch halo2proof benches * Update assigned.rs * Update halo2_gadgets/src/poseidon/primitives/mds.rs Co-authored-by: Han <[email protected]> * Update halo2_gadgets/src/poseidon/primitives/mds.rs Co-authored-by: Han <[email protected]> --------- Co-authored-by: Han <[email protected]> * Synchronize with upstream (privacy-scaling-explorations#199) * refactor: add default impl for `SyncDeps` for backward compatability * feat: pick changes from zcash#728 and changes of flag `test-dev-graph` * feat: pick changes from zcash#622 * feat: pick changes about mod `circuit` and mod `dev` * feat: pick rest changes of `halo2_proofs` * fix: when `--no-default-features` * ci: sync from upstream, and deduplicate jobs when push to `main`, and remove always failing job `codecov`. * fix: make `commit_zk` runnable when `--no-default-features` * chore: Update rust-toolchain to 1.66 for testing (privacy-scaling-explorations#208) * chore: Update rust-toolchain to 1.66 for testing Note that tests will not compile due to the silent MSRV bump in `blake2b_simd`. Hence, we need to use `1.66` as toolchain. Resolves: privacy-scaling-explorations#207 * change: UIpdate MSRVs in Cargo.toml * fix: clippy (privacy-scaling-explorations#203) * fix: clippy * fmt * fix: Final clippy complains & adjustments --------- Co-authored-by: CPerezz <[email protected]> * Implement Sum and Product for Expression (privacy-scaling-explorations#209) * Make it Eq to make it easier for tests * Implement Sum and Product for Expression * Make it readable * chore: update poseidon dependency * fix: compiling bug with feautes=parallel_syn * feat(MockProver): replace errors by asserts(privacy-scaling-explorations#150) * boundary offset lost when resolving conflict * disable multiphase prover * Sync halo2 lib 0.4.0 merging (#81) * Use thread pool for assign_regions (#57) * feat: use rayon threadpool * feat: add UT for many subregions * refact: move common struct out to module level * refact: reuse common configure code * fix ci errors --------- Co-authored-by: kunxian xia <[email protected]> * Move `env_logger` dependency to dev-depdendencies (only for test). (#69) * sync ff/group 0.13 * fix clippy * fix clippy * fmg * [FEAT] Upgrading table16 for SHA256 (#73) * upgrade sha256 * fix clippy * Bus auto (#72) * bus: expose global offset of regions * bus-auto: add query_advice and query_fixed function in witness generation * bus-auto: fix clippy --------- Co-authored-by: Aurélien Nicolas <[email protected]> * fix-tob-scroll-21 (#59) * fix-tob-scroll-21 * expose param field for re-randomization * enable accessing for table16 (#75) * chore: update poseidon link * merge sha256 gadget changes * Fix the CI errors (#78) * cargo fmt * fix clippy error * Feat: switch to logup scheme for lookup argument (#71) * Multi-input mv-lookup. (#49) * Add mv_lookup.rs * mv_lookup::prover, mv_lookup::verifier * Replace lookup with mv_lookup * replace halo2 with mv lookup Co-authored-by: ying tong <[email protected]> * cleanups Co-authored-by: ying tong <[email protected]> * ConstraintSystem: setup lookup_tracker Co-authored-by: Andrija <[email protected]> * mv_lookup::hybrid_prover Co-authored-by: Andrija <[email protected]> * WIP * mv_multi_lookup: enable lookup caching Co-authored-by: therealyingtong <[email protected]> * Rename hybrid_lookup -> lookup * Chunk lookups using user-provided minimum degree Co-authored-by: Andrija <[email protected]> * mv_lookup bench Co-authored-by: Andrija <[email protected]> * Introduce counter feature for FFTs and MSMs Co-authored-by: Andrija <[email protected]> * Fix off-by-one errors in chunk_lookup Co-authored-by: Andrija <[email protected]> * bench wip * time evaluate_h * KZG * more efficient batch inversion * extended lookup example * Finalize mv lookup Author: therealyingtong <[email protected]> * Remove main/ * Fix according to the comments * replace scan with parallel grand sum computation * Revert Cargo.lock * mv lookup Argument name * parallel batch invert --------- Co-authored-by: Andrija <[email protected]> Co-authored-by: ying tong <[email protected]> Co-authored-by: therealyingtong <[email protected]> * fmt * fix unit test * fix clippy errors * add todo in mv_lookup's prover * fmt and clippy * fix clippy * add detailed running time of steps in logup's prover * fmt * add more log hooks * more running time logs * use par invert * use sorted-vector to store how many times a table element occurs in input * par the process to get inputs_inv_sum * use par * fix par * add feature to skip inv sums * add new feature flag * fix clippy error --------- Co-authored-by: Sphere L <[email protected]> Co-authored-by: Andrija <[email protected]> Co-authored-by: ying tong <[email protected]> Co-authored-by: therealyingtong <[email protected]> * fix some simple building errs * upgrade pathfinder_simd to newer version as it can't compile on mac m1 pro * resolve merge conflict * fmt * clippy * more clippy fix * more lint fix * fmt * minor syntax fix * fix ipa multiopen test failure * fix clippy warning * fmt * fix par scan of log_inv diff * remove uncessary clone --------- Co-authored-by: alannotnerd <[email protected]> Co-authored-by: kunxian xia <[email protected]> Co-authored-by: Steven <[email protected]> Co-authored-by: Carlos Pérez <[email protected]> Co-authored-by: zhenfei <[email protected]> Co-authored-by: Ho <[email protected]> Co-authored-by: naure <[email protected]> Co-authored-by: Aurélien Nicolas <[email protected]> Co-authored-by: Sphere L <[email protected]> Co-authored-by: Andrija <[email protected]> Co-authored-by: ying tong <[email protected]> Co-authored-by: therealyingtong <[email protected]> --------- Co-authored-by: han0110 <[email protected]> Co-authored-by: Velaciela <[email protected]> Co-authored-by: Carlos Pérez <[email protected]> Co-authored-by: Eduard S <[email protected]> Co-authored-by: CeciliaZ030 <[email protected]> Co-authored-by: Brecht Devos <[email protected]> Co-authored-by: Enrico Bottazzi <[email protected]> Co-authored-by: Ethan-000 <[email protected]> Co-authored-by: dante <[email protected]> Co-authored-by: Mamy Ratsimbazafy <[email protected]> Co-authored-by: François Garillot <[email protected]> Co-authored-by: kilic <[email protected]> Co-authored-by: Thor <[email protected]> Co-authored-by: CPerezz <[email protected]> Co-authored-by: chokermaxx <[email protected]> Co-authored-by: Zhang Zhuo <[email protected]> Co-authored-by: alannotnerd <[email protected]> Co-authored-by: kunxian xia <[email protected]> Co-authored-by: Steven <[email protected]> Co-authored-by: Ho <[email protected]> Co-authored-by: naure <[email protected]> Co-authored-by: Aurélien Nicolas <[email protected]> Co-authored-by: Sphere L <[email protected]> Co-authored-by: Andrija <[email protected]> Co-authored-by: ying tong <[email protected]> Co-authored-by: therealyingtong <[email protected]>
If anyone wants to pick part of this up, I think the biggest thing missing is support for #![feature(associated_type_defaults)]
#![allow(unused)]
trait Trait {
type Foo = u8;
fn foo(&self) -> Self::Foo;
}
fn dyn_with_default_ty(a: &dyn Trait) -> u8 {
a.foo()
} Currently it fails with E0191 "the value of the associated type This can probably reuse a lot of the same logic as for generics Other than that, @nikomatsakis (or others) what is meant by trait Trait {
type Foo = u8;
// currently "associated type defaults can't be assumed inside the trait defining them"
const Bar: Self::Foo = 100;
} Which AIUI is rejected by the RFC, meaning the compiler handles it correctly. |
@Centril I don't know if you are still active, but maybe you have some insight as the author of that incredibly complete RFC :) |
Summary: If `A` and `B` represent the same starlark type, `A::Canonical == B::Canonical`. For example `String::Canonical == <&str>::Canonical`. This is useful with `ValueOfUnchecked::cast` in D59380757. This diff unfortunately makes starlark API harder to use. It would be better if we had [`associated_type_defaults`](rust-lang/rust#29661), but that won't happen soon. Reviewed By: JakobDegen Differential Revision: D59380699 fbshipit-source-id: 8b658203c2d46cd03ac722170ba3858d19eee001
Summary: If `A` and `B` represent the same starlark type, `A::Canonical == B::Canonical`. For example `String::Canonical == <&str>::Canonical`. This is useful with `ValueOfUnchecked::cast` in D59380757. This diff unfortunately makes starlark API harder to use. It would be better if we had [`associated_type_defaults`](rust-lang/rust#29661), but that won't happen soon. Reviewed By: JakobDegen Differential Revision: D59380699 fbshipit-source-id: 8b658203c2d46cd03ac722170ba3858d19eee001
I've just checked this feature against the compilation benchmarks of my proc-macro crate, that generates a big amount of repetitive trait implementations with associated types. The results of current benchmarks on stable are documented here. After I updated the macro impl to use associated type defaults this has removed the quadratic growth of the generated source code, and the compilation time improved:
I wonder if it would be possible to stabilize a conservative MVP subset of this feature that assumes:
This would still bring these compilation time improvements to stable |
This is a tracking issue for the RFC "Associated type defaults" (rust-lang/rfcs#2532) under the feature gate
#![feature(associated_type_defaults)]
.The associated item RFC included the ability to provide defaults for associated types, with some tricky rules about how that would influence defaulted methods.
The early implementation of this feature was gated, because there is a widespread feeling that we want a different semantics from the RFC -- namely, that default methods should not be able to assume anything about associated types. This is especially true given the specialization RFC, which provides a much cleaner way of tailoring default implementations.
The new RFC, rust-lang/rfcs#2532, specifies that this should be the new semantics but has not been implemented yet. The existing behavior under
#![feature(associated_type_defaults)]
is buggy and does not conform to the new RFC. Consult it for a discussion on changes that will be made.Steps:
associated_type_defaults
are unsound in the new solver trait-system-refactor-initiative#46Unresolved questions:
Test checklist
Originally created as a comment on #61812
type Foo = u8;
)dyn Trait
)dyn Trait<Foo = u16>
to thatu8
by invoking some method etcdyn Trait<Foo = u16>
)dyn Trait<Foo = u16>
to thatu8
by invoking some method etctype Foo = u8; type Bar;
)dyn Trait
) -- errorFoo
is specified (dyn Trait<Foo = u16>
) -- errorBar
is specified (dyn Trait<Bar = u32>
) -- ok, checkFoo
defaults tou8
dyn Trait<Foo = u16, Bar = u32>
) -- oktype Foo = u8; type Bar = Vec<Self::Foo>
)dyn Trait
) -- errorFoo
is specified (dyn Trait<Foo = u16>
) -- unclear, maybe an error?Bar
is specified (dyn Trait<Bar = u32>
) -- unclear, maybe an error?dyn Trait<Foo = u16, Bar = u32>
) -- oktype Foo = Self::Bar; type Bar = Self::Foo
)dyn Trait
)Foo
is specified (dyn Trait<Foo = u16>
)Bar
is specified (dyn Trait<Bar = u32>
)dyn Trait<Foo = u16, Bar = u32>
)type Foo = Vec<Self::Bar>; type Bar = Box<Self::Foo>;
)dyn Trait
)Foo
is specified (dyn Trait<Foo = u16>
)Bar
is specified (dyn Trait<Bar = u32>
)dyn Trait<Foo = u16, Bar = u32>
)type Foo = u8;
(defaults-in-other-trait-items.rs
)default type Foo = u8
, cannot rely on that internally (defaults-specialization.rs
)type Foo = u8
, cannot rely on that internally (defaults-specialization.rs
)type Foo = u8;
)associated-types/associated-types-overridden-default.rs
)impl Trait { }
) (associated-types/issue-54182-2.rs
)impl Trait { type Foo = u16; }
) (issue-54182-1.rs
)type Foo = u8; type Bar;
)impl Trait { }
) -- errorFoo
is specified (impl Trait { type Foo = u16; }
) -- errorBar
is specified (impl Trait { type Bar = u32; }
) -- okimpl Trait { type Foo = u16; type Bar = u32; }
) -- oktype Foo = u8; type Bar = Vec<Self::Foo>
) --defaults-in-other-trait-items-pass.rs
,defaults-in-other-trait-items-fail.rs
impl Trait { }
)Foo
is specified (impl Trait { type Foo = u16; }
)Bar
is specified (impl Trait { type Bar = u32; }
)impl Trait { type Foo = u16; type Bar = u32; }
)type Foo = Self::Bar; type Bar = Self::Foo
) --defaults-cyclic-fail.rs
,defaults-cyclic-pass.rs
impl Trait { }
)Foo
is specified (impl Trait { type Foo = u16; }
)Bar
is specified (impl Trait { type Bar = u32; }
)impl Trait { type Foo = u16; type Bar = u32; }
)type Foo = Vec<Self::Bar>; type Bar = Box<Self::Foo>;
)impl Trait { }
)Foo
is specified (impl Trait { type Foo = u16; }
)Bar
is specified (impl Trait { type Bar = u32; }
)impl Trait { type Foo = u16; type Bar = u32; }
)Foo
is specifiedBar
is specifiedFoo
is specifiedBar
is specifieddefaults-cyclic-fail.rs
,defaults-cyclic-pass.rs
impl Trait { }
)Foo
is specifiedBar
is specifiedFoo
is specifiedBar
is specifiedFoo
is specifiedBar
is specifieddefaults-suitability.rs
)type
in trait body, bound appears on the itemtype
in trait body, type not wftype
in trait body, bound appears as trait where clausedefault type
in impl, bound appears on the itemtype
in impl, bound appears on the itemtype
in default impl, bound appears on the itemtype
in trait body, conditionally wf depending on another defaulttype
in trait body, depends on another default whose bounds sufficeThe text was updated successfully, but these errors were encountered: