Skip to content

Commit

Permalink
Rollup merge of rust-lang#118681 - celinval:fix-foreign-item, r=ouz-a
Browse files Browse the repository at this point in the history
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 rust-lang#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`
  • Loading branch information
matthiaskrgr authored Dec 6, 2023
2 parents cf78a79 + 4a75d18 commit 3c1357c
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 8 deletions.
4 changes: 2 additions & 2 deletions compiler/rustc_smir/src/rustc_smir/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion compiler/stable_mir/src/compiler_interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion compiler/stable_mir/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<()> {
Expand Down
11 changes: 9 additions & 2 deletions compiler/stable_mir/src/mir/mono.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 6 additions & 2 deletions tests/ui-fulldeps/stable-mir/check_instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down

0 comments on commit 3c1357c

Please sign in to comment.