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.

Sign up for GitHub

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

Jump to bottom

Kicking off libproc_macro #35538

Merged
merged 1 commit into from
Aug 17, 2016
Merged

Kicking off libproc_macro #35538

merged 1 commit into from
Aug 17, 2016

Conversation

cgswords
Copy link
Contributor

@cgswords cgswords commented Aug 8, 2016 •
edited
Loading

This PR introduces libproc_macro, which is currently quite bare-bones (just a few macro construction tools and an initial quote! macro).

This PR also introduces a few test cases for it, and an additional shim file (at src/libsyntax/ext/proc_macro_shim.rs to allow a facsimile usage of Macros 2.0 today!

@rust-highfive
Copy link
Collaborator

r? @alexcrichton

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

@cgswords
Copy link
Contributor Author

cgswords commented Aug 8, 2016

r? @nrc

Whoops, forgot to tag @nrc for reviewer.

@rust-highfive rust-highfive assigned nrc and unassigned alexcrichton Aug 8, 2016
nrc
nrc reviewed Aug 9, 2016
View reviewed changes
mk/crates.mk
@@ -100,6 +100,7 @@ DEPS_test := std getopts term native:rust_test_helpers

DEPS_syntax := std term serialize log arena libc rustc_bitflags rustc_unicode rustc_errors syntax_pos
DEPS_syntax_ext := syntax syntax_pos rustc_errors fmt_macros
DEPS_proc_macro := syntax syntax_pos rustc_errors fmt_macros rustc_plugin
Copy link
Member

Choose a reason for hiding this comment

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

deps are inconsistent between here, the Cargo.toml, and the extern crates

@nrc
Copy link
Member

nrc commented Aug 10, 2016

I'm not sure I know what you mean.

I thought we landed your quote macro in libsyntax, which means with this PR there are two copies in the codebase. Can we remove the one from libsyntax? (Or am I misremembering landing it?)

@nrc
Copy link
Member

nrc commented Aug 10, 2016

r=me with tests passing and perhaps removing the old quote (if it exists)

@cgswords
Copy link
Contributor Author

cgswords commented Aug 10, 2016 •
edited
Loading

You're misremembering. This is the first quote macro.

@eddyb
Copy link
Member

eddyb commented Aug 16, 2016

@cgswords Can you squash?

@cgswords
Copy link
Contributor Author

@eddyb Done!

@eddyb
Copy link
Member

eddyb commented Aug 16, 2016

@cgswords Looking more at it, I think you'll need to start a rustbuild build to update Cargo.lock.

@cgswords
Copy link
Contributor Author

@eddyb That should do that.

@eddyb
Copy link
Member

eddyb commented Aug 16, 2016

@bors r=nrc

@bors
Copy link
Contributor

bors commented Aug 16, 2016

📌 Commit d9d6e8c has been approved by nrc

@bors
Copy link
Contributor

bors commented Aug 16, 2016

⌛ Testing commit d9d6e8c with merge 38002f1...

@cgswords cgswords force-pushed the libproc_macro branch 2 times, most recently from 7df1036 to 6ef1bbc Compare August 16, 2016 13:27
@cgswords
Copy link
Contributor Author

cgswords commented Aug 16, 2016 •
edited
Loading

Er, accidental re-push. Travis is failing because tidy reports that I have libproc_macro in librustc_driver's toml file, but don't use it in the driver. Is there a different Cargo.toml I can put it in? (I can't extern crate it because it defines a plugin.) (cc @alexcrichton for the proper fix)

@eddyb
Copy link
Member

eddyb commented Aug 16, 2016

@cgswords I would actually expect it to depend on nothing but libstd, and libsyntax to depend on it, in the long run.

@cgswords
Copy link
Contributor Author

@eddyb I can't imagine why libsyntax would rely on it; it's the user-facing abstraction over libsyntax, so it internally needs access to that.

@eddyb
Copy link
Member

eddyb commented Aug 16, 2016

@cgswords My intuition is reversed, having libsyntax be a grammar wrapper around libmacro.

Either way, can you make libsyntax_ext use it? That should solve the dependency rooting problem.

@cgswords
Copy link
Contributor Author

@eddyb That won't stop it from throwing the "plugin as library" warning. @alexcrichton how should we shim this?

@alexcrichton
Copy link
Member

@cgswords you can add an entry to the whitelist here

@cgswords
Copy link
Contributor Author

Whitelist entry added.

@eddyb
Copy link
Member

eddyb commented Aug 16, 2016

@bors r=nrc

@bors
Copy link
Contributor

bors commented Aug 16, 2016

📌 Commit 98c8e0a has been approved by nrc

@bors
Copy link
Contributor

bors commented Aug 16, 2016

⌛ Testing commit 98c8e0a with merge 1bf5fa3...

bors added a commit that referenced this pull request Aug 16, 2016
@bors
Auto merge of #35538 - cgswords:libproc_macro, r=nrc
1bf5fa3 
Kicking off libproc_macro

This PR introduces `libproc_macro`, which is currently quite bare-bones (just a few macro construction tools and an initial `quote!` macro).

This PR also introduces a few test cases for it, and an additional `shim` file (at `src/libsyntax/ext/proc_macro_shim.rs` to allow a facsimile usage of Macros 2.0 *today*!
@bors
Copy link
Contributor

bors commented Aug 17, 2016

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-cargotest, auto-linux-64-cross-freebsd, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-opt-no-mir, auto-linux-64-opt-rustbuild, auto-linux-64-x-android-t, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-opt, auto-mac-64-opt-rustbuild, auto-mac-cross-ios-opt, auto-win-gnu-32-opt, auto-win-gnu-32-opt-rustbuild, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-cargotest, auto-win-msvc-64-opt, auto-win-msvc-64-opt-no-mir, auto-win-msvc-64-opt-rustbuild
Approved by: nrc
Pushing 1bf5fa3 to master...

@bors bors merged commit 98c8e0a into rust-lang:master Aug 17, 2016
@cgswords
Copy link
Contributor Author

🤘

@aturon aturon mentioned this pull request Aug 26, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@nrc nrc

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@cgswords @rust-highfive @nrc @bors @TimNN @eddyb @alexcrichton