-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
borrowck: Prevent partial reinitialization of uninitialized structures #22079
Closed
Closed
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
// Copyright 2014-2015 The Rust Project Developers. See the COPYRIGHT | ||
// file at the top-level directory of this distribution and at | ||
// http://rust-lang.org/COPYRIGHT. | ||
// | ||
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or | ||
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license | ||
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your | ||
// option. This file may not be copied, modified, or distributed | ||
// except according to those terms. | ||
|
||
struct Test; | ||
|
||
struct Test2 { | ||
b: Option<Test>, | ||
} | ||
|
||
struct Test3(Option<Test>); | ||
|
||
impl Drop for Test { | ||
fn drop(&mut self) { | ||
println!("dropping!"); | ||
} | ||
} | ||
|
||
impl Drop for Test2 { | ||
fn drop(&mut self) {} | ||
} | ||
|
||
impl Drop for Test3 { | ||
fn drop(&mut self) {} | ||
} | ||
|
||
fn stuff() { | ||
let mut t = Test2 { b: None }; | ||
let u = Test; | ||
drop(t); | ||
t.b = Some(u); | ||
//~^ ERROR partial reinitialization of uninitialized structure `t` | ||
|
||
let mut t = Test3(None); | ||
let u = Test; | ||
drop(t); | ||
t.0 = Some(u); | ||
//~^ ERROR partial reinitialization of uninitialized structure `t` | ||
} | ||
|
||
fn main() { | ||
stuff() | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
// Copyright 2014-2015 The Rust Project Developers. See the COPYRIGHT | ||
// file at the top-level directory of this distribution and at | ||
// http://rust-lang.org/COPYRIGHT. | ||
// | ||
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or | ||
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license | ||
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your | ||
// option. This file may not be copied, modified, or distributed | ||
// except according to those terms. | ||
|
||
struct Test { | ||
a: isize, | ||
b: Option<Box<Test>>, | ||
} | ||
|
||
impl Drop for Test { | ||
fn drop(&mut self) { | ||
println!("Dropping {}", self.a); | ||
} | ||
} | ||
|
||
fn stuff() { | ||
let mut t = Test { a: 1, b: None}; | ||
let mut u = Test { a: 2, b: Some(Box::new(t))}; | ||
t.b = Some(Box::new(u)); | ||
//~^ ERROR partial reinitialization of uninitialized structure `t` | ||
println!("done"); | ||
} | ||
|
||
fn main() { | ||
stuff(); | ||
println!("Hello, world!") | ||
} | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT | ||
// file at the top-level directory of this distribution and at | ||
// http://rust-lang.org/COPYRIGHT. | ||
// | ||
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or | ||
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license | ||
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your | ||
// option. This file may not be copied, modified, or distributed | ||
// except according to those terms. | ||
use std::mem; | ||
|
||
struct Test { f: usize } | ||
impl Drop for Test { | ||
fn drop(&mut self) {} | ||
} | ||
|
||
fn main() { | ||
let mut x = (Test { f: 2 }, Test { f: 4 }); | ||
mem::drop(x.0); | ||
x.0.f = 3; | ||
//~^ ERROR partial reinitialization of uninitialized structure `x.0` | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT | ||
// file at the top-level directory of this distribution and at | ||
// http://rust-lang.org/COPYRIGHT. | ||
// | ||
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or | ||
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license | ||
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your | ||
// option. This file may not be copied, modified, or distributed | ||
// except according to those terms. | ||
|
||
struct Test; | ||
|
||
struct Test2(Option<Test>); | ||
|
||
impl Drop for Test { | ||
fn drop(&mut self) { | ||
println!("dropping!"); | ||
} | ||
} | ||
|
||
impl Drop for Test2 { | ||
fn drop(&mut self) {} | ||
} | ||
|
||
fn stuff() { | ||
let mut x : (Test2, Test2); | ||
(x.0).0 = Some(Test); | ||
//~^ ERROR partial reinitialization of uninitialized structure `x.0` | ||
} | ||
|
||
fn main() { | ||
stuff() | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code is an improvement on the prior PR that you rebased, thanks!
However, I do wonder whether we could simplify further and just call
self.check_if_path_is_moved(id, span, use_kind, lp_base);
after doing thety::has_dtor
check, the same way that the other cases below do.Does that produce an error message that is too obscure? (Or, is there some technical reason we cannot do that?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Off the top of my head, it was possible but the error was a more generic 'use of moved value
foo.bar
'.I reasoned that the improved error message from the original changeset might be preferable but I'm willing to change it for simpler code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about this: Keep your code the way it is, but throw in a FIXME here and above
fn check_if_path_is_moved
noting that the body ofcheck_if_path_is_moved
is largely the same as the code you put into this function. That way, if someone does this same thing again, maybe we will then be inspired to refactor the code, parameterizing over what error message to report (since that is AFAICT the only thing that is changing between the two fragements of code).(That, or you could add in such a refactoring yourself, as a followup commit post squash.)
That way, we keep a nice precision in the error messages, and we slightly reduce the risk that this code fragment will be cut-and-pasted into other contexts without an attempt to pull it all together.