Implement fallback for effect param #115727

fee1-dead · 2023-09-10T05:14:35Z

tracking issue for this ongoing work: #110395

rustbot · 2023-09-10T05:14:44Z

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

compiler-errors · 2023-09-10T05:23:49Z

compiler/rustc_middle/src/ty/sty.rs

@@ -1577,6 +1577,14 @@ pub struct ConstVid<'tcx> {
    pub phantom: PhantomData<&'tcx ()>,
 }

+/// An **effect** **v**ariable **ID**.


Why are effects being represented with a separate vid?

There's a weird dichotomy happening here where effects are kinda treated like const and kinda like their own kind, and it's not exactly clear why.

Shouldn't we be able to store whether a const vid comes from an effect parameter in the const vid table so it can participate in fallback later?

I kinda just followed how int and float type variables also have their own vids. We'd probably want to ensure that we know all variables instantiated for an effect param are actually for an effect param, and having a separate vid helped by catching things like relating a normal const var with an effect var which should never happen.

Yeah, maybe that's better. This definitely needs documentation though -- that's a design decision that (afaict? maybe i'm just out of the loop) isn't written anywhere, and it should be probably 😺

added a comment

compiler/rustc_infer/src/infer/mod.rs

compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs

compiler/rustc_hir_typeck/src/fallback.rs

oli-obk · 2023-09-11T11:07:17Z

compiler/rustc_middle/src/infer/canonical.rs

@@ -443,6 +448,14 @@ impl<'tcx> CanonicalVarValues<'tcx> {
                            };
                            ty::Region::new_late_bound(tcx, ty::INNERMOST, br).into()
                        }
+                        // todo eh?


leftover comment

oli-obk · 2023-09-11T11:09:17Z

compiler/rustc_metadata/src/rmeta/encoder.rs

@@ -873,14 +873,14 @@ fn should_encode_attrs(def_kind: DefKind) -> bool {
        | DefKind::AssocConst
        | DefKind::Macro(_)
        | DefKind::Field
-        | DefKind::Impl { .. } => true,
+        | DefKind::Impl { .. }
+        | DefKind::ConstParam => true,


do we need to encode const param attrs in metadata? I'd assume we'd get all the information from ty::Generics if it's from an upstream crate

oli-obk · 2023-09-11T11:15:31Z

compiler/rustc_infer/src/infer/mod.rs

+                // todo what about using effect var here
+                if self.tcx.has_attr(param.def_id, sym::rustc_host) {


hmm 🤔 I guess we could add a is_host_effect: bool field to GenericParamDefKind::Const to make this simpler. it's not going to increase the size of GenericParamDefKind, even if we change it to an enum at some point to encode different effect kinds

oli-obk · 2023-09-11T11:16:51Z

compiler/rustc_infer/src/infer/combine.rs

+fn effect_unification_error<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    a_is_expected: bool,
+    (a, b): (EffectVarValue<'tcx>, EffectVarValue<'tcx>),
+) -> TypeError<'tcx> {
+    TypeError::ConstMismatch(ExpectedFound::new(a_is_expected, a.as_const(tcx), b.as_const(tcx)))
+}


Can this be hit at all at present?

The tests pass with changing this to a bug so probably not (but we also don't test effects that much lol) I'll change this to panic and then see if we can get a better diagnostic if we actually hit it.

rustbot · 2023-09-11T13:18:51Z

This PR changes Stable MIR

cc @oli-obk, @celinval, @spastorino

oli-obk · 2023-09-11T13:34:14Z

@bors r+

bors · 2023-09-11T13:34:18Z

📌 Commit 9654d5c has been approved by oli-obk

It is now in the queue for this repository.

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#115335 (fix overflow in array length computation) - rust-lang#115440 (bootstrap/format: remove unnecessary paths.push) - rust-lang#115702 (Update mailmap) - rust-lang#115727 (Implement fallback for effect param) - rust-lang#115739 (Call `LateLintPass::check_attribute` from `with_lint_attrs`) - rust-lang#115743 (Point out if a local trait has no implementations) - rust-lang#115744 (Improve diagnostic for generic params from outer items (E0401)) - rust-lang#115752 (rustdoc: Add missing "Aliased type" title in the sidebar) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#115727 - fee1-dead-contrib:effect-fallback, r=oli-obk Implement fallback for effect param r? `@oli-obk` or `@lcnr` tracking issue for this ongoing work: rust-lang#110395

…ler-errors Remove a hack for effects Fallback was implemented in rust-lang#115727, which addresses the inference errors mentioned in the comments.

Rollup merge of rust-lang#118246 - fee1-dead-contrib:rm-hack, r=compiler-errors Remove a hack for effects Fallback was implemented in rust-lang#115727, which addresses the inference errors mentioned in the comments.

rustbot assigned oli-obk Sep 10, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative labels Sep 10, 2023

This comment has been minimized.

Sign in to view

compiler-errors reviewed Sep 10, 2023

View reviewed changes

Implement fallback for effect param

84a4907

fee1-dead force-pushed the effect-fallback branch from 0b9ab98 to 84a4907 Compare September 10, 2023 07:49

oli-obk requested changes Sep 11, 2023

View reviewed changes

add is_host_effect to GenericParamDefKind::Const and address review

9654d5c

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 11, 2023

fee1-dead mentioned this pull request Sep 11, 2023

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

Open

4 tasks

matthiaskrgr mentioned this pull request Sep 11, 2023

Rollup of 8 pull requests #115758

Merged

bors merged commit e7a347b into rust-lang:master Sep 11, 2023
11 checks passed

rustbot added this to the 1.74.0 milestone Sep 11, 2023

matthiaskrgr mentioned this pull request Sep 12, 2023

ICE: effects: is not fully resolved #115791

Closed

fmease mentioned this pull request Oct 13, 2023

Hide host effect params from docs #116670

Merged

fee1-dead deleted the effect-fallback branch November 24, 2023 14:51

fee1-dead mentioned this pull request Nov 24, 2023

Remove a hack for effects #118246

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement fallback for effect param #115727

Implement fallback for effect param #115727

fee1-dead commented Sep 10, 2023 •

edited by oli-obk

Loading

rustbot commented Sep 10, 2023

This comment has been minimized.

compiler-errors Sep 10, 2023 •

edited

Loading

fee1-dead Sep 10, 2023

compiler-errors Sep 10, 2023

fee1-dead Sep 10, 2023

oli-obk Sep 11, 2023

oli-obk Sep 11, 2023

oli-obk Sep 11, 2023

oli-obk Sep 11, 2023

fee1-dead Sep 11, 2023

rustbot commented Sep 11, 2023

oli-obk commented Sep 11, 2023

bors commented Sep 11, 2023

		// todo what about using effect var here
		if self.tcx.has_attr(param.def_id, sym::rustc_host) {

Implement fallback for effect param #115727

Implement fallback for effect param #115727

Conversation

fee1-dead commented Sep 10, 2023 • edited by oli-obk Loading

rustbot commented Sep 10, 2023

This comment has been minimized.

compiler-errors Sep 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rustbot commented Sep 11, 2023

oli-obk commented Sep 11, 2023

bors commented Sep 11, 2023

fee1-dead commented Sep 10, 2023 •

edited by oli-obk

Loading

compiler-errors Sep 10, 2023 •

edited

Loading