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

syntax: Box ast::MacArgs in attributes #66999

Closed
wants to merge 2 commits into from
Closed

syntax: Box ast::MacArgs in attributes #66999

wants to merge 2 commits into from

Conversation

petrochenkov
Copy link
Contributor

@petrochenkov petrochenkov commented Dec 3, 2019 •
edited
Loading

This reduces size of doc comments at cost of an allocation / indirection in regular attributes.
Fn-like macros already have MacArgs boxed.

Follow-up to #66935.

@rust-highfive
Copy link
Collaborator

r? @eddyb

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 3, 2019
@petrochenkov
Copy link
Contributor Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@petrochenkov petrochenkov added S-waiting-on-perf Status: Waiting on a perf run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 3, 2019
@bors
Copy link
Contributor

bors commented Dec 3, 2019

⌛ Trying commit 73f02fb with merge 737ec5e...

bors added a commit that referenced this pull request Dec 3, 2019
@bors
Auto merge of #66999 - petrochenkov:boxattr, r=<try>
737ec5e 
syntax: Box `ast::MacArgs` in attributes

This reduces size of doc comments at cost of an allocation / indirection in regular attributes.
Fn-like macros already have `MacArgs` boxed.
@bors
Copy link
Contributor

bors commented Dec 3, 2019

☀️ Try build successful - checks-azure
Build commit: 737ec5e (737ec5e77cfaa299dc9916ae99791aec6ad44ac8)

@rust-timer
Copy link
Collaborator

Queued 737ec5e with parent f577b0e, future comparison URL.

@Centril
Copy link
Contributor

Centril commented Dec 4, 2019

r? @Centril

r=me with improved perf

@rust-highfive rust-highfive assigned Centril and unassigned eddyb Dec 4, 2019
@rust-timer
Copy link
Collaborator

Finished benchmarking try commit 737ec5e, comparison URL.

@petrochenkov
Copy link
Contributor Author

Doesn't look like an improvement.
Perhaps when AST is allocated on arena this will make more sense.

@Centril
Copy link
Contributor

Centril commented Dec 4, 2019

Bummer; thanks for following up anyways!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Centril Centril

Labels
S-waiting-on-perf Status: Waiting on a perf run to be completed.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@petrochenkov @rust-highfive @rust-timer @bors @Centril @eddyb