Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow the use of syntax extensions when cross-compiling #13584

Closed
wants to merge 1 commit into from

Conversation

rcxdude
Copy link
Contributor

@rcxdude rcxdude commented Apr 17, 2014

This allows the use of syntax extensions when cross-compiling (fixing #12102). It does this by encoding the target triple in the crate metadata and checking it when searching for files. Currently the crate triple must match the host triple when there is a macro_registrar_fn, it must match the target triple when linking, and can match either when only macro_rules! macros are used.

due to carelessness, this is pretty much a duplicate of #13450.

@brson
Copy link
Contributor

brson commented Apr 17, 2014

This is an alternative to #13450

}
compile_aux_args(&aux_props);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes to compiletest are optional, I wrote them when attempting to get phase-syntax-link-does-resolve to work when cross-compiling, but currently there is the difficulty of libsyntax being required at runtime when this kind of crate is used. The capability may be useful in the future when this can be resolved, however.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I quite follow these changes, could you add some comments or explain them to me?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that requiring a target libsyntax is a bug and the tests should be updated to not do that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is difficult to do currently (specifically phase-syntax-link-does-resolve involves a crate from which normal functions and syntax extensions are loaded, which is a desirable feature). I think it could be done with conditional extern crate; and some #[cfg(no_syntax)] build or similar.

@rcxdude
Copy link
Contributor Author

rcxdude commented Apr 21, 2014

I've reworked the code as suggested, it involves a few more changes, but I think it's probably a bit cleaner overall (the most awkward bit is moved to load_crate in CrateLoader).

decoder::get_symbol(data.data.as_slice(), id)
let target_triple = self.env.sess.targ_cfg.target_strs.target_triple.as_slice();
let is_cross = target_triple == driver::host_triple();
let should_link = info.should_link && is_cross;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this condition use !is_cross instead of is_cross? It may also be clearer to get inlined below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be is_cross. It only makes sense to search twice if we're cross compiling and there are two sets of libraries to search.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I think I see what's going on, I think some comments may be in order (down on the if condition).

This also looks like if you have a host crate with macros (but no target crate), you will attempt to link against it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops, you are indeed correct, I misunderstood initially.

@alexcrichton
Copy link
Member

This is looking great, thanks again for this!

@rcxdude
Copy link
Contributor Author

rcxdude commented Apr 23, 2014

Squashed and rebased

@rcxdude
Copy link
Contributor Author

rcxdude commented Apr 23, 2014

Requested cleanup done.

@alexcrichton
Copy link
Member

This looks good to me, thanks!

I'm going to test the i686<->x86_64 configuration before landing to make sure our snap/nightly builders continue working (will post back here when it's done)

@alexcrichton
Copy link
Member

Looks like this does the trick! There's one more test (issue-13560.rs) which hasn't had the ignore-cross-compile which hasn't had the directive removed, could you remove that one as well?

This adds the target triple to the crate metadata.
When searching for a crate the phase (link, syntax) is taken into account.
During link phase only crates matching the target triple are considered.
During syntax phase, either the target or host triple will be accepted, unless
the crate defines a macro_registrar, in which case only the host triple will
match.
@rcxdude
Copy link
Contributor Author

rcxdude commented Apr 23, 2014

Done.

bors added a commit that referenced this pull request Apr 23, 2014
This allows the use of syntax extensions when cross-compiling (fixing #12102). It does this by encoding the target triple in the crate metadata and checking it when searching for files. Currently the crate triple must match the host triple when there is a macro_registrar_fn, it must match the target triple when linking, and can match either when only macro_rules! macros are used.

due to carelessness, this is pretty much a duplicate of #13450.
@bors bors closed this Apr 23, 2014
@rcxdude rcxdude deleted the cross-syntax-ext branch April 23, 2014 21:20
matthiaskrgr pushed a commit to matthiaskrgr/rust that referenced this pull request Nov 16, 2022
…r=jonas-schievink

fix: fix panic when computing signature of generic `FnOnce` callable

Fixes rust-lang/rust-analyzer#13579
flip1995 pushed a commit to flip1995/rust that referenced this pull request Nov 7, 2024
…rednet

borrow_deref_ref: do not trigger on `&raw` references

changelog: [`borrow_deref_ref`]: do not trigger on `&raw` references

Fix rust-lang#13584
flip1995 pushed a commit to flip1995/rust that referenced this pull request Nov 21, 2024
…rednet

borrow_deref_ref: do not trigger on `&raw` references

changelog: [`borrow_deref_ref`]: do not trigger on `&raw` references

Fix rust-lang#13584
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants