do not overwrite child def-id in place but rather remove/insert #52546

nikomatsakis · 2018-07-19T17:26:37Z

When inserting a node N into the tree of impls, we sometimes find than an existing node C should be replaced with N. We used to overwrite C in place with the new def-id N -- but since the lists of def-ids are separated by simplified type, that could lead to N being inserted in the wrong place. This meant we might miss conflicts. We are now not trying to be so smart -- we remove C and then add N later.

Fixes #52050

r? @aturon -- do you still remember this code at all? :)

rust-highfive · 2018-07-19T18:22:42Z

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:51:48] ....................................................................................................
[00:51:52] ....................................................................................................
[00:51:54] ....................................................................................................
[00:51:57] .......................................i............................................................
[00:52:00] .......F............................................................................................
[00:52:06] ....................................................................................................
[00:52:08] ......................................................................................
[00:52:08] failures:
[00:52:08] 
[00:52:08] 
[00:52:08] ---- [compile-fail] compile-fail/specialization/issue-52050.rs stdout ----
[00:52:08] 
[00:52:08] error: /checkout/src/test/compile-fail/specialization/issue-52050.rs:38: unexpected error: '38:1: 38:30: conflicting implementations of trait `IntoPyDictPointer` for type `()`: [E0119]'
[00:52:08] error: /checkout/src/test/compile-fail/specialization/issue-52050.rs:32: expected error not found: conflicting implementations
[00:52:08] 
[00:52:08] error: 1 unexpected errors found, 1 expected errors not found
[00:52:08] status: exit code: 1
[00:52:08] status: exit code: 1
[00 'main' panicked at 'Some tests failed', tools/compiletest/src/main.rs:498:22
[00:52:08] 
[00:52:08] 
[00:52: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/compile-fail" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/compile-fail" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "compile-fail" "--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:52:08] 
[00:52:08] 
[00:52:08] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[00:52:08] Build completed unsuccessfully in 0:09:18
[00:52:08] Build completed unsuccessfully in 0:09:18
[00:52:08] Makefile:58: recipe for target 'check' failed
[00:52:08] make: *** [check] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:232876b0
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
---
travis_time:end:2373d156:start=1532024555954942966,finish=1532024555963555883,duration=8612917
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:014dac4d
$ 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:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:1a626fb6
travis_time:start:1a626fb6
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.

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)

When inserting a node N into the tree of impls, we sometimes find than an existing node C should be replaced with N. We used to overwrite C in place with the new def-id N -- but since the lists of def-ids are separated by simplified type, that could lead to N being inserted in the wrong place. This meant we might miss conflicts. We are now not trying to be so smart -- we remove C and then add N later.

nikomatsakis · 2018-07-23T03:22:49Z

r? @eddyb

pnkfelix · 2018-07-26T14:17:48Z

r? @pnkfelix

pnkfelix

Why not make this a ui/ test?

pnkfelix · 2018-07-27T21:43:21Z

Ah compile-fail is fine, we’ll eventually move them all over to ui/

pnkfelix · 2018-07-27T21:44:04Z

@bors r+

bors · 2018-07-27T21:44:05Z

📌 Commit 4fd5aed has been approved by pnkfelix

bors · 2018-07-28T03:57:27Z

⌛ Testing commit 4fd5aed with merge c1b6bd2517d3d92e42c5ee0da82ff06c12c10db2...

bors · 2018-07-28T04:16:07Z

💔 Test failed - status-travis

kennytm · 2018-07-28T04:25:34Z

@bors retry travis-ci/travis-ci#4924

bors · 2018-07-28T06:59:28Z

⌛ Testing commit 4fd5aed with merge 67ca66eebb8c5e8de50576bcf207d5e5bffa7fe5...

bors · 2018-07-28T07:42:08Z

💔 Test failed - status-travis

kennytm · 2018-07-28T07:54:46Z

@bors retry travis-ci/travis-ci#4924

bors · 2018-07-28T13:40:44Z

⌛ Testing commit 4fd5aed with merge 17a666c322cf3f479daf4e1af3c2ed9aca707ea7...

bors · 2018-07-28T14:26:11Z

💔 Test failed - status-travis

kennytm · 2018-07-28T14:38:29Z

@bors retry travis-ci/travis-ci#4924

bors · 2018-07-28T16:37:27Z

⌛ Testing commit 4fd5aed with merge 67cd5ad53c24c613df36b49f0d3dbf3925090535...

bors · 2018-07-28T16:44:12Z

💔 Test failed - status-travis

kennytm · 2018-07-28T16:50:03Z

@bors retry #40474

This PR is being cursed or something.

[00:02:26]  Downloading unicode-segmentation v1.2.1
[00:02:57] warning: spurious network error (2 tries remaining): [28] Timeout was reached (Operation too slow. Less than 10 bytes/sec transferred the last 30 seconds)
[00:03:27] warning: spurious network error (1 tries remaining): [28] Timeout was reached (Operation too slow. Less than 10 bytes/sec transferred the last 30 seconds)
[00:03:57] error: unable to get packages from source
[00:03:57] 
[00:03:57] Caused by:
[00:03:57]   [28] Timeout was reached (Operation too slow. Less than 10 bytes/sec transferred the last 30 seconds)

bors · 2018-07-28T18:41:46Z

⌛ Testing commit 4fd5aed with merge 4234adf...

@aturon

do not overwrite child def-id in place but rather remove/insert When inserting a node N into the tree of impls, we sometimes find than an existing node C should be replaced with N. We used to overwrite C in place with the new def-id N -- but since the lists of def-ids are separated by simplified type, that could lead to N being inserted in the wrong place. This meant we might miss conflicts. We are now not trying to be so smart -- we remove C and then add N later. Fixes #52050 r? @aturon -- do you still remember this code at all? :)

bors · 2018-07-28T20:44:11Z

☀️ Test successful - status-appveyor, status-travis
Approved by: pnkfelix
Pushing 4234adf to master...

rust-highfive assigned aturon Jul 19, 2018

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

nikomatsakis force-pushed the issue-52050 branch from 2027584 to 4fd5aed Compare July 21, 2018 16:07

rust-highfive assigned eddyb and unassigned aturon Jul 23, 2018

nikomatsakis mentioned this pull request Jul 26, 2018

coherence check affected by module ordering #52050

Closed

rust-highfive assigned pnkfelix and unassigned eddyb Jul 26, 2018

pnkfelix reviewed Jul 27, 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 Jul 27, 2018

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 Jul 28, 2018

This comment has been minimized.

Sign in to view

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 Jul 28, 2018

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 Jul 28, 2018

This comment has been minimized.

Sign in to view

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 Jul 28, 2018

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 Jul 28, 2018

This comment has been minimized.

Sign in to view

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 Jul 28, 2018

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 Jul 28, 2018

This comment has been minimized.

Sign in to view

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 Jul 28, 2018

bors merged commit 4fd5aed into rust-lang:master Jul 28, 2018

pnkfelix mentioned this pull request Oct 9, 2018

error: internal compiler error: Accessing (*_310) with the kind Write(Move) shouldn't be possible #54597

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

do not overwrite child def-id in place but rather remove/insert #52546

do not overwrite child def-id in place but rather remove/insert #52546

nikomatsakis commented Jul 19, 2018 •

edited

Loading

rust-highfive commented Jul 19, 2018

nikomatsakis commented Jul 23, 2018

pnkfelix commented Jul 26, 2018

pnkfelix left a comment

pnkfelix commented Jul 27, 2018

pnkfelix commented Jul 27, 2018

bors commented Jul 27, 2018

bors commented Jul 28, 2018

bors commented Jul 28, 2018

This comment has been minimized.

kennytm commented Jul 28, 2018

bors commented Jul 28, 2018

bors commented Jul 28, 2018

This comment has been minimized.

kennytm commented Jul 28, 2018

bors commented Jul 28, 2018

bors commented Jul 28, 2018

This comment has been minimized.

kennytm commented Jul 28, 2018

bors commented Jul 28, 2018

bors commented Jul 28, 2018

This comment has been minimized.

kennytm commented Jul 28, 2018

bors commented Jul 28, 2018

bors commented Jul 28, 2018

do not overwrite child def-id in place but rather remove/insert #52546

do not overwrite child def-id in place but rather remove/insert #52546

Conversation

nikomatsakis commented Jul 19, 2018 • edited Loading

rust-highfive commented Jul 19, 2018

nikomatsakis commented Jul 23, 2018

pnkfelix commented Jul 26, 2018

pnkfelix left a comment

Choose a reason for hiding this comment

pnkfelix commented Jul 27, 2018

pnkfelix commented Jul 27, 2018

bors commented Jul 27, 2018

bors commented Jul 28, 2018

bors commented Jul 28, 2018

This comment has been minimized.

kennytm commented Jul 28, 2018

bors commented Jul 28, 2018

bors commented Jul 28, 2018

This comment has been minimized.

kennytm commented Jul 28, 2018

bors commented Jul 28, 2018

bors commented Jul 28, 2018

This comment has been minimized.

kennytm commented Jul 28, 2018

bors commented Jul 28, 2018

bors commented Jul 28, 2018

This comment has been minimized.

kennytm commented Jul 28, 2018

bors commented Jul 28, 2018

bors commented Jul 28, 2018

nikomatsakis commented Jul 19, 2018 •

edited

Loading