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

move async unsafe fn to distinct feature gate #62500

Closed
nikomatsakis opened this issue Jul 8, 2019 · 4 comments
Closed

move async unsafe fn to distinct feature gate #62500

nikomatsakis opened this issue Jul 8, 2019 · 4 comments
Assignees
Labels
A-async-await Area: Async & Await AsyncAwait-Polish Async-await issues that are part of the "polish" area E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Jul 8, 2019

Per @Centril's comment, we want to move async unsafe fn to its own feature gate:

I would also like to move async unsafe fn out of the MVP into its own feature gate because I think a) it has seen little use, b) it is not particularly well tested, c) it ostensibly behaves weirdly because the .await point is not where you write unsafe { ... } and this is understandable from "leaky implementation POV" but not so much from an effects POV, d) it has seen little discussion and was not included in the RFC nor this report, and e) we did this with const fn and it worked fine. (I can write up the feature gating PR)

This issue has been assigned to @delan via this comment.

@nikomatsakis nikomatsakis added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-async-await Area: Async & Await AsyncAwait-Polish Async-await issues that are part of the "polish" area labels Jul 8, 2019
@Centril
Copy link
Contributor

Centril commented Jul 8, 2019

Mentoring instructions:

  1. In libsyntax/parse/mod.rs there's a struct ParseSess. To it, you want to add: pub async_unsafe_spans: Lock<Vec<Span>>, and then modify fn with_span_handler as well in the same file.

  2. In libsyntax/feature_gate.rs you want to add a new active feature gate to the bottom (see the async_closure example) and then modify check_crate (also see async_closure_spans example).

  3. In parse_fn_front_matter inside libsyntax/parse/parser.rs you want to modify fn parse_fn_front_matter such that when asyncness.is_async() and unsafety == Unsafety::Unsafe coincide, you push the Span of the unsafe token to async_unsafe_spans.

  4. In parse_item_implementation inside libsyntax/parse/parser.rs you want to modify the if self.check_keyword(kw::Async) { case analogously.

  5. You then want to add a feature gate test to src/test/ui/async-await/feature-async-unsafe.rs which also checks that the feature gate is required even under #[cfg(FALSE)] on a function. Make sure you test both free functions and functions in implementations.

  6. You will also need to edit various tests which will now fail due to feature gating. Make sure to also move out async unsafe fn tests into standalone test files.

For an example of splitting a feature gate see #62292.

@Centril Centril added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels Jul 8, 2019
@Centril
Copy link
Contributor

Centril commented Jul 8, 2019

The tracking issue to use is #62501.

@delan
Copy link
Contributor

delan commented Jul 9, 2019

@rustbot claim

@Centril
Copy link
Contributor

Centril commented Jul 11, 2019

I was convinced not to feature gate this after all (see #62149 (comment)). Sorry about the noise!

@Centril Centril closed this as completed Jul 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-async-await Area: Async & Await AsyncAwait-Polish Async-await issues that are part of the "polish" area E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants