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

Introduce NonterminalKind for more type-safe mbe parsing #74826

Merged
merged 3 commits into from
Aug 2, 2020

Conversation

matklad
Copy link
Member

@matklad matklad commented Jul 27, 2020

It encapsulate the (part of) the interface between the parser and
macro by example (macro_rules) parser.

The second bit is somewhat more general parse_ast_fragment, which is
the reason why we keep some parse_xxx functions as public.

@rust-highfive
Copy link
Collaborator

r? @ecstatic-morse

(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 Jul 27, 2020
@matklad
Copy link
Member Author

matklad commented Jul 27, 2020

r? @petrochenkov

I also want to change MetaVarDecl(Span, Ident /* name to bind */, Ident /* kind of nonterminal */), to MetaVarDecl(Span, Ident /* name to bind */, Option<MbeFragment>),

@matklad
Copy link
Member Author

matklad commented Jul 27, 2020

I also want to change MetaVarDecl(Span, Ident /* name to bind /, Ident / kind of nonterminal /), to MetaVarDecl(Span, Ident / name to bind */, Option),

Ah, MetaVarDecl is also serializable. I think it makes sense to keep it as is then and not to mix serialization into parser's interface.

@matklad matklad changed the title WIP: Introduce MbeFragment Introduce MbeFragment Jul 27, 2020
@mark-i-m
Copy link
Member

This code is quite out of cache for me, but any improvements are very well! 🎉

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 28, 2020
@matklad
Copy link
Member Author

matklad commented Aug 1, 2020

Switched to NonterminalKind! This actually I think made it possible to use it in the MetaVarDecl, but that would be a somewhat bigger refactoring with higher risk of breaking something, so it might make sense to review the two separately.

@matklad matklad changed the title Introduce MbeFragment Introduce NonterminalKind for more type-safe mbe parsing Aug 1, 2020
@matklad matklad added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 1, 2020
@matklad
Copy link
Member Author

matklad commented Aug 1, 2020

Yup, so the second commit uses NonterminalKind for MetaVarDecl as well, ready for review now!

@matklad
Copy link
Member Author

matklad commented Aug 1, 2020

The remaining weirdness with Option is due to #40107 I belive. I'll tackle that next, seems fine to try to hard error now.

src/librustc_ast/token.rs Outdated Show resolved Hide resolved
src/librustc_parse/lib.rs Outdated Show resolved Hide resolved
@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 2, 2020
matklad added 3 commits August 2, 2020 14:09
It encapsulate the (part of) the interface between the parser and
macro by example (macro_rules) parser.

The second bit is somewhat more general `parse_ast_fragment`, which is
the reason why we keep some `parse_xxx` functions as public.
This is more type safe and allows us to remove a few dead branches
Seems to be a fallout from rustfmt transition
@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Aug 2, 2020

📌 Commit 1a2d07e has been approved by petrochenkov

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 2, 2020

use crate::lexer::UnmatchedBrace;
use diagnostics::Error;
pub use path::PathStyle;
Copy link
Member Author

Choose a reason for hiding this comment

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

Are there any word of mouth and/or written guidelines about the ordering of mods and uses? I've fixed this negatively, by not mixing uses and modules in random order, but i am wondering what the positive fix would look like. In rust-analyer, we are somewhat picky about this: https://github.com/rust-analyzer/rust-analyzer/blob/master/docs/dev/style.md#order-of-imports

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think there's an official guideline, but the most common order is

use crate::things;

use other_crate::things;

use std::things;

mod foo;

/* Other items */

@bors
Copy link
Contributor

bors commented Aug 2, 2020

⌛ Testing commit 1a2d07e with merge 8ae5d23b3623b4622364808137967732e0783d70...

@rust-log-analyzer
Copy link
Collaborator

The job i686-msvc-1 of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @rust-lang/infra. (Feature Requests)

@bors
Copy link
Contributor

bors commented Aug 2, 2020

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 2, 2020
@matklad
Copy link
Member Author

matklad commented Aug 2, 2020

Seems to be a spurious network error:

    Updating crates.io index
error: failed to get `cc` as a dependency of package `bootstrap v0.0.0 (D:\a\rust\rust\src\bootstrap)`

Caused by:
  failed to fetch `https://github.com/rust-lang/crates.io-index`

Caused by:
  error inflating zlib stream; class=Zlib (5)
failed to run: D:\a\rust\rust\build\i686-pc-windows-msvc\stage0\bin\cargo.exe build --manifest-path D:\a\rust\rust\src/bootstrap/Cargo.toml --locked
Build completed unsuccessfully in 0:00:16
make: *** [Makefile:60: prepare] Error 1
Command failed. Attempt 5/5:
    Updating crates.io index
error: failed to get `cc` as a dependency of package `bootstrap v0.0.0 (D:\a\rust\rust\src\bootstrap)`

Caused by:
  failed to fetch `https://github.com/rust-lang/crates.io-index`

Caused by:
  error inflating zlib stream; class=Zlib (5)

@bors retry

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 2, 2020
@bors
Copy link
Contributor

bors commented Aug 2, 2020

⌛ Testing commit 1a2d07e with merge f042d74...

@bors
Copy link
Contributor

bors commented Aug 2, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: petrochenkov
Pushing f042d74 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 2, 2020
@bors bors merged commit f042d74 into rust-lang:master Aug 2, 2020
matklad added a commit to matklad/rust that referenced this pull request Aug 31, 2020
…y, r=matklad

Restore public visibility on some parsing functions for rustfmt

In rust-lang#74826 the visibility of several parsing functions was reduced. However, rustfmt is an external consumer of some of these functions as well and needs the visibility to be public, similar to other elements in rustc_parse such as `parse_ident`

https://github.com/rust-lang/rust/blob/db534b3ac286cf45688c3bbae6aa6e77439e52d2/src/librustc_parse/parser/mod.rs#L433-L436
@cuviper cuviper added this to the 1.47.0 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants