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

Swap order of `unsafe async fn` to `async unsafe fn` #61319

Merged
merged 1 commit into from Jun 1, 2019

Conversation

Projects
None yet
6 participants
@Centril
Copy link
Member

commented May 29, 2019

Change the order of unsafe async fn to async unsafe fn.

I had intended to do this a while back but didn't get around to it...

This should be done because:

  • It is the order used by const unsafe fn so therefore it is consistent.
  • This keeps all the "effect/restriction" modifiers to the left of unsafe (which according to some is not an effect) instead of mixing them such that we are more forward compatible with some sort of effect system.

r? @cramertj

Show resolved Hide resolved src/libsyntax/parse/parser.rs Outdated
@cramertj

This comment has been minimized.

Copy link
Member

commented May 29, 2019

cc @rust-lang/lang

I don't particularly care about this either way.

@petrochenkov

This comment has been minimized.

Copy link
Contributor

commented May 29, 2019

This keeps all the "effect/restriction" modifiers to the left of unsafe (which according to some is not an effect) instead of mixing them such that we are more forward compatible with some sort of effect system.

Or looking from the other side, this keeps the entire function type together on the right (and non-type modifiers on the left).

@withoutboats

This comment has been minimized.

Copy link
Contributor

commented May 30, 2019

Seems fine

@bors

This comment was marked as resolved.

Copy link
Contributor

commented May 30, 2019

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

@cramertj

This comment has been minimized.

Copy link
Member

commented May 30, 2019

r=me with rebase and any bump-related changes you choose to make.

@Centril

This comment has been minimized.

Copy link
Member Author

commented May 30, 2019

We discussed this on the language team meeting and didn't find any problems with it. @nikomatsakis noted, I believe, that const unsafe as a rationale was compelling. I'll rebase in a bit. :)

@Centril Centril force-pushed the Centril:async-unsafe-fn-order branch from 86dd88f to 2ebfbb4 May 30, 2019

@Centril

This comment has been minimized.

Copy link
Member Author

commented May 30, 2019

@bors r=cramertj rollup

@bors

This comment has been minimized.

Copy link
Contributor

commented May 30, 2019

📌 Commit 2ebfbb4 has been approved by cramertj

pietroalbini added a commit to pietroalbini/rust that referenced this pull request May 31, 2019

Rollup merge of rust-lang#61319 - Centril:async-unsafe-fn-order, r=cr…
…amertj

Swap order of `unsafe async fn` to `async unsafe fn`

Change the order of `unsafe async fn` to `async unsafe fn`.

I had intended to do this a while back but didn't get around to it...

This should be done because:
- It is the order used by `const unsafe fn` so therefore it is consistent.
- This keeps all the "effect/restriction" modifiers to the left of `unsafe` (which according to some is not an effect) instead of mixing them such that we are more forward compatible with some sort of effect system.

r? @cramertj

bors added a commit that referenced this pull request May 31, 2019

Auto merge of #61394 - pietroalbini:rollup-lzugnb4, r=pietroalbini
Rollup of 11 pull requests

Successful merges:

 - #60897 (error: remove StringError from Debug output)
 - #61304 (Speed up Azure CI installing Windows dependencies)
 - #61319 (Swap order of `unsafe async fn` to `async unsafe fn`)
 - #61342 (Set ellipsis_inclusive_range_patterns lint to warn)
 - #61344 (Add regression test for const generics ICE)
 - #61359 (Fix links in Deref documentation)
 - #61363 (Stabilize iter_nth_back feature)
 - #61369 (Fixed lifetime misspelling)
 - #61372 (Migrate some books to mdbook version 0.2)
 - #61374 (Explicitly suggest 'type_ascription' feature)
 - #61382 (Fixed a typo in core::convert::AsMut)

Failed merges:

r? @ghost

bors added a commit that referenced this pull request May 31, 2019

Auto merge of #61394 - pietroalbini:rollup-lzugnb4, r=pietroalbini
Rollup of 11 pull requests

Successful merges:

 - #60897 (error: remove StringError from Debug output)
 - #61304 (Speed up Azure CI installing Windows dependencies)
 - #61319 (Swap order of `unsafe async fn` to `async unsafe fn`)
 - #61342 (Set ellipsis_inclusive_range_patterns lint to warn)
 - #61344 (Add regression test for const generics ICE)
 - #61359 (Fix links in Deref documentation)
 - #61363 (Stabilize iter_nth_back feature)
 - #61369 (Fixed lifetime misspelling)
 - #61372 (Migrate some books to mdbook version 0.2)
 - #61374 (Explicitly suggest 'type_ascription' feature)
 - #61382 (Fixed a typo in core::convert::AsMut)

Failed merges:

r? @ghost

bors added a commit that referenced this pull request Jun 1, 2019

Auto merge of #61394 - pietroalbini:rollup-lzugnb4, r=pietroalbini
Rollup of 11 pull requests

Successful merges:

 - #60897 (error: remove StringError from Debug output)
 - #61304 (Speed up Azure CI installing Windows dependencies)
 - #61319 (Swap order of `unsafe async fn` to `async unsafe fn`)
 - #61342 (Set ellipsis_inclusive_range_patterns lint to warn)
 - #61344 (Add regression test for const generics ICE)
 - #61359 (Fix links in Deref documentation)
 - #61363 (Stabilize iter_nth_back feature)
 - #61369 (Fixed lifetime misspelling)
 - #61372 (Migrate some books to mdbook version 0.2)
 - #61374 (Explicitly suggest 'type_ascription' feature)
 - #61382 (Fixed a typo in core::convert::AsMut)

Failed merges:

r? @ghost

@bors bors merged commit 2ebfbb4 into rust-lang:master Jun 1, 2019

1 check passed

Travis CI - Pull Request Build Passed
Details

@Centril Centril deleted the Centril:async-unsafe-fn-order branch Jun 1, 2019

@ehuss ehuss referenced this pull request Jun 29, 2019

Open

Swap unsafe/async #62

Centril added a commit to Centril/rust that referenced this pull request Jun 30, 2019

Rollup merge of rust-lang#62241 - Centril:fix-async-unsafe-order, r=p…
…etrochenkov

Always parse 'async unsafe fn' + properly ban in 2015

Parse `async unsafe fn` not `unsafe async fn` in implementations. We also take the opportunity to properly ban `async fn` in Rust 2015 when they are inside implementations.

Closes rust-lang#62232.

cc rust-lang#61319, rust-lang#62121, and rust-lang#62149.

r? @petrochenkov
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.