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

Expand macros in patterns #14298

Merged
merged 6 commits into from May 28, 2014
Merged

Expand macros in patterns #14298

merged 6 commits into from May 28, 2014

Conversation

kmcallister
Copy link
Contributor

First commit is an unrelated fix for a test suite warning I introduced last week.

@emberian
Copy link
Member

cc @paulstansifer

@huonw
Copy link
Member

huonw commented May 19, 2014

This should probably go through an RFC.

@lilyball
Copy link
Contributor

@huonw It seems like an RFC-worthy change, but is anyone actually against this? I thought we just didn't support it before because nobody had put in the effort, not because it's actually controversial.

I'm definitely in favor 👍

@lilyball
Copy link
Contributor

Looks like one of the commits here references #6830, so we actually have a year-old issue specifically asking for this. And it was filed by @huonw.

@kmcallister
Copy link
Contributor Author

Unfortunately this program ICEs my branch:

#![feature(macro_rules)]

macro_rules! foo ( () => (()) )

fn main() {
    match () {
        x @ foo!() => (),
    }
}

with

task 'rustc' failed at 'attempted to analyze unexpanded pattern', /home/keegan/proj/rust/rust/src/libsyntax/ast_util.rs:667

I'll think tomorrow about how to fix this, if nobody beats me to it.

@huonw
Copy link
Member

huonw commented May 20, 2014

Is it expanding patterns recursively? (E.g. does Some(foo!()) ICE too?)

@kballard: an RFC is not just "does this have widespread support", it is also a semi-formal documentation of expected semantics and a place to address any potential issues (e.g. hygiene and potential grammar ambiguities would warrant mentions, although I can't think of any problems with those two, off the top of my head). That issue was filed long before the RFC process was even a glint in Rust's eye.

@paulstansifer
Copy link
Contributor

This is cool!

@huonw: I also don't think that either of those things are going to be a problem (regarding hygiene, pattern variables are probably just another item on the long list of things that don't get to participate in hygiene until someone implements it for them), and I can't think of any other probable issue.

@brson
Copy link
Contributor

brson commented May 20, 2014

Agree this should be an RFC. I'm very afraid that our macros are becoming a house of cards due to ad-hoc features being added before the existing features even work reliably.

@kmcallister
Copy link
Contributor Author

Sounds good; I'll submit an RFC.

@brson: I agree that Rust macros have some rough edges, and I expect some early-adopter pain from using them so heavily in the HTML parser I'm writing. That's basically in-scope for the Servo project. Macros are feature-gated and it's fine if I have to rewrite them in the future, as long as I can still do basically the same things.

It seems that every large systems project contains some build-time code generators, scary C macros, and/or C++ metaprogramming weirdness. When you have exacting requirements at runtime, it's extremely useful to do complicated things at compile time. Or these projects just tolerate a lot of boilerplate. Macros are the main reason my tokenizer is 900 lines instead of 6,000 like the others I've seen. And the concise form is much closer to what's in the spec. I need pattern macros to realize similar gains in the other major parser component (the tree builder).

Sorry about the mini-rant ­— I know we all agree macros are useful. I just want to emphasize that powerful macros are an exciting and important feature for me, right up there with lifetime checking.

@kmcallister
Copy link
Contributor Author

This is rust-lang/rfcs#85.

@kmcallister
Copy link
Contributor Author

That last commit fixes the ICE I described above.

fld.cx.span_err(
pth.span,
format!(
"non-pattern macro in pattern pos: {}",
Copy link
Member

Choose a reason for hiding this comment

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

s/pos/position/

@huonw
Copy link
Member

huonw commented May 27, 2014

I'd like to see some tests for hygiene working (e.g. that let foo!() = 1; x + 1 fails but let bar!(x) = 1; x = 1 doesn't, for foo expanding to x and bar to its argument), and also the incomplete parse error (can add to src/test/compile-fail/macro-incomplete-parse.rs).

@kmcallister
Copy link
Contributor Author

Rebased, and comments addressed.

@huonw
Copy link
Member

huonw commented May 28, 2014

r=me after squashing some commits (or not, doesn't really matter, r=me in either case).

@kmcallister
Copy link
Contributor Author

I'll squash those.

@anasazi
Copy link
Contributor

anasazi commented May 28, 2014

r=huonw

bors added a commit that referenced this pull request May 28, 2014
First commit is an unrelated fix for a test suite warning I introduced last week.
@bors bors closed this May 28, 2014
@bors bors merged commit fd40d0c into rust-lang:master May 28, 2014
lnicola pushed a commit to lnicola/rust that referenced this pull request Mar 13, 2023
fix block with no termination in or patterns

fix rust-lang#14298
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-parser Area: The parsing of Rust source code to an AST. A-syntaxext Area: Syntax extensions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants