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

Replace mir_built query with a hook and use mir_const everywhere instead #122721

Merged
merged 3 commits into from
Mar 25, 2024
Merged
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
2 changes: 1 addition & 1 deletion compiler/rustc_incremental/src/persist/dirty_clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ const BASE_HIR: &[&str] = &[
const BASE_IMPL: &[&str] =
&[label_strs::associated_item_def_ids, label_strs::generics_of, label_strs::impl_trait_header];

/// DepNodes for mir_built/Optimized, which is relevant in "executable"
/// DepNodes for exported mir bodies, which is relevant in "executable"
/// code, i.e., functions+methods
const BASE_MIR: &[&str] = &[label_strs::optimized_mir, label_strs::promoted_mir];

Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_middle/src/hooks/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,4 +77,10 @@ declare_hooks! {
///
/// (Eligible functions might nevertheless be skipped for other reasons.)
hook is_eligible_for_coverage(key: LocalDefId) -> bool;

/// Create the MIR for a given `DefId` - this includes
/// unreachable code.
/// You do not want to call this yourself, instead use the cached version
/// via `mir_built`
hook build_mir(key: LocalDefId) -> mir::Body<'tcx>;
}
16 changes: 5 additions & 11 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -485,21 +485,15 @@ rustc_queries! {
separate_provide_extern
}

/// Fetch the MIR for a given `DefId` right after it's built - this includes
/// unreachable code.
/// Build the MIR for a given `DefId` and prepare it for const qualification.
///
/// See the [rustc dev guide] for more info.
///
/// [rustc dev guide]: https://rustc-dev-guide.rust-lang.org/mir/construction.html
query mir_built(key: LocalDefId) -> &'tcx Steal<mir::Body<'tcx>> {
desc { |tcx| "building MIR for `{}`", tcx.def_path_str(key) }
}

/// Fetch the MIR for a given `DefId` up till the point where it is
/// ready for const qualification.
///
/// See the README for the `mir` module for details.
query mir_const(key: LocalDefId) -> &'tcx Steal<mir::Body<'tcx>> {
desc { |tcx| "preparing `{}` for borrow checking", tcx.def_path_str(key) }
no_hash
}

/// Try to build an abstract representation of the given constant.
query thir_abstract_const(
key: DefId
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_build/src/build/custom/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
//! present, and if so we branch off into this module, which implements the attribute by
//! implementing a custom lowering from THIR to MIR.
//!
//! The result of this lowering is returned "normally" from the `mir_built` query, with the only
//! The result of this lowering is returned "normally" from the `build_mir` hook, with the only
//! notable difference being that the `injected` field in the body is set. Various components of the
//! MIR pipeline, like borrowck and the pass manager will then consult this field (via
//! `body.should_skip()`) to skip the parts of the MIR pipeline that precede the MIR phase the user
Expand Down
11 changes: 3 additions & 8 deletions compiler/rustc_mir_build/src/build/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use rustc_middle::hir::place::PlaceBase as HirPlaceBase;
use rustc_middle::middle::region;
use rustc_middle::mir::interpret::Scalar;
use rustc_middle::mir::*;
use rustc_middle::query::TyCtxtAt;
use rustc_middle::thir::{
self, BindingMode, ExprId, LintLevel, LocalVarId, Param, ParamId, PatKind, Thir,
};
Expand All @@ -30,13 +31,6 @@ use rustc_target::spec::abi::Abi;

use super::lints;

pub(crate) fn mir_built(
tcx: TyCtxt<'_>,
def: LocalDefId,
) -> &rustc_data_structures::steal::Steal<Body<'_>> {
tcx.alloc_steal_mir(mir_build(tcx, def))
}

pub(crate) fn closure_saved_names_of_captured_variables<'tcx>(
tcx: TyCtxt<'tcx>,
def_id: LocalDefId,
Expand All @@ -54,7 +48,8 @@ pub(crate) fn closure_saved_names_of_captured_variables<'tcx>(
}

/// Construct the MIR for a given `DefId`.
fn mir_build<'tcx>(tcx: TyCtxt<'tcx>, def: LocalDefId) -> Body<'tcx> {
pub(crate) fn mir_build<'tcx>(tcx: TyCtxtAt<'tcx>, def: LocalDefId) -> Body<'tcx> {
let tcx = tcx.tcx;
tcx.ensure_with_value().thir_abstract_const(def);
if let Err(e) = tcx.check_match(def) {
return construct_error(tcx, def, e);
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_mir_build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,14 @@ mod errors;
pub mod lints;
mod thir;

use rustc_middle::query::Providers;
use rustc_middle::util::Providers;

rustc_fluent_macro::fluent_messages! { "../messages.ftl" }

pub fn provide(providers: &mut Providers) {
providers.check_match = thir::pattern::check_match;
providers.lit_to_const = thir::constant::lit_to_const;
providers.mir_built = build::mir_built;
providers.hooks.build_mir = build::mir_build;
providers.closure_saved_names_of_captured_variables =
build::closure_saved_names_of_captured_variables;
providers.check_unsafety = check_unsafety::check_unsafety;
Expand Down
20 changes: 8 additions & 12 deletions compiler/rustc_mir_transform/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ pub fn provide(providers: &mut Providers) {
cross_crate_inline::provide(providers);
providers.queries = query::Providers {
mir_keys,
mir_const,
mir_built,
mir_const_qualif,
mir_promoted,
mir_drops_elaborated_and_const_checked,
Expand Down Expand Up @@ -259,9 +259,9 @@ fn mir_const_qualif(tcx: TyCtxt<'_>, def: LocalDefId) -> ConstQualifs {

// N.B., this `borrow()` is guaranteed to be valid (i.e., the value
// cannot yet be stolen), because `mir_promoted()`, which steals
// from `mir_const()`, forces this query to execute before
// from `mir_built()`, forces this query to execute before
// performing the steal.
let body = &tcx.mir_const(def).borrow();
let body = &tcx.mir_built(def).borrow();

if body.return_ty().references_error() {
// It's possible to reach here without an error being emitted (#121103).
Expand All @@ -279,19 +279,13 @@ fn mir_const_qualif(tcx: TyCtxt<'_>, def: LocalDefId) -> ConstQualifs {
validator.qualifs_in_return_place()
}

/// Make MIR ready for const evaluation. This is run on all MIR, not just on consts!
/// FIXME(oli-obk): it's unclear whether we still need this phase (and its corresponding query).
/// We used to have this for pre-miri MIR based const eval.
fn mir_const(tcx: TyCtxt<'_>, def: LocalDefId) -> &Steal<Body<'_>> {
fn mir_built(tcx: TyCtxt<'_>, def: LocalDefId) -> &Steal<Body<'_>> {
// MIR unsafety check uses the raw mir, so make sure it is run.
if !tcx.sess.opts.unstable_opts.thir_unsafeck {
tcx.ensure_with_value().mir_unsafety_check_result(def);
}

// has_ffi_unwind_calls query uses the raw mir, so make sure it is run.
tcx.ensure_with_value().has_ffi_unwind_calls(def);

let mut body = tcx.mir_built(def).steal();
let mut body = tcx.build_mir(def);

pass_manager::dump_mir_for_phase_change(tcx, &body);

Expand Down Expand Up @@ -339,7 +333,9 @@ fn mir_promoted(
| DefKind::AnonConst => tcx.mir_const_qualif(def),
_ => ConstQualifs::default(),
};
let mut body = tcx.mir_const(def).steal();
// has_ffi_unwind_calls query uses the raw mir, so make sure it is run.
tcx.ensure_with_value().has_ffi_unwind_calls(def);
let mut body = tcx.mir_built(def).steal();
if let Some(error_reported) = const_qualifs.tainted_by_errors {
body.tainted_by_errors = Some(error_reported);
}
Expand Down
64 changes: 32 additions & 32 deletions tests/ui/binding/issue-53114-safety-checks.stderr
Original file line number Diff line number Diff line change
@@ -1,3 +1,23 @@
error[E0793]: reference to packed field is unaligned
--> $DIR/issue-53114-safety-checks.rs:23:13
|
LL | let _ = &p.b;
| ^^^^
|
= note: packed structs are only aligned by one byte, and many modern architectures penalize unaligned field accesses
= note: creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
= help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)

error[E0793]: reference to packed field is unaligned
--> $DIR/issue-53114-safety-checks.rs:28:17
|
LL | let (_,) = (&p.b,);
| ^^^^
|
= note: packed structs are only aligned by one byte, and many modern architectures penalize unaligned field accesses
= note: creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
= help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)

error[E0133]: access to union field is unsafe and requires unsafe function or block
--> $DIR/issue-53114-safety-checks.rs:24:13
|
Expand Down Expand Up @@ -31,20 +51,20 @@ LL | let (_,) = (&u2.a,);
= note: the field may not be properly initialized: using uninitialized data will cause undefined behavior

error[E0793]: reference to packed field is unaligned
--> $DIR/issue-53114-safety-checks.rs:23:13
--> $DIR/issue-53114-safety-checks.rs:37:16
|
LL | let _ = &p.b;
| ^^^^
LL | let _: _ = &p.b;
| ^^^^
|
= note: packed structs are only aligned by one byte, and many modern architectures penalize unaligned field accesses
= note: creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
= help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)

error[E0793]: reference to packed field is unaligned
--> $DIR/issue-53114-safety-checks.rs:28:17
--> $DIR/issue-53114-safety-checks.rs:42:20
|
LL | let (_,) = (&p.b,);
| ^^^^
LL | let (_,): _ = (&p.b,);
| ^^^^
|
= note: packed structs are only aligned by one byte, and many modern architectures penalize unaligned field accesses
= note: creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
Expand Down Expand Up @@ -83,20 +103,20 @@ LL | let (_,): _ = (&u2.a,);
= note: the field may not be properly initialized: using uninitialized data will cause undefined behavior

error[E0793]: reference to packed field is unaligned
--> $DIR/issue-53114-safety-checks.rs:37:16
--> $DIR/issue-53114-safety-checks.rs:51:11
|
LL | let _: _ = &p.b;
| ^^^^
LL | match &p.b { _ => { } }
| ^^^^
|
= note: packed structs are only aligned by one byte, and many modern architectures penalize unaligned field accesses
= note: creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
= help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)

error[E0793]: reference to packed field is unaligned
--> $DIR/issue-53114-safety-checks.rs:42:20
--> $DIR/issue-53114-safety-checks.rs:56:12
|
LL | let (_,): _ = (&p.b,);
| ^^^^
LL | match (&p.b,) { (_,) => { } }
| ^^^^
|
= note: packed structs are only aligned by one byte, and many modern architectures penalize unaligned field accesses
= note: creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
Expand Down Expand Up @@ -134,26 +154,6 @@ LL | match (&u2.a,) { (_,) => { } }
|
= note: the field may not be properly initialized: using uninitialized data will cause undefined behavior

error[E0793]: reference to packed field is unaligned
--> $DIR/issue-53114-safety-checks.rs:51:11
|
LL | match &p.b { _ => { } }
| ^^^^
|
= note: packed structs are only aligned by one byte, and many modern architectures penalize unaligned field accesses
= note: creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
= help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)

error[E0793]: reference to packed field is unaligned
--> $DIR/issue-53114-safety-checks.rs:56:12
|
LL | match (&p.b,) { (_,) => { } }
| ^^^^
|
= note: packed structs are only aligned by one byte, and many modern architectures penalize unaligned field accesses
= note: creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
= help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)

error: aborting due to 18 previous errors

Some errors have detailed explanations: E0133, E0793.
Expand Down
5 changes: 0 additions & 5 deletions tests/ui/coroutine/clone-rpit.next.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,6 @@ note: ...which requires promoting constants in MIR for `foo::{closure#0}`...
|
LL | move |_: ()| {
| ^^^^^^^^^^^^
note: ...which requires preparing `foo::{closure#0}` for borrow checking...
--> $DIR/clone-rpit.rs:14:5
|
LL | move |_: ()| {
| ^^^^^^^^^^^^
note: ...which requires checking if `foo::{closure#0}` contains FFI-unwind calls...
--> $DIR/clone-rpit.rs:14:5
|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ LL | struct Dealigned<T>(u8, T);
|
= Box<dyn Any>
query stack during panic:
#0 [mir_const] preparing `<impl at $DIR/multiple_definitions_attribute_merging.rs:15:10: 15:19>::eq` for borrow checking
#1 [mir_promoted] promoting constants in MIR for `<impl at $DIR/multiple_definitions_attribute_merging.rs:15:10: 15:19>::eq`
#0 [mir_built] building MIR for `<impl at $DIR/multiple_definitions_attribute_merging.rs:15:10: 15:19>::eq`
#1 [check_unsafety] unsafety-checking `<impl at $DIR/multiple_definitions_attribute_merging.rs:15:10: 15:19>::eq`
end of query stack
error: aborting due to 2 previous errors

Expand Down
4 changes: 2 additions & 2 deletions tests/ui/resolve/proc_macro_generated_packed.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ LL | struct Dealigned<T>(u8, T);
|
= Box<dyn Any>
query stack during panic:
#0 [mir_const] preparing `<impl at $DIR/proc_macro_generated_packed.rs:15:10: 15:19>::eq` for borrow checking
#1 [mir_promoted] promoting constants in MIR for `<impl at $DIR/proc_macro_generated_packed.rs:15:10: 15:19>::eq`
#0 [mir_built] building MIR for `<impl at $DIR/proc_macro_generated_packed.rs:15:10: 15:19>::eq`
#1 [check_unsafety] unsafety-checking `<impl at $DIR/proc_macro_generated_packed.rs:15:10: 15:19>::eq`
end of query stack
error: aborting due to 1 previous error

Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,6 @@ note: ...which requires const checking `Alpha::V3::{constant#0}`...
|
LL | V3 = Self::V1 {} as u8 + 2,
| ^^^^^^^^^^^^^^^^^^^^^
note: ...which requires preparing `Alpha::V3::{constant#0}` for borrow checking...
--> $DIR/self-in-enum-definition.rs:5:10
|
LL | V3 = Self::V1 {} as u8 + 2,
| ^^^^^^^^^^^^^^^^^^^^^
note: ...which requires building MIR for `Alpha::V3::{constant#0}`...
--> $DIR/self-in-enum-definition.rs:5:10
|
Expand Down
Loading