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

macro_rules follow logic allocates uselessly in the happy path #61543

Closed
oli-obk opened this issue Jun 5, 2019 · 2 comments · Fixed by #61646
Closed

macro_rules follow logic allocates uselessly in the happy path #61543

oli-obk opened this issue Jun 5, 2019 · 2 comments · Fixed by #61646
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. WG-compiler-performance Working group: Compiler Performance

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Jun 5, 2019

E.g. in

let tokens = vec!["`=>`", "`,`", "`=`", "`|`", "`if`", "`in`"];
a vec is created, only to be thrown away if the macro actually works out. I think we can just turn most places into an early return in the happy path and then only allocate in the error path.

Do not work on this issue before #61541 is merged.

Also,

fn is_in_follow(tok: &quoted::TokenTree, frag: &str) -> IsInFollow {
hasn't had its docs updated to reflect the source changes.

@oli-obk oli-obk added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. C-cleanup Category: PRs that clean code up or issues documenting cleanup. S-blocked Status: Blocked on something else such as an RFC or other implementation work. WG-compiler-performance Working group: Compiler Performance labels Jun 5, 2019
@L117
Copy link
Contributor

L117 commented Jun 7, 2019

@oli-obk Are there any reasons to not use constant/static slices and convert them to vecs on demand?

Or maybe even convert

enum IsInFollow {
    Yes,
    No(Vec<&'static str>),
    Invalid(String, &'static str),
}

to something like

enum IsInFollow {
    Yes,
    No(&'static [&'static str]),
    Invalid(String, &'static str),
}

so there's no need to allocate vecs (In all cases) anymore?

@oli-obk
Copy link
Contributor Author

oli-obk commented Jun 7, 2019

That's a great idea! I haven't checked whether it'll work everywhere, but I think it should.

@oli-obk oli-obk removed the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Jun 7, 2019
bors added a commit that referenced this issue Jun 8, 2019
Remove useless allocations in macro_rules follow logic.

Closes  #61543
Centril added a commit to Centril/rust that referenced this issue Jun 8, 2019
Remove useless allocations in macro_rules follow logic.

Closes  rust-lang#61543
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. WG-compiler-performance Working group: Compiler Performance
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants