Cleanup nll #55151

ljedrz · 2018-10-17T14:59:43Z

improve allocations
improve format! calls
improve common patterns

rust-highfive · 2018-10-17T14:59:46Z

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

src/librustc_mir/borrow_check/nll/constraint_generation.rs

Mark-Simulacrum · 2018-10-17T16:36:39Z

src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs

@@ -254,7 +255,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
        &self,
        borrow_location: Location,
    ) -> bool {
-        let mut visited_locations = Vec::new();
+        let mut visited_locations = FxHashSet::default();


Hm, I'd rather make this change in a separate PR which would be benchmarked to make sure this is actually an optimization (it looks like it would be but I'm not sure).

estebank

LGTM, after addressing @Mark-Simulacrum's comments.

ljedrz · 2018-10-17T18:24:10Z

@estebank comments addressed.

rust-highfive · 2018-10-17T19:20:26Z

The job x86_64-gnu-llvm-5.0 of 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.

[00:53:55] .................................................................................................... 4200/4622
[00:53:58] ......i............................................................................................. 4300/4622
[00:54:02] .................................................................................................... 4400/4622
[00:54:05] .................................................................................................... 4500/4622
= Foo { x: None };\nx.x = Some(&y);\n```\n"},"level":"error","spans":[{"file_name":"/checkout/src/test/ui/nll/polonius-smoke-test.rs","byte_start":199,"byte_end":201,"line_start":7,"line_end":7,"column_start":5,"column_end":7,"is_primary":true,"text":[{"text":"    &x //~ ERROR","highlight_start":5,"highlight_end":7}],"label":"borrowed value does not live long enough","suggested_replacement":null,"suggestion_applicability":null,"expansion":null},{"file_name":"/checkout/src/test/ui/nll/polonius-smoke-test.rs","byte_start":212,"byte_end":213,"line_start":8,"line_end":8,"column_start":1,"column_end":2,"is_primary":false,"text":[{"text":"}","highlight_start":1,"highlight_end":2}],"label":"`x` dropped here while still borrowed","suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[{"message":"borrowed value must be valid for the static lifetime...","code":null,"level":"note","spans":[],"children":[],"rendered":null}],"rendered":"error[E0597]: `x` does not live long enough\n  --> /checkout/src/test/ui/nll/polonius-smoke-test.rs:7:5\n   |\nLL |     &x //~ ERROR\n   |     ^^ borrowed value does not live long enough\nLL | }\n   | - `x` dropped here while still borrowed\n   |\n   = note: borrowed value must be valid for the static lifetime...\n\n"}
[00:54:08] {"message":"cannot use `x` because it was mutably borrowed","code":{"code":"E0503","explanation":"\nA value was used after it was mutably borrowed.\n\nExample of erroneous code:\n\n```compile_fail,E0503\nfn main() {\n    let mut value = 3;\n    // Create a mutable borrow of `value`. This borrow\n    // lives until the end of this function.\n  to `_ref_to_val`.\nTo fix that you can do few different things:\n\n* Try to avoid moving the variable.\n* Release borrow before move.\n* Implement the `Copy` trait on the type.\n\nExamples:\n\n```\nstruct Value {}\n\nfn eat(val: &Value) {}\n\nfn main() {\n    let x = Value{};\n    {\n        let _ref_to_val: &Value = &x;\n        eat(&x); // pass by reference, if it's possible\n    }\n}\n```\n\nOr:\n\n```\nstruct Value {}\n\nfn eat(val: Value) {}\n\nfn main() {\n    let x = Value{};\n    {\n        let _ref_to_val: &Value = &x;\n    }\n    eat(x); // release borrow and then move it.\n}\n```\n\nOr:\n\n```\n#[derive(Clone, Copy)] // implement Copy trait\nstruct Value {}\n\nfn eat(val: Value) {}\n\nfn main() {\n    let x = Value{};\n    {\n        let _ref_to_val: &Value = &x;\n        eat(x); // it will be copied here.\n    }\n}\n```\n\nYou can find more information about borrowing in the rust-book:\nhttp://doc.rust-lang.org/stable/book/references-and-borrowing.html\n"},"level":"error","spans":[{"file_name":"/checkout/src/test/ui/nll/polonius-smoke-test.rs","byte_start":385,"byte_end":392,"line_start":18,"line_end":18,"column_start":13,"column_end":20,"is_primary":false,"text":[{"text":"    let y = &mut *x;","highlight_start":13,"highlight_end":20}],"label":"borrow of `*x` occurs here","suggested_replacement":null,"suggestion_applicability":null,"expansion":null},{"file_name":"/checkout/src/test/ui/nll/polonius-smoke-test.rs","byte_start":406,"byte_end":407,"line_start":19,"line_end":19,"column_start":13,"column_end":14,"is_primary":true,"text":[{"text":"    let z = x; //~ ERROR","highlight_start":13,"highlight_end":14}],"labelr a backtrace.
[00:54:08] 
[00:54:08] failures:
[00:54:08]     [ui] ui/nll/polonius-smoke-test.rs
[00:54:08] 
[00:54:08] 
[00:54:08] test result: FAILED. 4601 passed; 1 failed; 20 ignored; 0 measured; 0 filtered out
[00:54:08] 
[00:54:08] thread 'main' panicked at 'Some tests failed', tools/compiletest/src/main.rs:501:22
[00:54:08] 
[00:54:08] 
[00:54:08] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/ui" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "ui" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-5.0/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Zunstable-options " "--target-rustcflags" "-Crpath -O -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "5.0.0\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
[00:54:08] 
[00:54:08] 
[0
travis_time:end:220c9c90:start=1539800773144546658,finish=1539804021692434916,duration=3248547888258

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)

estebank · 2018-10-17T20:34:33Z

[00:54:08] ---- [ui] ui/nll/polonius-smoke-test.rs stdout ----
[00:54:08] diff of stderr:
[00:54:08] 
[00:54:08] 28	LL |     y
[00:54:08] 29	   |     - borrow later used here
[00:54:08] 30	
[00:54:08] -	error[E0505]: cannot move out of `s` because it is borrowed
[00:54:08] -	  --> $DIR/polonius-smoke-test.rs:43:5
[00:54:08] -	   |
[00:54:08] -	LL |     let r = &mut *s;
[00:54:08] -	   |             ------- borrow of `*s` occurs here
[00:54:08] -	LL |     let tmp = foo(&r);
[00:54:08] -	LL |     s; //~ ERROR
[00:54:08] -	   |     ^ move out of `s` occurs here
[00:54:08] -	LL |     tmp;
[00:54:08] -	   |     --- borrow later used here
[00:54:08] -	
[00:54:08] -	error: aborting due to 4 previous errors
[00:54:08] +	error: aborting due to 3 previous errors
[00:54:08] 43	
[00:54:08] 44	Some errors occurred: E0503, E0505, E0597.
[00:54:08] 45	For more information about an error, try `rustc --explain E0503`.

ljedrz · 2018-10-17T20:37:37Z

Yes, but how? I only changed size_hint to count (plus the related iterator stuff).

Mark-Simulacrum · 2018-10-17T20:46:32Z

Count will exhaust the iterator (by_ref doesn't clone it) regardless of whether it's implemented to be more efficient, I believe.

src/librustc_mir/borrow_check/nll/constraint_generation.rs

estebank · 2018-10-17T23:49:30Z

@bors r+ rollup

bors · 2018-10-17T23:49:31Z

📌 Commit ffecbc5 has been approved by estebank

Cleanup nll - improve allocations - improve `format!` calls - improve common patterns

Rollup of 18 pull requests Successful merges: - #54646 (improve documentation on std::thread::sleep) - #54933 (Cleanup the rest of codegen_llvm) - #54964 (Run both lldb and gdb tests) - #55016 (Deduplicate some code and compile-time values around vtables) - #55031 (Improve verify_llvm_ir config option) - #55050 (doc std::fmt: the Python inspiration is already mentioned in precedin…) - #55077 (rustdoc: Use dyn keyword when rendering dynamic traits) - #55080 (Detect if access to localStorage is forbidden by the user's browser) - #55090 (regression test for move out of borrow via pattern) - #55102 (resolve: Do not skip extern prelude during speculative resolution) - #55104 (Add test for #34229) - #55111 ([Rustc Book] Explain --cfg's arguments) - #55122 (Cleanup mir/borrowck) - #55127 (Remove HybridBitSet::dummy) - #55128 (Fix LLVMRustInlineAsmVerify return type mismatch) - #55142 (miri: layout should not affect CTFE checks (outside of validation)) - #55151 (Cleanup nll) - #55161 ([librustdoc] Disable spellcheck for search field)

rust-highfive assigned estebank Oct 17, 2018

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

Mark-Simulacrum reviewed Oct 17, 2018

View reviewed changes

estebank approved these changes Oct 17, 2018

View reviewed changes

ljedrz force-pushed the cleanup_nll branch from 5d70139 to e6911bb Compare October 17, 2018 18:23

estebank reviewed Oct 17, 2018

View reviewed changes

src/librustc_mir/borrow_check/nll/constraint_generation.rs Show resolved Hide resolved

ljedrz added 3 commits October 17, 2018 22:57

nll: improve allocations

e62ce98

nll: improve format operations

2bda0c1

nll: improve common patterns

ffecbc5

ljedrz force-pushed the cleanup_nll branch from e6911bb to ffecbc5 Compare October 17, 2018 20:58

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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 17, 2018

kennytm mentioned this pull request Oct 18, 2018

Rollup of 18 pull requests #55171

Merged

kennytm added a commit to kennytm/rust that referenced this pull request Oct 18, 2018

Rollup merge of rust-lang#55151 - ljedrz:cleanup_nll, r=estebank

919f434

Cleanup nll - improve allocations - improve `format!` calls - improve common patterns

bors merged commit ffecbc5 into rust-lang:master Oct 18, 2018

ljedrz deleted the cleanup_nll branch October 18, 2018 10:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cleanup nll #55151

Cleanup nll #55151

ljedrz commented Oct 17, 2018 •

edited

Loading

rust-highfive commented Oct 17, 2018

Mark-Simulacrum Oct 17, 2018

estebank left a comment

ljedrz commented Oct 17, 2018

rust-highfive commented Oct 17, 2018

estebank commented Oct 17, 2018

ljedrz commented Oct 17, 2018 •

edited

Loading

Mark-Simulacrum commented Oct 17, 2018

estebank commented Oct 17, 2018

bors commented Oct 17, 2018

Cleanup nll #55151

Cleanup nll #55151

Conversation

ljedrz commented Oct 17, 2018 • edited Loading

rust-highfive commented Oct 17, 2018

Mark-Simulacrum Oct 17, 2018

Choose a reason for hiding this comment

estebank left a comment

Choose a reason for hiding this comment

ljedrz commented Oct 17, 2018

rust-highfive commented Oct 17, 2018

estebank commented Oct 17, 2018

ljedrz commented Oct 17, 2018 • edited Loading

Mark-Simulacrum commented Oct 17, 2018

estebank commented Oct 17, 2018

bors commented Oct 17, 2018

ljedrz commented Oct 17, 2018 •

edited

Loading

ljedrz commented Oct 17, 2018 •

edited

Loading