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

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

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

let e = hir::ExprKind::ConstBlock(this.lower_expr(c));
this.coroutine_kind = coroutine_kind;
e
})
}
ExprKind::Repeat(expr, count) => {
let expr = self.lower_expr(expr);
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2131,6 +2131,7 @@ impl LoopSource {
}

#[derive(Copy, Clone, Debug, PartialEq, HashStable_Generic)]
// FIXME: eagerly report these errors and remove this enum and the wfcheck.
pub enum LoopIdError {
OutsideLoopScope,
UnlabeledCfInWhileCondition,
Expand Down
33 changes: 26 additions & 7 deletions compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
ExprKind::DropTemps(e) => self.check_expr_with_expectation(e, expected),
ExprKind::Array(args) => self.check_expr_array(args, expected, expr),
ExprKind::ConstBlock(ref block) => self.check_expr_with_expectation(block, expected),
ExprKind::ConstBlock(ref block) => {
self.check_expr_const_block(block, expr.hir_id, expr.span, expected)
}
ExprKind::Repeat(element, ref count) => {
self.check_expr_repeat(element, count, expected, expr)
}
Expand Down Expand Up @@ -903,15 +905,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// We are inside a function body, so reporting "return statement
// outside of function body" needs an explanation.

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));
Comment on lines -906 to +912
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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.

err.encl_fn_span = Some(*encl_fn_span);
}

Expand Down Expand Up @@ -1458,6 +1456,27 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}

fn check_expr_const_block(
&self,
block: &'tcx hir::Expr<'tcx>,
hir_id: HirId,
span: Span,
expected: Expectation<'tcx>,
) -> Ty<'tcx> {
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);
Comment on lines +1466 to +1468
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps yeah

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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:

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

Copy link
Member

Choose a reason for hiding this comment

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

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.

self.typeck_results.borrow_mut().inline_consts.insert(hir_id.local_id, feed.def_id());

// Create a new function context.
let fcx = FnCtxt::new(self, self.param_env, feed.key());

let ty = fcx.check_expr_with_expectation(block, expected);
fcx.require_type_is_sized(ty, block.span, ObligationCauseCode::ConstSized);
fcx.write_ty(block.hir_id, ty);
ty
}

fn check_expr_repeat(
&self,
element: &'tcx hir::Expr<'tcx>,
Expand Down
12 changes: 4 additions & 8 deletions compiler/rustc_hir_typeck/src/writeback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
// generic parameters.

use crate::FnCtxt;
use hir::def::DefKind;
use rustc_data_structures::unord::ExtendUnord;
use rustc_errors::{ErrorGuaranteed, StashKey};
use rustc_hir as hir;
Expand All @@ -17,7 +16,7 @@ use rustc_middle::ty::fold::{TypeFoldable, TypeFolder};
use rustc_middle::ty::visit::TypeVisitableExt;
use rustc_middle::ty::TypeSuperFoldable;
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_span::symbol::{kw, sym};
use rustc_span::symbol::sym;
use rustc_span::Span;
use rustc_trait_selection::solve;
use rustc_trait_selection::traits::error_reporting::TypeErrCtxtExt;
Expand Down Expand Up @@ -84,6 +83,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
wbcx.typeck_results.treat_byte_string_as_slice =
mem::take(&mut self.typeck_results.borrow_mut().treat_byte_string_as_slice);

wbcx.typeck_results.inline_consts =
mem::take(&mut self.typeck_results.borrow_mut().inline_consts);

debug!("writeback: typeck results for {:?} are {:#?}", item_def_id, wbcx.typeck_results);

self.tcx.arena.alloc(wbcx.typeck_results)
Expand Down Expand Up @@ -296,12 +298,6 @@ impl<'cx, 'tcx> Visitor<'tcx> for WritebackCx<'cx, 'tcx> {
hir::ExprKind::Field(..) | hir::ExprKind::OffsetOf(..) => {
self.visit_field_id(e.hir_id);
}
hir::ExprKind::ConstBlock(_) => {
let feed = self.tcx().create_def(self.fcx.body_id, kw::Empty, DefKind::InlineConst);
feed.def_span(e.span);
feed.local_def_id_to_hir_id(e.hir_id);
self.typeck_results.inline_consts.insert(e.hir_id.local_id, feed.def_id());
}
_ => {}
}

Expand Down
39 changes: 39 additions & 0 deletions tests/ui/inline-const/cross_const_control_flow.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
//@edition:2021

fn foo() {
const { return }
//~^ ERROR: return statement outside of function body
}

fn labelled_block_break() {
'a: { const { break 'a } }
//~^ ERROR: `break` outside of a loop or labeled block
//~| ERROR: use of unreachable label
}

fn loop_break() {
loop {
const { break }
//~^ ERROR: `break` outside of a loop or labeled block
}
}

fn continue_to_labelled_block() {
'a: { const { continue 'a } }
//~^ ERROR: `continue` outside of a loop
//~| ERROR: use of unreachable label
}

fn loop_continue() {
loop {
const { continue }
//~^ ERROR: `continue` outside of a loop
}
}

async fn await_across_const_block() {
const { async {}.await }
//~^ ERROR: `await` is only allowed inside `async` functions and blocks
}

fn main() {}
67 changes: 67 additions & 0 deletions tests/ui/inline-const/cross_const_control_flow.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
error[E0767]: use of unreachable label `'a`
--> $DIR/cross_const_control_flow.rs:9:25
|
LL | 'a: { const { break 'a } }
| -- ^^ unreachable label `'a`
| |
| unreachable label defined here
|
= note: labels are unreachable through functions, closures, async blocks and modules

error[E0767]: use of unreachable label `'a`
--> $DIR/cross_const_control_flow.rs:22:28
|
LL | 'a: { const { continue 'a } }
| -- ^^ unreachable label `'a`
| |
| unreachable label defined here
|
= note: labels are unreachable through functions, closures, async blocks and modules

error[E0728]: `await` is only allowed inside `async` functions and blocks
--> $DIR/cross_const_control_flow.rs:35:22
|
LL | const { async {}.await }
| -----------------^^^^^--
| | |
| | only allowed inside `async` functions and blocks
| this is not `async`

error[E0268]: `break` outside of a loop or labeled block
--> $DIR/cross_const_control_flow.rs:9:19
|
LL | 'a: { const { break 'a } }
| ^^^^^^^^ cannot `break` outside of a loop or labeled block

error[E0268]: `break` outside of a loop or labeled block
--> $DIR/cross_const_control_flow.rs:16:17
|
LL | const { break }
| ^^^^^ cannot `break` outside of a loop or labeled block

error[E0268]: `continue` outside of a loop
--> $DIR/cross_const_control_flow.rs:22:19
|
LL | 'a: { const { continue 'a } }
| ^^^^^^^^^^^ cannot `continue` outside of a loop

error[E0268]: `continue` outside of a loop
--> $DIR/cross_const_control_flow.rs:29:17
|
LL | const { continue }
| ^^^^^^^^ cannot `continue` outside of a loop

error[E0572]: return statement outside of function body
--> $DIR/cross_const_control_flow.rs:4:13
|
LL | / fn foo() {
LL | | const { return }
| | --------^^^^^^-- the return is part of this body...
LL | |
LL | | }
| |_- ...not the enclosing function body

error: aborting due to 8 previous errors

Some errors have detailed explanations: E0268, E0572, E0728, E0767.
For more information about an error, try `rustc --explain E0268`.
Loading