MIR progress report -- when to open a PR?

I just wanted to mention that the MIR branch is proceeding pretty well. I hope to open a PR soon, but I’m not sure how far to get before doing so. I list some options at the end of this post.

The current branch creates MIR but does not consume it in any way. The branch is currently able to build MIR for everything in the standard library and compiler. It dies processing librustdoc because slice patterns are not yet suppored (e.g., [foo, ..bar]). In fact, they raise some interesting questions, which I think I will put to a distinct thread later today.

If you’d like to see some examples, you can attach a #[rustc_mir] annotation to a fn. Here is an example of some Rust code that uses the annotation. The annotation triggers MIR to be created and dumped in graphviz form into the given file. I rendered the graphviz into SVG files and uploaded them to this directory. Note that in the case of closures, there are multiple graphs, one for the main function, and one for each closure. The closure graphs begin with a node id (e.g., 187-dot_product.dot.svg).

As you can see, there is still low-hanging fruit in terms of improvements: for example, this version of MIR is still generating drops unconditionally; I should modify it to elide drops for copy data. Similarly, we consolidate cleanup paths for panics, but not other kinds of control flow. These changes should be relatively straightforward I would think.

Regarding landing, the question is how far to go before opening a PR. The advantage of opening a PR early is of course that it eases the pain of rebasing and helps parallelize the effort. The disadvantage is that, at least at present, there aren’t really any tests for MIR, so this would be landing untested code. I originally wanted to create a suite of MIR-specific unit tests, but I haven’t gotten around to it yet – frankly, I’ve not really found many bugs in the translation itself, which is fairly straightforward, so it hasn’t really seemed worth the trouble yet. (But then, I’m not consuming MIR yet, so most bugs probably have yet to show their head.)

My current plan is to wait until MIR can build the entire bootstrap cycle (including rustdoc). I will then enable it by default when bootstrapping. This forms a kind of unit test of sorts: if you screw things up, you will hopefully get a bug or something else. (In case you’re curious, the time to construct MIR is basically negligible: e.g., for rustc_typeck, MIR construction across the entire crate takes 0.371s, as compared to 0.708 for macro expansion and ~8s for type check, and much more for translation and LLVM.)

Other options I’ve considered:

  • Just open a PR now, even without any unit tests, and keep pressing on to build some. I can imagine there will be a long review cycle, so there is also some time to make progress before landing. Also appealing.
  • Wait until a crater run with MIR enabled passes. This is a stronger form of unit test, and it means we could enable MIR construction by default across the board. However, it seems like a stronger condition that necessary.
  • Port the borrow checker or trans to use MIR and land then. Appealing because having a consumer validates the MIR design, but this is even stronger than the preivous bullet point.
  • Port the borrow checker to use MIR, but keep the old AST one around until MIR is able to take over. This is somewhat appealing, though having two copies of the borrow checker – even if one is deprecated – feels like it could be a maintenance burden. It may make sense though. But probably not quite yet.
  • Develop a type-checker for MIR and land that together with MIR itself. This forms a stronger sanity check, which is very appealing, and it But it also seems like work that can be done asynchronously.

Thoughts?

5 Likes

I would open a PR when MIR construction succeeds on rustc (that’s what I did with AdtDef, anyway). Hopefully you won’t have any nasty bugs when consuming it.

1 Like

I'm all for this!

Land it ASAP - I don’t see any disadvantage to it being in tree.

1 Like

My general software engineering philosophy is to make lots of small changes, which suggests landing this ASAP, like most other people have also suggested. :slight_smile:

1 Like

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.