Pass rustflags to artifacts built with implicit targets when using target-applies-to-host

[gmorenz]

What this PR does

This fixes #10744, a long-standing bug with target-applies-to-host=false where RUSTFLAGS are not being passed to artifact-units when built with cargo build (as opposed to cargo build --target <host>).

It doesn't fix a similar problem with linker and links. If the architecture in this PR is accepted, I expect I will be able to fix linker and links in the same way in a subsequent PR.

Below is a hopefully useful table of what flags are passed when, with new behavior bolded (without these changes the flag is not passed). I've only changed values in the top right cell, I've included the whole table for completeness and to hopefully make clear what exactly this feature is doing (which took me awhile to understand).

The table was generated with a host of x86_64-unknown-linux-gnu. "Flag" refers to values in RUSTFLAGS. "Linker" refers to the value of --config target.<host>.linker . The table was built with this repo and then hand modify to bold changed values.

	target_applies_to_host=true	target_applies_to_host=false
no --target flag	Flag passed to bin Flag passed to shared dep from bin Linker passed to bin Flag passed to proc macro Flag passed to shared dep from proc macro Linker passed to proc macro Built with 5 invocations Without rustflags, built with 5 invocations	Flag passed to bin Flag passed to shared dep from bin Linker not passed to bin Flag not passed to proc macro Flag not passed to shared dep from proc macro Linker not passed to proc macro Built with 6 invocations Without rustflags, built with 5 invocations
--target x86_64-unknown-linux-gnu	Flag passed to bin Flag passed to shared dep from bin Linker passed to bin Flag not passed to proc macro Flag not passed to shared dep from proc macro Linker passed to proc macro Built with 6 invocations Without rustflags, built with 6 invocations	Flag passed to bin Flag passed to shared dep from bin Linker passed to bin Flag not passed to proc macro Flag not passed to shared dep from proc macro Linker not passed to proc macro Built with 6 invocations Without rustflags, built with 6 invocations
--target i686-unknown-linux-gnu	Flag passed to bin Flag passed to shared dep from bin Linker not passed to bin Flag not passed to proc macro Flag not passed to shared dep from proc macro Linker passed to proc macro Built with 6 invocations Without rustflags, built with 6 invocations	Flag passed to bin Flag passed to shared dep from bin Linker not passed to bin Flag not passed to proc macro Flag not passed to shared dep from proc macro Linker not passed to proc macro Built with 6 invocations Without rustflags, built with 6 invocations

How it is implemented

There are two stages in the UnitGraph's life. It gets built, with correct CompileKind: CompileKind::Host for host units, CompileKind::Target(HostTriple) for artifact units. Then it gets rebuilt with artifact units that are intended for the host having their kind changed to CompileKind::Host.

This rebuilding step serves to de-duplicate host and artifact units. A step that we want to maintain where possible. Because de-duplicating host and artifact dependencies is only possible if their rustflags don't differ we must calculate rustflags for units before this step, and this step necessarily throws away the information necessary to calculate them.

The possible rustflags have already been determined before creating units. "Calculating rustflags for units" just means determining which set of rustflags go with a particular unit, and storing that somewhere. The obvious place to do that is in the unit itself, so that's what this PR does, which has the convenient side affect of also handing the de-duplication logic for us. If the rustflags are the same, two units can be merged, if they differ, they cannot.

In the above table that's why it takes 5 invocations to build the repo without rustflags, but 6 with them. The shared_dependency merges when it is built without rustflags, and not when it is built with rustflags.

Related PRs, comments and issues

fixes #10744

#9453 - Tracking issue

#9753 - Stabilization PR

#9634 - I believe this was another attempt (going down another implementation route) to fix the same issue

Comments from Alex Crichton noting that this must be fixed 1 2

My own comment describing exactly how I ran into this

Documentation for the feature

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

[gmorenz]

@rustbot label: Z-target-applies-to-host

The original work for this feature was done under Alex Crichton, and no team member has seemingly picked up the feature after he stepped down. Since the original bug isn't marked as "S-accepted" I'm rather ignoring the contributor documentation by submitting this PR in the first place (sorry).

If this isn't the direction whatever team member picks this up would like to go in, I'm happy to do something else. If no team member picks this up (which is what the contributing docs suggest should happen)... I guess the PR sits here harmlessly ¯\_(ツ)_/¯ (I'll probably show up at office hours in that case and see if I can prompt some movement).

[gmorenz]

If I'm reading the test error correctly, it's a formatting change in the benchmark harness completely unrelated to this PR

It's expecting

test bin_bench ... bench:           0 ns/iter (+/- 0)

It's finding

test bin_bench ... bench:           0.00 ns/iter (+/- 0.00)

With extra .00s

Fixed in #13901

[ehuss]

If you rebase on latest master, the test issue should be fixed.

[gmorenz]

Rebased on master. Also merged the commit fixing the doc links into the commits moving the the docs around, since I noticed comments on other PRs asking that every commit pass tests.