Skip to content
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

enforce user annotations in closure signatures #55229

Merged
merged 10 commits into from
Oct 23, 2018

Conversation

nikomatsakis
Copy link
Contributor

@nikomatsakis nikomatsakis commented Oct 20, 2018

Not quite ready yet but I'm opening anyway. Still have to finish running tests locally.

Fixes #54692
Fixes #54124

r? @matthewjasper

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 20, 2018
@rust-highfive
Copy link
Collaborator

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.

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)

@nikomatsakis
Copy link
Contributor Author

Hmm I seem to be hitting some local errors that I didn't see at first :(

@nikomatsakis nikomatsakis force-pushed the issue-54692-closure-signatures branch from 1cc3d84 to 8c89a70 Compare October 20, 2018 19:03
@nikomatsakis
Copy link
Contributor Author

OK, should work now. We'll see what Travis thinks.

@rust-highfive
Copy link
Collaborator

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:50:23] .................................................................................................... 2200/4660
[00:50:27] ....................................i............................................................... 2300/4660
[00:50:31] .................................................................................................... 2400/4660
[00:50:34] .................................................................................................... 2500/4660
[00:50:38] ...................................................iiiiiiiii........................................ 2600/4660
[00:50:44] .................................................................................................... 2800/4660
[00:50:48] .................................................................................................... 2900/4660
[00:50:51] .................................................................................i.................. 3000/4660
[00:50:54] .................................................................................................... 3100/4660
---
travis_time:start:test_codegen
Check compiletest suite=codegen mode=codegen (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[01:03:50] 
[01:03:50] running 111 tests
[01:03:53] i..ii...iii.......i...i.........i..iii...........i.....i.....ii...i.i.ii..............i...ii..ii.i.. 100/111
[01:03:53] ..iiii.....
[01:03:53] 
[01:03:53]  finished in 3.492
[01:03:53] travis_fold:end:test_codegen

---
travis_time:start:test_incremental
Check compiletest suite=incremental mode=incremental (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[01:03:55] 
[01:03:55] running 92 tests
hes/closure_expressions/closure_expressions.inc" "-Z" "incremental-verify-ich" "-Z" "incremental-queries" "--error-format" "json" "-Zui-testing" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/incremental/hashes/closure_expressions/a" "-Crpath" "-O" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-Z" "query-dep-graph" "-Zincremental-ignore-spans" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/incremental/hashes/closure_expressions/auxiliary"
[01:04:08] ------------------------------------------
[01:04:08] 
[01:04:08] ------------------------------------------
[01:04:08] stderr:
[01:04:08] stderr:
[01:04:08] ------------------------------------------
[01:04:08] {"message":"`TypeckTables(add_type_ascription_to_parameter)` should be clean but is not","code":null,"level":"error","spans":[{"file_name":"/checkout/src/test/incremental/hashes/closure_expressions.rs","byte_start":2588,"byte_end":2699,"line_start":100,"line_end":103,"column_start":1,"column_end":2,"is_primary":true,"text":[{"text":"pub fn add_type_ascription_to_parameter() {","highlight_start":1,"highlight_end":44},{"text":"    let closure = |x: u32| x + 1u32;","highlight_start":1,"highlight_end":37},{"text":"    let _: u32 = closure(1);","highlight_start":1,"highlight_end":29},{"text":"}","highlight_start":1,"highlight_end":2}],"label":null,"suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[],"rendered":"error: `TypeckTables(add_type_ascription_to_parameter)` should be clean but is not\n  --> /checkout/src/test/incremental/hashes/closure_expressions.rs:100:1\n   |\nLL | / pub fn add_type_ascription_to_parameter() {\nLL | |     let closure = |x: u32| x + 1u32;\nLL | |     let _: u32 = closure(1);\nLL | | }\n   | |_^\n\n"}
[01:04:08] {"message":"aborting due to previous error","code":null,"level":"error","spans":[],"children":[],"rendered":"error: aborting due to previous error\n\n"}
[01:04:08] ------------------------------------------
[01:04:08] 
[01:04:08] thread '[incremental] incremental/hashes/closure_expressions.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:3284:9
[01:04:08] note: Run with `RUST_BACKTRACE=1` for a backtrace.
---
[01:04:08] 
[01:04:08] thread 'main' panicked at 'Some tests failed', tools/compiletest/src/main.rs:503:22
[01:04:08] 
[01:04:08] 
[01:04: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/incremental" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/incremental" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "incremental" "--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"
[01:04:08] 
[01:04:08] 
[01:04:08] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[01:04:08] Build completed unsuccessfully in 0:18:29
[01:04:08] Build completed unsuccessfully in 0:18:29
[01:04:08] Makefile:58: recipe for target 'check' failed
[01:04:08] make: *** [check] Error 1
75788 ./obj/build/x86_64-unknown-linux-gnu/stage0-sysroot/lib/rustlib/x86_64-unknown-linux-gnu/lib
72532 ./src/llvm/lib
70532 ./obj/build/x86_64-unknown-linux-gnu/native
70300 ./obj/build/x86_64-unknown-linux-gnu/native/jemalloc
---
travis_time:end:0f82f73d:start=1540066258740294292,finish=1540066258747934118,duration=7639826
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:25dd73bc
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashl

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)

Copy link
Contributor

@matthewjasper matthewjasper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing that I'm not sure about. r=me with that addressed

@bors
Copy link
Contributor

bors commented Oct 21, 2018

☔ The latest upstream changes (presumably #52984) made this pull request unmergeable. Please resolve the merge conflicts.

@nikomatsakis nikomatsakis force-pushed the issue-54692-closure-signatures branch from 979fa31 to 2921fba Compare October 22, 2018 14:11
@nikomatsakis
Copy link
Contributor Author

@bors r=MatthewJasper p=1

Giving p=1 because this is an RC2 blocker.

In particular, after the first for a given region variable. This
suppresses a lot of duplicate errors.
@nikomatsakis nikomatsakis force-pushed the issue-54692-closure-signatures branch from 8bcb79b to 64b5599 Compare October 22, 2018 15:53
@nikomatsakis
Copy link
Contributor Author

@bors p=1

@nikomatsakis
Copy link
Contributor Author

@bors r=MatthewJasper

@matthewjasper
Copy link
Contributor

matthewjasper commented Oct 22, 2018

I think bors is back now. maybe not @bors r=matthewjasper

@bors
Copy link
Contributor

bors commented Oct 22, 2018

📌 Commit 64b5599 has been approved by MatthewJasper

@bors 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 22, 2018
@bors
Copy link
Contributor

bors commented Oct 22, 2018

📌 Commit 64b5599 has been approved by MatthewJasper

@bors
Copy link
Contributor

bors commented Oct 22, 2018

📌 Commit 64b5599 has been approved by matthewjasper

1 similar comment
@bors
Copy link
Contributor

bors commented Oct 22, 2018

📌 Commit 64b5599 has been approved by matthewjasper

@bors
Copy link
Contributor

bors commented Oct 22, 2018

⌛ Testing commit 64b5599 with merge 5639651ab8552188c4bc360761a82c602cefc114...

@bors 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 22, 2018
@bors
Copy link
Contributor

bors commented Oct 22, 2018

⌛ Testing commit 64b5599 with merge 7fa5c7fcf77cf759bb0114afeac5b47ba3397b7f...

@bors
Copy link
Contributor

bors commented Oct 23, 2018

💔 Test failed - status-appveyor

@bors 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 Oct 23, 2018
@matthewjasper
Copy link
Contributor

@bors retry

@bors 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 23, 2018
@bors
Copy link
Contributor

bors commented Oct 23, 2018

⌛ Testing commit 64b5599 with merge 1344e6f8fd074c9856840b6e5c14b80292f11018...

@bors
Copy link
Contributor

bors commented Oct 23, 2018

💔 Test failed - status-appveyor

@bors 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 Oct 23, 2018
@nikomatsakis
Copy link
Contributor Author

Is this the reason the run failed?

The state of "rls" has changed from "test-pass" to "test-fail"
The state of "rls" has regressed from "test-pass" to "test-fail"

If so...interesting and unexpected. =)

This fixes `issue-28848.rs` -- it also handles another case that the
AST region checker gets wrong (`wf-self-type.rs`).  I don't actually
think that this is the *right way* to be enforcing this constraint --
I think we should probably do it more generally, perhaps by editing
`predicates_of` for the impl itself. The chalk-style implied bounds
setup ought to fix this.
@rust-highfive

This comment has been minimized.

@nikomatsakis
Copy link
Contributor Author

@bors r=MatthewJasper

Let's see if this is spurious or not.

@bors
Copy link
Contributor

bors commented Oct 23, 2018

📌 Commit 4394c83 has been approved by MatthewJasper

@bors 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 23, 2018
@nikomatsakis
Copy link
Contributor Author

I can't reproduce locally.

@bors
Copy link
Contributor

bors commented Oct 23, 2018

⌛ Testing commit 4394c83 with merge f99911a...

bors added a commit that referenced this pull request Oct 23, 2018
…=MatthewJasper

enforce user annotations in closure signatures

Not *quite* ready yet but I'm opening anyway. Still have to finish running tests locally.

Fixes #54692
Fixes #54124

r? @matthewjasper
@bors
Copy link
Contributor

bors commented Oct 23, 2018

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants