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

Implement a explicit_generic_args_with_impl_trait feature gate #86176

Merged
merged 1 commit into from
Aug 2, 2021

Conversation

nbdd0121
Copy link
Contributor

@nbdd0121 nbdd0121 commented Jun 9, 2021

Implements #83701

When this gate is enabled, explicit generic arguments can be specified even if impl Trait is used in argument position. Generic arguments can only be specified for explicit generic parameters but not for the synthetic type parameters from impl Trait

So code like this will be accepted:

#![feature(explicit_generic_args_with_impl_trait)]

fn foo<T: ?Sized>(_f: impl AsRef<T>) {}
fn main() {
    foo::<str>("".to_string());
}

@rust-highfive
Copy link
Collaborator

r? @steveklabnik

(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 Jun 9, 2021
@rust-log-analyzer

This comment has been minimized.

@nbdd0121 nbdd0121 force-pushed the explicit-generic-args branch from 0d25b89 to d03079c Compare June 9, 2021 21:37
@rust-log-analyzer

This comment has been minimized.

@nbdd0121 nbdd0121 force-pushed the explicit-generic-args branch from d03079c to 52a8798 Compare June 9, 2021 22:47
@bors
Copy link
Contributor

bors commented Jun 10, 2021

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

@nbdd0121 nbdd0121 force-pushed the explicit-generic-args branch from 52a8798 to 43a3064 Compare June 10, 2021 23:39
@rust-log-analyzer

This comment has been minimized.

@nbdd0121 nbdd0121 force-pushed the explicit-generic-args branch from 43a3064 to ebefa0c Compare June 10, 2021 23:52
@crlf0710 crlf0710 added A-impl-trait Area: `impl Trait`. Universally / existentially quantified anonymous types with static dispatch. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 26, 2021
@crlf0710
Copy link
Member

r? @matthewjasper

@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 12, 2021
@bors
Copy link
Contributor

bors commented Jul 27, 2021

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

@nbdd0121 nbdd0121 force-pushed the explicit-generic-args branch 2 times, most recently from 15a1422 to 35b80d9 Compare July 28, 2021 01:07
@bors
Copy link
Contributor

bors commented Jul 28, 2021

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

@nbdd0121 nbdd0121 force-pushed the explicit-generic-args branch from 35b80d9 to ffb2892 Compare July 28, 2021 20:19
@JohnTitor
Copy link
Member

r? @bjorn3 as per GitHub suggestions

@bjorn3
Copy link
Member

bjorn3 commented Jul 28, 2021

I have no experience with the typechecker, maybe

r? @lcnr

(another github suggestion)

@rust-highfive rust-highfive assigned lcnr and unassigned bjorn3 Jul 28, 2021
@jackh726
Copy link
Member

I will take this review if @lcnr can't.

That being said, I think it's prudent to get a sign off from the lang team here. I imagine this probably falls under the "make more reversible decisions" part of the initiative process. So probably just need someone to give the okay. So cc @rust-lang/lang

@joshtriplett
Copy link
Member

I agree---since this is nightly-only, I think it's fine to go ahead with it.

I'll nominate this for consideration at a future meeting, but in the interim, go ahead.

@bors
Copy link
Contributor

bors commented Jul 30, 2021

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

@nbdd0121 nbdd0121 force-pushed the explicit-generic-args branch from ffb2892 to 5527810 Compare July 31, 2021 19:37
Copy link
Member

@jackh726 jackh726 left a comment

Choose a reason for hiding this comment

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

A couple nits, but r=me for this after those are addressed.

#83701 needs modified to "become" a tracking issue before/when this lands (or a new issue could be opened)

@@ -687,6 +687,9 @@ declare_features! (
/// Trait upcasting is casting, e.g., `dyn Foo -> dyn Bar` where `Foo: Bar`.
(incomplete, trait_upcasting, "1.56.0", Some(65991), None),

/// Allows explicit generic arguments specification with `impl Trait` present.
(active, explicit_generic_args_with_impl_trait, "1.56.0", Some(83701), None),
Copy link
Member

Choose a reason for hiding this comment

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

I suppose we can just co-opt that issue to be a tracking issue, yeah.

@@ -459,7 +459,28 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {

let default_counts = gen_params.own_defaults();
let param_counts = gen_params.own_counts();
let named_type_param_count = param_counts.types - has_self as usize;
let synth_type_param_count = if tcx.features().explicit_generic_args_with_impl_trait {
Copy link
Member

Choose a reason for hiding this comment

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

I'd like a small comment here just explaining what we're doing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

src/test/ui/impl-trait/explicit-generic-args-for-impl.rs Outdated Show resolved Hide resolved
@@ -0,0 +1,9 @@
error[E0632]: cannot provide explicit generic arguments when `impl Trait` is used in argument position
Copy link
Member

Choose a reason for hiding this comment

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

...in fact, I imagine the longer for that part of the diagnostic will probably co-opt some of this logic

@jackh726
Copy link
Member

jackh726 commented Aug 2, 2021

r? @jackh726

@rust-highfive rust-highfive assigned jackh726 and unassigned lcnr Aug 2, 2021
@jackh726 jackh726 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, 2021
When this gate is enabled, explicit generic arguments can be specified even
if `impl Trait` is used in argument position. Generic arguments can only be
specified for explicit generic parameters but not for the synthetic type
parameters from  `impl Trait`
@nbdd0121 nbdd0121 force-pushed the explicit-generic-args branch from 5527810 to 9b90e7e Compare August 2, 2021 03:17
@jackh726
Copy link
Member

jackh726 commented Aug 2, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Aug 2, 2021

📌 Commit 9b90e7e has been approved by jackh726

@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, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 2, 2021
Rollup of 6 pull requests

Successful merges:

 - rust-lang#86176 (Implement a `explicit_generic_args_with_impl_trait` feature gate)
 - rust-lang#87654 (Add documentation for the order of Option and Result)
 - rust-lang#87659 (Fix invalid suggestions for non-ASCII characters in byte constants)
 - rust-lang#87673 (Tweak opaque type mismatch error)
 - rust-lang#87687 (Inline some macros)
 - rust-lang#87690 (Add missing "allocated object" doc link to `<*mut T>::add`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 14f3418 into rust-lang:master Aug 2, 2021
@rustbot rustbot added this to the 1.56.0 milestone Aug 2, 2021
@nbdd0121 nbdd0121 deleted the explicit-generic-args branch August 2, 2021 22:14
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Feb 26, 2022
…ackh726

Provide extra note if synthetic type args are specified

Implement the unresolved question in rust-lang#83701 as suggested in rust-lang#86176 (comment).

r? `@jackh726`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 26, 2022
…ackh726

Provide extra note if synthetic type args are specified

Implement the unresolved question in rust-lang#83701 as suggested in rust-lang#86176 (comment).

r? ``@jackh726``
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Jun 11, 2022
…1,nagisa

Stabilize explicit_generic_args_with_impl_trait

This is a stabilisation PR for `explicit_generic_args_with_impl_trait`.

* [tracking issue](rust-lang#83701)
  - [Stabilisation report](rust-lang#83701 (comment))
  - [FCP entered](rust-lang#83701 (comment))
* [implementation PR](rust-lang#86176)
* [Reference PR](rust-lang/reference#1212)
* There is no mention of using the turbofish operator in the book (other than an entry in the operator list in the appendix), so there is no documentation to change/add there, unless we felt like we should add a section on using turbofish, but that seems orthogonal to `explicit_generic_args_with_impl_trait`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-impl-trait Area: `impl Trait`. Universally / existentially quantified anonymous types with static dispatch. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. 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.