rust-lang / compiler-team

A home for compiler team planning documents, meeting minutes, and other such things.
https://rust-lang.github.io/compiler-team/
Apache License 2.0
387 stars 69 forks source link

Don't track `--emit=` options as part of crate SVH #769

Closed chbaker0 closed 2 months ago

chbaker0 commented 3 months ago

Proposal

The list of artifact types passed to --emit= does not affect the contents of those artifacts (with one exception). If foo.rlib depends on bar.rlib which was generated with --emit=link, then overwritten with e.g. --emit=link,dep-info, a downstream rustc invocation should still be able to use the newer bar.rlib.

The one exception is with --emit=metadata,...: if metadata is passed alone the .rmeta output does not contain MIR, so it is not suitable for building .rlib dependents. However, this results in an obvious compile error like error: missing optimized MIR for an item in the crate foo, and cannot cause the incorrectness that the SVH check prevents.

Excluding the --emit list from the SVH allows more flexibility when building Rust code in third-party build systems.

Mentors or Reviewers

TBD

Process

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

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 commented 3 months ago

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.

Concerns or objections to the proposal should be discussed on Zulip and formally registered here by adding a comment with the following syntax:

 @rustbot concern reason-for-concern 
 <description of the concern> 

Concerns can be lifted with:

 @rustbot resolve reason-for-concern 

See documentation at https://forge.rust-lang.org

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

cjgillot commented 2 months ago

From the zulip discussion and my own reflexion, this should go ahead provided the questions around --emit metadata get answered. @rustbot second

apiraino commented 2 months ago

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