Use better generic type parameter names for Extend and FromIterator - #161379
Conversation
|
r? @JohnTitor rustbot has assigned @JohnTitor. Use Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
This failure might be spurious, for all I can tell, so let's retry. I might as well rebase now to reduce back the number of commits (integrating the previous two fixes for test failure with the respective change they correspond to) to trigger another run ^^ Edit: Yup, it was a spurious failure, the same test passed now. |
ae45a17 to
3060fe3
Compare
Extend trait's generic param name some other generics, tooExtend and FromIterator
3060fe3 to
9bca736
Compare
|
@bors r=JohnTitor |
|
@steffahn: 🔑 Insufficient privileges: not in review users |
|
Oh sorry I thought you had, @bors r+ |
Rollup of 8 pull requests Successful merges: - #161301 (libcore: expose volatile atomic operations) - #161379 (Use better generic type parameter names for `Extend` and `FromIterator`) - #161926 (borrowck: Restore alias rigidity from HIR typeck) - #161956 (Remove unused `perform_locally_with_next_solver`) - #162026 (Emit delayed bug instead of ICEing when `TypeOutlives` goal fails) - #162034 (Make the LLVM version mismatch ICE a fatal error) - #162037 (LLVM wrapper cleanups) - #162043 (_ an unused parameter)
Rollup merge of #161379 - steffahn:extend_trait_param_name, r=JohnTitor Use better generic type parameter names for `Extend` and `FromIterator` Change ```rs pub trait Extend<A> { fn extend<T: IntoIterator<Item = A>>(&mut self, iter: T); … } ``` to ```rs pub trait Extend<T> { fn extend<I: IntoIterator<Item = T>>(&mut self, iter: I); … } ``` to be more consistent with common use of `T` as the item type in containers, and their `Extend` impls; and use `I` as the name for the `IntoIterator` type. also do the basically the same thing for `FromIterator<A>`→`FromIterator<T>` as well cases of these parameters in trait impls for the abovementioned traits. These changes obviously don't change/break behavior of the traits and its users, but the improve the way the documentation of the traits renders (the improvement is the more sensible and more consistent names for the parameters). The only downside I'm aware of (besides that this is touching quite a few files..) is that this does affect some link anchor names in *some* cases. E.g. ```diff - impl<A, V: FromIterator<A>> FromIterator<Option<A>> for Option<V> { + impl<T, V: FromIterator<T>> FromIterator<Option<T>> for Option<V> { ``` would mean that [doc.rust-lang.org/core/iter/trait.FromIterator.html#impl-FromIterator<Option\<A>>-for-Option\<V>](https://doc.rust-lang.org/core/iter/trait.FromIterator.html#impl-FromIterator%3COption%3CA%3E%3E-for-Option%3CV%3E) no longer works, as it changes `…#impl-FromIterator<Option<A>>-for-Option<V>` to `…#impl-FromIterator<Option<T>>-for-Option<V>`
|
Note This PR was benchmarked as part of triage of its containing rollup: triage URL. Finished benchmarking commit (ea92208): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis perf run didn't have relevant results for this metric. Max RSS (memory usage)Results (secondary -0.9%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesThis perf run didn't have relevant results for this metric. Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: missing data |
Okay, first some context:
When talking about the
Extendtrait in a recent meeting, it got really confusing to talk about properties of theExtendtrait for a moment because the naming convention on the trait was so weird. It's defined asExtend<A>and has a methodextend<T>. But most generic implementations of it look likeimpl<T> Extend<T> for …iterator-type…, calling the item typeT.I think
Tis quite sensible for the item type here, so let's change it in the trait definition as well!That means that
<T>for theextendmethod must go away.. what is that anyway? Yes, it't theIntoIteratorparameter that you extend with. Why isn't that justIanyway? Let's choose that! It will be more consistent with existing things likeimpl<I: Iterator> IntoIterator for Ifor instance. Or e.g. all of these… 🦀I've then also noticed
FromIteratorhas the same problem of calling the itemAand usingT: IntoIterator. Let's change that, too.....and then of course, there are a “handful” of implementations of these methods that did just copy the signature for
fn extend<T>orfn from_iter<T>, so I guess I'm fixing these as well.TL;DR:
Change
to
to be more consistent with common use of
Tas the item type in containers, and theirExtendimpls; and useIas the name for theIntoIteratortype.(first commit)
then also do the basically the same thing for
FromIterator<A>→FromIterator<T>as well(second commit)
Finally, adjust
a fewa lot of cases of these parameters in trait impls for the abovementioned traits.(third commit)
These changes obviously don't change/break behavior of the traits and its users, but the improve the way the documentation of the traits renders (the improvement is the more sensible and more consistent names for the parameters). The only downside I'm aware of (besides that this is touching quite a few files.. though I don't feel quite as strongly about all those impls needing change as about the trait definition itself) is that this does affect some link anchor names in some cases. E.g.
would mean that doc.rust-lang.org/core/iter/trait.FromIterator.html#impl-FromIterator<Option<A>>-for-Option<V> no longer works, as it changes
…#impl-FromIterator<Option<A>>-for-Option<V>to…#impl-FromIterator<Option<T>>-for-Option<V>