Skip to content

libsyntax: miscellaneous cleanup #33943

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 5 commits into from
Jun 28, 2016
Merged

libsyntax: miscellaneous cleanup #33943

merged 5 commits into from
Jun 28, 2016

Conversation

jseyfried
Copy link
Contributor

syntax-[breaking-change] cc #31645
Miscellaneous low priority cleanup in libsyntax for the next breaking batch.
r? @nrc

@jseyfried
Copy link
Contributor Author

cc @Manishearth
I don't know if these cleanups are likely to cause breakage for extensions in the wild.

@jseyfried jseyfried force-pushed the libsyntax_cleanup branch from 7e014ee to e3e3057 Compare May 29, 2016 22:17
@nrc
Copy link
Member

nrc commented May 30, 2016

r=me, could probably land early doesn't look like it will break much, but I guess we may as well wait for a batch if there is no urgency.

cc @cgswords

@Manishearth
Copy link
Member

This probably will break aster.

@jseyfried jseyfried force-pushed the libsyntax_cleanup branch 5 times, most recently from 463955a to 366b01d Compare June 4, 2016 10:00
@jseyfried jseyfried force-pushed the libsyntax_cleanup branch 4 times, most recently from 68ab0c8 to 6e7d690 Compare June 12, 2016 12:04
@bors
Copy link
Collaborator

bors commented Jun 14, 2016

☔ The latest upstream changes (presumably #33749) made this pull request unmergeable. Please resolve the merge conflicts.

@jseyfried jseyfried force-pushed the libsyntax_cleanup branch from 6e7d690 to 0644aba Compare June 14, 2016 07:40
jseyfried
jseyfried reviewed Jun 16, 2016
View reviewed changes
src/libsyntax/ext/expand.rs
}

