reimplement `~const Trait` bounds via a fourth kind of generic param by oli-obk · Pull Request #101900 · rust-lang/rust

oli-obk · 2022-09-16T13:12:26Z

tracking issue: #102090

Deduplicate two functions that would soon have been three rust-lang#101900 would have added another copy of this for effects

bors · 2022-09-23T16:50:55Z

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

compiler/rustc_feature/src/active.rs

oli-obk · 2022-09-29T10:06:43Z

Looks like src/tools/jsondoclint is not covered by ./x.py check

bors · 2022-09-29T14:05:43Z

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

lcnr · 2023-01-12T12:17:04Z

compiler/rustc_hir_analysis/src/collect/generics_of.rs

        }
    }

+    if has_constness_effect {


has_host_effect.

lcnr · 2023-01-12T12:37:15Z

compiler/rustc_infer/src/infer/canonical/canonicalizer.rs

+        let flags = FlagComputation::for_effect(e);
+        if flags.intersects(self.needs_canonical_flags) { e.super_fold_with(self) } else { e }


why is that not simply happening in the _ => {} branch and we stop explicitly using return for the other branches.

lcnr · 2023-01-12T12:38:38Z

compiler/rustc_infer/src/infer/canonical/canonicalizer.rs

+    ) -> ty::Effect<'tcx> {
+        let infcx = self.infcx;
+        let bound_to = infcx.shallow_resolve(effect_var);
+        if bound_to != effect_var {


can you make that an assert instead? shouldn't we shallow resolve in fold_effect already?

lcnr · 2023-01-12T12:39:28Z

compiler/rustc_infer/src/infer/canonical/query_response.rs

+                }
+                GenericArgKind::Effect(result_value) => {
+                    if let ty::EffectValue::Bound(debrujin, b) = result_value.val {
+                        // ...in which case we would set `canonical_vars[0]` to `Some(const X)`.


Some(const x)?

lcnr · 2023-01-12T12:42:36Z

compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs

                    GenericArgKind::Lifetime(_) => 0, // erased
                    GenericArgKind::Type(ty) => self.ty_cost(ty),
                    GenericArgKind::Const(_) => 3, // some non-zero value
+                    GenericArgKind::Effect(_) => 3, // some non-zero value


would put them together with regions, you cannot explicitly mention effects so they don't have a cost

lcnr · 2023-01-12T12:43:32Z

compiler/rustc_infer/src/infer/mod.rs

+                EffectVariableValue::Unknown { .. } => {}
+                EffectVariableValue::Known { .. } => continue,
+            }
+            let value = self.tcx.mk_effect(ty::EffectValue::Rigid { on: true }, origin.effect_kind);


not applying vs ty::EffectValue::Rigid { on: true },. afaict either the function comment or the impl is wrong

lcnr · 2023-01-12T12:52:10Z

compiler/rustc_infer/src/infer/sub.rs

+        b: ty::Effect<'tcx>,
+    ) -> RelateResult<'tcx, ty::Effect<'tcx>> {
+        match (a.val, b.val) {
+            // Any effect matches the rigid "on" effect. For example: `T: ~const Trait` (off) implies `T: Trait` (on)


effects and subtyping seems very sus. What's the default variance for effect parameters?

lcnr · 2023-01-12T12:57:39Z

compiler/rustc_infer/src/traits/util.rs

            }
            ty::PredicateKind::Clause(ty::Clause::Projection(..)) => {
-                // Nothing to elaborate in a projection predicate.
+                // `<T as ~const Trait>::TYPE` implies `<T as Trait>::TYPE`


can you add a comment that this is only correct if the predicate has exactly 1 effect param? If there were multiple params i could imagine us wanting to add all possible combinations of const + ~const.

lcnr · 2023-01-12T13:06:47Z

compiler/rustc_middle/src/ty/generics.rs

+                GenericParamDefKind::Effect { kind } => kind == effect_kind,
+                _ => false,
+            })
+            .or_else(|| tcx.generics_of(self.parent?).effect_param(effect_kind, tcx))


i am not certain whether we will always only have 1 effect param of the same kind, or I can at least imagine future extensions which will break this code.

Can you add a Debug_assert to generics_of whichi check for this?

lcnr · 2023-01-12T13:07:53Z

