Skip to content

Conversation

@nham
Copy link
Contributor

@nham nham commented Jul 8, 2014

This adds a new method, append_all_move, to Vec. It's similar to push_all_move, but returns the current vector so it can be used again.

@lilyball
Copy link
Contributor

lilyball commented Jul 8, 2014

So far we have append() and append_one() that behave in this same way. I think that's sufficient precedent for an append_all_move(), although since append() is the equivalent of push_all() then it should probably be named append_move().

That said, I'm a bit leery about adding more stuff to Vec that's literally the existing stuff done in a functional style. What's next, a function version of reverse()? How about sort_by()?

Perhaps it's better to investigate an alternative approach. Once we have by-value closures, I would suggest something like

pub fn with(mut self, f: |&mut Vec<T>|) -> Vec<T> {
    f(&mut self);
    self
}

This will abstract away all the mutating methods so they can work in a functional style. So instead of return_a_vec().append(slice) you'd say return_a_vec().with(|v| v.push_all(slice)).

Without by-value closures, this would work for things like that push_all() above, but it won't work for e.g. your append_all_move(). But by-value closures are coming soon.

@nham
Copy link
Contributor Author

nham commented Jul 11, 2014

I've renamed it to append_move. Even with such a with function, I think this would be nice to have. The operation is common enough and return_a_vec().append_move(slice) looks nicer than return_a_vec().with(|v| v.push_all_move(slice))

@Kimundi
Copy link
Contributor

Kimundi commented Jul 11, 2014

Hm, a few random thoughts:

  • I'd very much like to see any append/push method that clones from a slice be removed and replaced with something that just takes a iterator. More composable, and less API duplication. But it might need something like a iterable trait to be ergonomical.
  • In the same way, I'd very much like to see all the append methods go away and be expressed through some composable way with push() (or with the iterator extend()). Again, the current way duplicates API for a different use case that exists for all types in Rust.
  • with might solve that and could be a generic trait impl for all types, but that might be overkill.

@alexcrichton
Copy link
Member

Closing due to in activity, but I think that we'll definitely shake out some conventions for methods like this during API triage.

bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 18, 2023
…-formatting, r=Veykril

On type format '(', by adding closing ')' automatically

If I understand right, `()` can surround pretty much the same `{}` can, so add another on type formatting pair for convenience: sometimes it's not that pleasant to write parenthesis in `Some(2).map(|i| (i, i+1))` cases and I would prefer r-a to do that for me.

One note: currently, https://github.com/rust-lang/rust-analyzer/blob/b06503b6ec98c9ed44698870cbf3302b8560b442/crates/rust-analyzer/src/handlers/request.rs#L357 fires always.
Should we remove the assertion entirely now, since apparently things work in release despite that check?
flip1995 added a commit to flip1995/rust that referenced this pull request Sep 4, 2025
)

`assign_op_pattern` can be used in a `const` context if the trait
definition as well as the implementation of the corresponding `Assign`
pattern is `const` as well.

The lint was temporarily deactivated in the compiler repository for all
`const` contexts to avoid false positives when operators were
potentially constified. This reenables it when appropriate now that the
repositories have been synced.

Closes rust-lang/rust-clippy#15285

changelog: [`assign_op_pattern`]: trigger in `const` contexts when
appropriate

r? @flip1995
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.

4 participants