reject partial init and reinit of uninitialized data #54941

pnkfelix · 2018-10-09T21:33:23Z

Reject partial initialization of uninitialized structured types (i.e. structs and tuples) and also reject partial reinitialization of such types.

Fix #54986

Fix #54499

cc #21232

rust-highfive · 2018-10-09T21:33:26Z

r? @varkor

(rust_highfive has picked a reviewer for you, use r? to override)

pnkfelix · 2018-10-09T22:53:35Z

This PR is almost certainly subsumed by the work of @spastorino on PR #54621

I have put this PR up here mostly as a point of comparison (e.g. to double-check the tests and to compare the choice of diagnostic wording under the two approaches).

rust-highfive · 2018-10-12T00:10:55Z

Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

/usr/local/lib/python2.7/dist-packages/pip/_vendor/requests/packages/urllib3/util/ssl_.py:122: InsecurePlatformWarning: A true SSLContext object is not available. This prevents urllib3 from configuring SSL appropriately and may cause certain SSL connections to fail. You can upgrade to a newer version of Python to solve this. For more information, see https://urllib3.readthedocs.io/en/latest/security.html#insecureplatformwarning.
  InsecurePlatformWarning
/usr/local/lib/python2.7/dist-packages/pip/_vendor/requests/packages/urllib3/util/ssl_.py:122: InsecurePlatformWarning: A true SSLContext object is not available. This prevents urllib3 from configuring SSL appropriately and may cause certain SSL connections to fail. You can upgrade to a newer version of Python to solve this. For more information, see https://urllib3.readthedocs.io/en/latest/security.html#insecureplatformwarning.
  InsecurePlatformWarning
  Downloading https://files.pythonhosted.org/packages/ac/12/38a00649e6d56d80aeee31dd42343d0795da92a7862b1af7b275f8979613/awscli-1.16.32-py2.py3-none-any.whl (1.4MB)
    0% |▎                               | 10kB 11.7MB/s eta 0:00:01
    1% |▌                               | 20kB 1.9MB/s eta 0:00:01
    2% |▊                               | 30kB 2.2MB/s eta 0:00:01
    3% |█                               | 40kB 2.0MB/s eta 0:00:01
---
curl: (7) Failed to connect to s3-us-west-1.amazonaws.com port 443: Connection timed out
The command "curl -fo /home/travis/stamp https://s3-us-west-1.amazonaws.com/rust-lang-ci2/rust-ci-mirror/2017-03-17-stamp-x86_64-unknown-linux-musl" failed 3 times.
travis_time:end:102d9668:start=1539301314540595134,finish=1539301699395577127,duration=384854981993

The command "case "$TRAVIS_OS_NAME" in linux) travis_retry curl -fo $HOME/stamp https://s3-us-west-1.amazonaws.com/rust-lang-ci2/rust-ci-mirror/2017-03-17-stamp-x86_64-unknown-linux-musl && chmod +x $HOME/stamp && export PATH=$PATH:$HOME ;; osx) if [[ "$RUST_CHECK_TARGET" == dist ]]; then travis_retry brew update && travis_retry brew install xz && travis_retry brew install swig; fi && travis_retry curl -fo /usr/local/bin/sccache https://s3-us-west-1.amazonaws.com/rust-lang-ci2/rust-ci-mirror/2018-04-02-sccache-x86_64-apple-darwin && chmod +x /usr/local/bin/sccache && travis_retry curl -fo /usr/local/bin/stamp https://s3-us-west-1.amazonaws.com/rust-lang-ci2/rust-ci-mirror/2017-03-17-stamp-x86_64-apple-darwin && chmod +x /usr/local/bin/stamp && travis_retry curl -f http://releases.llvm.org/6.0.0/clang+llvm-6.0.0-x86_64-apple-darwin.tar.xz | tar xJf - && export CC=`pwd`/clang+llvm-6.0.0-x86_64-apple-darwin/bin/clang && export CXX=`pwd`/clang+llvm-6.0.0-x86_64-apple-darwin/bin/clang++ && export AR=ar ;; esac" failed and exited with 7 during .
Your build has been stopped.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

pnkfelix · 2018-10-15T13:38:50Z

(I am still working on this.

I need to extend it to cover #54499 (even if that means "just" adding tests)

…clarify why we need the former.

…#54986. Treat attempt to partially intialize local `l` as uses of a `mut` in `let mut l;`, to fix rust-lang#54499.

…current point in flow.

…aks. (This makes it a little easier to add instrumentation of the entry and exit by adding `debug!` at the beginning and end, though note that the function body *does* use the `?` operator...)

… uninitialized struct. Under the semantics of rust-lang#54986 (our short term plan), the partial initialization itself will signal an error. We don't need to add noise to the output by also complaining about `mut`. (In particular, the user may well revise their code in a way that does not require `mut`.)

…e-fail under rust-lang#54986.

pnkfelix · 2018-10-16T15:12:40Z

this is now ready for review

pnkfelix · 2018-10-16T15:13:00Z

r? @nikomatsakis

nikomatsakis

r=me with update

nikomatsakis · 2018-10-16T15:42:53Z

src/librustc_mir/borrow_check/mod.rs

-        let init_indices = &self.move_data.init_path_map[mpi];
-        let first_init_index = init_indices.iter().find(|&ii| flow_state.ever_inits.contains(*ii));
-        if let Some(&init_index) = first_init_index {
+        if let Some(init_index) = self.is_local_ever_initialized(local, flow_state) {


nice place for a helper fn 💯

src/test/ui/nll/issue-21232-partial-init-and-erroneous-use.rs

pnkfelix · 2018-10-17T00:26:34Z

@bors r=nikomatsakis

bors · 2018-10-17T00:26:35Z

📌 Commit 258f9cab12c08a3d43661a48b947b3e4a61ffb58 has been approved by nikomatsakis

pnkfelix · 2018-10-17T00:27:50Z

@bors r-

Oops: I added the //~ ERROR annotations to a bunch of my new tests, but not the particular one where @nikomatsakis asked for it...

pnkfelix · 2018-10-17T00:34:12Z

@bors r=nikomatsakis

bors · 2018-10-17T00:34:12Z

📌 Commit 233fdb4 has been approved by nikomatsakis

pnkfelix · 2018-10-17T00:40:55Z

@bors r-

@nikomatsakis had warned about not breaking unions here: #21232 (comment)

but I'm worried that I may have broken that exact case.

pnkfelix · 2018-10-17T00:43:23Z

@bors r=nikomatsakis

I double-checked: the code that @nikomatsakis wrote was actually (I think) referring to the hypothetical world of #54987. This PR is solely about #54986. As far as I can tell, this change does not break union.

bors · 2018-10-17T00:43:24Z

📌 Commit 233fdb4 has been approved by nikomatsakis

bors · 2018-10-17T09:13:58Z

⌛ Testing commit 233fdb4 with merge cbbd70d...

…nikomatsakis reject partial init and reinit of uninitialized data Reject partial initialization of uninitialized structured types (i.e. structs and tuples) and also reject partial *reinitialization* of such types. Fix #54986 Fix #54499 cc #21232

bors · 2018-10-17T11:47:47Z

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing cbbd70d to master...

rust-highfive assigned varkor Oct 9, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 9, 2018

pnkfelix added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 9, 2018

pnkfelix mentioned this pull request Oct 9, 2018

Fix variable does not need to be mutable warning #54621

Closed

pnkfelix changed the title ~~[WIP] Issue 21232 reject partial reinit~~ [WIP] Issue 54986 reject partial reinit Oct 15, 2018

pnkfelix changed the title ~~[WIP] Issue 54986 reject partial reinit~~ [WIP] reject partial init and reinit of uninitialized adts+tuples Oct 15, 2018

pnkfelix changed the title ~~[WIP] reject partial init and reinit of uninitialized adts+tuples~~ [WIP] reject partial init and reinit of uninitialized data Oct 15, 2018

pnkfelix mentioned this pull request Oct 16, 2018

NLL: Reject partial init of uninitialized record (struct or tuple) #54986

Closed

pnkfelix added 8 commits October 16, 2018 16:57

Add Place::base_local method and improve doc for Place::local to …

44a2f68

…clarify why we need the former.

Make us error consistently in issue rust-lang#21232, to fix rust-lang…

bc61541

…#54986. Treat attempt to partially intialize local `l` as uses of a `mut` in `let mut l;`, to fix rust-lang#54499.

Add helper method to determine if local had ever been initialized at …

d0416cc

…current point in flow.

Cleanup fn is_mutable by removing some unnecessary control-flow bre…

57c4678

…aks. (This makes it a little easier to add instrumentation of the entry and exit by adding `debug!` at the beginning and end, though note that the function body *does* use the `?` operator...)

Add ignore-compare-mode-nll to some run-pass tests that become compil…

4229739

…e-fail under rust-lang#54986.

updates to existing ui/borrowck tests.

b10beb3

updates to expected output for other ui tests.

5b74843

pnkfelix force-pushed the issue-21232-reject-partial-reinit branch from d52c071 to 936d9c0 Compare October 16, 2018 15:12

pnkfelix changed the title ~~[WIP] reject partial init and reinit of uninitialized data~~ reject partial init and reinit of uninitialized data Oct 16, 2018

rust-highfive assigned nikomatsakis and unassigned varkor Oct 16, 2018

nikomatsakis requested changes Oct 16, 2018

View reviewed changes

pnkfelix force-pushed the issue-21232-reject-partial-reinit branch from 936d9c0 to 258f9ca Compare October 17, 2018 00:21

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 17, 2018

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 17, 2018

Some new tests I added.

233fdb4

pnkfelix force-pushed the issue-21232-reject-partial-reinit branch from 258f9ca to 233fdb4 Compare October 17, 2018 00:33

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 17, 2018

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 17, 2018

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 17, 2018

bors merged commit 233fdb4 into rust-lang:master Oct 17, 2018

This was referenced Oct 17, 2018

Prefer unwrap_or_else to unwrap_or in case of function calls/allocations #55014

Merged

Cleanup mir/borrowck #55122

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

reject partial init and reinit of uninitialized data #54941

reject partial init and reinit of uninitialized data #54941

pnkfelix commented Oct 9, 2018 •

edited

Loading

rust-highfive commented Oct 9, 2018

pnkfelix commented Oct 9, 2018

rust-highfive commented Oct 12, 2018

pnkfelix commented Oct 15, 2018 •

edited

Loading

pnkfelix commented Oct 16, 2018

pnkfelix commented Oct 16, 2018

nikomatsakis left a comment

nikomatsakis Oct 16, 2018

pnkfelix commented Oct 17, 2018

bors commented Oct 17, 2018

pnkfelix commented Oct 17, 2018

pnkfelix commented Oct 17, 2018

bors commented Oct 17, 2018

pnkfelix commented Oct 17, 2018

pnkfelix commented Oct 17, 2018

bors commented Oct 17, 2018

bors commented Oct 17, 2018

bors commented Oct 17, 2018

reject partial init and reinit of uninitialized data #54941

reject partial init and reinit of uninitialized data #54941

Conversation

pnkfelix commented Oct 9, 2018 • edited Loading

rust-highfive commented Oct 9, 2018

pnkfelix commented Oct 9, 2018

rust-highfive commented Oct 12, 2018

pnkfelix commented Oct 15, 2018 • edited Loading

pnkfelix commented Oct 16, 2018

pnkfelix commented Oct 16, 2018

nikomatsakis left a comment

Choose a reason for hiding this comment

nikomatsakis Oct 16, 2018

Choose a reason for hiding this comment

pnkfelix commented Oct 17, 2018

bors commented Oct 17, 2018

pnkfelix commented Oct 17, 2018

pnkfelix commented Oct 17, 2018

bors commented Oct 17, 2018

pnkfelix commented Oct 17, 2018

pnkfelix commented Oct 17, 2018

bors commented Oct 17, 2018

bors commented Oct 17, 2018

bors commented Oct 17, 2018

pnkfelix commented Oct 9, 2018 •

edited

Loading

pnkfelix commented Oct 15, 2018 •

edited

Loading