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

Make vec!() macro create a vector with precise capacity #16204

Closed

Conversation

bkoropoff
Copy link
Contributor

This seemed like a small but obvious optimization. Feedback questions:

  • Is the count_args!() macro a good fit for libstd? It seems like it might be generally useful for macro writers.
  • I can add a few unit tests, but where should I put them?

@lilyball
Copy link
Contributor

lilyball commented Aug 2, 2014

I'll review the code later, but just going by the description, we haven't done this precisely because of the issue of introducing a new macro that isn't necessarily a good design. There's an RFC I filed (can't look up the number now) that modifies the macro system to, among other things, provide counting. I encourage you to find and read through it.

/// comma-separated expression parameters passed to it.
#[macro_export]
macro_rules! count_args(
($e:expr $(,$e_rest:expr)*) => (1u + count_args!($($e_rest),*));
Copy link
Contributor

Choose a reason for hiding this comment

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

This pattern looks weird, shouldn't it be ($e:expr, $(e_rest:expr),*)?

Copy link
Member

Choose a reason for hiding this comment

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

That doesn't work with one argument i.e. count_args!(foo).

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah. You can always add a one-argument case though. But I suppose if this pattern works, then it doesn't matter.

@lilyball
Copy link
Contributor

lilyball commented Aug 5, 2014

Yeah, this is basically what I thought. It's a macro that only works for expr arguments, and produces a 1+1+1+1+... expression. And it requires being publicly exported.

The RFC I mentioned is rust-lang/rfcs#88, which introduces new syntax for counting repetitions directly into the macro parser. That avoids the need for a new macro (which is only useful to other macros), supports all nonterminal kinds, and produces a single literal instead of an expression.

@bkoropoff
Copy link
Contributor Author

It's possible to avoid a separate macro by being a little creative:

/// Create a `std::vec::Vec` containing the arguments.
#[macro_export]
macro_rules! vec(
    ($($e:expr),*) => ({
        // leading _ to allow empty construction without a warning.
        let mut _temp = ::std::vec::Vec::with_capacity(vec!(c: $($e),*));
        $(_temp.push($e);)*
        _temp
    });
    ($($e:expr),+,) => (vec!($($e),+));
    // These syntactic forms are for internal use only
    (c: $e:expr $(,$e_rest:expr)*) => (1 + vec!(c: $($e_rest),*));
    (c: ) => (0);
    (c: $($e:expr),*,) => (vec!(c: $($e),*))
)

Of course, this requires consumers to abstain from using the internal syntactic forms themselves. I'm guessing this optimization is not pressing enough to justify a hacky interim solution pending the adoption of an RFC like the one you mentioned, so I'll close the PR.

@bkoropoff bkoropoff closed this Aug 6, 2014
@lilyball
Copy link
Contributor

lilyball commented Aug 6, 2014

@bkoropoff Yeah that internal format is also something that's been suggested before. My personal belief is that it's not pressing enough to warrant doing things like that.

Hopefully the RFC will be accepted. If not, then we can consider workarounds.

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.

3 participants