compiler/rustc_middle/src/ty/instance.rs

                bug!("Instance::mono: {:?} has const parameters", def_id)
            }
+            ty::GenericParamDefKind::Effect { kind } => {
+                // FIXME: this is necessary for `panic` and friends in the monomorphization collector.


Suggested change

// FIXME: this is necessary for `panic` and friends in the monomorphization collector.

// FIXME(effects): this is necessary for `panic` and friends in the monomorphization collector.

compiler/rustc_middle/src/ty/sty.rs

lcnr · 2023-01-12T13:27:29Z

compiler/rustc_middle/src/ty/visit.rs

+        // hook in here to catch this case (annoying...), but
+        // otherwise we do want to remember to visit the rest of the
+        // effect, as it has types/regions embedded in a lot of other
+        // places.


i don't understand that comment 🤔

I think I copy pasted it from constants without looking at the details

lcnr · 2023-01-12T13:47:37Z

compiler/rustc_trait_selection/src/traits/util.rs

    tcx: TyCtxt<'tcx>,
    fn_trait_def_id: DefId,
    self_ty: Ty<'tcx>,
+    // Make this an array once the effect feature is always active


Suggested change

// Make this an array once the effect feature is always active

// FIXME(effects): Make this an array once the effect feature is always active

lcnr · 2023-01-12T13:47:58Z

compiler/rustc_trait_selection/src/traits/util.rs

+        };
+        let trait_ref = tcx.mk_trait_ref(
+            fn_trait_def_id,
+            // FIXME: have the caller pass the effects here and use them.


Suggested change

// FIXME: have the caller pass the effects here and use them.

// FIXME(effects): have the caller pass the effects here and use them.

lcnr · 2023-01-12T13:48:10Z

compiler/rustc_trait_selection/src/traits/util.rs

    tcx: TyCtxt<'tcx>,
    fn_trait_def_id: DefId,
    self_ty: Ty<'tcx>,
+    // Make this an array once the effect feature is always active


Suggested change

// Make this an array once the effect feature is always active

// FIXME(effects): Make this an array once the effect feature is always active

lcnr · 2023-01-12T13:49:45Z

library/core/src/alloc/layout.rs

-    #[rustc_const_unstable(feature = "const_alloc_layout", issue = "67521")]
    #[inline]
-    pub const fn extend(&self, next: Self) -> Result<(Self, usize), LayoutError> {
+    pub fn extend(&self, next: Self) -> Result<(Self, usize), LayoutError> {


why this change?

lcnr · 2023-01-12T13:50:01Z

library/core/src/array/mod.rs


    #[inline]
-    fn index(&self, index: I) -> &Self::Output {
+    fn index(&self, index: I) -> &<[T] as Index<I>>::Output {


same here, why these libs changes?

Because Self didn't work ^^ I'm still figuring out that part

lcnr · 2023-01-27T11:38:18Z

src/test/ui/rfc-2632-const-trait-impl/inherent-impl.rs


 impl const S {}
 //~^ ERROR inherent impls cannot be `const`
+//~| ERROR `host` is not constrained


adding an effect param for inherent impls seems bad ui wise ^^

yea I hate everything about it ^^ I'll remove it and just require everyone to repeat trait bounds on the functions for now

apiraino · 2023-02-01T22:38:44Z

If I am correct this can be switched to waiting on author. Feel free to request a review with @rustbot ready, thanks!

@rustbot author

oli-obk · 2023-03-14T22:25:23Z

This PR has become impossible to rebase. I'm starting from scratch on a new branch with a smaller MVP

Various cleanups This PR pulls changes out of rust-lang/rust#101900 that can land on master immediately r? ``@fee1-dead``

rust-highfive assigned lcnr Sep 16, 2022

rustbot added T-compiler Relevant to the compiler 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 Sep 16, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 16, 2022

This comment has been minimized.

Sign in to view

oli-obk force-pushed the const_trait_take_two branch from 9963569 to 815442a Compare September 16, 2022 16:02

rustbot added the A-rustdoc-json Area: Rustdoc JSON backend label Sep 16, 2022

This comment has been minimized.

Sign in to view

This was referenced Sep 21, 2022

[WIP] Rework the entire const trait system #96077

Closed

Deduplicate two functions that would soon have been three #102095

Merged

oli-obk force-pushed the const_trait_take_two branch from 6dc21df to c68c262 Compare September 22, 2022 09:35

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 22, 2022

Rollup merge of rust-lang#102095 - oli-obk:simplify, r=TaKO8Ki

fc50878

Deduplicate two functions that would soon have been three rust-lang#101900 would have added another copy of this for effects

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 22, 2022

Rollup merge of rust-lang#102095 - oli-obk:simplify, r=TaKO8Ki

8f4accc

Deduplicate two functions that would soon have been three rust-lang#101900 would have added another copy of this for effects

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 23, 2022

Rollup merge of rust-lang#102095 - oli-obk:simplify, r=TaKO8Ki

dac0f6e

Deduplicate two functions that would soon have been three rust-lang#101900 would have added another copy of this for effects

oli-obk force-pushed the const_trait_take_two branch 2 times, most recently from 3ec01fc to 2e40ec7 Compare September 23, 2022 10:48

This comment has been minimized.

Sign in to view

fee1-dead reviewed Sep 28, 2022

View reviewed changes

compiler/rustc_feature/src/active.rs Outdated Show resolved Hide resolved

oli-obk force-pushed the const_trait_take_two branch 2 times, most recently from 28262b2 to 74e72c0 Compare September 28, 2022 19:07

This comment has been minimized.

Sign in to view

oli-obk force-pushed the const_trait_take_two branch 2 times, most recently from 04641ab to 098e59c Compare September 29, 2022 09:46

This comment has been minimized.

Sign in to view

oli-obk force-pushed the const_trait_take_two branch from 098e59c to b98d0d3 Compare September 30, 2022 12:33

oli-obk mentioned this pull request Sep 30, 2022

Allow ~const bounds on non-const functions #102273

Merged

oli-obk force-pushed the const_trait_take_two branch from 37df7dc to 8de7869 Compare October 4, 2022 10:10

lcnr reviewed Jan 12, 2023

View reviewed changes

compiler/rustc_hir_analysis/src/collect/generics_of.rs

}

}

if has_constness_effect {

Copy link

Contributor

lcnr Jan 12, 2023

Choose a reason for hiding this comment

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

has_host_effect.

lcnr reviewed Jan 12, 2023

View reviewed changes

compiler/rustc_middle/src/ty/sty.rs Show resolved Hide resolved

lcnr reviewed Jan 12, 2023

View reviewed changes

oli-obk mentioned this pull request Jan 20, 2023

add a Callable trait that is implemented for unsafe functions, too #107123

Closed

lcnr reviewed Jan 27, 2023

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 1, 2023

felix91gr mentioned this pull request Mar 9, 2023

I wonder if all the methods of the crate could be made const stephaneyfx/enum-iterator#24

Closed

oli-obk closed this Mar 14, 2023

fee1-dead mentioned this pull request Apr 16, 2023

Tracking Issue for removing impl const and ~const in the standard library #110395

Closed

4 tasks

thomcc pushed a commit to tcdi/postgrestd that referenced this pull request May 31, 2023

Rollup merge of #105960 - oli-obk:effect_cleanup, r=fee1-dead

3b85756

Various cleanups This PR pulls changes out of rust-lang/rust#101900 that can land on master immediately r? ``@fee1-dead``

		let flags = FlagComputation::for_effect(e);
		if flags.intersects(self.needs_canonical_flags) { e.super_fold_with(self) } else { e }

	// FIXME: this is necessary for `panic` and friends in the monomorphization collector.
	// FIXME(effects): this is necessary for `panic` and friends in the monomorphization collector.

	// Make this an array once the effect feature is always active
	// FIXME(effects): Make this an array once the effect feature is always active

	// FIXME: have the caller pass the effects here and use them.
	// FIXME(effects): have the caller pass the effects here and use them.

Uh oh!

Conversation

oli-obk commented Sep 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

bors commented Sep 23, 2022

Uh oh!

This comment has been minimized.

Uh oh!

This comment has been minimized.

oli-obk commented Sep 29, 2022

Uh oh!

bors commented Sep 29, 2022

Uh oh!

This comment has been minimized.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

apiraino commented Feb 1, 2023

Uh oh!

oli-obk commented Mar 14, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

oli-obk commented Sep 16, 2022 •

edited

Loading