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

parse: unify function front matter parsing #69023

Merged
merged 13 commits into from
Feb 13, 2020
Merged

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Feb 10, 2020

Part of #68728.

  • const extern fn feature gating is now done post-expansion such that we do not have conditional compatibilities of function qualifiers in parsing.

  • The FnFrontMatter grammar becomes:

    Extern = "extern" StringLit ;
    FnQual = "const"? "async"? "unsafe"? Extern? ;
    FnFrontMatter = FnQual "fn" ;

    That is, all item contexts now syntactically allow const async unsafe extern "C" fn and use semantic restrictions to rule out combinations previously prevented syntactically. The semantic restrictions include in particular:

    • fns in extern { ... } can have no qualifiers.
    • const and async cannot be combined.
  • We change ast::{Unsafety, Spanned<Constness>}> into enum ast::{Unsafe, Const} { Yes(Span), No } respectively. This change in formulation allow us to exclude Span in the case of No, which facilitates parsing. Moreover, we also add a Span to IsAsync which is renamed to Async. The new Spans in Unsafety and Async are then taken advantage of for better diagnostics. A reason this change was made is to have a more uniform and clear naming scheme.

    The HIR keeps the structures in AST (with those definitions moved into HIR) for now to avoid regressing perf.

r? @petrochenkov

@petrochenkov petrochenkov added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 10, 2020
src/librustc_ast_passes/ast_validation.rs Outdated Show resolved Hide resolved
src/librustc_ast_passes/ast_validation.rs Outdated Show resolved Hide resolved
src/librustc_ast_passes/ast_validation.rs Outdated Show resolved Hide resolved
src/librustc_ast_passes/ast_validation.rs Outdated Show resolved Hide resolved
src/librustc_hir/print.rs Outdated Show resolved Hide resolved
src/librustc_ast_passes/ast_validation.rs Outdated Show resolved Hide resolved
src/librustc_builtin_macros/test.rs Outdated Show resolved Hide resolved
src/librustc_parse/parser/item.rs Outdated Show resolved Hide resolved
src/librustc_ast_passes/feature_gate.rs Show resolved Hide resolved
src/librustc_parse/parser/item.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 Feb 10, 2020
@bors

This comment has been minimized.

@Centril Centril 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 Feb 11, 2020
@Centril
Copy link
Contributor Author

Centril commented Feb 11, 2020

Addressed the comments. :)

@bors

This comment has been minimized.

@petrochenkov
Copy link
Contributor

r=me after rebase

@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 Feb 11, 2020
@Centril
Copy link
Contributor Author

Centril commented Feb 13, 2020

@bors r=petrochenkov

@bors
Copy link
Contributor

bors commented Feb 13, 2020

📌 Commit 9828559 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 Feb 13, 2020
@bors
Copy link
Contributor

bors commented Feb 13, 2020

⌛ Testing commit 9828559 with merge be493fe...

bors added a commit that referenced this pull request Feb 13, 2020
parse: unify function front matter parsing

Part of #68728.

- `const extern fn` feature gating is now done post-expansion such that we do not have conditional compatibilities of function qualifiers *in parsing*.

- The `FnFrontMatter` grammar becomes:
   ```rust
   Extern = "extern" StringLit ;
   FnQual = "const"? "async"? "unsafe"? Extern? ;
   FnFrontMatter = FnQual "fn" ;
   ```

   That is, all item contexts now *syntactically* allow `const async unsafe extern "C" fn` and use semantic restrictions to rule out combinations previously prevented syntactically. The semantic restrictions include in particular:

   - `fn`s in `extern { ... }` can have no qualifiers.
   - `const` and `async` cannot be combined.

- We change `ast::{Unsafety, Spanned<Constness>}>` into `enum ast::{Unsafe, Const} { Yes(Span), No }` respectively. This change in formulation allow us to exclude `Span` in the case of `No`, which facilitates parsing. Moreover, we also add a `Span` to `IsAsync` which is renamed to `Async`. The new `Span`s in `Unsafety` and `Async` are then taken advantage of for better diagnostics. A reason this change was made is to have a more uniform and clear naming scheme.

  The HIR keeps the structures in AST (with those definitions moved into HIR) for now to avoid regressing perf.

r? @petrochenkov
@bors
Copy link
Contributor

bors commented Feb 13, 2020

☀️ Test successful - checks-azure
Approved by: petrochenkov
Pushing be493fe to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 13, 2020
@bors bors merged commit 9828559 into rust-lang:master Feb 13, 2020
@Centril Centril deleted the parse_fn branch February 13, 2020 12:54
@rust-highfive
Copy link
Collaborator

📣 Toolstate changed by #69023!

Tested on commit be493fe.
Direct link to PR: #69023

💔 rustc-guide on linux: test-pass → test-fail (cc @JohnTitor @amanjeev @spastorino @mark-i-m, @rust-lang/infra).

rust-highfive added a commit to rust-lang-nursery/rust-toolstate that referenced this pull request Feb 13, 2020
Tested on commit rust-lang/rust@be493fe.
Direct link to PR: <rust-lang/rust#69023>

💔 rustc-guide on linux: test-pass → test-fail (cc @JohnTitor @amanjeev @spastorino @mark-i-m, @rust-lang/infra).
@JohnTitor
Copy link
Member

(Toolstate failure is spurious, can be ignored.)

@Centril Centril added relnotes Marks issues that should be documented in the release notes of the next release. T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Mar 24, 2020
@Centril Centril added this to the 1.43 milestone Mar 24, 2020
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. relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants