search

rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.57k stars 12.74k forks source link

Clarify stages of MIR pipeline, and make MIR lints consistent #72515

Open ecstatic-morse opened 4 years ago

ecstatic-morse commented 4 years ago

The rustc_mir::transform module defines the pipeline of analysis passes and transformations that run on the MIR after it is lowered but before it is passed to codegen. However, it's not always clear whether certain invariants apply to each stage of the pipeline. For example, some terminators (FalseEdges, DropAndReplace) are removed entirely and do not appear beyond a certain point and the Return terminator, which initially appears only once in the entire MIR body may appear multiple times after the generator transform.

Additionally, it can be hard to find the right place for new error-emitting passes like the lint in #72270 or #71824. Is there a point at which the MIR is no longer meant for analysis but only for codegen? Should these be part of a specific query? Finally, the names of the mir_* queries have lost their meaning over time: mir_const can probably be removed entirely, and optimized_mir and promoted_mir should probably be renamed to mir_optimized and mir_promoted_fragments_optimized respectively.

I've listed quite a few issues above, but to resolve them I think we first need to figure out who the stakeholders are. Is there anyone who "owns" the current structure and has a clear idea of how things should look?

RalfJung commented 4 years ago

the Return terminator, which initially appears only once in the entire MIR body may appear multiple times after the generator transform.

Ah I didn't know this; this should also be reflected in the docs for TerminatorKind::Return.

RalfJung commented 4 years ago

mir_promoted_fragments_optimized

I don't think I understand what this name means. What would be the doc comment that goes with it?

ecstatic-morse commented 4 years ago

promoted_mir doesn't do promotion (I believe that happens in mir_validated?). It steals the promoted MIR and optimizes it.

RalfJung commented 4 years ago

Why does it need to be a separate query?

RalfJung commented 4 years ago

@oli-obk added infrastructure to MIR validation to distinguish different "stages".

But we still do not have a clear place to put MIR lints, and we currently do this inconsistently. https://github.com/rust-lang/rust/pull/72270 added a "MIR transform" that is just a lint; there is a lints::check call in MIR construction, and unsafety checking is its own whole thing.

RalfJung commented 4 years ago

Also Cc @rust-lang/wg-mir-opt for the stakeholders and code owner questions in the OP.

camsteffen commented 2 years ago

Is this solved by #91475?

RalfJung commented 2 years ago

I don't think so. We still have mir_const, mir_promoted, mir_for_ctfe, mir_drops_elaborated_and_const_checked, run_optimization_passes.

For lints, the situation seems to have improved -- we now have MirLint, that can be turned into passes. On the other hand, there also still is some mention of lints during MIR building, and check_unsafety is its own special thing and not a MIR pass.