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

[WIP] Rework the entire const trait system #96077

Closed
wants to merge 51 commits into from

Conversation

fee1-dead
Copy link
Member

@fee1-dead fee1-dead commented Apr 15, 2022

Instead of storing constness in trait predicates and tracking the environment in the ParamEnv, we store it as substitutions to a trait.

r? @ghost

cc @oli-obk

This is a work in progress. I opened this draft such that we can track the progress and have something that can be pointed to when this blocks other PRs.

  • Add GenericArgKind::Constness
    • Handle this case in all dependent crates
  • Enforce that all traits automatically get the generic argument = default to non-const if not present
    • Ensure the implementation of generics_of is correct
  • Review interaction with #[min_specialization]
  • Remove #[default_method_body_is_const]
  • Add #[const_trait]
  • Add support for ~const super traits
    • Tests
  • Update tests

@rustbot rustbot added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Apr 15, 2022
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@fee1-dead
Copy link
Member Author

@bors rollup=never

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented May 9, 2022

☔ The latest upstream changes (presumably #96473) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label May 9, 2022
@fee1-dead
Copy link
Member Author

I didn't write a correct generics_of implementation 😅

@oli-obk
Copy link
Contributor

oli-obk commented May 11, 2022

rebased over master

@oli-obk
Copy link
Contributor

oli-obk commented May 11, 2022

Status update on libcore (will dig into it now):

error[E0221]: ambiguous associated type `Item` in bounds of `I`
   --> library/core/src/array/mod.rs:770:77
    |
770 | unsafe fn collect_into_array_unchecked<I, const N: usize>(iter: &mut I) -> [I::Item; N]
    |                                                                             ^^^^^^^ ambiguous associated type `Item`
    |
   ::: library/core/src/iter/traits/iterator.rs:65:5
    |
65  |     type Item;
    |     ----------
    |     |
    |     ambiguous `Item` from `Iterator<>`
    |     ambiguous `Item` from `Iterator<>`
    |
help: use fully qualified syntax to disambiguate
    |
770 | unsafe fn collect_into_array_unchecked<I, const N: usize>(iter: &mut I) -> [<I as Iterator<>>::Item; N]
    |                                                                             ~~~~~~~~~~~~~~~~~~~
help: use fully qualified syntax to disambiguate
    |
770 | unsafe fn collect_into_array_unchecked<I, const N: usize>(iter: &mut I) -> [<I as Iterator<>>::Item; N]
    |                                                                             ~~~~~~~~~~~~~~~~~~~

error[E0221]: ambiguous associated type `Item` in bounds of `A`
   --> library/core/src/iter/adapters/zip.rs:165:52
    |
140 | / macro_rules! zip_impl_general_defaults {
141 | |     () => {
142 | |         default fn new(a: A, b: B) -> Self {
143 | |             Zip {
...   |
165 | |         default fn next_back(&mut self) -> Option<(A::Item, B::Item)>
    | |                                                    ^^^^^^^ ambiguous associated type `Item`
...   |
194 | |     };
195 | | }
    | |_- in this expansion of `zip_impl_general_defaults!`
...
206 |       zip_impl_general_defaults! {}
    |       ----------------------------- in this macro invocation
    |
   ::: library/core/src/iter/traits/iterator.rs:65:5
    |
65  |       type Item;
    |       ----------
    |       |
    |       ambiguous `Item` from `Iterator<>`
    |       ambiguous `Item` from `Iterator<>`
    |
help: use fully qualified syntax to disambiguate
    |
165 |         default fn next_back(&mut self) -> Option<(<A as Iterator<>>::Item, B::Item)>
    |                                                    ~~~~~~~~~~~~~~~~~~~
help: use fully qualified syntax to disambiguate
    |
165 |         default fn next_back(&mut self) -> Option<(<A as Iterator<>>::Item, B::Item)>
    |                                                    ~~~~~~~~~~~~~~~~~~~

error[E0221]: ambiguous associated type `Item` in bounds of `B`
   --> library/core/src/iter/adapters/zip.rs:165:61
    |
140 | / macro_rules! zip_impl_general_defaults {
141 | |     () => {
142 | |         default fn new(a: A, b: B) -> Self {
143 | |             Zip {
...   |
165 | |         default fn next_back(&mut self) -> Option<(A::Item, B::Item)>
    | |                                                             ^^^^^^^ ambiguous associated type `Item`
...   |
194 | |     };
195 | | }
    | |_- in this expansion of `zip_impl_general_defaults!`
...
206 |       zip_impl_general_defaults! {}
    |       ----------------------------- in this macro invocation
    |
   ::: library/core/src/iter/traits/iterator.rs:65:5
    |
65  |       type Item;
    |       ----------
    |       |
    |       ambiguous `Item` from `Iterator<>`
    |       ambiguous `Item` from `Iterator<>`
    |
help: use fully qualified syntax to disambiguate
    |
165 |         default fn next_back(&mut self) -> Option<(A::Item, <B as Iterator<>>::Item)>
    |                                                             ~~~~~~~~~~~~~~~~~~~
help: use fully qualified syntax to disambiguate
    |
165 |         default fn next_back(&mut self) -> Option<(A::Item, <B as Iterator<>>::Item)>
    |                                                             ~~~~~~~~~~~~~~~~~~~

error[E0221]: ambiguous associated type `Item` in bounds of `A`
   --> library/core/src/iter/adapters/zip.rs:165:52
    |
140 | / macro_rules! zip_impl_general_defaults {
141 | |     () => {
142 | |         default fn new(a: A, b: B) -> Self {
143 | |             Zip {
...   |
165 | |         default fn next_back(&mut self) -> Option<(A::Item, B::Item)>
    | |                                                    ^^^^^^^ ambiguous associated type `Item`
...   |
194 | |     };
195 | | }
    | |_- in this expansion of `zip_impl_general_defaults!`
...
239 |       zip_impl_general_defaults! {}
    |       ----------------------------- in this macro invocation
    |
   ::: library/core/src/iter/traits/iterator.rs:65:5
    |
65  |       type Item;
    |       ----------
    |       |
    |       ambiguous `Item` from `Iterator<>`
    |       ambiguous `Item` from `Iterator<>`
    |
help: use fully qualified syntax to disambiguate
    |
165 |         default fn next_back(&mut self) -> Option<(<A as Iterator<>>::Item, B::Item)>
    |                                                    ~~~~~~~~~~~~~~~~~~~
help: use fully qualified syntax to disambiguate
    |
165 |         default fn next_back(&mut self) -> Option<(<A as Iterator<>>::Item, B::Item)>
    |                                                    ~~~~~~~~~~~~~~~~~~~

error[E0221]: ambiguous associated type `Item` in bounds of `B`
   --> library/core/src/iter/adapters/zip.rs:165:61
    |
140 | / macro_rules! zip_impl_general_defaults {
141 | |     () => {
142 | |         default fn new(a: A, b: B) -> Self {
143 | |             Zip {
...   |
165 | |         default fn next_back(&mut self) -> Option<(A::Item, B::Item)>
    | |                                                             ^^^^^^^ ambiguous associated type `Item`
...   |
194 | |     };
195 | | }
    | |_- in this expansion of `zip_impl_general_defaults!`
...
239 |       zip_impl_general_defaults! {}
    |       ----------------------------- in this macro invocation
    |
   ::: library/core/src/iter/traits/iterator.rs:65:5
    |
65  |       type Item;
    |       ----------
    |       |
    |       ambiguous `Item` from `Iterator<>`
    |       ambiguous `Item` from `Iterator<>`
    |
help: use fully qualified syntax to disambiguate
    |
165 |         default fn next_back(&mut self) -> Option<(A::Item, <B as Iterator<>>::Item)>
    |                                                             ~~~~~~~~~~~~~~~~~~~
help: use fully qualified syntax to disambiguate
    |
165 |         default fn next_back(&mut self) -> Option<(A::Item, <B as Iterator<>>::Item)>
    |                                                             ~~~~~~~~~~~~~~~~~~~

error[E0221]: ambiguous associated type `Item` in bounds of `A`
   --> library/core/src/iter/adapters/zip.rs:330:40
    |
330 |     fn next_back(&mut self) -> Option<(A::Item, B::Item)>
    |                                        ^^^^^^^ ambiguous associated type `Item`
    |
   ::: library/core/src/iter/traits/iterator.rs:65:5
    |
65  |     type Item;
    |     ----------
    |     |
    |     ambiguous `Item` from `Iterator<>`
    |     ambiguous `Item` from `Iterator<>`
    |
help: use fully qualified syntax to disambiguate
    |
330 |     fn next_back(&mut self) -> Option<(<A as Iterator<>>::Item, B::Item)>
    |                                        ~~~~~~~~~~~~~~~~~~~
help: use fully qualified syntax to disambiguate
    |
330 |     fn next_back(&mut self) -> Option<(<A as Iterator<>>::Item, B::Item)>
    |                                        ~~~~~~~~~~~~~~~~~~~

error[E0221]: ambiguous associated type `Item` in bounds of `B`
   --> library/core/src/iter/adapters/zip.rs:330:49
    |
330 |     fn next_back(&mut self) -> Option<(A::Item, B::Item)>
    |                                                 ^^^^^^^ ambiguous associated type `Item`
    |
   ::: library/core/src/iter/traits/iterator.rs:65:5
    |
65  |     type Item;
    |     ----------
    |     |
    |     ambiguous `Item` from `Iterator<>`
    |     ambiguous `Item` from `Iterator<>`
    |
help: use fully qualified syntax to disambiguate
    |
330 |     fn next_back(&mut self) -> Option<(A::Item, <B as Iterator<>>::Item)>
    |                                                 ~~~~~~~~~~~~~~~~~~~
help: use fully qualified syntax to disambiguate
    |
330 |     fn next_back(&mut self) -> Option<(A::Item, <B as Iterator<>>::Item)>
    |                                                 ~~~~~~~~~~~~~~~~~~~

error: internal compiler error: compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs:196:37: MethodCallee {
                                    def_id: DefId(0:3401 ~ core[cd04]::ops::function::FnOnce::call_once),
                                    substs: [
                                        impl FnMut(A, B) -> T,
                                        (_, _),
                                        Param,
                                    ],
                                    sig: ([impl FnMut(A, B) -> T, (_, _)]; c_variadic: false)-><impl FnMut(A, B) -> T as ops::function::FnOnce>::Output<>,
                                }
                                Generics {
                                    parent: Some(
                                        DefId(0:3398 ~ core[cd04]::ops::function::FnOnce),
                                    ),
                                    parent_count: 2,
                                    params: [
                                        GenericParamDef {
                                            name: "<constness>",
                                            def_id: DefId(0:3401 ~ core[cd04]::ops::function::FnOnce::call_once),
                                            index: 3,
                                            pure_wrt_drop: false,
                                            kind: Constness,
                                        },
                                    ],
                                    param_def_id_to_index: {
                                        DefId(0:3401 ~ core[cd04]::ops::function::FnOnce::call_once): 3,
                                    },
                                    has_self: true,
                                    has_late_bound_regions: None,
                                }
                                i: 3
   --> library/core/src/ops/try_trait.rs:383:39
    |
383 |         move |a, b| NeverShortCircuit(f(a, b))
    |               

@fee1-dead
Copy link
Member Author

For now we could also append the ConstnessArg when printing to make debugging these errors easier

@oli-obk
Copy link
Contributor

oli-obk commented May 11, 2022

For now we could also append the ConstnessArg when printing to make debugging these errors easier

ah good idea.

I'm currently looking at generics_of and am now wondering about

if parent has constness param, we do not inherit it from the parent, and we
get it in the end instead of the middle.

This means we'll end up with multiple constness params in the generics list, that seems odd(-ish)?

@fee1-dead
Copy link
Member Author

fee1-dead commented May 11, 2022

if parent has constness param, we do not inherit it from the parent, and we
get it in the end instead of the middle.

this exactly means that we don't want to end up with multiple constness params. The constness param from the parent must not be cloned to the child, and the child has to push the constness param after their generics is built.

We can also talk on Zulip if you have more questions :)

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented May 12, 2022

☔ The latest upstream changes (presumably #95562) made this pull request unmergeable. Please resolve the merge conflicts.

bors added a commit to rust-lang-ci/rust that referenced this pull request May 30, 2022
…rrors

Replace `#[default_method_body_is_const]` with `#[const_trait]`

pulled out of rust-lang#96077

related issues:  rust-lang#67792 and rust-lang#92158

cc `@fee1-dead`

This is groundwork to only allowing `impl const Trait` for traits that are marked with `#[const_trait]`. This is necessary to prevent adding a new default method from becoming a breaking change (as it could be a non-const fn).
@oli-obk
Copy link
Contributor

oli-obk commented Sep 21, 2022

We learned a lot in this PR, and I'm rewriting it from scratch with what we learned in #101900

The most notable difference will be that instead of eagerly figuring out what kind of constness parameter is needed, we create inference variables and let the inference engine figure it out where it can. Then, at fallback time, we insert the right kind of constness depending on which scope we're in (which fallback does know, in contrast to all the other sites where we had to thread stuff through in this PR)

Thanks @fee1-dead for pushing on this!

@oli-obk oli-obk closed this Sep 21, 2022
@fee1-dead fee1-dead deleted the const_trait branch September 28, 2022 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants