Filesearch fixes for crosses.

[DiamondLovesYou]

Currently Rustc has no way to differentiate crates based on the crate's target. That is, getting Rustc to use host platform crates for syntax phase dependencies (by using -L paths) whilst also using the same filesearch paths to resolve link phase crates results in either version mismatches or multiple matching crate errors. A truly vexing problem whilst cross compiling.

To remedy this, I added a new metadata tag, tag_crate_target, and added it to the crate metadata. I also slightly modified the build pipeline so that resolving syntax extensions for crosses will search through rustc's set of crates, instead of always searching though the target set (but only while cross compiling). Additionally, while cross compiling, rustc will always reset its cstore, lest host platform dep libraries leak in.

This isn't user visible, but I also added a Triple structure, which for the most part mimics LLVM's behavior. I realize it wasn't strictly speaking necessarily, but it made working with the triple in filesearch && loader a tad bit easier.

[alexcrichton]

Thanks for looking into this! This is something that's been a thorn in our side for quite some time now.

Could you split apart the second commit into a few more commits? There's quite a lot going on in there, and it'd be much easier to review if all the changes were split apart from each other.

This will also need some tests before merging. There's a number of force-host tests, but this commit should in theory allow lifting that restriction. It would be nice to have tests of some other form as well, but removing the force-host parameter would be a good start.

cc #12102

[DiamondLovesYou]

I'll have to make some extra changes to compiletest to support compiling an aux crate twice when needed (once for the host/syntax extensions, once for link). I'll need to add a tag, something like needs-host. A better solution would be to parse the test crate and look at the test's extern crates to decide what targets an aux crate needs to be built for; though I believe this would be a much bigger change than just adding a new tag.
Anyhow, I'll start splitting that second commit up.

[DiamondLovesYou]

I've refactored the second commit into bite-ish sized chunks. As such, the core changes are ready to review.
I'll now start working on the compiletest changes.

[DiamondLovesYou]

@alexcrichton This PR is ready for review.

[huonw]

(Needs a rebase.)

[DiamondLovesYou]

@huonw Rebased, though the tests haven't finished running locally yet.

[alexcrichton]

Whoa, this changed a lot since I last looked at it (sorry it took awhile!).

Even after separation, there are quite a lot of changes going on here, and I don't think we need to deal with them all at once. I would very much like to get cross compilation working with syntax extensions, but much of the rest of this work seems only tangentially related.

Can you split out this new Triple structure to a new PR (as well as libmachine), and leave libsyntax in the HOST_CRATES instead of TARGET_CRATES? (it shouldn't be built for cross-compiled architectures).

[DiamondLovesYou]

Sure, but the second request poses a bit of a problem: as a requirement of removing some ignore-* in the tests that import a crate in both phases, libsyntax has to be available for both host & target so that extern crate syntax can resolve, despite the fact that it isn't used during link/runtime. Additionally, some tools, like rust-bindgen, need access to libsyntax anyway, which currently stipulates an -L argument to rustc's set of crates; I would argue that Rust would benefit from libsyntax co-existing as a target crate & host crate.

Frankly, I feel that this exposes a larger issue (note I'm only referring to #[macro_registrar] crates here; macro_rules! extensions obviously don't suffer from this issue): data does not cross the phase boundary. They clearly don't belong in the same crate together, so why should it even be allowed?

I actually resolved to creating a RFC wherein I propose a distinct crate type for syntax extensions, but blah! When ever is there time?

[DiamondLovesYou]

As for libmachine: I created it specifically so compiletest would not have to depend on all of libsyntax &| librustc. However, as rustc doesn't use it's own set of crates (outside syntax extensions) to find extern crates, libmachine also needed to exist as a target crate. Furthermore, because libsyntax depends on libmachine, when I duplicated libsyntax in TARGET_CRATES, had also had to duplicate libmachine in TARGET_CRATES.

[alexcrichton]

I don't follow why libsyntax is needed by the tests (for the target). That should be considered a bug because if I have an x86 compiler targeting arm there's no reason that an arm libsyntax should be available.

[DiamondLovesYou]

Its a result of the fact that rustc doesn't consider syntax phase crates separate from link phase crates except during extern crate time. You're right though; it is an issue, and a design level issue at that.

Consider this test: https://github.com/DiamondLovesYou/rust/blob/filesearch/src/test/run-pass-fulldeps/phase-syntax-link-does-resolve.rs. This test references an aux crate: https://github.com/DiamondLovesYou/rust/blob/filesearch/src/test/auxiliary/macro_crate_test.rs. Now, speaking on a strictly theoretical basis, you're right, it doesn't depend on libsyntax during link phase. However, due to rustc's lack of crate role differentiation (the issue to which I referred to in the second paragraph in #13450 (comment)), rustc stipulates target access to libsyntax, which means libsyntax needs to also be a target crate.

[alexcrichton]

That test is requesting that libsyntax be available at runtime, but that is generally not guaranteed. I believe that test should be rewritten as 'require-host' or not requiring libsyntax at runtime.

[DiamondLovesYou]

macro_crate_test::foo() doesn't do anything that requires libsyntax. Neither do any of the other tests that test #[phase(syntax, link)] functionality. IDK if that was intentional or not, however in my experience (others may have differing resultant conclusions, verily) the roles/functions of syntax and link phases in crates is a disjoint set, though typically in support of one another. And then only the syntax phase side actually needs libsyntax. Again, I'm referring exclusively to #[macro_registrar].

At any rate, user crates wishing to cross compile while importing crates in both phases will run into the same problems (without additional changes): libsyntax won't be available as a target crate, stipulating an additional -L argument.

But back to your comment, it would defeat the whole purpose of this PR. EDIT: Well, the desired goal.

I'm curious though, what, exactly, are the objections to including libsyntax in the set of TARGET_CRATES, even if only as a temporary measure util a better general solution is proposed, discussed, and ratified by the community/Rust team?

[DiamondLovesYou]

Correction: Actually, for cross compiled syntax extension crates, without libsyntax in both HOST_CRATES && TARGET_CRATES, importing in both phases won't work at all (libsyntax won't be available for the target, even if unused).

[alexcrichton]

Hm, let me explain myself a bit more.

In today's world, a crate has a dependent set of crates. These are all declared with extern crate. If you link to a crate, you link to all of its dependencies as well. The #[phase(syntax)] indicator means do not link but rather only read metadata and dlopen() the crate if necessary. If #[phase(syntax, link)] is provided, then you both dlopen the crate and link to the target crate afterwards.

This does not make sense for syntax extensions using procedural macros. There is rarely a need to have a runtime dependency on libsyntax. Any tests currently doing this are getting lucky, and they should not be doing this for cross compilation.

In a future world, this will not be the case, but we aren't quite there yet. This patch is fixing a real problem, however, which is that when you are targeting arm form x86, you will attempt to dlopen() arm syntax extension crates. You should instead dlopen() x86 syntax extension crates.

Does that make sense? There is no need to support linking to libsyntax on targets, that's not the underlying bug which I thought this was trying to fix.

[DiamondLovesYou]

Ah, I see where you're coming from.

[DiamondLovesYou]

As per your request, @alexcrichton, I've removed libsyntax && libmachine from TARGET_CRATES and have updated the tests accordingly.

[alexcrichton]

Could you also split out the addition of Triple and libmachine? I'd like to leave that to a later PR.

[DiamondLovesYou]

It would be pretty rough; I wrote the core changes in rustc::metadata & compiletest under the assumption of its presence.

How about I first push through a PR containing the libmachine & related changes and then I revisit this PR?

[alexcrichton]

I have some concerns about the necessity and implementation of Triple (commented on a previous commit, but github seems to have lost it). I would also be surprised that compiletest needs to know that much information about Triple that it needs its own crate.

If you want to submit the PR first, that's certainly ok!

[DiamondLovesYou]

Triple is in its own crate for compiletest, to avoid pulling all of librustc into compiletest for that one structure.

[alexcrichton]

Now that #13584 has landed, I think that the major parts of this are now present in #13605, so I'm going to close this one. If there are still remaining bits though, feel free to reopen!