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

Support macros expanding to multiple items #10649

Merged
merged 3 commits into from
Nov 27, 2013
Merged

Conversation

sfackler
Copy link
Member

The majority of this change is modifying some of the ast_visit methods to return multiple values.

It's prohibitively expensive to allocate a ~[Foo] every time a statement, declaration, item, etc is visited, especially since the vast majority will have 0 or 1 elements. I've added a SmallVector class that avoids allocation in the 0 and 1 element cases to take care of that.

pub enum SmallVector<T> {
priv Zero,
priv One(T),
priv Many(~[T]),
Copy link
Member

Choose a reason for hiding this comment

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

Why priv? Since this is mainly a fill-in until we have a proper small-vector, I'd think it's ok to just use them directly and not have the one statement constructor functions?

Copy link
Member Author

Choose a reason for hiding this comment

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

The names would probably have to change to ZeroVec or whatever. I was initially keeping them hidden in case this ends up being restructured into something like

pub struct SmallVector<T, N: uint> {
    priv size: uint,
    priv start: [Option<T>, ..N],
    priv rest: Option<~[T]>
}

but I don't know if that's worth it.

@sfackler
Copy link
Member Author

@huonw I've cleaned up SmallVector a bit. Poking around the parser, I don't see an easy way to get at a bunch of statements in a reasonable way. I'm kind of inclined to remove the multiple statement macro commit entirely for now. There doesn't seem to be as much of a need for that as multi-item macros in any case.

@huonw
Copy link
Member

huonw commented Nov 26, 2013

That's probably best.

Also, just looking quickly now, it appears that there are some unused/can be private functions on SmallVector: .iter is unused, and .pop and .get are unused outside that module. I'd say that they don't need to exist (or don't need to be public) for now?

@sfackler
Copy link
Member Author

I'd lean towards keeping iter, pop and get around since they're pretty fundamental operations that I'm guessing will be looked for at some point. Is there any reason to make them private?

@sfackler
Copy link
Member Author

@huonw removed multi-statement macros.

@huonw
Copy link
Member

huonw commented Nov 26, 2013

Well, removing them/folding them into their point-of-use would reduce the amount of patch-over-library-holes code this adds (we already have one too many vectors defined in libsyntax, i.e. opt_vec); but yeah, no particular reason other than that.

@sfackler
Copy link
Member Author

@huonw Updated

}
}

pub fn expect_one(mut self, err: &'static str) -> T {
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this mut unnecessary? (Doesn't it warn?)

@huonw
Copy link
Member

huonw commented Nov 26, 2013

(Windows needs pub fn main for the run-pass tests.)

@sfackler
Copy link
Member Author

@huonw fixed

bors added a commit that referenced this pull request Nov 26, 2013
The majority of this change is modifying some of the `ast_visit` methods to return multiple values.

It's prohibitively expensive to allocate a `~[Foo]` every time a statement, declaration, item, etc is visited, especially since the vast majority will have 0 or 1 elements. I've added a `SmallVector` class that avoids allocation in the 0 and 1 element cases to take care of that.
@bors bors closed this Nov 27, 2013
@bors bors merged commit c403c1f into rust-lang:master Nov 27, 2013
@sfackler sfackler deleted the multi-macro branch December 23, 2013 03:53
flip1995 pushed a commit to flip1995/rust that referenced this pull request May 5, 2023
Spelling

This PR corrects misspellings identified by the [check-spelling action](https://github.com/marketplace/actions/check-spelling).

The misspellings have been reported at https://github.com/jsoref/rust-clippy/actions/runs/4710771873#summary-12776860721

The action reports that the changes in this PR would make it happy: https://github.com/jsoref/rust-clippy/actions/runs/4710771874#summary-12776860722

changelog: none
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.

4 participants