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

Macro reform #20482

Closed
wants to merge 24 commits into from
Closed

Macro reform #20482

wants to merge 24 commits into from

Conversation

kmcallister
Copy link
Contributor

Fixes #20008.

This is a [breaking-change]. I've written a guide to migrating your code.

@rust-highfive
Copy link
Collaborator

r? @aturon

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

@steveklabnik
Copy link
Member

Does any of this need [breaking-change]

@kmcallister
Copy link
Contributor Author

@steveklabnik: Yeah, good point. Since things change a few times in the Git history, maybe I'll just tag the doc commit and provide a link to the migration guide.

@kmcallister
Copy link
Contributor Author

@nikomatsakis is going to review this.

@nikomatsakis nikomatsakis assigned nikomatsakis and unassigned aturon Jan 4, 2015
@nikomatsakis
Copy link
Contributor

f? @sfackler @cmr

@sfackler
Copy link
Member

sfackler commented Jan 4, 2015

I'll start looking at this tonight.

One quick note from looking at the migration guide - The behavior of #[plugin] runtime linking the crate by default seems wrong. It pulls in a huge runtime dependency (the entire compiler), and all syntax extension libraries I can think of off the top of my head have separate plugin and runtime crates. My gut feeling is that #[plugin] should only link to it as a plugin, and users could have a second un-annotated extern crate foo if they actually want to do link to it at runtime.

Apologies if this was already discussed in the RFC, I didn't look at it too closely at the time.

@emberian
Copy link
Member

emberian commented Jan 4, 2015

All the code looks good, though to @sfackler's point, there is supposed to be a method on the Registry(or, I would assume, a flag instead, per-syntax-extension) that determines whether the crate gets linked at runtime.

@emberian
Copy link
Member

emberian commented Jan 4, 2015

(r=me with that fixed, and a rebase)

@sfackler
Copy link
Member

sfackler commented Jan 4, 2015

It's not totally clear to me when a syntax extension would ever want to get linked at runtime, unless we add some machinery to build two versions of the crate, one with the rustc dependency and plugin_registrar and the other without.

@huonw
Copy link
Member

huonw commented Jan 4, 2015

@sfackler I could vaguely imagine some syntax extension that extern crates another one and uses its functionality for something.

@sfackler
Copy link
Member

sfackler commented Jan 4, 2015

But then you'd only be importing it at runtime, not compile time in the extension, right? In any case, it seems like it'd make more sense to have the 99.999% case (not wanting to link at runtime) to be "automatic", not the other way around.

@kmcallister
Copy link
Contributor Author

there is supposed to be a method on the Registry(or, I would assume, a flag instead, per-syntax-extension) that determines whether the crate gets linked at runtime.

Yeah, this was the original plan, but it's a bit tricky to pull off. I'm going to work on that during the alpha period. Plugins aren't part of the stable language, so I don't think the issue is worth blocking this PR with only 5 days left before 1.0-alpha.

@emberian
Copy link
Member

emberian commented Jan 5, 2015

Needs a rebase. f+.

@alexcrichton
Copy link
Member

This may want to hold off on removing the macro_rules feature gate until rust-lang/rfcs#550 is resolved.

@nikomatsakis
Copy link
Contributor

I agree with @alexcrichton about feature gate and had planned to say the same thing.

@kmcallister
Copy link
Contributor Author

It isn't required to use {...} with macros, right? (fwiw, tuple structs are declared with () and ;, so the existing usages were not so foreign.)

Correct, and that's a fair point about tuple structs. Still I think {...} is the best style default.

@kmcallister
Copy link
Contributor Author

This may want to hold off on removing the macro_rules feature gate until rust-lang/rfcs#550 is resolved.

I think it's fine to un-gate now, because we are committing to ungating for 1.0-final. In fact it would be bad if alpha users get a warning: macros are experimental message, as it implies they won't be available in 1.0. rust-lang/rfcs#550 is an important issue for the long-term future-proofing of the language, but it's also something of a subtle detail that most users will never encounter during the alpha period.

I talked to @cmr briefly and he seemed roughly agreed with this.

@emberian
Copy link
Member

emberian commented Jan 5, 2015

Indeed.

On Mon, Jan 5, 2015 at 12:53 PM, Keegan McAllister <notifications@github.com

wrote:

This may want to hold off on removing the macro_rules feature gate until
rust-lang/rfcs#550 rust-lang/rfcs#550 is
resolved.

I think it's fine to un-gate now, because we are committing to ungating
for 1.0-final. In fact it would be bad if alpha users get a warning:
macros are experimental message, as it implies they won't be available in
1.0. rust-lang/rfcs#550 rust-lang/rfcs#550 is
an important issue for the long-term future-proofing of the language, but
it's also something of a subtle detail that most users will never encounter
during the alpha period.

I talked to @cmr https://github.com/cmr briefly and he seemed roughly
agreed with this.

—
Reply to this email directly or view it on GitHub
#20482 (comment).

http://octayn.net/

Keegan McAllister added 9 commits January 5, 2015 11:38
@kmcallister
Don't test codegen-units errors on stage1 (c.f. rust-lang#20184)
d1cf1b1
@kmcallister
Replace LetSyntaxTT with MacroRulesTT
5e5924b 
The implementation of LetSyntaxTT was specialized to macro_rules! in various
ways. This gets rid of the false generality and simplifies the code.
@kmcallister
Add a special macro nonterminal $crate
ad7c647
@kmcallister
Allow leading :: in use items
e2a9c04
@kmcallister
Implement macro re-export
5382881 
Fixes rust-lang#17103.
@kmcallister
Remove unused if_ok! macro
1c2fddc
@kmcallister
Use $crate and macro reexport to reduce duplicated code
73806dd 
Many of libstd's macros are now re-exported from libcore and libcollections.
Their libstd definitions have moved to a macros_stage0 module and can disappear
after the next snapshot.

Where the two crates had already diverged, I took the libstd versions as
they're generally newer and better-tested. See e.g. d3c831b, which was a fix to
libstd's assert_eq!() that didn't make it into libcore's.

Fixes rust-lang#16806.
@kmcallister
Stop using macro_escape as an inner attribute
fc58479 
In preparation for the rename.
@kmcallister
Rename macro_escape to macro_use
5bf385b 
In the future we want to support

    #[macro_use(foo, bar)]
    mod macros;

but it's not an essential part of macro reform.  Reserve the syntax for now.
Keegan McAllister added 3 commits January 5, 2015 12:00
@kmcallister
creader: Convert free functions to Env methods
5171b32
@kmcallister
creader: Use a single struct
24aa7f0
@kmcallister
Reformat metadata for exported macros
677b7ca 
Instead of copy-pasting the whole macro_rules! item from the original .rs file,
we serialize a separate name, attributes list, and body, the latter as
pretty-printed TTs.  The compilation of macro_rules! macros is decoupled
somewhat from the expansion of macros in item position.

This filters out comments, and facilitates selective imports.
@kmcallister
Copy link
Contributor Author

Rebased here, testing now.

@kmcallister
Copy link
Contributor Author

Bisecting locally to find the Android build failure.

Keegan McAllister added 11 commits January 5, 2015 18:21
@kmcallister
creader: Load parts of plugin metadata on demand
f314e2c
@kmcallister
Replace #[phase] with #[plugin] / #[macro_use] / #[no_link]
60be2f5
@kmcallister
Move #[macro_reexport] to extern crate
0816255
@kmcallister
Allow selective macro import
aa69cbd
@kmcallister
Reserve the keyword 'macro'
c9f0ff3
@kmcallister
Modernize macro_rules! invocations
416137e 
macro_rules! is like an item that defines a macro.  Other items don't have a
trailing semicolon, or use a paren-delimited body.

If there's an argument for matching the invocation syntax, e.g. parentheses for
an expr macro, then I think that applies more strongly to the *inner*
delimiters on the LHS, wrapping the individual argument patterns.
@kmcallister
Pass the #[plugin(...)] meta item to the registrar
d0163d3
@kmcallister
Un-gate macro_rules
c2e2697
@kmcallister
Forbid '#[macro_use] extern crate' outside the crate root
bbbb85a
@kmcallister
Add a test case for accidental macro re-export
34b995d
@kmcallister
Update docs
78e841d
@kmcallister
Copy link
Contributor Author

Pushed with a fix for the cross compilation issue in creader: Load parts of plugin metadata on demand, which makes that patch smaller.

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Jan 6, 2015
@alexcrichton
rollup merge of rust-lang#20482: kmcallister/macro-reform
7975fd9 
Conflicts:
	src/libflate/lib.rs
	src/libstd/lib.rs
	src/libstd/macros.rs
	src/libsyntax/feature_gate.rs
	src/libsyntax/parse/parser.rs
	src/libsyntax/show_span.rs
	src/test/auxiliary/macro_crate_test.rs
	src/test/compile-fail/lint-stability.rs
	src/test/run-pass/intrinsics-math.rs
	src/test/run-pass/tcp-connect-timeouts.rs
@alexcrichton
Copy link
Member

Landed in #20610

@alexcrichton
Copy link
Member

Merge commit: 7975fd9

@alexcrichton alexcrichton mentioned this pull request Jan 8, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@nikomatsakis nikomatsakis

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Macro reform (Tracking issue for RFC 453)
9 participants
@kmcallister @rust-highfive @steveklabnik @nikomatsakis @sfackler @emberian @huonw @alexcrichton @aturon