about summary refs log tree commit diff
diff options
context:
space:
mode:
authorMatthias Krüger <matthias.krueger@famsik.de>2023-12-06 21:52:33 +0100
committerGitHub <noreply@github.com>2023-12-06 21:52:33 +0100
commit3c1357ca6b32fef50ab0d16d50fa54168c384cd6 (patch)
tree22ab643856916e8f257e72187ba842c5bacffc46
parentcf78a79020d4f04ceeed755db2d28ee99baad683 (diff)
parent4a75d1893eb65baa5d65c0e1eb511daa8247c85e (diff)
downloadrust-3c1357ca6b32fef50ab0d16d50fa54168c384cd6.tar.gz
rust-3c1357ca6b32fef50ab0d16d50fa54168c384cd6.zip
Rollup merge of #118681 - celinval:fix-foreign-item, r=ouz-a
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`. I also added a method to check if the instance has body, since the function already existed and it just wasn't exposed via public APIs. This makes it much cheaper for the user to check if the instance has body.

## 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`.

r? `@ouz-a`
-rw-r--r--compiler/rustc_smir/src/rustc_smir/context.rs4
-rw-r--r--compiler/stable_mir/src/compiler_interface.rs2
-rw-r--r--compiler/stable_mir/src/lib.rs2
-rw-r--r--compiler/stable_mir/src/mir/mono.rs11
-rw-r--r--tests/ui-fulldeps/stable-mir/check_instance.rs8
5 files changed, 19 insertions, 8 deletions
diff --git a/compiler/rustc_smir/src/rustc_smir/context.rs b/compiler/rustc_smir/src/rustc_smir/context.rs
index 93d49038fc1..6dc569b8f6a 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 d8a3d4eda4b..5c91ec15455 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 2099c485c6f..1e7495009d8 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 10270c82e63..11b849868e0 100644
--- a/compiler/stable_mir/src/mir/mono.rs
+++ b/compiler/stable_mir/src/mir/mono.rs
@@ -39,9 +39,16 @@ 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 {
-        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 976dfee774b..5cb07eabf41 100644
--- a/tests/ui-fulldeps/stable-mir/check_instance.rs
+++ b/tests/ui-fulldeps/stable-mir/check_instance.rs
@@ -64,8 +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!(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 */