Fix behaviour of divergence in while loop conditions #51049

varkor · 2018-05-24T22:26:52Z

This fixes 'a: while break 'a {}; being treated as diverging, by tracking break expressions in the same way as in loop expressions.

Fixes #50856.

r? @nikomatsakis

nikomatsakis · 2018-05-25T12:50:21Z

src/librustc_typeck/check/mod.rs

              };

-              self.with_breakable_ctxt(expr.id, ctxt, || {
+              let (ctxt, ()) = self.with_breakable_ctxt(expr.id, ctxt, || {
                  self.check_expr_has_type_or_error(&cond, tcx.types.bool);
                  let cond_diverging = self.diverges.get();


So what's confusing me here: there is somehow an implicit break should the condition be false (this is why may_break used to be considered true), and I can't see where that is accounted for.

For example, does this code accept:

let x: ! = { while false { return; }; };

( Do we have a test for this? It annoys me that this question is so hard to answer =) )

Traditionally we have considered while loops as always potentially breaking — but it seems like your change could allow us to handle cases like this one (for example)

'a: loop { let x: ! = { while (continue 'a) { }; }; }

For example, does this code accept:

No, this code fails to type-check both in the current version and after this PR. I'll add a test for it, though. The divergence of the body of the loop is completely ignored at the moment — it's just the condition that is taken into account. This PR ignores divergence in the condition too if break was involved.

Actually, even a case like:

let _: ! = 'a: while continue 'a {};

doesn't type-check at the moment, though intuitively it could do. (This isn't affected by this PR.) I think such a change would be more in scope for #51053, though. It's not breaking anything at the moment (as it's conservative).

nikomatsakis · 2018-05-25T12:50:42Z

src/librustc_typeck/check/mod.rs

@@ -3867,6 +3867,12 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                  self.diverges.set(cond_diverging);
              });

+              if ctxt.may_break {


It seems like not having this before was the obvious bug...

varkor · 2018-05-25T14:30:30Z

I've added a couple more tests. The actual error messages look a little weird due to #50085, but that's an unrelated problem.

rust-highfive · 2018-05-25T15:20:32Z

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

travis_time:start:test_ui
Check compiletest suite=ui mode=ui (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[00:47:07] 
[00:47:07] running 1464 tests
[00:47:11] .......................................................F.................................i..........
[00:47:20] ....................................................................................................
[00:47:24] ....................................................................................................
[00:47:27] ....................................................................................................
[00:47:30] ....................................................................................................
---
[00:47:54] .........................................................i..........................................
[00:47:58] ............................................................................ii......................
[00:48:04] ....................................................................................................
[00:48:10] ......................................................................................i.............
[00:48:13] ....iiiiiiiii...................................................
[00:48:13] 
[00:48:13] ---- [ui] ui/break-while-condition.rs stdout ----
[00:48:13] 
[00:48:13] 
[00:48:13] error: /checkout/src/test/ui/break-while-condition.rs:26: unexpected error: '26:13: 28:14: mismatched types [E0308]'
[00:48:13] 
[00:48:13] error: /checkout/src/test/ui/break-while-condition.rs:34: unexpected error: '34:13: 36:14: mismatched types [E0308]'
[00:48:13] 
[00:48:13] error: /checkout/src/test/ui/break-while-condition.rs:25: expected error not found: mismatched types
[00:48:13] 
[00:48:13] error: /checkout/src/test/ui/break-while-condition.rs:33: expected error not found: mismatched types
[00:48:13] 
[00:48:13] error: 2 unexpected errors found, 2 expected errors not found
[00:48:13] status: exit code: 101
[00:48:13] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/break-whil 
[00:48:13]     [ui] ui/break-while-condition.rs
[00:48:13] 
[00:48:13] test result: FAILED. 1447 passed; 1 failed; 16 ignored; 0 measured; 0 filtered out
[00:48:13] 
[00:48:13] 
[00:48:13] thread 'main' panicked at 'Some tests failed', tools/compiletest/src/main.rs:498:22
[00:48:13] 
[00:48:13] 
[00:48:13] 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-3.9/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" "3.9.1\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:48:13] 
[00:48:13] 
[00:48:13] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[00:48:13] Build completed unsuccessfully in 0:02:40
[00:48:13] Build completed unsuccessfully in 0:02:40
[00:48:13] Makefile:58: recipe for target 'check' failed
[00:48:13] make: *** [check] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:06748ff0
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)

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)

This fixes `'a: while break 'a {};` being treated as diverging, by tracking break expressions in the same way as in `loop` expressions.

nikomatsakis · 2018-05-25T20:35:33Z

src/librustc_typeck/check/mod.rs

@@ -3853,6 +3853,12 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                  self.diverges.set(cond_diverging);


Ah, this is the bit I was missing before (which ignores the divergence of the body). Very good.

nikomatsakis · 2018-05-25T20:35:56Z

@bors r+

bors · 2018-05-25T20:35:57Z

📌 Commit d5bf4de has been approved by nikomatsakis

@nikomatsakis

…omatsakis Fix behaviour of divergence in while loop conditions This fixes `'a: while break 'a {};` being treated as diverging, by tracking break expressions in the same way as in `loop` expressions. Fixes rust-lang#50856. r? @nikomatsakis

Rollup of 3 pull requests Successful merges: - #51049 (Fix behaviour of divergence in while loop conditions) - #51057 (make ui tests robust with respect to NLL) - #51092 ([master] Release notes for 1.26.1) Failed merges:

bors · 2018-05-26T21:26:29Z

⌛ Testing commit d5bf4de with merge 5015fa3...

bors · 2018-05-27T01:26:29Z

💥 Test timed out

rust-highfive assigned nikomatsakis May 24, 2018

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

nikomatsakis requested changes May 25, 2018

View reviewed changes

varkor added 2 commits May 25, 2018 16:53

Fix behaviour of divergence in while loop conditions

a38a1ce

This fixes `'a: while break 'a {};` being treated as diverging, by tracking break expressions in the same way as in `loop` expressions.

Add a test for returning inside a while loop

03d816f

varkor force-pushed the break-while-condition branch from 6003dd2 to d5bf4de Compare May 25, 2018 16:30

--bless the tests

d5bf4de

nikomatsakis approved these changes May 25, 2018

View reviewed changes

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 May 25, 2018

Mark-Simulacrum mentioned this pull request May 26, 2018

Rollup of 3 pull requests #51094

Merged

bors merged commit d5bf4de into rust-lang:master May 26, 2018

varkor deleted the break-while-condition branch May 26, 2018 21:50

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 27, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix behaviour of divergence in while loop conditions #51049

Fix behaviour of divergence in while loop conditions #51049

varkor commented May 24, 2018

nikomatsakis May 25, 2018

nikomatsakis May 25, 2018

varkor May 25, 2018 •

edited

Loading

nikomatsakis May 25, 2018

varkor commented May 25, 2018

rust-highfive commented May 25, 2018

nikomatsakis May 25, 2018

nikomatsakis commented May 25, 2018

bors commented May 25, 2018

bors commented May 26, 2018

bors commented May 27, 2018

		@@ -3853,6 +3853,12 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
		self.diverges.set(cond_diverging);

Fix behaviour of divergence in while loop conditions #51049

Fix behaviour of divergence in while loop conditions #51049

Conversation

varkor commented May 24, 2018

nikomatsakis May 25, 2018

Choose a reason for hiding this comment

nikomatsakis May 25, 2018

Choose a reason for hiding this comment

varkor May 25, 2018 • edited Loading

Choose a reason for hiding this comment

nikomatsakis May 25, 2018

Choose a reason for hiding this comment

varkor commented May 25, 2018

rust-highfive commented May 25, 2018

nikomatsakis May 25, 2018

Choose a reason for hiding this comment

nikomatsakis commented May 25, 2018

bors commented May 25, 2018

bors commented May 26, 2018

bors commented May 27, 2018

varkor May 25, 2018 •

edited

Loading