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

Add more content to impl-trait.md #1017

Merged
merged 8 commits into from
May 24, 2021
Merged

Add more content to impl-trait.md #1017

merged 8 commits into from
May 24, 2021

Conversation

tlyu
Copy link
Contributor

@tlyu tlyu commented May 6, 2021

Much of this comes from the edition-guide. Some information comes from the RFCs about the impl Trait feature. (rust-lang/rfcs#1522, rust-lang/rfcs#1951, and rust-lang/rfcs#2250)

Some finer points from the RFCs might need to be added, but maybe a new issue should be opened to enumerate and track those? I guess the biggest one would be checking that the syntax changes of rust-lang/rfcs#2250 are adequately documented in the reference. Lifetime defaults, especially for impl Trait in return position, should probably also be checked.

CC #276.

tlyu added 6 commits May 5, 2021 20:19
Add impl-trait text from edition-guide, mostly unchanged except for
reformatting to one sentence per line.
Other documentation makes it reasonably clear that `impl Trait` in
argument position allows the _caller_ to specify the type.
Reword a lot of things to sound less like an update and more like a
reference. Add local links in a few places.
Rename and rework the "Using `impl Trait` in more places"
into a "Limitations" section.
Rework "More details" to focus on the differences between `impl Trait`
and generic type parameters in return position. Delete the `.parse()`
examples because they're somewhat verbose for a reference.
Use only the closure example to illustrate the difference between
returning trait objects vs `impl Trait`. Also mention usefulness in
returning iterators.
Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! The placeholders have been hanging around for quite a long time.

This is written in a little more of a guide style, whereas we generally try to keep the reference closer to a specification style. However, having this is better than not, so I'm fine with merging it. However, I'll probably keep #276 open to further that work.

@@ -5,33 +5,120 @@
>
> _ImplTraitTypeOneBound_ : `impl` [_TraitBound_]

## Anonymous type parameters
> **Edition differences**: `impl Trait` is new in the 2018 edition.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
> **Edition differences**: `impl Trait` is new in the 2018 edition.

The edition differences are only for things that change compilation based on the edition. The old style of the edition guide presenting all new features as part of the "edition" is being phased out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review! I had seen other parts of the reference using language like this. Is text like "impl Trait is not available in the 2015 edition" better? I think I saw some style guide saying to prefer that, but I can't seem to find it now.

Copy link
Contributor

Choose a reason for hiding this comment

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

impl trait can be used on both 2015 and 2018. The only differences between the editions are listed here: https://doc.rust-lang.org/edition-guide/rust-2018/edition-changes.html

The reference conventions are listed here and there is a style guide here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I see. Apparently some new features were enabled in the 2015 edition after 2015, and only some (with syntax backward-incompatibilities?) were gated on edition=2018? I'm fine with deleting that note, then. (Though I also think the edition guide could use some better explanation about how new features found their way into the 2015 edition.)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yea. Usually new features work on all editions if possible. Only if it is backwards-incompatible in some way does it get gated on a new edition. FWIW, the edition guide is in the process of being rewritten.

> With a generic parameter such as `<T: Trait>`, the caller has the option to explicitly specify the generic argument for `T` at the call site using [_GenericArgs_], for example, `foo::<usize>(1)`.
> If `impl Trait` is the type of a function argument, then the caller can't ever specify the type of that argument by using a generic argument.
>
> Therefore, changing the function signature from either one to the other can constitute a breaking change for the callers of a function.
Copy link
Contributor

Choose a reason for hiding this comment

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

I can see how changing from generic parameter to impl trait would be a breaking change, but can you explain how the reverse is also a breaking change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this was based on text that I copied from the edition guide. For the reverse direction, I'm not especially familiar with using turbofish when there are multiple type parameters, e.g., what if we went from

fn foo<T: PartialOrd>(a: T, b: impl PartialOrd) {}
foo::<i32>(0, 1.0);

to

fn foo<T: PartialOrd, U: PartialOrd>(a: T, b: U) {}
foo::<i32>(0, 1.0);

Is it valid to omit the second type parameter? How does the compiler know which one is omitted? (I can't find any text in the reference about this case.) I'm sorry if this is a bad example.

Copy link
Contributor Author

@tlyu tlyu May 19, 2021

Choose a reason for hiding this comment

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

I experimented a bit in the playground, and it seems like in practice:

  • If there are multiple type parameters, either all or none must be specified by turbofish
  • If there is a mixture of type parameters and impl Trait in argument position, turbofish may not be used for any type arguments at all (edit: apparently this includes type parameters for the return type!)

Side note: are we avoiding using the term "turbofish" in the reference?

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. The second parameter can be omitted if it is defaulted, like <T, U=T>, but that can't be done with functions.

The reference normally doesn't discuss semver compatibility since that is a cargo concept (documented here). It's probably fine to leave this as-is, though.

There is one mention of turbofish, plus another in the glossary, but otherwise it usually doesn't come up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some further experimentation suggests that both my proposed text and the edition guide are differently wrong:

  • impl Trait in return position doesn't prevent the use of turbofish, contrary to the edition guide, which says that "In the case of impl Trait, if it is used anywhere in the function definition, then you can't use turbo-fish at all"
  • impl Trait in any parameter type prevents the caller from providing any generic arguments at all, including return type and even const generics (No way to use fn with both const generics and impl Trait rust#85475)! This is contrary to my text, which says it only prevents turbofish for that parameter.

@tlyu
Copy link
Contributor Author

tlyu commented May 20, 2021

While making the latest updates, I noticed that the text I copied from the edition guide uses "argument" in some places where it should use "parameter". The style guide doesn't specify explicitly (nor does either term seem to appear in the glossary), but most styles I've seen distinguish between "parameter" as an abstract input to a function, etc., and "argument" as the concrete input provided by a caller.

There's already some inconsistency out there in the Rust documentation, especially in phrasing like "impl Trait in argument position". Do we want to make any changes to make the usage more consistent on this page?

@ehuss
Copy link
Contributor

ehuss commented May 20, 2021

Yea, it definitely should be consistent. You are correct that parameter is the variable declaration and argument is the value passed to a function. If I were writing it, I would phrase things like "impl Trait can only appear as a parameter or return type"

Be more consistent about using "argument" to mean a concrete value
provided by a function caller, and "parameter" to mean the abstract
input to the function.

Retain a few instances of the phrasing "in argument position",
because it appears in the RFCs and implementation. Also make a note
about the historical terminology inconsistency.
@tlyu
Copy link
Contributor Author

tlyu commented May 20, 2021

Thanks! I've made the terminology more consistent, while keeping a few historical phrasings and adding a note about the use of wording like "impl Trait in argument position".

Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

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

Thanks!

@ehuss ehuss merged commit 81ad2cf into rust-lang:master May 24, 2021
@tlyu tlyu deleted the impl-trait branch May 25, 2021 22:32
JohnTitor added a commit to JohnTitor/rust that referenced this pull request May 26, 2021
Update books

## reference

4 commits in 5aa457bf1b54bd2cd5d4cf49797f29299bdf89a7..9c68af3ce6ccca2395e1868addef26a0542e9ddd
2021-05-05 08:39:22 -0700 to 2021-05-24 09:53:32 -0700
- missing parameter name in Trait Implementations (rust-lang/reference#1030)
- Add more content to impl-trait.md (rust-lang/reference#1017)
- Document extended key-value attributes (rust-lang/reference#1029)
- Document raw pointer &lt;-&gt; usize casts. (rust-lang/reference#970)

## rust-by-example

1 commits in 5f8c6da200ada77760a2fe1096938ef58151c9a6..805e016c5792ad2adabb66e348233067d5ea9f10
2021-04-29 08:08:01 -0300 to 2021-05-20 17:08:34 -0300
- Update structs.md (rust-lang/rust-by-example#1440)

## rustc-dev-guide

4 commits in 1e6c7fb..50de7f0
2021-05-10 13:38:24 +0900 to 2021-05-20 15:02:20 +0200
- update rustfmt references to reflect change from submod to subtree (rust-lang/rustc-dev-guide#1129)
- Remove `--stage 1` argument from `doc` invocations (rust-lang/rustc-dev-guide#1125)
- Update coverage docs (rust-lang/rustc-dev-guide#1122)
- Document -Zunpretty=thir-tree (rust-lang/rustc-dev-guide#1128)

## edition-guide

1 commits in 1da3c411f17adb1ba5de1683bb6acee83362b54a..302a115e8f71876dfc884aebb0ca5ccb02b8a962
2021-02-16 16:46:40 -0800 to 2021-05-21 10:46:11 -0400
- Minimize the edition guide (rust-lang/edition-guide#232)

## embedded-book

1 commits in 569c3391f5c0cc43433bc77831d17f8ff4d76602..7349d173fa28a0bb834cf0264a05286620ef0923
2021-04-07 08:32:11 +0000 to 2021-05-25 13:59:05 +0000
- Remove $ from cargo-binutils  (rust-embedded/book#292)
JohnTitor added a commit to JohnTitor/rust that referenced this pull request May 26, 2021
Update books

## reference

4 commits in 5aa457bf1b54bd2cd5d4cf49797f29299bdf89a7..9c68af3ce6ccca2395e1868addef26a0542e9ddd
2021-05-05 08:39:22 -0700 to 2021-05-24 09:53:32 -0700
- missing parameter name in Trait Implementations (rust-lang/reference#1030)
- Add more content to impl-trait.md (rust-lang/reference#1017)
- Document extended key-value attributes (rust-lang/reference#1029)
- Document raw pointer &lt;-&gt; usize casts. (rust-lang/reference#970)

## rust-by-example

1 commits in 5f8c6da200ada77760a2fe1096938ef58151c9a6..805e016c5792ad2adabb66e348233067d5ea9f10
2021-04-29 08:08:01 -0300 to 2021-05-20 17:08:34 -0300
- Update structs.md (rust-lang/rust-by-example#1440)

## rustc-dev-guide

4 commits in 1e6c7fb..50de7f0
2021-05-10 13:38:24 +0900 to 2021-05-20 15:02:20 +0200
- update rustfmt references to reflect change from submod to subtree (rust-lang/rustc-dev-guide#1129)
- Remove `--stage 1` argument from `doc` invocations (rust-lang/rustc-dev-guide#1125)
- Update coverage docs (rust-lang/rustc-dev-guide#1122)
- Document -Zunpretty=thir-tree (rust-lang/rustc-dev-guide#1128)

## edition-guide

1 commits in 1da3c411f17adb1ba5de1683bb6acee83362b54a..302a115e8f71876dfc884aebb0ca5ccb02b8a962
2021-02-16 16:46:40 -0800 to 2021-05-21 10:46:11 -0400
- Minimize the edition guide (rust-lang/edition-guide#232)

## embedded-book

1 commits in 569c3391f5c0cc43433bc77831d17f8ff4d76602..7349d173fa28a0bb834cf0264a05286620ef0923
2021-04-07 08:32:11 +0000 to 2021-05-25 13:59:05 +0000
- Remove $ from cargo-binutils  (rust-embedded/book#292)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants