Simplify rustc_codegen_llvm::debuginfo::metadata

[michaelwoerister]

Proposal

Type debuginfo generation in rustc_codegen_llvm::debuginfo::metadata is become very messy over the years. It was originally written before Rust 1.0 and the infrastructure in it has become a bit of a callback hell. At the same time our debuginfo has become more complex and has outgrown the infrastructure's capabilities in a number of ways. As a result the code is now very hard to read and extend.

I propose to refactor the module and in particular:

• get rid of MemberDescription, MemberDescriptionFactory, RecursiveTypeDescription, and the like. These exist solely for breaking recursion cycles in the debuginfo node graph -- something with can be done in simpler fashion by relying on closures (see below).
• completely separate enum debuginfo generation for DWARF-based and for "C++-like" targets. The two cases are quite different but at the moment they are all handled by the same set of functions with many, hard-to-untangle if cpp_like_debuginfo { ... } else { ... } branches in them.

Dealing with recursion

For types like struct Foo { x: *const Foo } we need to create cycles in the graph of LLVM metadata nodes that encodes debuginfo. The way to do this in LLVM is to allocate a "stub" node for Foo that does not have the list of fields yet, create the fields (which can now transitively reference the stub node), and then mutate the stub node in-place (that is, attach the list of fields to it).

So far this is implemented via RecursiveTypeDescription, which holds the stub node and a MemberDescriptionFactory that is then used to create the members in a delayed fashion. This has worked OK for enforcing the protocol -- but it has the disadvantage that the creation of debuginfo for all types that have fields is split into a prepare_xyz() -> RecursiveTypeDescription function and an implementation of the MemberDescriptionFactory trait (in the case of enums and generators across two levels), which makes it hard to read the code.

The proposal is to implement this via a single helper function that looks like:

fn build_type_with_fields(cx: Context, type_id: TypeId, stub: DIType, make_fields: FnOnce(cx, stub) -> Vec<fields>) -> DIType {
   assert!(type_map(cx).insert(type_id, stub).is_none());
   let fields = make_fields(cx, stub);
   llvm::attach_fields(stub, fields);
   return stub;
}

As a result one can write things more naturally, like:

fn build_struct_type_info(cx, struct_type) -> DIType {
  build_type_with_fields(
    cx,
    TypeId::from(struct_type),
    stub(name, size, align, ...),
    |cx, stub| {
      struct_type.fields.map(|field| build_field_debuginfo(cx, stub, field))
    })
}

This approach would also reduce the amount of boilerplate code needed to do things that can also be done directly via the LLVM APIs.

The refactoring would be straightforward for the most part (we still follow the same basic principle for handling graph cycles after all) but the module would look rather different afterwards. It is also likely to uncover inconsistencies in the debuginfo we generate.

Mentors or Reviewers

@wesleywiser volunteered to review the changes.

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.

[rustbot]

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.

cc @rust-lang/compiler @rust-lang/compiler-contributors

[wesleywiser]

@rustbot second

[michaelwoerister]

@apiraino, the FCP has passed without objections, correct?

[apiraino]

Indeed, this can be closed as accepted.

@rustbot label -final-comment-period +major-change-accepted