From b1585983ccd83c8bf15e5e19ec594c00b8a37a2e Mon Sep 17 00:00:00 2001 From: Kirby Linvill Date: Thu, 2 Nov 2023 09:05:35 -0600 Subject: [PATCH 1/8] Add stable MIR Projections support based on MIR structure This commit includes richer projections for both Places and UserTypeProjections. However, the tests only touch on Places. There are also outstanding TODOs regarding how projections should be resolved to produce Place types, and regarding if UserTypeProjections should just contain ProjectionElem<(),()> objects as in MIR. --- compiler/rustc_smir/src/rustc_smir/mod.rs | 69 +++++++++++- compiler/stable_mir/src/mir/body.rs | 117 ++++++++++++++++++++- tests/ui-fulldeps/stable-mir/crate-info.rs | 103 ++++++++++++++++++ 3 files changed, 283 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_smir/src/rustc_smir/mod.rs b/compiler/rustc_smir/src/rustc_smir/mod.rs index 27596c08f1c1e..83bacfb8263f8 100644 --- a/compiler/rustc_smir/src/rustc_smir/mod.rs +++ b/compiler/rustc_smir/src/rustc_smir/mod.rs @@ -682,10 +682,39 @@ impl<'tcx> Stable<'tcx> for mir::ConstOperand<'tcx> { impl<'tcx> Stable<'tcx> for mir::Place<'tcx> { type T = stable_mir::mir::Place; - fn stable(&self, _: &mut Tables<'tcx>) -> Self::T { + fn stable(&self, tables: &mut Tables<'tcx>) -> Self::T { stable_mir::mir::Place { local: self.local.as_usize(), - projection: format!("{:?}", self.projection), + projection: self.projection.iter().map(|e| e.stable(tables)).collect(), + } + } +} + +impl<'tcx> Stable<'tcx> for mir::PlaceElem<'tcx> { + type T = stable_mir::mir::ProjectionElem; + fn stable(&self, tables: &mut Tables<'tcx>) -> Self::T { + use mir::ProjectionElem::*; + match self { + Deref => stable_mir::mir::ProjectionElem::Deref, + Field(idx, ty) => { + stable_mir::mir::ProjectionElem::Field(idx.stable(tables), ty.stable(tables)) + } + Index(local) => stable_mir::mir::ProjectionElem::Index(local.stable(tables)), + ConstantIndex { offset, min_length, from_end } => { + stable_mir::mir::ProjectionElem::ConstantIndex { + offset: *offset, + min_length: *min_length, + from_end: *from_end, + } + } + Subslice { from, to, from_end } => stable_mir::mir::ProjectionElem::Subslice { + from: *from, + to: *to, + from_end: *from_end, + }, + Downcast(_, idx) => stable_mir::mir::ProjectionElem::Downcast(idx.stable(tables)), + OpaqueCast(ty) => stable_mir::mir::ProjectionElem::OpaqueCast(ty.stable(tables)), + Subtype(ty) => stable_mir::mir::ProjectionElem::Subtype(ty.stable(tables)), } } } @@ -693,8 +722,40 @@ impl<'tcx> Stable<'tcx> for mir::Place<'tcx> { impl<'tcx> Stable<'tcx> for mir::UserTypeProjection { type T = stable_mir::mir::UserTypeProjection; - fn stable(&self, _: &mut Tables<'tcx>) -> Self::T { - UserTypeProjection { base: self.base.as_usize(), projection: format!("{:?}", self.projs) } + fn stable(&self, tables: &mut Tables<'tcx>) -> Self::T { + UserTypeProjection { + base: self.base.as_usize(), + projection: self.projs.iter().map(|e| e.stable(tables)).collect(), + } + } +} + +// ProjectionKind is nearly identical to PlaceElem, except its generic arguments are units. We +// therefore don't need to resolve any arguments with the generic types. +impl<'tcx> Stable<'tcx> for mir::ProjectionKind { + type T = stable_mir::mir::ProjectionElem<(), ()>; + fn stable(&self, tables: &mut Tables<'tcx>) -> Self::T { + use mir::ProjectionElem::*; + match self { + Deref => stable_mir::mir::ProjectionElem::Deref, + Field(idx, ty) => stable_mir::mir::ProjectionElem::Field(idx.stable(tables), *ty), + Index(local) => stable_mir::mir::ProjectionElem::Index(*local), + ConstantIndex { offset, min_length, from_end } => { + stable_mir::mir::ProjectionElem::ConstantIndex { + offset: *offset, + min_length: *min_length, + from_end: *from_end, + } + } + Subslice { from, to, from_end } => stable_mir::mir::ProjectionElem::Subslice { + from: *from, + to: *to, + from_end: *from_end, + }, + Downcast(_, idx) => stable_mir::mir::ProjectionElem::Downcast(idx.stable(tables)), + OpaqueCast(ty) => stable_mir::mir::ProjectionElem::OpaqueCast(*ty), + Subtype(ty) => stable_mir::mir::ProjectionElem::Subtype(*ty), + } } } diff --git a/compiler/stable_mir/src/mir/body.rs b/compiler/stable_mir/src/mir/body.rs index 0693378368534..69e83efdf43a3 100644 --- a/compiler/stable_mir/src/mir/body.rs +++ b/compiler/stable_mir/src/mir/body.rs @@ -398,22 +398,133 @@ pub enum Operand { pub struct Place { pub local: Local, /// projection out of a place (access a field, deref a pointer, etc) - pub projection: String, + pub projection: Vec>, +} + +// TODO(klinvill): in MIR ProjectionElem is parameterized on the second Field argument and the Index +// argument. This is so it can be used for both the rust provided Places (for which the projection +// elements are of type ProjectionElem) and user-provided type annotations (for which the +// projection elements are of type ProjectionElem<(), ()>). Should we do the same thing in Stable MIR? +#[derive(Clone, Debug)] +pub enum ProjectionElem { + /// Dereference projections (e.g. `*_1`) project to the address referenced by the base place. + Deref, + + /// A field projection (e.g., `f` in `_1.f`) project to a field in the base place. The field is + /// referenced by source-order index rather than the name of the field. The fields type is also + /// given. + Field(FieldIdx, T), + + /// Index into a slice/array. The value of the index is computed at runtime using the `V` + /// argument. + /// + /// Note that this does not also dereference, and so it does not exactly correspond to slice + /// indexing in Rust. In other words, in the below Rust code: + /// + /// ```rust + /// let x = &[1, 2, 3, 4]; + /// let i = 2; + /// x[i]; + /// ``` + /// + /// The `x[i]` is turned into a `Deref` followed by an `Index`, not just an `Index`. The same + /// thing is true of the `ConstantIndex` and `Subslice` projections below. + Index(V), + + /// Index into a slice/array given by offsets. + /// + /// These indices are generated by slice patterns. Easiest to explain by example: + /// + /// ```ignore (illustrative) + /// [X, _, .._, _, _] => { offset: 0, min_length: 4, from_end: false }, + /// [_, X, .._, _, _] => { offset: 1, min_length: 4, from_end: false }, + /// [_, _, .._, X, _] => { offset: 2, min_length: 4, from_end: true }, + /// [_, _, .._, _, X] => { offset: 1, min_length: 4, from_end: true }, + /// ``` + ConstantIndex { + /// index or -index (in Python terms), depending on from_end + offset: u64, + /// The thing being indexed must be at least this long. For arrays this + /// is always the exact length. + min_length: u64, + /// Counting backwards from end? This is always false when indexing an + /// array. + from_end: bool, + }, + + /// Projects a slice from the base place. + /// + /// These indices are generated by slice patterns. If `from_end` is true, this represents + /// `slice[from..slice.len() - to]`. Otherwise it represents `array[from..to]`. + Subslice { + from: u64, + to: u64, + /// Whether `to` counts from the start or end of the array/slice. + from_end: bool, + }, + + /// "Downcast" to a variant of an enum or a coroutine. + // + // TODO(klinvill): MIR includes an Option argument that is the name of the variant, used + // for printing MIR. However I don't see it used anywhere. Is such a field needed or can we just + // include the VariantIdx which could be used to recover the field name if needed? + Downcast(VariantIdx), + + /// Like an explicit cast from an opaque type to a concrete type, but without + /// requiring an intermediate variable. + OpaqueCast(T), + + /// A `Subtype(T)` projection is applied to any `StatementKind::Assign` where + /// type of lvalue doesn't match the type of rvalue, the primary goal is making subtyping + /// explicit during optimizations and codegen. + /// + /// This projection doesn't impact the runtime behavior of the program except for potentially changing + /// some type metadata of the interpreter or codegen backend. + Subtype(T), } #[derive(Clone, Debug, Eq, PartialEq)] pub struct UserTypeProjection { pub base: UserTypeAnnotationIndex, - pub projection: String, + + /// `UserTypeProjection` projections need neither the `V` parameter for `Index` nor the `T` for + /// `Field`. + pub projection: Vec>, } pub type Local = usize; pub const RETURN_LOCAL: Local = 0; +/// The source-order index of a field in a variant. +/// +/// For example, in the following types, +/// ```rust +/// enum Demo1 { +/// Variant0 { a: bool, b: i32 }, +/// Variant1 { c: u8, d: u64 }, +/// } +/// struct Demo2 { e: u8, f: u16, g: u8 } +/// ``` +/// `a`'s `FieldIdx` is `0`, +/// `b`'s `FieldIdx` is `1`, +/// `c`'s `FieldIdx` is `0`, and +/// `g`'s `FieldIdx` is `2`. type FieldIdx = usize; /// The source-order index of a variant in a type. +/// +/// For example, in the following types, +/// ```rust +/// enum Demo1 { +/// Variant0 { a: bool, b: i32 }, +/// Variant1 { c: u8, d: u64 }, +/// } +/// struct Demo2 { e: u8, f: u16, g: u8 } +/// ``` +/// `a` is in the variant with the `VariantIdx` of `0`, +/// `c` is in the variant with the `VariantIdx` of `1`, and +/// `g` is in the variant with the `VariantIdx` of `0`. pub type VariantIdx = usize; type UserTypeAnnotationIndex = usize; @@ -536,6 +647,8 @@ impl Constant { } impl Place { + // TODO(klinvill): What is the expected behavior of this function? Should it resolve down the + // chain of projections so that `*(_1.f)` would end up returning the type referenced by `f`? pub fn ty(&self, locals: &[LocalDecl]) -> Ty { let _start_ty = locals[self.local].ty; todo!("Implement projection") diff --git a/tests/ui-fulldeps/stable-mir/crate-info.rs b/tests/ui-fulldeps/stable-mir/crate-info.rs index ed6b786f5e1de..c6aaea1487202 100644 --- a/tests/ui-fulldeps/stable-mir/crate-info.rs +++ b/tests/ui-fulldeps/stable-mir/crate-info.rs @@ -23,6 +23,7 @@ use rustc_hir::def::DefKind; use rustc_middle::ty::TyCtxt; use rustc_smir::rustc_internal; use stable_mir::mir::mono::Instance; +use stable_mir::mir::{ProjectionElem, Rvalue, StatementKind}; use stable_mir::ty::{RigidTy, TyKind}; use std::assert_matches::assert_matches; use std::io::Write; @@ -163,6 +164,99 @@ fn test_stable_mir(_tcx: TyCtxt<'_>) -> ControlFlow<()> { stable_mir::ty::TyKind::RigidTy(stable_mir::ty::RigidTy::Bool) ); + let projections_fn = get_item(&items, (DefKind::Fn, "projections")).unwrap(); + let body = projections_fn.body(); + assert_eq!(body.blocks.len(), 4); + // The first statement assigns `&s.c` to a local. The projections include a deref for `s`, since + // `s` is passed as a reference argument, and a field access for field `c`. + match &body.blocks[0].statements[0].kind { + StatementKind::Assign( + stable_mir::mir::Place { local: _, projection: local_proj }, + Rvalue::Ref(_, _, stable_mir::mir::Place { local: _, projection: r_proj }), + ) => { + // We can't match on vecs, only on slices. Comparing for equality wouldn't be any easier + // since we'd then have to add in the expected local and region values instead of + // matching on wildcards. + assert_matches!(local_proj[..], []); + match &r_proj[..] { + // Similarly we can't match against a type, only against its kind. + [ProjectionElem::Deref, ProjectionElem::Field(2, ty)] => assert_matches!( + ty.kind(), + TyKind::RigidTy(RigidTy::Uint(stable_mir::ty::UintTy::U8)) + ), + other => panic!( + "Unable to match against expected rvalue projection. Expected the projection \ + for `s.c`, which is a Deref and u8 Field. Got: {:?}", + other + ), + }; + } + other => panic!( + "Unable to match against expected Assign statement with a Ref rvalue. Expected the \ + statement to assign `&s.c` to a local. Got: {:?}", + other + ), + }; + // This statement assigns `slice[1]` to a local. The projections include a deref for `slice`, + // since `slice` is a reference, and an index. + match &body.blocks[2].statements[0].kind { + StatementKind::Assign( + stable_mir::mir::Place { local: _, projection: local_proj }, + Rvalue::Use(stable_mir::mir::Operand::Copy(stable_mir::mir::Place { + local: _, + projection: r_proj, + })), + ) => { + // We can't match on vecs, only on slices. Comparing for equality wouldn't be any easier + // since we'd then have to add in the expected local values instead of matching on + // wildcards. + assert_matches!(local_proj[..], []); + assert_matches!(r_proj[..], [ProjectionElem::Deref, ProjectionElem::Index(_)]); + } + other => panic!( + "Unable to match against expected Assign statement with a Use rvalue. Expected the \ + statement to assign `slice[1]` to a local. Got: {:?}", + other + ), + }; + // The first terminator gets a slice of an array via the Index operation. Specifically it + // performs `&vals[1..3]`. There are no projections in this case, the arguments are just locals. + match &body.blocks[0].terminator.kind { + stable_mir::mir::TerminatorKind::Call { args, .. } => + // We can't match on vecs, only on slices. Comparing for equality wouldn't be any easier + // since we'd then have to add in the expected local values instead of matching on + // wildcards. + { + match &args[..] { + [ + stable_mir::mir::Operand::Move(stable_mir::mir::Place { + local: _, + projection: arg1_proj, + }), + stable_mir::mir::Operand::Move(stable_mir::mir::Place { + local: _, + projection: arg2_proj, + }), + ] => { + assert_matches!(arg1_proj[..], []); + assert_matches!(arg2_proj[..], []); + } + other => { + panic!( + "Unable to match against expected arguments to Index call. Expected two \ + move operands. Got: {:?}", + other + ) + } + } + } + other => panic!( + "Unable to match against expected Call terminator. Expected a terminator that calls \ + the Index operation. Got: {:?}", + other + ), + }; + ControlFlow::Continue(()) } @@ -242,6 +336,15 @@ fn generate_input(path: &str) -> std::io::Result<()> { }} else {{ 'b' }} + }} + + pub struct Struct1 {{ _a: u8, _b: u16, c: u8 }} + + pub fn projections(s: &Struct1) -> u8 {{ + let v = &s.c; + let vals = [1, 2, 3, 4]; + let slice = &vals[1..3]; + v + slice[1] }}"# )?; Ok(()) From 98228e67bc11a15e03c4db1bd61ffda6b4620ecb Mon Sep 17 00:00:00 2001 From: Kirby Linvill Date: Thu, 9 Nov 2023 17:59:24 -0700 Subject: [PATCH 2/8] Move SMIR projections tests to new file --- tests/ui-fulldeps/stable-mir/crate-info.rs | 103 ------------ tests/ui-fulldeps/stable-mir/projections.rs | 175 ++++++++++++++++++++ 2 files changed, 175 insertions(+), 103 deletions(-) create mode 100644 tests/ui-fulldeps/stable-mir/projections.rs diff --git a/tests/ui-fulldeps/stable-mir/crate-info.rs b/tests/ui-fulldeps/stable-mir/crate-info.rs index c6aaea1487202..ed6b786f5e1de 100644 --- a/tests/ui-fulldeps/stable-mir/crate-info.rs +++ b/tests/ui-fulldeps/stable-mir/crate-info.rs @@ -23,7 +23,6 @@ use rustc_hir::def::DefKind; use rustc_middle::ty::TyCtxt; use rustc_smir::rustc_internal; use stable_mir::mir::mono::Instance; -use stable_mir::mir::{ProjectionElem, Rvalue, StatementKind}; use stable_mir::ty::{RigidTy, TyKind}; use std::assert_matches::assert_matches; use std::io::Write; @@ -164,99 +163,6 @@ fn test_stable_mir(_tcx: TyCtxt<'_>) -> ControlFlow<()> { stable_mir::ty::TyKind::RigidTy(stable_mir::ty::RigidTy::Bool) ); - let projections_fn = get_item(&items, (DefKind::Fn, "projections")).unwrap(); - let body = projections_fn.body(); - assert_eq!(body.blocks.len(), 4); - // The first statement assigns `&s.c` to a local. The projections include a deref for `s`, since - // `s` is passed as a reference argument, and a field access for field `c`. - match &body.blocks[0].statements[0].kind { - StatementKind::Assign( - stable_mir::mir::Place { local: _, projection: local_proj }, - Rvalue::Ref(_, _, stable_mir::mir::Place { local: _, projection: r_proj }), - ) => { - // We can't match on vecs, only on slices. Comparing for equality wouldn't be any easier - // since we'd then have to add in the expected local and region values instead of - // matching on wildcards. - assert_matches!(local_proj[..], []); - match &r_proj[..] { - // Similarly we can't match against a type, only against its kind. - [ProjectionElem::Deref, ProjectionElem::Field(2, ty)] => assert_matches!( - ty.kind(), - TyKind::RigidTy(RigidTy::Uint(stable_mir::ty::UintTy::U8)) - ), - other => panic!( - "Unable to match against expected rvalue projection. Expected the projection \ - for `s.c`, which is a Deref and u8 Field. Got: {:?}", - other - ), - }; - } - other => panic!( - "Unable to match against expected Assign statement with a Ref rvalue. Expected the \ - statement to assign `&s.c` to a local. Got: {:?}", - other - ), - }; - // This statement assigns `slice[1]` to a local. The projections include a deref for `slice`, - // since `slice` is a reference, and an index. - match &body.blocks[2].statements[0].kind { - StatementKind::Assign( - stable_mir::mir::Place { local: _, projection: local_proj }, - Rvalue::Use(stable_mir::mir::Operand::Copy(stable_mir::mir::Place { - local: _, - projection: r_proj, - })), - ) => { - // We can't match on vecs, only on slices. Comparing for equality wouldn't be any easier - // since we'd then have to add in the expected local values instead of matching on - // wildcards. - assert_matches!(local_proj[..], []); - assert_matches!(r_proj[..], [ProjectionElem::Deref, ProjectionElem::Index(_)]); - } - other => panic!( - "Unable to match against expected Assign statement with a Use rvalue. Expected the \ - statement to assign `slice[1]` to a local. Got: {:?}", - other - ), - }; - // The first terminator gets a slice of an array via the Index operation. Specifically it - // performs `&vals[1..3]`. There are no projections in this case, the arguments are just locals. - match &body.blocks[0].terminator.kind { - stable_mir::mir::TerminatorKind::Call { args, .. } => - // We can't match on vecs, only on slices. Comparing for equality wouldn't be any easier - // since we'd then have to add in the expected local values instead of matching on - // wildcards. - { - match &args[..] { - [ - stable_mir::mir::Operand::Move(stable_mir::mir::Place { - local: _, - projection: arg1_proj, - }), - stable_mir::mir::Operand::Move(stable_mir::mir::Place { - local: _, - projection: arg2_proj, - }), - ] => { - assert_matches!(arg1_proj[..], []); - assert_matches!(arg2_proj[..], []); - } - other => { - panic!( - "Unable to match against expected arguments to Index call. Expected two \ - move operands. Got: {:?}", - other - ) - } - } - } - other => panic!( - "Unable to match against expected Call terminator. Expected a terminator that calls \ - the Index operation. Got: {:?}", - other - ), - }; - ControlFlow::Continue(()) } @@ -336,15 +242,6 @@ fn generate_input(path: &str) -> std::io::Result<()> { }} else {{ 'b' }} - }} - - pub struct Struct1 {{ _a: u8, _b: u16, c: u8 }} - - pub fn projections(s: &Struct1) -> u8 {{ - let v = &s.c; - let vals = [1, 2, 3, 4]; - let slice = &vals[1..3]; - v + slice[1] }}"# )?; Ok(()) diff --git a/tests/ui-fulldeps/stable-mir/projections.rs b/tests/ui-fulldeps/stable-mir/projections.rs new file mode 100644 index 0000000000000..d1b4fc5b28ed9 --- /dev/null +++ b/tests/ui-fulldeps/stable-mir/projections.rs @@ -0,0 +1,175 @@ +// run-pass +// Tests the Stable MIR projections API + +// ignore-stage1 +// ignore-cross-compile +// ignore-remote +// ignore-windows-gnu mingw has troubles with linking https://github.com/rust-lang/rust/pull/116837 +// edition: 2021 + +#![feature(rustc_private)] +#![feature(assert_matches)] +#![feature(control_flow_enum)] + +extern crate rustc_hir; +extern crate rustc_middle; +#[macro_use] +extern crate rustc_smir; +extern crate rustc_driver; +extern crate rustc_interface; +extern crate stable_mir; + +use rustc_hir::def::DefKind; +use rustc_middle::ty::TyCtxt; +use rustc_smir::rustc_internal; +use stable_mir::mir::{ProjectionElem, Rvalue, StatementKind}; +use stable_mir::ty::{RigidTy, TyKind}; +use std::assert_matches::assert_matches; +use std::io::Write; +use std::ops::ControlFlow; + +const CRATE_NAME: &str = "input"; + +/// This function uses the Stable MIR APIs to get information about the test crate. +fn test_projections(_tcx: TyCtxt<'_>) -> ControlFlow<()> { + // Find items in the local crate. + let items = stable_mir::all_local_items(); + + let body = get_item(&items, (DefKind::Fn, "projections")).unwrap().body(); + assert_eq!(body.blocks.len(), 4); + // The first statement assigns `&s.c` to a local. The projections include a deref for `s`, since + // `s` is passed as a reference argument, and a field access for field `c`. + match &body.blocks[0].statements[0].kind { + StatementKind::Assign( + stable_mir::mir::Place { local: _, projection: local_proj }, + Rvalue::Ref(_, _, stable_mir::mir::Place { local: _, projection: r_proj }), + ) => { + // We can't match on vecs, only on slices. Comparing statements for equality wouldn't be + // any easier since we'd then have to add in the expected local and region values + // instead of matching on wildcards. + assert_matches!(local_proj[..], []); + match &r_proj[..] { + // Similarly we can't match against a type, only against its kind. + [ProjectionElem::Deref, ProjectionElem::Field(2, ty)] => assert_matches!( + ty.kind(), + TyKind::RigidTy(RigidTy::Uint(stable_mir::ty::UintTy::U8)) + ), + other => panic!( + "Unable to match against expected rvalue projection. Expected the projection \ + for `s.c`, which is a Deref and u8 Field. Got: {:?}", + other + ), + }; + } + other => panic!( + "Unable to match against expected Assign statement with a Ref rvalue. Expected the \ + statement to assign `&s.c` to a local. Got: {:?}", + other + ), + }; + // This statement assigns `slice[1]` to a local. The projections include a deref for `slice`, + // since `slice` is a reference, and an index. + match &body.blocks[2].statements[0].kind { + StatementKind::Assign( + stable_mir::mir::Place { local: _, projection: local_proj }, + Rvalue::Use(stable_mir::mir::Operand::Copy(stable_mir::mir::Place { + local: _, + projection: r_proj, + })), + ) => { + // We can't match on vecs, only on slices. Comparing for equality wouldn't be any easier + // since we'd then have to add in the expected local values instead of matching on + // wildcards. + assert_matches!(local_proj[..], []); + assert_matches!(r_proj[..], [ProjectionElem::Deref, ProjectionElem::Index(_)]); + } + other => panic!( + "Unable to match against expected Assign statement with a Use rvalue. Expected the \ + statement to assign `slice[1]` to a local. Got: {:?}", + other + ), + }; + // The first terminator gets a slice of an array via the Index operation. Specifically it + // performs `&vals[1..3]`. There are no projections in this case, the arguments are just locals. + match &body.blocks[0].terminator.kind { + stable_mir::mir::TerminatorKind::Call { args, .. } => + // We can't match on vecs, only on slices. Comparing for equality wouldn't be any easier + // since we'd then have to add in the expected local values instead of matching on + // wildcards. + { + match &args[..] { + [ + stable_mir::mir::Operand::Move(stable_mir::mir::Place { + local: _, + projection: arg1_proj, + }), + stable_mir::mir::Operand::Move(stable_mir::mir::Place { + local: _, + projection: arg2_proj, + }), + ] => { + assert_matches!(arg1_proj[..], []); + assert_matches!(arg2_proj[..], []); + } + other => { + panic!( + "Unable to match against expected arguments to Index call. Expected two \ + move operands. Got: {:?}", + other + ) + } + } + } + other => panic!( + "Unable to match against expected Call terminator. Expected a terminator that calls \ + the Index operation. Got: {:?}", + other + ), + }; + + ControlFlow::Continue(()) +} + +// Use internal API to find a function in a crate. +fn get_item<'a>( + items: &'a stable_mir::CrateItems, + item: (DefKind, &str), +) -> Option<&'a stable_mir::CrateItem> { + items.iter().find(|crate_item| { + crate_item.kind().to_string() == format!("{:?}", item.0) && crate_item.name() == item.1 + }) +} + +/// This test will generate and analyze a dummy crate using the stable mir. +/// For that, it will first write the dummy crate into a file. +/// Then it will create a `StableMir` using custom arguments and then +/// it will run the compiler. +fn main() { + let path = "input.rs"; + generate_input(&path).unwrap(); + let args = vec![ + "rustc".to_string(), + "--crate-type=lib".to_string(), + "--crate-name".to_string(), + CRATE_NAME.to_string(), + path.to_string(), + ]; + run!(args, tcx, test_projections(tcx)).unwrap(); +} + +fn generate_input(path: &str) -> std::io::Result<()> { + let mut file = std::fs::File::create(path)?; + write!( + file, + r#" + pub struct Struct1 {{ _a: u8, _b: u16, c: u8 }} + + pub fn projections(s: &Struct1) -> u8 {{ + let v = &s.c; + let vals = [1, 2, 3, 4]; + let slice = &vals[1..3]; + v + slice[1] + }}"# + )?; + Ok(()) +} From 30d6733eb3dd4c393c517056fd3cc276b288b57f Mon Sep 17 00:00:00 2001 From: Kirby Linvill Date: Thu, 9 Nov 2023 18:05:10 -0700 Subject: [PATCH 3/8] Replace match assertions against empty slices with is_empty assertions Asserting is_empty is slightly more concise. --- tests/ui-fulldeps/stable-mir/projections.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/tests/ui-fulldeps/stable-mir/projections.rs b/tests/ui-fulldeps/stable-mir/projections.rs index d1b4fc5b28ed9..c3fbf4b6e9129 100644 --- a/tests/ui-fulldeps/stable-mir/projections.rs +++ b/tests/ui-fulldeps/stable-mir/projections.rs @@ -30,11 +30,9 @@ use std::ops::ControlFlow; const CRATE_NAME: &str = "input"; -/// This function uses the Stable MIR APIs to get information about the test crate. -fn test_projections(_tcx: TyCtxt<'_>) -> ControlFlow<()> { - // Find items in the local crate. +/// Tests projections within Place objects +fn test_place_projections(_tcx: TyCtxt<'_>) -> ControlFlow<()> { let items = stable_mir::all_local_items(); - let body = get_item(&items, (DefKind::Fn, "projections")).unwrap().body(); assert_eq!(body.blocks.len(), 4); // The first statement assigns `&s.c` to a local. The projections include a deref for `s`, since @@ -47,7 +45,7 @@ fn test_projections(_tcx: TyCtxt<'_>) -> ControlFlow<()> { // We can't match on vecs, only on slices. Comparing statements for equality wouldn't be // any easier since we'd then have to add in the expected local and region values // instead of matching on wildcards. - assert_matches!(local_proj[..], []); + assert!(local_proj.is_empty()); match &r_proj[..] { // Similarly we can't match against a type, only against its kind. [ProjectionElem::Deref, ProjectionElem::Field(2, ty)] => assert_matches!( @@ -80,7 +78,7 @@ fn test_projections(_tcx: TyCtxt<'_>) -> ControlFlow<()> { // We can't match on vecs, only on slices. Comparing for equality wouldn't be any easier // since we'd then have to add in the expected local values instead of matching on // wildcards. - assert_matches!(local_proj[..], []); + assert!(local_proj.is_empty()); assert_matches!(r_proj[..], [ProjectionElem::Deref, ProjectionElem::Index(_)]); } other => panic!( @@ -108,8 +106,8 @@ fn test_projections(_tcx: TyCtxt<'_>) -> ControlFlow<()> { projection: arg2_proj, }), ] => { - assert_matches!(arg1_proj[..], []); - assert_matches!(arg2_proj[..], []); + assert!(arg1_proj.is_empty()); + assert!(arg2_proj.is_empty()); } other => { panic!( From 2e70d95cdba8ea185c5d695c410744590785e4ac Mon Sep 17 00:00:00 2001 From: Kirby Linvill Date: Thu, 9 Nov 2023 20:34:30 -0700 Subject: [PATCH 4/8] Remove rich UserTypeProjection projections in SMIR It's not clear to me (klinvill) that UserTypeProjections are produced anymore with the removal of type ascriptions as per https://github.com/rust-lang/rfcs/pull/3307. Furthermore, it's not clear to me which variants of ProjectionElem could appear in such projections. For these reasons, I'm reverting projections in UserTypeProjections to simple strings until I can get more clarity on UserTypeProjections. --- compiler/rustc_smir/src/rustc_smir/mod.rs | 38 ++------------------- compiler/stable_mir/src/mir/body.rs | 25 +++++++------- tests/ui-fulldeps/stable-mir/projections.rs | 2 +- 3 files changed, 16 insertions(+), 49 deletions(-) diff --git a/compiler/rustc_smir/src/rustc_smir/mod.rs b/compiler/rustc_smir/src/rustc_smir/mod.rs index 83bacfb8263f8..a4d83debb57bc 100644 --- a/compiler/rustc_smir/src/rustc_smir/mod.rs +++ b/compiler/rustc_smir/src/rustc_smir/mod.rs @@ -691,7 +691,7 @@ impl<'tcx> Stable<'tcx> for mir::Place<'tcx> { } impl<'tcx> Stable<'tcx> for mir::PlaceElem<'tcx> { - type T = stable_mir::mir::ProjectionElem; + type T = stable_mir::mir::ProjectionElem; fn stable(&self, tables: &mut Tables<'tcx>) -> Self::T { use mir::ProjectionElem::*; match self { @@ -722,40 +722,8 @@ impl<'tcx> Stable<'tcx> for mir::PlaceElem<'tcx> { impl<'tcx> Stable<'tcx> for mir::UserTypeProjection { type T = stable_mir::mir::UserTypeProjection; - fn stable(&self, tables: &mut Tables<'tcx>) -> Self::T { - UserTypeProjection { - base: self.base.as_usize(), - projection: self.projs.iter().map(|e| e.stable(tables)).collect(), - } - } -} - -// ProjectionKind is nearly identical to PlaceElem, except its generic arguments are units. We -// therefore don't need to resolve any arguments with the generic types. -impl<'tcx> Stable<'tcx> for mir::ProjectionKind { - type T = stable_mir::mir::ProjectionElem<(), ()>; - fn stable(&self, tables: &mut Tables<'tcx>) -> Self::T { - use mir::ProjectionElem::*; - match self { - Deref => stable_mir::mir::ProjectionElem::Deref, - Field(idx, ty) => stable_mir::mir::ProjectionElem::Field(idx.stable(tables), *ty), - Index(local) => stable_mir::mir::ProjectionElem::Index(*local), - ConstantIndex { offset, min_length, from_end } => { - stable_mir::mir::ProjectionElem::ConstantIndex { - offset: *offset, - min_length: *min_length, - from_end: *from_end, - } - } - Subslice { from, to, from_end } => stable_mir::mir::ProjectionElem::Subslice { - from: *from, - to: *to, - from_end: *from_end, - }, - Downcast(_, idx) => stable_mir::mir::ProjectionElem::Downcast(idx.stable(tables)), - OpaqueCast(ty) => stable_mir::mir::ProjectionElem::OpaqueCast(*ty), - Subtype(ty) => stable_mir::mir::ProjectionElem::Subtype(*ty), - } + fn stable(&self, _tables: &mut Tables<'tcx>) -> Self::T { + UserTypeProjection { base: self.base.as_usize(), projection: format!("{:?}", self.projs) } } } diff --git a/compiler/stable_mir/src/mir/body.rs b/compiler/stable_mir/src/mir/body.rs index 69e83efdf43a3..49b85d665b1b8 100644 --- a/compiler/stable_mir/src/mir/body.rs +++ b/compiler/stable_mir/src/mir/body.rs @@ -398,22 +398,23 @@ pub enum Operand { pub struct Place { pub local: Local, /// projection out of a place (access a field, deref a pointer, etc) - pub projection: Vec>, + pub projection: Vec, } -// TODO(klinvill): in MIR ProjectionElem is parameterized on the second Field argument and the Index -// argument. This is so it can be used for both the rust provided Places (for which the projection -// elements are of type ProjectionElem) and user-provided type annotations (for which the -// projection elements are of type ProjectionElem<(), ()>). Should we do the same thing in Stable MIR? +// In MIR ProjectionElem is parameterized on the second Field argument and the Index argument. This +// is so it can be used for both Places (for which the projection elements are of type +// ProjectionElem) and user-provided type annotations (for which the projection elements +// are of type ProjectionElem<(), ()>). In SMIR we don't need this generality, so we just use +// ProjectionElem for Places. #[derive(Clone, Debug)] -pub enum ProjectionElem { +pub enum ProjectionElem { /// Dereference projections (e.g. `*_1`) project to the address referenced by the base place. Deref, /// A field projection (e.g., `f` in `_1.f`) project to a field in the base place. The field is /// referenced by source-order index rather than the name of the field. The fields type is also /// given. - Field(FieldIdx, T), + Field(FieldIdx, Ty), /// Index into a slice/array. The value of the index is computed at runtime using the `V` /// argument. @@ -429,7 +430,7 @@ pub enum ProjectionElem { /// /// The `x[i]` is turned into a `Deref` followed by an `Index`, not just an `Index`. The same /// thing is true of the `ConstantIndex` and `Subslice` projections below. - Index(V), + Index(Local), /// Index into a slice/array given by offsets. /// @@ -472,7 +473,7 @@ pub enum ProjectionElem { /// Like an explicit cast from an opaque type to a concrete type, but without /// requiring an intermediate variable. - OpaqueCast(T), + OpaqueCast(Ty), /// A `Subtype(T)` projection is applied to any `StatementKind::Assign` where /// type of lvalue doesn't match the type of rvalue, the primary goal is making subtyping @@ -480,16 +481,14 @@ pub enum ProjectionElem { /// /// This projection doesn't impact the runtime behavior of the program except for potentially changing /// some type metadata of the interpreter or codegen backend. - Subtype(T), + Subtype(Ty), } #[derive(Clone, Debug, Eq, PartialEq)] pub struct UserTypeProjection { pub base: UserTypeAnnotationIndex, - /// `UserTypeProjection` projections need neither the `V` parameter for `Index` nor the `T` for - /// `Field`. - pub projection: Vec>, + pub projection: String, } pub type Local = usize; diff --git a/tests/ui-fulldeps/stable-mir/projections.rs b/tests/ui-fulldeps/stable-mir/projections.rs index c3fbf4b6e9129..9c649a2effc75 100644 --- a/tests/ui-fulldeps/stable-mir/projections.rs +++ b/tests/ui-fulldeps/stable-mir/projections.rs @@ -152,7 +152,7 @@ fn main() { CRATE_NAME.to_string(), path.to_string(), ]; - run!(args, tcx, test_projections(tcx)).unwrap(); + run!(args, tcx, test_place_projections(tcx)).unwrap(); } fn generate_input(path: &str) -> std::io::Result<()> { From 998aa383ba159bc7c6a6eba13ce020c191988f82 Mon Sep 17 00:00:00 2001 From: Kirby Linvill Date: Thu, 9 Nov 2023 20:43:05 -0700 Subject: [PATCH 5/8] Defer Place ty implementation in Stable Mir to later PR --- compiler/stable_mir/src/mir/body.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/compiler/stable_mir/src/mir/body.rs b/compiler/stable_mir/src/mir/body.rs index 49b85d665b1b8..625e52c983ed0 100644 --- a/compiler/stable_mir/src/mir/body.rs +++ b/compiler/stable_mir/src/mir/body.rs @@ -646,8 +646,10 @@ impl Constant { } impl Place { - // TODO(klinvill): What is the expected behavior of this function? Should it resolve down the - // chain of projections so that `*(_1.f)` would end up returning the type referenced by `f`? + // FIXME(klinvill): This function is expected to resolve down the chain of projections to get + // the type referenced at the end of it. E.g. calling `ty()` on `*(_1.f)` should end up + // returning the type referenced by `f`. The information needed to do this may not currently be + // present in Stable MIR since at least an implementation for AdtDef is probably needed. pub fn ty(&self, locals: &[LocalDecl]) -> Ty { let _start_ty = locals[self.local].ty; todo!("Implement projection") From d517a1cbda49309c8364daaedfee040371e616fb Mon Sep 17 00:00:00 2001 From: Kirby Linvill Date: Fri, 10 Nov 2023 11:21:46 -0700 Subject: [PATCH 6/8] Add SMIR visitor for Places and projections --- compiler/stable_mir/src/mir/body.rs | 2 +- compiler/stable_mir/src/mir/visit.rs | 33 +++++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/compiler/stable_mir/src/mir/body.rs b/compiler/stable_mir/src/mir/body.rs index 625e52c983ed0..48d4a3d37e2c2 100644 --- a/compiler/stable_mir/src/mir/body.rs +++ b/compiler/stable_mir/src/mir/body.rs @@ -406,7 +406,7 @@ pub struct Place { // ProjectionElem) and user-provided type annotations (for which the projection elements // are of type ProjectionElem<(), ()>). In SMIR we don't need this generality, so we just use // ProjectionElem for Places. -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Eq, PartialEq)] pub enum ProjectionElem { /// Dereference projections (e.g. `*_1`) project to the address referenced by the base place. Deref, diff --git a/compiler/stable_mir/src/mir/visit.rs b/compiler/stable_mir/src/mir/visit.rs index 806dced71ff3e..475d6e9763d91 100644 --- a/compiler/stable_mir/src/mir/visit.rs +++ b/compiler/stable_mir/src/mir/visit.rs @@ -76,6 +76,15 @@ pub trait MirVisitor { self.super_place(place, ptx, location) } + fn visit_projection_elem( + &mut self, + elem: &ProjectionElem, + ptx: PlaceContext, + location: Location, + ) { + self.super_projection_elem(elem, ptx, location); + } + fn visit_local(&mut self, local: &Local, ptx: PlaceContext, location: Location) { let _ = (local, ptx, location); } @@ -264,7 +273,29 @@ pub trait MirVisitor { fn super_place(&mut self, place: &Place, ptx: PlaceContext, location: Location) { let _ = location; let _ = ptx; - visit_opaque(&Opaque(place.projection.clone())); + self.visit_local(&place.local, ptx, location); + + for elem in &place.projection { + self.visit_projection_elem(elem, ptx, location); + } + } + + fn super_projection_elem( + &mut self, + elem: &ProjectionElem, + ptx: PlaceContext, + location: Location, + ) { + match elem { + ProjectionElem::Deref => {} + ProjectionElem::Field(_idx, ty) => self.visit_ty(ty, location), + ProjectionElem::Index(local) => self.visit_local(local, ptx, location), + ProjectionElem::ConstantIndex { offset: _, min_length: _, from_end: _ } => {} + ProjectionElem::Subslice { from: _, to: _, from_end: _ } => {} + ProjectionElem::Downcast(_idx) => {} + ProjectionElem::OpaqueCast(ty) => self.visit_ty(ty, location), + ProjectionElem::Subtype(ty) => self.visit_ty(ty, location), + } } fn super_rvalue(&mut self, rvalue: &Rvalue, location: Location) { From ae1726bfceffa271131d9ef852c1d553688aa6a4 Mon Sep 17 00:00:00 2001 From: Kirby Linvill Date: Fri, 10 Nov 2023 17:18:59 -0700 Subject: [PATCH 7/8] Ignore FieldIdx and VariantIdx examples in docs --- compiler/stable_mir/src/mir/body.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/stable_mir/src/mir/body.rs b/compiler/stable_mir/src/mir/body.rs index 48d4a3d37e2c2..a5b51ce6a34db 100644 --- a/compiler/stable_mir/src/mir/body.rs +++ b/compiler/stable_mir/src/mir/body.rs @@ -498,7 +498,7 @@ pub const RETURN_LOCAL: Local = 0; /// The source-order index of a field in a variant. /// /// For example, in the following types, -/// ```rust +/// ```ignore(illustrative) /// enum Demo1 { /// Variant0 { a: bool, b: i32 }, /// Variant1 { c: u8, d: u64 }, @@ -514,7 +514,7 @@ type FieldIdx = usize; /// The source-order index of a variant in a type. /// /// For example, in the following types, -/// ```rust +/// ```ignore(illustrative) /// enum Demo1 { /// Variant0 { a: bool, b: i32 }, /// Variant1 { c: u8, d: u64 }, From c036a10ed501cd2c4b501af03c920af3f28d360f Mon Sep 17 00:00:00 2001 From: Kirby Linvill Date: Tue, 14 Nov 2023 19:19:35 -0700 Subject: [PATCH 8/8] Make UserTypeProjection projections Opaque Also shifts comments explaining why Stable MIR drops an optional variant name field, for `Downcast` projection elements, to the `Place::stable` function. --- compiler/rustc_smir/src/rustc_smir/mod.rs | 7 ++++++- compiler/stable_mir/src/mir/body.rs | 6 +----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_smir/src/rustc_smir/mod.rs b/compiler/rustc_smir/src/rustc_smir/mod.rs index a4d83debb57bc..1bdcc805f63c7 100644 --- a/compiler/rustc_smir/src/rustc_smir/mod.rs +++ b/compiler/rustc_smir/src/rustc_smir/mod.rs @@ -712,6 +712,11 @@ impl<'tcx> Stable<'tcx> for mir::PlaceElem<'tcx> { to: *to, from_end: *from_end, }, + // MIR includes an `Option` argument for `Downcast` that is the name of the + // variant, used for printing MIR. However this information should also be accessible + // via a lookup using the `VariantIdx`. The `Option` argument is therefore + // dropped when converting to Stable MIR. A brief justification for this decision can be + // found at https://github.com/rust-lang/rust/pull/117517#issuecomment-1811683486 Downcast(_, idx) => stable_mir::mir::ProjectionElem::Downcast(idx.stable(tables)), OpaqueCast(ty) => stable_mir::mir::ProjectionElem::OpaqueCast(ty.stable(tables)), Subtype(ty) => stable_mir::mir::ProjectionElem::Subtype(ty.stable(tables)), @@ -723,7 +728,7 @@ impl<'tcx> Stable<'tcx> for mir::UserTypeProjection { type T = stable_mir::mir::UserTypeProjection; fn stable(&self, _tables: &mut Tables<'tcx>) -> Self::T { - UserTypeProjection { base: self.base.as_usize(), projection: format!("{:?}", self.projs) } + UserTypeProjection { base: self.base.as_usize(), projection: opaque(&self.projs) } } } diff --git a/compiler/stable_mir/src/mir/body.rs b/compiler/stable_mir/src/mir/body.rs index a5b51ce6a34db..351e7bb69c3f7 100644 --- a/compiler/stable_mir/src/mir/body.rs +++ b/compiler/stable_mir/src/mir/body.rs @@ -465,10 +465,6 @@ pub enum ProjectionElem { }, /// "Downcast" to a variant of an enum or a coroutine. - // - // TODO(klinvill): MIR includes an Option argument that is the name of the variant, used - // for printing MIR. However I don't see it used anywhere. Is such a field needed or can we just - // include the VariantIdx which could be used to recover the field name if needed? Downcast(VariantIdx), /// Like an explicit cast from an opaque type to a concrete type, but without @@ -488,7 +484,7 @@ pub enum ProjectionElem { pub struct UserTypeProjection { pub base: UserTypeAnnotationIndex, - pub projection: String, + pub projection: Opaque, } pub type Local = usize;