Add more information for `ForeignDef`

[momvart]

ForeignDef items may correspond to functions, statics, or types. Based on rustc_hir::ForeignItem, we need an equivalent of ForeignItemKind.

For these variants:

• Fn: Can FnDef be reused?
• Static: The existing information on CrateDef should be enough.
• Ty: Not sure if it is stable yet (#rust/43467)

Similar to existing kind types, the following changes are expected:

• adding method foreign_item_kind to Context
• adding method kind to FnDef calling the method above

[celinval]

Do you mean adding kind function to ForeignDef?

[momvart]

Do you mean adding kind function to ForeignDef?

That's right.

[celinval]

@oli-obk wondering if you know what kind of information users get from the ForeignDef besides name? I couldn't find any other use case in MIRI, rustc codegen or Kani. I am tempted to just have a simple ForeignItemKind enum for now to provide the kind.

[oli-obk]

Yea, you can't obtain more than the kind (and thus information like a static's type or a function's signature). Not sure that information is useful to anyone, as you can't rely on the other end of the foreign item to actually match

[celinval]

In theory, I think it would make sense for kind to be:

enum ForeignItemKind {
    Fn(FnDef),
    Static(StaticDef),
    Ty(Ty),
}

That said, we would need to ensure all the methods of FnDef and StaticDef work as expected. I believe we currently implement impl From<StaticDef> for CrateItem which wouldn't hold since CrateItem has to have a body. That said, I suspect that it doesn't hold for static objects that are defined outside of the compiled crate, and we should use TryFrom instead where we only convert if the item has a body.

[momvart]

which wouldn't hold since CrateItem has to have a body

The same happens for FnDef. Should we make the body function optional for these or introduce a super type?

we should use TryFrom instead where we only convert if the item has a body.

But it means that StaticDef (and FnDef) has to be associated with a body, which means they shouldn't be used in ForeignItemKind then.

[celinval]

FnDef body is already optional: https://github.com/rust-lang/rust/blob/e7bbe8ce933123a8e327c79e38bcb790595e6a65/compiler/stable_mir/src/ty.rs#L554-L556

[celinval]

I believe StaticDef doesn't necessarily need to have a body either, so that's why the conversion might be the place to fix at this point. CrateItem today has to have a body, but we have an issue open to polish that type (#34).

[momvart]

@rustbot claim