Skip to content

Commit 9ea1063

Browse files
authored
Rollup merge of rust-lang#123655 - celinval:smir-fix-binop-ty, r=compiler-errors
Remove unimplemented!() from BinOp::ty() function To reduce redundancy, we now internalize the BinOp instead of duplicating the `ty()` function body.
2 parents 7e9eaba + 0a4f4a3 commit 9ea1063

File tree

5 files changed

+196
-39
lines changed

5 files changed

+196
-39
lines changed

Diff for: compiler/rustc_smir/src/rustc_internal/internal.rs

+33-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use rustc_span::Symbol;
1010
use stable_mir::abi::Layout;
1111
use stable_mir::mir::alloc::AllocId;
1212
use stable_mir::mir::mono::{Instance, MonoItem, StaticDef};
13-
use stable_mir::mir::{Mutability, Place, ProjectionElem, Safety};
13+
use stable_mir::mir::{BinOp, Mutability, Place, ProjectionElem, Safety};
1414
use stable_mir::ty::{
1515
Abi, AdtDef, Binder, BoundRegionKind, BoundTyKind, BoundVariableKind, ClosureKind, Const,
1616
DynKind, ExistentialPredicate, ExistentialProjection, ExistentialTraitRef, FloatTy, FnSig,
@@ -551,6 +551,38 @@ impl RustcInternal for ProjectionElem {
551551
}
552552
}
553553

554+
impl RustcInternal for BinOp {
555+
type T<'tcx> = rustc_middle::mir::BinOp;
556+
557+
fn internal<'tcx>(&self, _tables: &mut Tables<'_>, _tcx: TyCtxt<'tcx>) -> Self::T<'tcx> {
558+
match self {
559+
BinOp::Add => rustc_middle::mir::BinOp::Add,
560+
BinOp::AddUnchecked => rustc_middle::mir::BinOp::AddUnchecked,
561+
BinOp::Sub => rustc_middle::mir::BinOp::Sub,
562+
BinOp::SubUnchecked => rustc_middle::mir::BinOp::SubUnchecked,
563+
BinOp::Mul => rustc_middle::mir::BinOp::Mul,
564+
BinOp::MulUnchecked => rustc_middle::mir::BinOp::MulUnchecked,
565+
BinOp::Div => rustc_middle::mir::BinOp::Div,
566+
BinOp::Rem => rustc_middle::mir::BinOp::Rem,
567+
BinOp::BitXor => rustc_middle::mir::BinOp::BitXor,
568+
BinOp::BitAnd => rustc_middle::mir::BinOp::BitAnd,
569+
BinOp::BitOr => rustc_middle::mir::BinOp::BitOr,
570+
BinOp::Shl => rustc_middle::mir::BinOp::Shl,
571+
BinOp::ShlUnchecked => rustc_middle::mir::BinOp::ShlUnchecked,
572+
BinOp::Shr => rustc_middle::mir::BinOp::Shr,
573+
BinOp::ShrUnchecked => rustc_middle::mir::BinOp::ShrUnchecked,
574+
BinOp::Eq => rustc_middle::mir::BinOp::Eq,
575+
BinOp::Lt => rustc_middle::mir::BinOp::Lt,
576+
BinOp::Le => rustc_middle::mir::BinOp::Le,
577+
BinOp::Ne => rustc_middle::mir::BinOp::Ne,
578+
BinOp::Ge => rustc_middle::mir::BinOp::Ge,
579+
BinOp::Gt => rustc_middle::mir::BinOp::Gt,
580+
BinOp::Cmp => rustc_middle::mir::BinOp::Cmp,
581+
BinOp::Offset => rustc_middle::mir::BinOp::Offset,
582+
}
583+
}
584+
}
585+
554586
impl<T> RustcInternal for &T
555587
where
556588
T: RustcInternal,

Diff for: compiler/rustc_smir/src/rustc_smir/context.rs

+10-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use stable_mir::abi::{FnAbi, Layout, LayoutShape};
1919
use stable_mir::compiler_interface::Context;
2020
use stable_mir::mir::alloc::GlobalAlloc;
2121
use stable_mir::mir::mono::{InstanceDef, StaticDef};
22-
use stable_mir::mir::{Body, Place};
22+
use stable_mir::mir::{BinOp, Body, Place};
2323
use stable_mir::target::{MachineInfo, MachineSize};
2424
use stable_mir::ty::{
2525
AdtDef, AdtKind, Allocation, ClosureDef, ClosureKind, Const, FieldDef, FnDef, ForeignDef,
@@ -668,6 +668,15 @@ impl<'tcx> Context for TablesWrapper<'tcx> {
668668
let tcx = tables.tcx;
669669
format!("{:?}", place.internal(&mut *tables, tcx))
670670
}
671+
672+
fn binop_ty(&self, bin_op: BinOp, rhs: Ty, lhs: Ty) -> Ty {
673+
let mut tables = self.0.borrow_mut();
674+
let tcx = tables.tcx;
675+
let rhs_internal = rhs.internal(&mut *tables, tcx);
676+
let lhs_internal = lhs.internal(&mut *tables, tcx);
677+
let ty = bin_op.internal(&mut *tables, tcx).ty(tcx, rhs_internal, lhs_internal);
678+
ty.stable(&mut *tables)
679+
}
671680
}
672681

673682
pub struct TablesWrapper<'tcx>(pub RefCell<Tables<'tcx>>);

Diff for: compiler/stable_mir/src/compiler_interface.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use std::cell::Cell;
88
use crate::abi::{FnAbi, Layout, LayoutShape};
99
use crate::mir::alloc::{AllocId, GlobalAlloc};
1010
use crate::mir::mono::{Instance, InstanceDef, StaticDef};
11-
use crate::mir::{Body, Place};
11+
use crate::mir::{BinOp, Body, Place};
1212
use crate::target::MachineInfo;
1313
use crate::ty::{
1414
AdtDef, AdtKind, Allocation, ClosureDef, ClosureKind, Const, FieldDef, FnDef, ForeignDef,
@@ -211,6 +211,9 @@ pub trait Context {
211211

212212
/// Get a debug string representation of a place.
213213
fn place_pretty(&self, place: &Place) -> String;
214+
215+
/// Get the resulting type of binary operation.
216+
fn binop_ty(&self, bin_op: BinOp, rhs: Ty, lhs: Ty) -> Ty;
214217
}
215218

216219
// A thread local variable that stores a pointer to the tables mapping between TyCtxt

Diff for: compiler/stable_mir/src/mir/body.rs

+2-36
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use crate::compiler_interface::with;
12
use crate::mir::pretty::function_body;
23
use crate::ty::{
34
AdtDef, ClosureDef, Const, CoroutineDef, GenericArgs, Movability, Region, RigidTy, Ty, TyKind,
@@ -337,42 +338,7 @@ impl BinOp {
337338
/// Return the type of this operation for the given input Ty.
338339
/// This function does not perform type checking, and it currently doesn't handle SIMD.
339340
pub fn ty(&self, lhs_ty: Ty, rhs_ty: Ty) -> Ty {
340-
match self {
341-
BinOp::Add
342-
| BinOp::AddUnchecked
343-
| BinOp::Sub
344-
| BinOp::SubUnchecked
345-
| BinOp::Mul
346-
| BinOp::MulUnchecked
347-
| BinOp::Div
348-
| BinOp::Rem
349-
| BinOp::BitXor
350-
| BinOp::BitAnd
351-
| BinOp::BitOr => {
352-
assert_eq!(lhs_ty, rhs_ty);
353-
assert!(lhs_ty.kind().is_primitive());
354-
lhs_ty
355-
}
356-
BinOp::Shl | BinOp::ShlUnchecked | BinOp::Shr | BinOp::ShrUnchecked => {
357-
assert!(lhs_ty.kind().is_primitive());
358-
assert!(rhs_ty.kind().is_primitive());
359-
lhs_ty
360-
}
361-
BinOp::Offset => {
362-
assert!(lhs_ty.kind().is_raw_ptr());
363-
assert!(rhs_ty.kind().is_integral());
364-
lhs_ty
365-
}
366-
BinOp::Eq | BinOp::Lt | BinOp::Le | BinOp::Ne | BinOp::Ge | BinOp::Gt => {
367-
assert_eq!(lhs_ty, rhs_ty);
368-
let lhs_kind = lhs_ty.kind();
369-
assert!(lhs_kind.is_primitive() || lhs_kind.is_raw_ptr() || lhs_kind.is_fn_ptr());
370-
Ty::bool_ty()
371-
}
372-
BinOp::Cmp => {
373-
unimplemented!("Should cmp::Ordering be a RigidTy?");
374-
}
375-
}
341+
with(|ctx| ctx.binop_ty(*self, lhs_ty, rhs_ty))
376342
}
377343
}
378344

Diff for: tests/ui-fulldeps/stable-mir/check_binop.rs

+147
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
//@ run-pass
2+
//! Test information regarding binary operations.
3+
4+
//@ ignore-stage1
5+
//@ ignore-cross-compile
6+
//@ ignore-remote
7+
//@ ignore-windows-gnu mingw has troubles with linking https://github.com/rust-lang/rust/pull/116837
8+
9+
#![feature(rustc_private)]
10+
11+
extern crate rustc_hir;
12+
#[macro_use]
13+
extern crate rustc_smir;
14+
extern crate rustc_driver;
15+
extern crate rustc_interface;
16+
extern crate stable_mir;
17+
18+
use rustc_smir::rustc_internal;
19+
use stable_mir::mir::mono::Instance;
20+
use stable_mir::mir::visit::{Location, MirVisitor};
21+
use stable_mir::mir::{LocalDecl, Rvalue, Statement, StatementKind, Terminator, TerminatorKind};
22+
use stable_mir::ty::{RigidTy, TyKind};
23+
use std::collections::HashSet;
24+
use std::convert::TryFrom;
25+
use std::io::Write;
26+
use std::ops::ControlFlow;
27+
28+
/// This function tests that we can correctly get type information from binary operations.
29+
fn test_binops() -> ControlFlow<()> {
30+
// Find items in the local crate.
31+
let items = stable_mir::all_local_items();
32+
let mut instances =
33+
items.into_iter().map(|item| Instance::try_from(item).unwrap()).collect::<Vec<_>>();
34+
while let Some(instance) = instances.pop() {
35+
// The test below shouldn't have recursion in it.
36+
let Some(body) = instance.body() else {
37+
continue;
38+
};
39+
let mut visitor = Visitor { locals: body.locals(), calls: Default::default() };
40+
visitor.visit_body(&body);
41+
instances.extend(visitor.calls.into_iter());
42+
}
43+
ControlFlow::Continue(())
44+
}
45+
46+
struct Visitor<'a> {
47+
locals: &'a [LocalDecl],
48+
calls: HashSet<Instance>,
49+
}
50+
51+
impl<'a> MirVisitor for Visitor<'a> {
52+
fn visit_statement(&mut self, stmt: &Statement, _loc: Location) {
53+
match &stmt.kind {
54+
StatementKind::Assign(place, Rvalue::BinaryOp(op, rhs, lhs)) => {
55+
let ret_ty = place.ty(self.locals).unwrap();
56+
let op_ty = op.ty(rhs.ty(self.locals).unwrap(), lhs.ty(self.locals).unwrap());
57+
assert_eq!(ret_ty, op_ty, "Operation type should match the assigned place type");
58+
}
59+
_ => {}
60+
}
61+
}
62+
63+
fn visit_terminator(&mut self, term: &Terminator, _loc: Location) {
64+
match &term.kind {
65+
TerminatorKind::Call { func, .. } => {
66+
let TyKind::RigidTy(RigidTy::FnDef(def, args)) =
67+
func.ty(self.locals).unwrap().kind()
68+
else {
69+
return;
70+
};
71+
self.calls.insert(Instance::resolve(def, &args).unwrap());
72+
}
73+
_ => {}
74+
}
75+
}
76+
}
77+
78+
/// This test will generate and analyze a dummy crate using the stable mir.
79+
/// For that, it will first write the dummy crate into a file.
80+
/// Then it will create a `StableMir` using custom arguments and then
81+
/// it will run the compiler.
82+
fn main() {
83+
let path = "binop_input.rs";
84+
generate_input(&path).unwrap();
85+
let args = vec!["rustc".to_string(), "--crate-type=lib".to_string(), path.to_string()];
86+
run!(args, test_binops).unwrap();
87+
}
88+
89+
fn generate_input(path: &str) -> std::io::Result<()> {
90+
let mut file = std::fs::File::create(path)?;
91+
write!(
92+
file,
93+
r#"
94+
macro_rules! binop_int {{
95+
($fn:ident, $typ:ty) => {{
96+
pub fn $fn(lhs: $typ, rhs: $typ) {{
97+
let eq = lhs == rhs;
98+
let lt = lhs < rhs;
99+
let le = lhs <= rhs;
100+
101+
let sum = lhs + rhs;
102+
let mult = lhs * sum;
103+
let shift = mult << 2;
104+
let bit_or = shift | rhs;
105+
let cmp = lhs.cmp(&bit_or);
106+
107+
// Try to avoid the results above being pruned
108+
std::hint::black_box(((eq, lt, le), cmp));
109+
}}
110+
}}
111+
}}
112+
113+
binop_int!(binop_u8, u8);
114+
binop_int!(binop_i64, i64);
115+
116+
pub fn binop_bool(lhs: bool, rhs: bool) {{
117+
let eq = lhs == rhs;
118+
let or = lhs | eq;
119+
let lt = lhs < or;
120+
let cmp = lhs.cmp(&rhs);
121+
122+
// Try to avoid the results above being pruned
123+
std::hint::black_box((lt, cmp));
124+
}}
125+
126+
pub fn binop_char(lhs: char, rhs: char) {{
127+
let eq = lhs == rhs;
128+
let lt = lhs < rhs;
129+
let cmp = lhs.cmp(&rhs);
130+
131+
// Try to avoid the results above being pruned
132+
std::hint::black_box(([eq, lt], cmp));
133+
}}
134+
135+
pub fn binop_ptr(lhs: *const char, rhs: *const char) {{
136+
let eq = lhs == rhs;
137+
let lt = lhs < rhs;
138+
let cmp = lhs.cmp(&rhs);
139+
let off = unsafe {{ lhs.offset(2) }};
140+
141+
// Try to avoid the results above being pruned
142+
std::hint::black_box(([eq, lt], cmp, off));
143+
}}
144+
"#
145+
)?;
146+
Ok(())
147+
}

0 commit comments

Comments
 (0)