Skip to content

Update rustc-ap-syntax #2512

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

Merged
merged 9 commits into from
Mar 8, 2018
Merged

Update rustc-ap-syntax #2512

merged 9 commits into from
Mar 8, 2018

Conversation

topecongiro
Copy link
Contributor

@topecongiro topecongiro commented Mar 6, 2018

This PR updates rustc-ap-syntax to the latest version (currently 58.0.0), and fixes some breaking changes:

  1. beginning_vert field is removed from ast::Arm.
  2. ast::Visibility is changed to Spanned.
  3. Multiple patterns are allowed in if let and while let.
  4. Parentheses is allowed in patterns.

Closes #2398.
Closes #2511.

Bump `rustc-ap-syntax` and `rustc-ap-rustc_errors` to `57.0.0`.
With some refactorings to avoid duplicated code.
`ast::Visibility` is changed to `codemap::Spanned` whose node is
`ast::VisibilityKind`. This commit fixes it.

Closes rust-lang#2398.
`ast::Arm` used to have `beginning_vert` field whose type is `Option<Span>`
and holds a span of the beginning `|` if available. This field is now removed.
This commit works around that.

Since we only need a `BytePos` of the `|`, the type of `beginning_vert` in
`ArmWrapper` is `Option<BytePos>`.
@topecongiro
Copy link
Contributor Author

I think that we should periodically update rustc-ap-syntax so that we do not have to deal with breaking changes from libsyntax at once. Probably once a week or so?

@nrc
Copy link
Member

nrc commented Mar 8, 2018

I think that we should periodically update rustc-ap-syntax so that we do not have to deal with breaking changes from libsyntax at once. Probably once a week or so?

Yeah, I think that would be good. I would like to update regularly, but leave any new syntax as unimplemented! (with tracking issues and instructions) rather than fixing since these are typically great first issues for new contributors to tackle.

Copy link
Member

@nrc nrc left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good other than the minor code style query.

src/expr.rs Outdated
@@ -730,7 +730,7 @@ struct ControlFlow<'a> {
block: &'a ast::Block,
else_block: Option<&'a ast::Expr>,
label: Option<ast::Label>,
pat: Option<&'a ast::Pat>,
pats: Option<Vec<&'a ast::Pat>>,
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to have an Option<Vec<_>> here? Is there a difference between None and an empty Vec?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I blindly added Vec to the previous definition, but a simple Vec<_> is enough.

@topecongiro
Copy link
Contributor Author

Thank you for your review! I have updated the PR to use Vec<_> over Option<Vec<_>>.

@nrc nrc merged commit 06d509c into rust-lang:master Mar 8, 2018
@topecongiro topecongiro deleted the rustc-ap-syntax branch March 8, 2018 20:55
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.

rustfmt cannot format code with if_while_or_patterns feature pub(in path), pub(super), and pub(self) are broken
2 participants