Rust: Tracking issue for RFC#1685: Deprecate anonymous parameters

Created on 2 May 2017  ·  34Comments  ·  Source: rust-lang/rust

This is a tracking issue for the RFC "Deprecate anonymous parameters " (rust-lang/rfcs#1685).

Steps:

  • [x] Implement the RFC (cc @rust-lang/compiler -- can anyone write up mentoring instructions?)
  • [x] Adjust documentation (see instructions on forge)

Unresolved questions:

  • [x] The precise deprecation strategy
B-RFC-approved C-tracking-issue T-lang disposition-merge finished-final-comment-period

Most helpful comment

:bell: This is now entering its final comment period, as per the review above. :bell:

All 34 comments

I could work on this. From reading the RFC discussion, I think the "soft" deprecation strategy has been chosen? So on the compiler side, we'll need an allow by default (deprecation) lint for anonymous parameters, right?

@est31 Awesome! And yes, that's correct. This will be most effective if we add the lint to Clippy as well.

(The open question is around the criteria for moving to a warn-by-default strategy in the future.)

About the clippy lint, I'll leave that to someone else. Given #41692, it should be mostly copy paste, but maybe with official rust claiming the lint name one has to chose another one. No idea.

IJR inspection: https://github.com/intellij-rust/intellij-rust/pull/1211

I'll try to add an automatic fix to rustfmt as well. Not sure it's a great idea, but I want to try :)

It doesn't appear that this is linting: https://play.rust-lang.org/?gist=b818c6024acb95add60b339e7708d132&version=nightly

Also, we might want to try to finish this for the 2018 epoch, so we should start linting _now_

Oh, duh 🤦‍

It's an allow by default lint.

Sorry for the multiple rapid posts.

After having read a bit more of the discussion that I can find, I propose a more aggressive deprecation strategy:

  • We make the lint warn-by-default as soon as possible
  • We make anon parameters a hard error at the epoch boundary

cc @matklad @est31 @aturon

Should this also apply to fn(Foo) function pointer types?

@SimonSapin I would say no, because the situation for function pointers is just the opposite: nontrivial patterns are forbidden:

error[E0561]: patterns aren't allowed in function pointer types
 --> main.rs:1:13
  |
1 | type F = fn(&x: &i32);
  |             ^^

Also, I think the original RFC doesn't mention fn pointers...

@SimonSapin
The problem solved by this RFC is unique to trait methods - they may have or not have body, so they may need or not need the full power of patterns in parameters (and full pattern support is incompatible with anonymous parameters).
Function pointers never have body, so they never need non-trivial patterns, so they can freely use anonymous parameters as well.

Shall it be addressed before Rust 2018?

Maybe there should be a Github label like "rust-2018-checklist"? Or it should be a Github milestone?

Do the current plan is to make this warn by default starting at the edition and deprecate it in a later edition. (See https://github.com/rust-lang/rust/issues/48309)

In RFC 2522, I argue that to support the cases in that RFC we should expedite the schedule of RFC 1685
such that writing trait Foo { fn bar(MyType) { ... } } becomes a warning in Rust 2015 and a hard error in Rust 2018.

cc @nrc @nikomatsakis

@mark-i-m would you be up for prep'ing a PR to make that change? I think that was what you originally wanted to do. As long as there is a migration lint, it seems fine -- that is, as long as we can automatically migrate 2015 code.

Sure, opened #53272.

It just changes the lint from being "warn in 2018" to "warn now" and adds a migration suggestion. Question: do we want to backport it to 1.29? Otherwise it will start warning just a month before the edition release.

@rfcbot merge

I propose that we adopt the deprecation strategy of warning in Rust 2015 and making this a hard error in Rust 2018 per my note over at https://github.com/rust-lang/rust/issues/41686#issuecomment-411966709.

(this means the edition-2018 lint group)

Team member @Centril has proposed to merge this. The next step is review by the rest of the tagged teams:

  • [x] @Centril
  • [x] @aturon
  • [x] @cramertj
  • [x] @eddyb
  • [x] @joshtriplett
  • [x] @nikomatsakis
  • [x] @nrc
  • [x] @pnkfelix
  • [x] @scottmcm
  • [ ] @withoutboats

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

:bell: This is now entering its final comment period, as per the review above. :bell:

The final comment period, with a disposition to merge, as per the review above, is now complete.

The OP mentions some adjusting of documentation. Does anyone know specifically which documentation needs to be adjusted?

@mark-i-m The forge has a section on updating documentation, but I don't think there's much to do here for deprecations other than to make sure that the book / reference / RBE aren't using the feature.

@cramertj @mark-i-m We should probably note this in the edition guide as well.

Triage: We still need to update the documentation.
ping @mark-i-m

Sorry for the delay. I have been a bit swamped lately and mostly keeping up from my phone, so not much opportunity to write PRs :stuck_out_tongue:.

Reference: https://github.com/rust-lang-nursery/reference/pull/420
Edition Guide: https://github.com/rust-lang-nursery/edition-guide/pull/105

I did a brief search through the nomicon, book, RBE sections on traits and didn't see any other anon params, but please let me know if I missed one.

Docs have all been merged. This is good to go.

Before we close the tracking issue; we still need to update the reference with notes about the actual edition-breaking change.

@Centril @steveklabnik I opened https://github.com/rust-lang-nursery/reference/pull/421

Does this look like what you were thinking of?

@mark-i-m I was thinking of specifying somewhat more formally (BNF, etc.) but this is good in the interim :)

@Centril I didn't realize there was a BNF spec of rust anywhere. Could you point me to it so I can update it?

@mark-i-m I don't think there is a BNF spec of Rust anywhere that covers all of Rust and is up to date;

However, we do specify some parts of the language in BNF; for example: https://doc.rust-lang.org/reference/expressions/literal-expr.html.

I think the most up-to-date specification is in parser-lalr.y but it isn't fully updated either.

We don't need to spec it in BNF right now; but as a long term goal it would be good to do so.

@Centril Perhaps we can spin that off into its own specific issue?

Currently, I think the last piece of documentation that is mandatory for this issue would be https://github.com/rust-lang-nursery/reference/pull/421. Right?

@mark-i-m nah; no issue needed; we should define a canonical grammar for Rust at some point, hopefully soon.

Once the reference PR lands we can close this issue.

The reference PR has been merged, so I am closing this.

Was this page helpful?
0 / 5 - 0 ratings