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

Check for proc_macro before -Ctarget-features=+crt-static #74179

Closed
wants to merge 1 commit into from

Conversation

bjorn3
Copy link
Member

@bjorn3 bjorn3 commented Jul 9, 2020

This makes it possible to compile proc macros when -Ctarget-features=+crt-static is used

This makes it possible to compile proc macros when -Ctarget-features=+crt-static is used
@rust-highfive
Copy link
Collaborator

r? @oli-obk

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 9, 2020
@oli-obk
Copy link
Contributor

oli-obk commented Jul 9, 2020

Do you think it's reasonable to build a test for this behaviour? I worry that other subtle changes may break this again, but that may be overcautious.

@petrochenkov
Copy link
Contributor

petrochenkov commented Jul 9, 2020

r? @petrochenkov
This is a tricky case with future-compatibility implications, cc #71651 #72510.
I'd prefer not to touch it for now.

(I think one proper solution here is supporting per crate or per crate type rustc flags in cargo.)

@rust-highfive rust-highfive assigned petrochenkov and unassigned oli-obk Jul 9, 2020
@bjorn3
Copy link
Member Author

bjorn3 commented Jul 9, 2020

This only changes behaviour for proc-macros, which are not guaranteed to be usable for anything other than rustc usage anyway. We could make proc-macros executables if we wanted or even use our own object format. If this were to apply to dylibs this could indead break stuff, but for proc-macros this is solely a bug fix.

@petrochenkov
Copy link
Contributor

petrochenkov commented Jul 9, 2020

Well, yeah, but rust-lang/cargo#7811 and rust-lang/cargo#8441 are on its way, and I'd want to develop some holistic solution to this on rustc side, so I'll take a liberty to close this for now, it's not super urgent.

In general, I still think rustc is a wrong abstraction level here, it the user explicitly says to build a proc macro crate with +crt-static, then the compiler should obey, perhaps there are reasons.
A higher level build system should ensure that specifying +crt-static for executables doesn't do collateral damage on other crates, and it's unfortunate that Cargo sometimes shies away from being a proper build system. If it supports build targets (in CMake terminology), then it should support per-target configuration as well.

@bjorn3 bjorn3 deleted the fix_crt_static_proc_macro branch July 9, 2020 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants