Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom THIR -> MIR parser for testing purposes #564

Closed
1 of 3 tasks
JakobDegen opened this issue Oct 25, 2022 · 3 comments
Closed
1 of 3 tasks

Custom THIR -> MIR parser for testing purposes #564

JakobDegen opened this issue Oct 25, 2022 · 3 comments
Labels
major-change A proposal to make a major change to rustc major-change-accepted A major change proposal that was accepted T-compiler Add this label so rfcbot knows to poll the compiler team

Comments

@JakobDegen
Copy link

Proposal

Problem

Borrowck, mir optimizations, ctfe, and codegen are currently difficult to test because it is difficult to create test cases that generate exactly the MIR that the author wants. People will generally do their best to write some similar looking Rust code and then check that this results in the desired MIR. At best this is brittle, because it means that innocuous changes elsewhere in the compiler can break the test. At worst this is not possible, because the author is interested in some pattern that rustc cannot be tricked into generating.

Solution

I propose to solve this by creating a custom MIR parser that replaces the MIR building step. Users will write code like this:

#![feature(custom_mir, core_intrinsics)]

extern crate core;
use core::intrinsics::mir::*;

#[custom_mir(dialect = "built")]
pub fn simple(x: i32) -> i32 {
    mir!(
        let temp1: i32;
        let temp2: _;

        {
            temp1 = x;
            Goto(exit)
        }

        exit = {
            temp2 = Move(temp1);
            RET = temp2;
            Return()
        }
    )
}

The mir! macro performs some light transformations that 1) ensure that the result parses, resolves, and type-checks, and 2) make the THIR that arrives at MIR building slightly easier to parse. The entire MIR building logic will then be replaced by a new parser that does a "trivial" translation of the THIR into MIR. Each statement in the source will become one statement in MIR, each local in the source will become one local in MIR, each of the braced blocks will become one basic block, etc. This should allow generating arbitrary MIR.

The core::intrinsics::mir module will include the mir! macro and a number of helper functions that receive special treatment by the parser and are used to represent MIR constructs that would be difficult to represent with normal Rust syntax.

The syntax that will be used for this is not specified in this MCP. The intent is that native Rust syntax should be used where it's reasonably easy to implement and matches nicely with the meaning in MIR, and special functions will be used everywhere else.

The custom_mir attribute accepts a dialect and phase that indicates when this MIR should be "injected." Concretely, this becomes a field on the MIR body and all normal operations in MIR before the given dialect + phase will be disabled.

In addition to all that, a couple other changes are needed to make this work:

  • Typeck adjustments are not applied at HIR -> THIR building time. These introduce lots of extra operations that are undesirable.
  • Unsafeck is turned off unconditionally. I may revisit this but for now trying to support unsafe blocks is a bit of a pain.

Drawbacks and Alternatives

  1. Stable MIR is likely to be a superior alternative to this mechanism at some point in the future. However, it seems too far out to be a solution to the acute problem on hand.

  2. The ergonomics of this feature are never going to be great. I'll make a reasonable effort to avoid setting off ICEs unnecessarily, but it's expected that any serious attempt to use this in ways that weren't intentionally supported will lead to all kinds of wrong behavior.

  3. This only supports writing MIR for whole functions at a time. An inline_mir! like macro might be significantly more useful. However, it is also much more difficult to design and implement.

Mentors or Reviewers

I'm happy to implement this myself. There's a working branch rust-lang/rust#103464 . I also don't expect this to be an excessive amount of code, so hopefully reviewers won't be hard to find.

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.

@JakobDegen JakobDegen added major-change A proposal to make a major change to rustc T-compiler Add this label so rfcbot knows to poll the compiler team labels Oct 25, 2022
@rustbot
Copy link
Collaborator

rustbot commented Oct 25, 2022

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

@rustbot rustbot added the to-announce Announce this issue on triage meeting label Oct 25, 2022
@oli-obk
Copy link
Contributor

oli-obk commented Oct 25, 2022

@rustbot second

@rustbot rustbot added the final-comment-period The FCP has started, most (if not all) team members are in agreement label Oct 25, 2022
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Oct 28, 2022
Manishearth added a commit to Manishearth/rust that referenced this issue Nov 8, 2022
Add support for custom mir

This implements rust-lang/compiler-team#564 . Details about the design, motivation, etc. can be found in there.

r? `@oli-obk`
@apiraino
Copy link
Contributor

apiraino commented Nov 9, 2022

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

@apiraino apiraino closed this as completed Nov 9, 2022
@rustbot rustbot added major-change-accepted A major change proposal that was accepted to-announce Announce this issue on triage meeting and removed final-comment-period The FCP has started, most (if not all) team members are in agreement labels Nov 9, 2022
Manishearth added a commit to Manishearth/rust that referenced this issue Nov 9, 2022
Add support for custom mir

This implements rust-lang/compiler-team#564 . Details about the design, motivation, etc. can be found in there.

r? `@oli-obk`
Manishearth added a commit to Manishearth/rust that referenced this issue Nov 9, 2022
Add support for custom mir

This implements rust-lang/compiler-team#564 . Details about the design, motivation, etc. can be found in there.

r? ``@oli-obk``
Manishearth added a commit to Manishearth/rust that referenced this issue Nov 9, 2022
Add support for custom mir

This implements rust-lang/compiler-team#564 . Details about the design, motivation, etc. can be found in there.

r? ```@oli-obk```
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Nov 18, 2022
thomcc pushed a commit to tcdi/postgrestd that referenced this issue Feb 10, 2023
Add support for custom mir

This implements rust-lang/compiler-team#564 . Details about the design, motivation, etc. can be found in there.

r? ```@oli-obk```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
major-change A proposal to make a major change to rustc major-change-accepted A major change proposal that was accepted T-compiler Add this label so rfcbot knows to poll the compiler team
Projects
None yet
Development

No branches or pull requests

4 participants