Skip to content

Add infrastructure to "compute the ABI of a Rust type, described as a C type" #672

Closed
@RalfJung

Description

@RalfJung

Proposal

Platform ABIs are typically defined in terms of C types. When adding support for a platform in Rust, the ABI adjustment code needs to figure out how to map Rust types into such a description. For many ABIs this is fairly simple, but some ABIs do complicated type-directed traversals, and that easily leads to issues:

Literally every single ABI that we have implemented and that does something non-trivial got it wrong! This is clearly a systematic issue. I don't have high confidence that after fixing the above bugs, those targets are "good" -- these ABIs tend to special-case specific arcane conditions and it is really hard to be sure that tests like rust-lang/rust#115372 cover all of them.

So I propose that we systematically attack this issue, by introducing a type for "C ABI descriptions" that roughly looks like this (this is a first draft, this will almost certainly need to be extended/adjusted):

/// The C type that describes the ABI of a Rust type.
enum CAbiType {
  Primitive(abi::Primitive),
  Struct(Vec<CAbiType>),
  PackedStruct(Vec<CAbiType>),
  Union(Vec<CAbiType>),
  Array { elem: Box<CAbiType>, len: usize },
  /// A kind of type that doesn't exist in C such as a zero-sized type, an unsized type,
  /// or some enums. This can never exist as the field of a struct/union/array,
  /// it is used only as a top-level ABI type.
  /// (This is the most likely place for changes to be needed.)
  Opaque,
}

We could then have a single shared function that computes the CAbiType for a Rust TyAndLayout, and all targets that need to make subtle adjustments should be ported to use that shared function instead of traversing the type/layout themselves. This shared function would, in particular, know to skip repr(transparent) wrappers.

Alternative

Instead of making this a completely new type that is derived from TyAndLayout, we could also alter the existing Abi type to be able to represent C structs, unions, and arrays. The intent of that Abi type was to capture everything relevant for the ABI, but unfortunately it turns out that the current Abi::Aggregate case just loses too much information.

Mentors or Reviewers

I won't have time to implement this or to serve as the sole mentor, but I'd be happy to co-mentor with someone else.

Process

The main points of the Major Change Process are as follows:

  • File an issue describing the proposal.
  • A compiler team member or contributor who is knowledgeable in the area can second by writing @rustbot second.
    • Finding a "second" suffices for internal changes. If however, you are proposing a new public-facing feature, such as a -C flag, then full team check-off is required.
    • Compiler team members can initiate a check-off via @rfcbot fcp merge on either the MCP or the PR.
  • Once an MCP is seconded, the Final Comment Period begins. If no objections are raised after 10 days, the MCP is considered approved.

You can read more about Major Change Proposals on forge.

Comments

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

Metadata

Metadata

Assignees

No one assigned

    Labels

    T-compilerAdd this label so rfcbot knows to poll the compiler teammajor-changeA proposal to make a major change to rustcmajor-change-acceptedA major change proposal that was accepted

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions