fn push_fn_err(s :&str)->String{
s.to_string().push_str("new_chunk");
s.to_string()
}
fn push_str_expect(s:&str)->String{
let mut output = s.to_string();
output.push_str("new_chunk");
output.to_string()
}
fn main() {
assert_eq!(push_fn_err(""),push_str_expect("")) ;
}
I don't see an error from clippy.
The assertion of course fails, because in push_fn_err the line s.to_string().push_str("new_chunk"); creates a temporary owned String which is immediately dropped.
Then a new owned String with the same contents of s is returned, which is not equal to the correctly modified string in push_str_expect.
s.to_string().push_str("new_chunk");
^ ^
| +---- Modifies the temporary owned String
+---- Creates a new owned String from s
s.to_string()
^
+---- Creates yet another new owned String from s
In push_str_expect the second call to to_string() is superfluous:
fn push_str_expect(s:&str)->String{
let mut output = s.to_string(); // Creates a new owned String
output.push_str("new_chunk"); // Modifies the owned string
output // Returns the owned String
}
1 Like
I see. This isn't an error from clippy. your answer is very accurate
But clippy should diagnose this ,shouldn't it?
for instance
fn main(){
let a= 5; // clippy warning is needing use _ for this statement because a haven't used
/// and with
5; // clippy give warning too
}
It would have to be type and method specific to not have false positives. In some cases, creating a value. calling a &mut method on it, and doing nothing else with the value can still have meaningful results.
(On mobile so I won't code it up, but say, get StdOut and write to it.)
String is common enough it may still be worth it for cases like this.
1 Like
I think Rust’s “mutability story” would be notably better if functions were in fact annotated to make this distinction, between “only modifies the T this &mut Self owns”, and “has side effects which happen to require exclusive access to this Self”. If these annotations exist, then the compiler could reliably give a warning (or error) whenever a method of the first sort is applied to a temporary or a variable that is not read further, in the same way that it warns you if you assign a value to a variable and then never read that variable.
(P.S. I don't have an idea for how such a modification could reasonably be incorporated, though. Attributes on the function or arguments are not well-defined in complex cases. A purely advisory, coercible-away flag on the &mut type, perhaps?)
3 Likes
The could work by allowing the #[must_use] attribute on &mut parameters for things that must be used after being modified.
4 Likes
Interesting.
must_use wouldn't suggest for unit type. So This is not useful in push_str case. It doesn't work with some thing like Something::new().some_function();
Am I right?
I believe they meant that it would be possible to introduce a compiler feature to allow the attribute on the function parameter &mut self to indicate that the mutated value must not be discarded after the call.
2 Likes
similar with your answer before.
because something(??) available in memory. we allowed to accept and modify it by a function( for the thing called side effect)
because of this, it can’t consider as a error.
so after all , I pleasure. Indicating features might created or not but it’s avoidable . (it isn’t supposed an unexpected behavior )
If some method has side effects, its self parameter will not be annotated.
2 Likes
No, I have checked it. parameter of write have a self. but if you impl about annotated self (if fn is side effect ) by attribute,.. maybe it can be hard
Here is how this feature would work:
impl String {
fn push_str(#[must_use] &mut self, s: &str) { ... }
}
warning: `String` unused after `push_str`
| s.to_string().push_str("new_chunk");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
3 Likes
I just remembered that there is already a draft RFC for this: https://github.com/rust-lang/rfcs/pull/3773
3 Likes
This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.