ast::ExprKind::While(cond, body, opt_ident) => {
let cond = fld.fold_expr(cond);
let (body, opt_ident) = expand_loop_block(body, opt_ident, fld);
fld.cx.expr(expr.span, ast::ExprKind::While(cond, body, opt_ident))
.with_attrs(fold_thin_attrs(expr.attrs, fld))
expr.node = ast::ExprKind::While(cond, body, opt_ident);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

fold_thin_attrs became the identity after this commit.

@jseyfried
Copy link
Contributor Author

Rebased and added 3 new commits (r? @nrc)

@nrc
Copy link
Member

nrc commented Jun 16, 2016

r=me

@bors
Copy link
Collaborator

bors commented Jun 25, 2016

☔ The latest upstream changes (presumably #34464) made this pull request unmergeable. Please resolve the merge conflicts.

jseyfried added a commit to jseyfried/rust that referenced this pull request Jun 26, 2016
@jseyfried
Rollup merge of rust-lang#33943 - jseyfried:libsyntax_cleanup, r=nrc
33ea1e3 
Miscellaneous low priority cleanup in `libsyntax`.
@jseyfried jseyfried mentioned this pull request Jun 26, 2016
Merged
bors added a commit that referenced this pull request Jun 26, 2016
@bors
Auto merge of #34424 - jseyfried:breaking_batch, r=<try>
dff9860 
Batch up libsyntax breaking changes

Batch of the following syntax-[breaking-change] changes:
 - #34213: Add a variant `Macro` to `TraitItemKind`
 - #34368: Merge the variant `QPath` of `PatKind` into the variant `PatKind::Path`
 - #34385: Move `syntax::ast::TokenTree` into a new module `syntax::tokenstream`
 - #33943:
  - Remove the type parameter from `visit::Visitor`
  - Remove `attr::WithAttrs` -- use `attr::HasAttrs` instead.
  - Change `fold_tt`/`fold_tts` to take token trees by value and avoid wrapping token trees in `Rc`.
  - Remove the field `ctxt` of `ast::Mac_`
  - Remove inherent method `attrs()` of types -- use the method `attrs` of `HasAttrs` instead.
 - #34316:
  - Remove `ast::Decl`/`ast::DeclKind` and add variants `Local` and `Item` to `StmtKind`.
  - Move the node id for statements from the `StmtKind` variants to a field of `Stmt` (making `Stmt` a struct instead of an alias for `Spanned<StmtKind>`)
  - Rename `ast::ExprKind::Again` to `Continue`.
 - #34339: Generalize and abstract `ThinAttributes` to `ThinVec<Attribute>`
  - Use `.into()` in convert between `Vec<Attribute>` and `ThinVec<Attribute>`
  - Use autoderef instead of `.as_attr_slice()`
 - #34436: Remove the optional expression from `ast::Block` and instead use a `StmtKind::Expr` at the end of the statement list.
 - #34403: Move errors into a separate crate (unlikely to cause breakage)
bors added a commit that referenced this pull request Jun 27, 2016
@bors
Auto merge of #34424 - jseyfried:breaking_batch, r=Manishearth
ea0dc92 
Batch up libsyntax breaking changes

Batch of the following syntax-[breaking-change] changes:
 - #34213: Add a variant `Macro` to `TraitItemKind`
 - #34368: Merge the variant `QPath` of `PatKind` into the variant `PatKind::Path`
 - #34385: Move `syntax::ast::TokenTree` into a new module `syntax::tokenstream`
 - #33943:
  - Remove the type parameter from `visit::Visitor`
  - Remove `attr::WithAttrs` -- use `attr::HasAttrs` instead.
  - Change `fold_tt`/`fold_tts` to take token trees by value and avoid wrapping token trees in `Rc`.
  - Remove the field `ctxt` of `ast::Mac_`
  - Remove inherent method `attrs()` of types -- use the method `attrs` of `HasAttrs` instead.
 - #34316:
  - Remove `ast::Decl`/`ast::DeclKind` and add variants `Local` and `Item` to `StmtKind`.
  - Move the node id for statements from the `StmtKind` variants to a field of `Stmt` (making `Stmt` a struct instead of an alias for `Spanned<StmtKind>`)
  - Rename `ast::ExprKind::Again` to `Continue`.
 - #34339: Generalize and abstract `ThinAttributes` to `ThinVec<Attribute>`
  - Use `.into()` in convert between `Vec<Attribute>` and `ThinVec<Attribute>`
  - Use autoderef instead of `.as_attr_slice()`
 - #34436: Remove the optional expression from `ast::Block` and instead use a `StmtKind::Expr` at the end of the statement list.
 - #34403: Move errors into a separate crate (unlikely to cause breakage)
@bors bors merged commit 0644aba into rust-lang:master Jun 28, 2016
@jseyfried jseyfried mentioned this pull request Jul 5, 2016
Merged
bors added a commit that referenced this pull request Jul 6, 2016
@bors
Auto merge of #34652 - jseyfried:fix_expansion_perf, r=nrc
70e1a95 
Fix expansion performance regression

**syntax-[breaking-change] cc #31645**

This fixes #34630 by reverting commit 5bf7970 of PR #33943, which landed in #34424.

By removing the `Rc<_>` wrapping around `Delimited` and `SequenceRepetition` in `TokenTree`, 5bf7970 made cloning `TokenTree`s more expensive. While this had no measurable performance impact on the compiler's crates, it caused an order of magnitude performance regression on some macro-heavy code in the wild. I believe this is due to clones of `TokenTree`s in `macro_parser.rs` and/or `macro_rules.rs`.

r? @nrc
bors added a commit that referenced this pull request Jul 7, 2016
@bors
Auto merge of #34652 - jseyfried:fix_expansion_perf, r=nrc
de78655 
Fix expansion performance regression

**syntax-[breaking-change] cc #31645**

This fixes #34630 by reverting commit 5bf7970 of PR #33943, which landed in #34424.

By removing the `Rc<_>` wrapping around `Delimited` and `SequenceRepetition` in `TokenTree`, 5bf7970 made cloning `TokenTree`s more expensive. While this had no measurable performance impact on the compiler's crates, it caused an order of magnitude performance regression on some macro-heavy code in the wild. I believe this is due to clones of `TokenTree`s in `macro_parser.rs` and/or `macro_rules.rs`.

r? @nrc
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers
No reviews
Assignees

@nrc nrc

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jseyfried @nrc @Manishearth @bors