Create const blocks on the fly during typeck instead of waiting until writeback. #125806

oli-obk · 2024-05-31T08:32:14Z

This allows us to spawn a nested FnCtxt, creating a barrier that control flow statements cannot cross.

The issues were caused by #124650 where I failed to account for the lack of changing scopes for const blocks

… writeback. This allows us to spawn a nested FnCtxt, creating a barrier that control flow statements cannot cross.

rustbot · 2024-05-31T08:32:23Z

rustbot has assigned @fee1-dead.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

oli-obk · 2024-05-31T08:33:24Z

compiler/rustc_hir_typeck/src/expr.rs

-            let encl_body_owner_id = self.tcx.hir().enclosing_body_owner(expr.hir_id);
-
            // If this didn't hold, we would not have to report an error in
            // the first place.
-            assert_ne!(encl_item_id.def_id, encl_body_owner_id);
+            assert_ne!(encl_item_id.def_id, self.body_id);

-            let encl_body = self.tcx.hir().body_owned_by(encl_body_owner_id);
-
-            err.encl_body_span = Some(encl_body.value.span);
+            err.encl_body_span = Some(self.tcx.def_span(self.body_id));


This change is because enclosing_body_owner does not yield const blocks anymore. I'm addressing this separately to remove this footgun, but the code here could have been written simpler anyway, so I just did that.

enclosing_body_owner being out of sync is somewhat sketch 💀 why doesn't it? it should be as much of a body as a closure is, for example.

compiler-errors

I think this change definitely improves things, but I'm still somewhat skeptical that we can't improve it more... Thoughts?

compiler-errors · 2024-05-31T14:20:24Z

compiler/rustc_ast_lowering/src/expr.rs

@@ -76,7 +76,12 @@ impl<'hir> LoweringContext<'_, 'hir> {
                ExprKind::Array(exprs) => hir::ExprKind::Array(self.lower_exprs(exprs)),
                ExprKind::ConstBlock(c) => {
                    self.has_inline_consts = true;
-                    hir::ExprKind::ConstBlock(self.lower_expr(c))
+                    self.with_new_scopes(e.span, |this| {
+                        let coroutine_kind = this.coroutine_kind.take();


Kind of strange that coroutine_kind is not handled in with_new_scopes. Is there some other with_body_*-like function that does the coroutine kind handling too?

I have been looking into this in a branch, but didn't want to add more stuff to this PR.

we can get it all working by merging with_new_scopes into lower_body, but it needs some managing of the right spans to maintain diagnostic quality.

compiler-errors · 2024-05-31T14:21:34Z

compiler/rustc_hir_typeck/src/expr.rs

-            let encl_body_owner_id = self.tcx.hir().enclosing_body_owner(expr.hir_id);
-
            // If this didn't hold, we would not have to report an error in
            // the first place.
-            assert_ne!(encl_item_id.def_id, encl_body_owner_id);
+            assert_ne!(encl_item_id.def_id, self.body_id);

-            let encl_body = self.tcx.hir().body_owned_by(encl_body_owner_id);
-
-            err.encl_body_span = Some(encl_body.value.span);
+            err.encl_body_span = Some(self.tcx.def_span(self.body_id));


enclosing_body_owner being out of sync is somewhat sketch 💀 why doesn't it? it should be as much of a body as a closure is, for example.

compiler-errors · 2024-05-31T14:22:59Z

compiler/rustc_hir_typeck/src/expr.rs

+        let feed = self.tcx().create_def(self.body_id, kw::Empty, DefKind::InlineConst);
+        feed.def_span(span);
+        feed.local_def_id_to_hir_id(hir_id);


I feel like const blocks should be first-class nested bodies (liek closures) all the way from the AST-level (i.e. having a def id, being treated as a hir owner, etc) instead of having to synthesize a def-id here.

that's what they used to be (before #124650), but it meant we couldn't feed the type_of query. Turns out I couldn't do that anyway, so now I think I should probably just revert #124650 and try again from scratch

Perhaps yeah

though we'll def need to lazily create the DefId of anon consts of method calls' const generic params, so that we can feed type_of. That's why I'm trying to figure out how to do this better.

right now we are breaking query system invariants and are feeding anon const type_of from typeck, even though typeck did not create the DefId.

Also, why do we need to feed the type_of query at all?

The type_of for a const block should operate like a type_of for a closure -- for closures, it calls typeck_results on the parent and extracts it from the expr_ty:

rust/compiler/rustc_hir_analysis/src/collect/type_of.rs

Lines 483 to 485 in 2a2c29a

Node::Expr(&Expr { kind: ExprKind::Closure { .. }, .. }) => {

tcx.typeck(def_id).node_type(hir_id)

}

though we'll def need to lazily create the DefId of anon consts of method calls' const generic params, so that we can feed type_of. That's why I'm trying to figure out how to do this better.

I don't understand why this needs to be entangled with inline const exprs. Perhaps we should just distinguish these two rather than treating them the same.

oli-obk · 2024-05-31T15:55:45Z

Just gonna revert #124650 and start over

…r-errors Revert: create const block bodies in typeck via query feeding as per the discussion in rust-lang#125806 (comment) It was a mistake to try to shoehorn const blocks and some specific anon consts into the same box and feed them during typeck. It turned out not simplifying anything (my hope was that we could feed `type_of` to start avoiding the huge HIR matcher, but that didn't work out), but instead making a few things more fragile. reverts the const-block-specific parts of rust-lang#124650 `@bors` rollup=never had a small perf impact previously fixes rust-lang#125846 r? `@compiler-errors`

…r-errors Revert: create const block bodies in typeck via query feeding as per the discussion in rust-lang#125806 (comment) It was a mistake to try to shoehorn const blocks and some specific anon consts into the same box and feed them during typeck. It turned out not simplifying anything (my hope was that we could feed `type_of` to start avoiding the huge HIR matcher, but that didn't work out), but instead making a few things more fragile. reverts the const-block-specific parts of rust-lang#124650 ``@bors`` rollup=never had a small perf impact previously fixes rust-lang#125846 r? ``@compiler-errors``

…r-errors Revert: create const block bodies in typeck via query feeding as per the discussion in rust-lang#125806 (comment) It was a mistake to try to shoehorn const blocks and some specific anon consts into the same box and feed them during typeck. It turned out not simplifying anything (my hope was that we could feed `type_of` to start avoiding the huge HIR matcher, but that didn't work out), but instead making a few things more fragile. reverts the const-block-specific parts of rust-lang#124650 ```@bors``` rollup=never had a small perf impact previously fixes rust-lang#125846 r? ```@compiler-errors```

…errors Revert: create const block bodies in typeck via query feeding as per the discussion in rust-lang#125806 (comment) It was a mistake to try to shoehorn const blocks and some specific anon consts into the same box and feed them during typeck. It turned out not simplifying anything (my hope was that we could feed `type_of` to start avoiding the huge HIR matcher, but that didn't work out), but instead making a few things more fragile. reverts the const-block-specific parts of rust-lang#124650 `@bors` rollup=never had a small perf impact previously fixes rust-lang#125846 r? `@compiler-errors`

Revert: create const block bodies in typeck via query feeding as per the discussion in rust-lang/rust#125806 (comment) It was a mistake to try to shoehorn const blocks and some specific anon consts into the same box and feed them during typeck. It turned out not simplifying anything (my hope was that we could feed `type_of` to start avoiding the huge HIR matcher, but that didn't work out), but instead making a few things more fragile. reverts the const-block-specific parts of rust-lang/rust#124650 `@bors` rollup=never had a small perf impact previously fixes rust-lang/rust#125846 r? `@compiler-errors`

oli-obk added 2 commits May 31, 2024 08:29

Create const blocks on the fly during typeck instead of waiting until…

9f04fc6

… writeback. This allows us to spawn a nested FnCtxt, creating a barrier that control flow statements cannot cross.

Properly report await errors

5e3f578

rustbot assigned fee1-dead May 31, 2024

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. labels May 31, 2024

oli-obk commented May 31, 2024

View reviewed changes

compiler-errors reviewed May 31, 2024

View reviewed changes

compiler-errors assigned compiler-errors and unassigned fee1-dead May 31, 2024

oli-obk closed this May 31, 2024

oli-obk mentioned this pull request Jun 3, 2024

Revert: create const block bodies in typeck via query feeding #125918

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create const blocks on the fly during typeck instead of waiting until writeback. #125806

Create const blocks on the fly during typeck instead of waiting until writeback. #125806

oli-obk commented May 31, 2024

rustbot commented May 31, 2024

oli-obk May 31, 2024

compiler-errors May 31, 2024

compiler-errors left a comment

compiler-errors May 31, 2024

oli-obk May 31, 2024

compiler-errors May 31, 2024

compiler-errors May 31, 2024

oli-obk May 31, 2024

compiler-errors May 31, 2024

oli-obk May 31, 2024

oli-obk May 31, 2024

compiler-errors May 31, 2024

compiler-errors May 31, 2024

oli-obk commented May 31, 2024

	Node::Expr(&Expr { kind: ExprKind::Closure { .. }, .. }) => {
	tcx.typeck(def_id).node_type(hir_id)
	}

Create const blocks on the fly during typeck instead of waiting until writeback. #125806

Create const blocks on the fly during typeck instead of waiting until writeback. #125806

Conversation

oli-obk commented May 31, 2024

rustbot commented May 31, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

compiler-errors left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oli-obk commented May 31, 2024