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.

Sign up for GitHub

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

Jump to bottom

edition lint: unnecessary T: 'a annotations #52042

Closed
nrc opened this issue Jul 3, 2018 · 1 comment
Closed

edition lint: unnecessary T: 'a annotations #52042

nrc opened this issue Jul 3, 2018 · 1 comment
Labels
A-edition-2018-lints Area: Lints supporting the 2018 edition
Milestone
Edition 2018 RC 2

Comments

@nrc
Copy link
Member

nrc commented Jul 3, 2018

We might want to wait until #51858 is implemented before landing (not sure), but we can begin implementation.
@nrc nrc added the A-edition-2018-lints Area: Lints supporting the 2018 edition label Jul 3, 2018
@nrc nrc mentioned this issue Jul 4, 2018
Closed
15 tasks
@nikomatsakis
Copy link
Contributor

nikomatsakis commented Jul 19, 2018 •
edited
Loading

Getting this 100% right would probably be fairly hard, but we can probably start with something very simple: essentially, we just identify T: 'a where-clauses in structs and .. don't do them.

struct Foo<T: 'a> { .. }

becomes

struct Foo<T> { .. }

I tried this by hand on Rayon and it all worked great ;).

@zackmdavis zackmdavis mentioned this issue Aug 3, 2018
Merged
@nikomatsakis nikomatsakis mentioned this issue Sep 13, 2018
Closed
3 tasks
zackmdavis added a commit to zackmdavis/rust that referenced this issue Sep 28, 2018
@zackmdavis
in which inferable outlives-requirements are linted
032d97f 
RFC 2093 (tracking issue rust-lang#44493) lets us leave off
commonsensically inferable `T: 'a` outlives requirements. (A separate
feature-gate was split off for the case of 'static lifetimes, for
which questions still remain.) Detecting these was requested as an
idioms-2018 lint.

It turns out that issuing a correct, autofixable suggestion here is
somewhat subtle in the presence of other bounds and generic
parameters. Basically, we want to handle these three cases:

 • One outlives-bound. We want to drop the bound altogether, including
   the colon—

   MyStruct<'a, T: 'a>
                 ^^^^ help: remove this bound

 • An outlives bound first, followed by a trait bound. We want to
   delete the outlives bound and the following plus sign (and
   hopefully get the whitespace right, too)—

   MyStruct<'a, T: 'a + MyTrait>
                   ^^^^^ help: remove this bound

 • An outlives bound after a trait bound. We want to delete the
   outlives lifetime and the preceding plus sign—

   MyStruct<'a, T: MyTrait + 'a>
                          ^^^^^ help: remove this bound

This gets (slightly) even more complicated in the case of where
clauses, where we want to drop the where clause altogether if there's
just the one bound. Hopefully the comments are enough to explain
what's going on!

A script (in Python, sorry) was used to generate the
hopefully-sufficiently-exhaustive UI test input. Some of these are
split off into a different file because rust-lang/rustfix#141
(and, causally upstream of that, rust-lang#53934) prevents them from being
`run-rustfix`-tested.

We also make sure to include a UI test of a case (copied from RFC
2093) where the outlives-bound can't be inferred. Special thanks to
Niko Matsakis for pointing out the `inferred_outlives_of` query,
rather than blindly stripping outlives requirements as if we weren't a
production compiler and didn't care.

This concerns rust-lang#52042.
bors added a commit that referenced this issue Sep 29, 2018
@bors
Auto merge of #53013 - zackmdavis:infer_outlints, r=nikomatsakis
a0a6e43 
in which inferable outlives-requirements are linted

RFC 2093 (tracking issue #44493) lets us leave off these
commonsensically inferable `T: 'a` outlives requirements. (A separate
feature-gate was split off for the case of 'static lifetimes, for
which questions still remain.) Detecting these was requested as an
idioms-2018 lint.

Resolves #52042, an item under the fabulous metaïssue #52047.

It's plausible that this shouldn't land until after `infer_outlives_requirements` has been stabilized ([final comment period started](#44493 (comment)) 4 days ago), but I think there's also a strong case to not-wait in order to maximize the time that [Edition Preview 2](https://internals.rust-lang.org/t/rust-2018-release-schedule-and-extended-beta/8076) users have to kick at it. (It's allow by default, so there's no impact unless you explicitly turn it or the rust-2018-idioms group up to `warn` or higher.)

Questions—

 * Is `explicit-outlives-requirements` a good name? (I chose it as an [RFC 344](https://github.com/rust-lang/rfcs/blob/master/text/0344-conventions-galore.md#lints)-compliant "inversion" of the feature-gate name, `infer_outlives_requirements`, but I could imagine someone arguing that the word `struct` should be part of the name somewhere, for specificity.)

 * Are there any false-positives or false-negatives? @nikomatsakis [said that](#52042 (comment)) getting this right would be "fairly hard", which makes me nervous that I'm missing something. The UI test in the initial submission of this pull request just exercises the examples [given in the Edition Guide](https://rust-lang-nursery.github.io/edition-guide/2018/transitioning/ownership-and-lifetimes/struct-inference.html).

![infer_outlints](https://user-images.githubusercontent.com/1076988/43625740-6bf43dca-96a3-11e8-9dcf-793ac83d424d.png)

r? @alexcrichton
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-edition-2018-lints Area: Lints supporting the 2018 edition
Projects
None yet
Milestone
Edition 2018 RC 2
Development

No branches or pull requests
4 participants
@alexcrichton @nikomatsakis @nrc @Mark-Simulacrum