Skip to content

Avoid loading needless proc-macro dependencies #38024

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

Merged
merged 1 commit into from
Nov 28, 2016

Conversation

jseyfried
Copy link
Contributor

Fixes #37958 when no proc-macros are exported; in particular, without pub extern crate proc_macros;, #![feature(macro_reexport)], or #![feature(use_extern_macros)].

I opened rust-lang/cargo#3334 for exported proc macros.

r? @alexcrichton

@jseyfried
Copy link
Contributor Author

cc @nrc

@jseyfried jseyfried force-pushed the avoid_needless_proc_macro_deps branch from 83f9ab2 to 08ef9ff Compare November 27, 2016 09:00
@nox
Copy link
Contributor

nox commented Nov 27, 2016

Could this be reviewed and prioritised? This is blocking rustup in Servo.

Copy link
Member

@nrc nrc left a comment

Choose a reason for hiding this comment

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

r = me with the comment expanded.

@@ -67,6 +67,8 @@ pub struct CrateSource {

#[derive(RustcEncodable, RustcDecodable, Copy, Clone, Ord, PartialOrd, Eq, PartialEq, Debug)]
pub enum DepKind {
/// A dependency that is only used for its macros, none of which are visible from other crates.
UnexportedMacrosOnly,
Copy link
Member

Choose a reason for hiding this comment

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

Are these only used by monomorphised functions or is there some reason to include them. Would be good to comment in any case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

UnexportedMacrosOnly are only included in the metadata as placeholders (the CrateNum of a dependency is computed from its position in the dependency list) -- I'll add a comment.

Does that answer your question? (not sure how monomorphised functions are relevant)

Copy link
Contributor Author

@jseyfried jseyfried Nov 28, 2016

Choose a reason for hiding this comment

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

Oh, I see -- monomorphised functions from extern crates might need to access non-visible functions. I don't think this is relevant to macros though, since expansion finishes before monomorphization.

@jseyfried jseyfried force-pushed the avoid_needless_proc_macro_deps branch from 08ef9ff to 1fd9041 Compare November 28, 2016 03:37
@jseyfried
Copy link
Contributor Author

@bors r=nrc

@bors
Copy link
Collaborator

bors commented Nov 28, 2016

📌 Commit 1fd9041 has been approved by nrc

@jseyfried
Copy link
Contributor Author

@bors p=1

@bors
Copy link
Collaborator

bors commented Nov 28, 2016

⌛ Testing commit 1fd9041 with merge 39c267a...

bors added a commit that referenced this pull request Nov 28, 2016
Avoid loading needless proc-macro dependencies

Fixes #37958 when no proc-macros are exported; in particular, without `pub extern crate proc_macros;`, `#![feature(macro_reexport)]`, or `#![feature(use_extern_macros)]`.

I opened rust-lang/cargo#3334 for exported proc macros.

r? @alexcrichton
@bors
Copy link
Collaborator

bors commented Nov 28, 2016

⛄ The build was interrupted to prioritize another pull request.

@bors bors merged commit 1fd9041 into rust-lang:master Nov 28, 2016
@jseyfried jseyfried deleted the avoid_needless_proc_macro_deps branch February 12, 2017 13:04
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.

5 participants