From 1bcd162465dc98a2264138d145ecf3033d7e2108 Mon Sep 17 00:00:00 2001
From: "Celina G. Val" <celinval@amazon.com>
Date: Wed, 6 Dec 2023 10:48:18 -0800
Subject: [PATCH 1/2] Fix `is_foreign_item` for StableMIR instance
Change the implementation of `Instance::is_foreign_item` to directly
query the compiler for the instance `def_id` instead of incorrectly
relying on the conversion to `CrateItem`.
Background:
- In pull https://github.com/rust-lang/rust/pull/118524, I fixed the
conversion from Instance to CrateItem to avoid the conversion if the
instance didn't have a body available. This broke the `is_foreign_item`.
---
compiler/rustc_smir/src/rustc_smir/context.rs | 4 ++--
compiler/stable_mir/src/compiler_interface.rs | 2 +-
compiler/stable_mir/src/lib.rs | 2 +-
compiler/stable_mir/src/mir/mono.rs | 3 +--
tests/ui-fulldeps/stable-mir/check_instance.rs | 3 ++-
5 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/compiler/rustc_smir/src/rustc_smir/context.rs b/compiler/rustc_smir/src/rustc_smir/context.rs
index 0bd640ee1e3b0..301ba79bfbe55 100644
--- a/compiler/rustc_smir/src/rustc_smir/context.rs
+++ b/compiler/rustc_smir/src/rustc_smir/context.rs
@@ -187,9 +187,9 @@ impl<'tcx> Context for TablesWrapper<'tcx> {
new_item_kind(tables.tcx.def_kind(tables[item.0]))
}
- fn is_foreign_item(&self, item: CrateItem) -> bool {
+ fn is_foreign_item(&self, item: DefId) -> bool {
let tables = self.0.borrow();
- tables.tcx.is_foreign_item(tables[item.0])
+ tables.tcx.is_foreign_item(tables[item])
}
fn adt_kind(&self, def: AdtDef) -> AdtKind {
diff --git a/compiler/stable_mir/src/compiler_interface.rs b/compiler/stable_mir/src/compiler_interface.rs
index daf4465963edd..1bbcc257dfc4e 100644
--- a/compiler/stable_mir/src/compiler_interface.rs
+++ b/compiler/stable_mir/src/compiler_interface.rs
@@ -60,7 +60,7 @@ pub trait Context {
fn item_kind(&self, item: CrateItem) -> ItemKind;
/// Returns whether this is a foreign item.
- fn is_foreign_item(&self, item: CrateItem) -> bool;
+ fn is_foreign_item(&self, item: DefId) -> bool;
/// Returns the kind of a given algebraic data type
fn adt_kind(&self, def: AdtDef) -> AdtKind;
diff --git a/compiler/stable_mir/src/lib.rs b/compiler/stable_mir/src/lib.rs
index 2099c485c6f8f..1e7495009d8de 100644
--- a/compiler/stable_mir/src/lib.rs
+++ b/compiler/stable_mir/src/lib.rs
@@ -120,7 +120,7 @@ impl CrateItem {
}
pub fn is_foreign_item(&self) -> bool {
- with(|cx| cx.is_foreign_item(*self))
+ with(|cx| cx.is_foreign_item(self.0))
}
pub fn dump<W: io::Write>(&self, w: &mut W) -> io::Result<()> {
diff --git a/compiler/stable_mir/src/mir/mono.rs b/compiler/stable_mir/src/mir/mono.rs
index 10270c82e6323..541a8376a62ea 100644
--- a/compiler/stable_mir/src/mir/mono.rs
+++ b/compiler/stable_mir/src/mir/mono.rs
@@ -40,8 +40,7 @@ impl Instance {
}
pub fn is_foreign_item(&self) -> bool {
- let item = CrateItem::try_from(*self);
- item.as_ref().is_ok_and(CrateItem::is_foreign_item)
+ with(|cx| cx.is_foreign_item(self.def.def_id()))
}
/// Get the instance type with generic substitutions applied and lifetimes erased.
diff --git a/tests/ui-fulldeps/stable-mir/check_instance.rs b/tests/ui-fulldeps/stable-mir/check_instance.rs
index 976dfee774b27..2b590d2eff7a2 100644
--- a/tests/ui-fulldeps/stable-mir/check_instance.rs
+++ b/tests/ui-fulldeps/stable-mir/check_instance.rs
@@ -65,7 +65,8 @@ fn test_body(body: mir::Body) {
let instance = Instance::resolve(def, &args).unwrap();
let mangled_name = instance.mangled_name();
let body = instance.body();
- assert!(body.is_some() || mangled_name == "setpwent", "Failed: {func:?}");
+ assert!(body.is_some() || (mangled_name == "setpwent"), "Failed: {func:?}");
+ assert!(body.is_some() ^ instance.is_foreign_item());
}
Goto { .. } | Assert { .. } | SwitchInt { .. } | Return | Drop { .. } => {
/* Do nothing */
From 4a75d1893eb65baa5d65c0e1eb511daa8247c85e Mon Sep 17 00:00:00 2001
From: "Celina G. Val" <celinval@amazon.com>
Date: Wed, 6 Dec 2023 11:00:30 -0800
Subject: [PATCH 2/2] Also add an API to check if an instance has body
This is much cheaper than building a body just for the purpose of
checking if the body exists.
---
compiler/stable_mir/src/mir/mono.rs | 8 ++++++++
tests/ui-fulldeps/stable-mir/check_instance.rs | 9 ++++++---
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/compiler/stable_mir/src/mir/mono.rs b/compiler/stable_mir/src/mir/mono.rs
index 541a8376a62ea..11b849868e009 100644
--- a/compiler/stable_mir/src/mir/mono.rs
+++ b/compiler/stable_mir/src/mir/mono.rs
@@ -39,6 +39,14 @@ impl Instance {
with(|context| context.instance_body(self.def))
}
+ /// Check whether this instance has a body available.
+ ///
+ /// This call is much cheaper than `instance.body().is_some()`, since it doesn't try to build
+ /// the StableMIR body.
+ pub fn has_body(&self) -> bool {
+ with(|cx| cx.has_body(self.def.def_id()))
+ }
+
pub fn is_foreign_item(&self) -> bool {
with(|cx| cx.is_foreign_item(self.def.def_id()))
}
diff --git a/tests/ui-fulldeps/stable-mir/check_instance.rs b/tests/ui-fulldeps/stable-mir/check_instance.rs
index 2b590d2eff7a2..5cb07eabf41d3 100644
--- a/tests/ui-fulldeps/stable-mir/check_instance.rs
+++ b/tests/ui-fulldeps/stable-mir/check_instance.rs
@@ -64,9 +64,12 @@ fn test_body(body: mir::Body) {
let RigidTy::FnDef(def, args) = ty else { unreachable!() };
let instance = Instance::resolve(def, &args).unwrap();
let mangled_name = instance.mangled_name();
- let body = instance.body();
- assert!(body.is_some() || (mangled_name == "setpwent"), "Failed: {func:?}");
- assert!(body.is_some() ^ instance.is_foreign_item());
+ assert!(instance.has_body() || (mangled_name == "setpwent"), "Failed: {func:?}");
+ assert!(instance.has_body() ^ instance.is_foreign_item());
+ if instance.has_body() {
+ let body = instance.body().unwrap();
+ assert!(!body.locals().is_empty(), "Body must at least have a return local");
+ }
}
Goto { .. } | Assert { .. } | SwitchInt { .. } | Return | Drop { .. } => {
/* Do nothing */