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

Specialization is unsound #71420

Merged
merged 3 commits into from
Jun 20, 2020
Merged

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Apr 22, 2020

As discussed in #31844 (comment), it might be a good idea to warn users of specialization that the feature they are using is unsound.

I also expanded the "incomplete feature" warning to link the user to the tracking issue.

@rust-highfive
Copy link
Collaborator

r? @eddyb

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 22, 2020
@rust-highfive

This comment has been minimized.

@rust-highfive

This comment has been minimized.

@RalfJung
Copy link
Member Author

RalfJung commented Apr 22, 2020

Turns out feature(specialization) is used in quite a few rustc crates. @matthewjasper I am not sure what the plan is for those, should they all move to min_specialization?

src/libserialize/lib.rs
13:#![feature(specialization)]

src/librustc_ast_lowering/lib.rs
36:#![feature(specialization)]

src/librustc_data_structures/lib.rs
15:#![feature(specialization)]

src/librustc_hir/lib.rs
11:#![feature(specialization)]

src/librustc_metadata/lib.rs
10:#![feature(specialization)]

src/librustc_middle/lib.rs
43:#![feature(specialization)]

src/librustc_mir/lib.rs
20:#![feature(specialization)]

src/librustc_query_system/lib.rs
7:#![feature(specialization)]

src/librustc_span/lib.rs
15:#![feature(specialization)]

@matthewjasper
Copy link
Contributor

They probably should, but I don't know how feasible it is. Once stage0 has min_specialization I could see how easy it is to switch.

@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-8 of your PR failed (pretty log, 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.
2020-04-22T10:15:25.4531470Z ========================== Starting Command Output ===========================
2020-04-22T10:15:25.4534279Z [command]/bin/bash --noprofile --norc /home/vsts/work/_temp/570d6fc4-6216-40ca-bee1-dcd6c4bc39ad.sh
2020-04-22T10:15:25.4534524Z 
2020-04-22T10:15:25.4538648Z ##[section]Finishing: Disable git automatic line ending conversion
2020-04-22T10:15:25.4557508Z ##[section]Starting: Checkout rust-lang/rust@refs/pull/71420/merge to s
2020-04-22T10:15:25.4560748Z Task         : Get sources
2020-04-22T10:15:25.4561024Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
2020-04-22T10:15:25.4561297Z Version      : 1.0.0
2020-04-22T10:15:25.4561500Z Author       : Microsoft
---
2020-04-22T10:15:26.7025144Z ##[command]git remote add origin https://github.com/rust-lang/rust
2020-04-22T10:15:26.7034817Z ##[command]git config gc.auto 0
2020-04-22T10:15:26.7040774Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2020-04-22T10:15:26.7046065Z ##[command]git config --get-all http.proxy
2020-04-22T10:15:26.7056191Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/71420/merge:refs/remotes/pull/71420/merge
---
2020-04-22T10:17:44.9944957Z  ---> 318032b5f0e2
2020-04-22T10:17:44.9945721Z Step 5/8 : ENV RUST_CONFIGURE_ARGS       --build=x86_64-unknown-linux-gnu       --llvm-root=/usr/lib/llvm-8       --enable-llvm-link-shared       --set rust.thin-lto-import-instr-limit=10
2020-04-22T10:17:44.9950406Z  ---> Using cache
2020-04-22T10:17:44.9950797Z  ---> d44a858fd1ce
2020-04-22T10:17:44.9951702Z Step 6/8 : ENV SCRIPT python2.7 ../x.py test --exclude src/tools/tidy &&            python2.7 ../x.py test src/test/mir-opt --pass=build                                   --target=armv5te-unknown-linux-gnueabi &&            python2.7 ../x.py test src/tools/tidy
2020-04-22T10:17:44.9957126Z  ---> 58b910f50f5a
2020-04-22T10:17:44.9957329Z Step 7/8 : ENV NO_DEBUG_ASSERTIONS=1
2020-04-22T10:17:44.9961794Z  ---> Using cache
2020-04-22T10:17:44.9962177Z  ---> ee7702aadba1
---
2020-04-22T10:17:45.0403916Z Looks like docker image is the same as before, not uploading
2020-04-22T10:17:52.9168020Z [CI_JOB_NAME=x86_64-gnu-llvm-8]
2020-04-22T10:17:52.9378759Z [CI_JOB_NAME=x86_64-gnu-llvm-8]
2020-04-22T10:17:52.9407070Z == clock drift check ==
2020-04-22T10:17:52.9422066Z   local time: Wed Apr 22 10:17:52 UTC 2020
2020-04-22T10:17:53.2397166Z   network time: Wed, 22 Apr 2020 10:17:53 GMT
2020-04-22T10:17:53.2422497Z Starting sccache server...
2020-04-22T10:17:53.3287464Z configure: processing command line
2020-04-22T10:17:53.3288268Z configure: 
2020-04-22T10:17:53.3289541Z configure: rust.dist-src        := False
---
2020-04-22T10:23:14.2806383Z    Compiling rustc_feature v0.0.0 (/checkout/src/librustc_feature)
2020-04-22T10:23:15.7403862Z    Compiling fmt_macros v0.0.0 (/checkout/src/libfmt_macros)
2020-04-22T10:23:17.3778220Z    Compiling rustc_ast_pretty v0.0.0 (/checkout/src/librustc_ast_pretty)
2020-04-22T10:23:18.2962509Z    Compiling rustc_hir v0.0.0 (/checkout/src/librustc_hir)
2020-04-22T10:23:27.3610299Z    Compiling rustc_query_system v0.0.0 (/checkout/src/librustc_query_system)
2020-04-22T10:23:29.4377241Z    Compiling rustc_hir_pretty v0.0.0 (/checkout/src/librustc_hir_pretty)
2020-04-22T10:23:33.7914814Z    Compiling rustc_attr v0.0.0 (/checkout/src/librustc_attr)
2020-04-22T10:23:37.9142948Z    Compiling rustc_parse v0.0.0 (/checkout/src/librustc_parse)
2020-04-22T10:23:47.5081847Z    Compiling rustc_ast_lowering v0.0.0 (/checkout/src/librustc_ast_lowering)
---
2020-04-22T10:43:31.2073294Z    |
2020-04-22T10:43:31.2074249Z 13 | #![feature(specialization)]
2020-04-22T10:43:31.2075304Z    |            ^^^^^^^^^^^^^^
2020-04-22T10:43:31.2076135Z    |
2020-04-22T10:43:31.2077168Z    = note: `-D incomplete-features` implied by `-D warnings`
2020-04-22T10:43:31.2078825Z    = note: see issue #31844 <***/issues/31844> for more information
2020-04-22T10:43:32.1924256Z error: aborting due to previous error
2020-04-22T10:43:32.1924523Z 
2020-04-22T10:43:32.1974710Z error: could not compile `serialize`.
2020-04-22T10:43:32.1974977Z 
---
2020-04-22T10:43:32.3250570Z expected success, got: exit code: 101
2020-04-22T10:43:32.3264540Z failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test --exclude src/tools/tidy
2020-04-22T10:43:32.3264972Z Build completed unsuccessfully in 0:23:55
2020-04-22T10:43:32.3324345Z == clock drift check ==
2020-04-22T10:43:32.3343873Z   local time: Wed Apr 22 10:43:32 UTC 2020
2020-04-22T10:43:32.5017354Z   network time: Wed, 22 Apr 2020 10:43:32 GMT
2020-04-22T10:43:33.3395838Z 
2020-04-22T10:43:33.3395838Z 
2020-04-22T10:43:33.3473696Z ##[error]Bash exited with code '1'.
2020-04-22T10:43:33.3488497Z ##[section]Finishing: Run build
2020-04-22T10:43:33.3538769Z ##[section]Starting: Checkout rust-lang/rust@refs/pull/71420/merge to s
2020-04-22T10:43:33.3544934Z Task         : Get sources
2020-04-22T10:43:33.3545268Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
2020-04-22T10:43:33.3545553Z Version      : 1.0.0
2020-04-22T10:43:33.3545776Z Author       : Microsoft
2020-04-22T10:43:33.3545776Z Author       : Microsoft
2020-04-22T10:43:33.3546110Z Help         : [More Information](https://go.microsoft.com/fwlink/?LinkId=798199)
2020-04-22T10:43:33.3546474Z ==============================================================================
2020-04-22T10:43:33.6905494Z Cleaning any cached credential from repository: rust-lang/rust (GitHub)
2020-04-22T10:43:33.6949920Z ##[section]Finishing: Checkout rust-lang/rust@refs/pull/71420/merge to s
2020-04-22T10:43:33.7044182Z Cleaning up task key
2020-04-22T10:43:33.7046243Z Start cleaning up orphan processes.
2020-04-22T10:43:33.7236508Z Terminate orphan process: pid (4007) (python)
2020-04-22T10:43:33.7399875Z ##[section]Finishing: Finalize Job

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 @rust-lang/infra. (Feature Requests)

@RalfJung
Copy link
Member Author

Once stage0 has min_specialization

That shouldn't be too long. :D

I am going to wait for some more feedback on this PR before trying to get the build through.

@bors

This comment has been minimized.

@RalfJung
Copy link
Member Author

This is a rather conflict-heavy PR, would be good if I got quick feedback on this.

@rust-highfive

This comment has been minimized.

@RalfJung
Copy link
Member Author

RalfJung commented May 1, 2020

@matthewjasper stage 0 has min_specialization now. How do we proceed here? Should I add the allow(incomplete_features) for now (do people even want this PR), and you can remove it again when you get to porting those crates?

RalfJung added a commit to RalfJung/rust that referenced this pull request May 11, 2020
…ochenkov

Incomplete features can also be unsound

Some incomplete features do not just ICE, they are also currently unsound (e.g. rust-lang#72029, and also `specialization` -- which is not yet marked incomplete but [should be](rust-lang#71420)). This makes the message reflect that.

While at it I also added a link to the tracking issue, which hopefully should explain what is incomplete/unsound about the feature.
@joelpalmer joelpalmer added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 12, 2020
RalfJung added a commit to RalfJung/rust that referenced this pull request May 14, 2020
…ochenkov

Incomplete features can also be unsound

Some incomplete features do not just ICE, they are also currently unsound (e.g. rust-lang#72029, and also `specialization` -- which is not yet marked incomplete but [should be](rust-lang#71420)). This makes the message reflect that.

While at it I also added a link to the tracking issue, which hopefully should explain what is incomplete/unsound about the feature.
RalfJung added a commit to RalfJung/rust that referenced this pull request May 16, 2020
…ochenkov

Incomplete features can also be unsound

Some incomplete features do not just ICE, they are also currently unsound (e.g. rust-lang#72029, and also `specialization` -- which is not yet marked incomplete but [should be](rust-lang#71420)). This makes the message reflect that.

While at it I also added a link to the tracking issue, which hopefully should explain what is incomplete/unsound about the feature.
@bors

This comment has been minimized.

@RalfJung
Copy link
Member Author

Rebased across my other PR which changes the message to include unsoundness.

@matthewjasper should I just add allow(incomplete_features) to the remaining rustc crates?

@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-8 of your PR failed (pretty log, 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.
##[section]Starting: Linux x86_64-gnu-llvm-8
##[section]Starting: Initialize job
Agent name: 'Azure Pipelines 2'
Agent machine name: 'fv-az619'
Current agent version: '2.168.2'
##[group]Operating System
16.04.6
LTS
LTS
##[endgroup]
##[group]Virtual Environment
Environment: ubuntu-16.04
Version: 20200430.2
Included Software: https://github.com/actions/virtual-environments/blob/ubuntu16/20200430.2/images/linux/Ubuntu1604-README.md
##[endgroup]
Agent running as: 'vsts'
Prepare build directory.
Set build variables.
Download all required tasks.
Download all required tasks.
Downloading task: Bash (3.163.2)
Checking job knob settings.
   Knob: AgentToolsDirectory = /opt/hostedtoolcache Source: ${AGENT_TOOLSDIRECTORY} 
   Knob: AgentPerflog = /home/vsts/perflog Source: ${VSTS_AGENT_PERFLOG} 
Start tracking orphan processes.
##[section]Finishing: Initialize job
##[section]Starting: Configure Job Name
==============================================================================
---
========================== Starting Command Output ===========================
[command]/bin/bash --noprofile --norc /home/vsts/work/_temp/cd5facce-dd70-462a-b664-5e57e821a07f.sh

##[section]Finishing: Disable git automatic line ending conversion
##[section]Starting: Checkout rust-lang/rust@refs/pull/71420/merge to s
Task         : Get sources
Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
Version      : 1.0.0
Author       : Microsoft
---
##[command]git remote add origin https://github.com/rust-lang/rust
##[command]git config gc.auto 0
##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
##[command]git config --get-all http.proxy
##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/71420/merge:refs/remotes/pull/71420/merge
---
 ---> cb2676f08729
Step 5/8 : ENV RUST_CONFIGURE_ARGS       --build=x86_64-unknown-linux-gnu       --llvm-root=/usr/lib/llvm-8       --enable-llvm-link-shared       --set rust.thin-lto-import-instr-limit=10
 ---> Using cache
 ---> df25ce111862
Step 6/8 : ENV SCRIPT python2.7 ../x.py test --exclude src/tools/tidy &&            python2.7 ../x.py test src/test/mir-opt --pass=build                                   --target=armv5te-unknown-linux-gnueabi &&            python2.7 ../x.py test src/tools/tidy
 ---> 599b9ac96b27
Step 7/8 : ENV NO_DEBUG_ASSERTIONS=1
 ---> Using cache
 ---> 091087e35a36
---
   Compiling fmt_macros v0.0.0 (/checkout/src/libfmt_macros)
   Compiling rustc_ast_pretty v0.0.0 (/checkout/src/librustc_ast_pretty)
   Compiling chalk-rust-ir v0.10.0
   Compiling rustc_hir v0.0.0 (/checkout/src/librustc_hir)
   Compiling rustc_query_system v0.0.0 (/checkout/src/librustc_query_system)
   Compiling chalk-solve v0.10.0
   Compiling rustc_hir_pretty v0.0.0 (/checkout/src/librustc_hir_pretty)
   Compiling rustc_parse v0.0.0 (/checkout/src/librustc_parse)
   Compiling rustc_ast_lowering v0.0.0 (/checkout/src/librustc_ast_lowering)
---
   |
13 | #![feature(specialization)] // FIXME: min_specialization does not work
   |            ^^^^^^^^^^^^^^
   |
   = note: `-D incomplete-features` implied by `-D warnings`
   = note: see issue #31844 <***/issues/31844> for more information
   Compiling getopts v0.2.21
error: aborting due to previous error

error: could not compile `serialize`.
---
  local time: Sun May 17 09:01:12 UTC 2020
  network time: Sun, 17 May 2020 09:01:12 GMT
== end clock drift check ==

##[error]Bash exited with code '1'.
##[section]Finishing: Run build
##[section]Starting: Checkout rust-lang/rust@refs/pull/71420/merge to s
Task         : Get sources
Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
Version      : 1.0.0
Author       : Microsoft
Author       : Microsoft
Help         : [More Information](https://go.microsoft.com/fwlink/?LinkId=798199)
==============================================================================
Cleaning any cached credential from repository: rust-lang/rust (GitHub)
##[section]Finishing: Checkout rust-lang/rust@refs/pull/71420/merge to s
Cleaning up task key
Start cleaning up orphan processes.
Terminate orphan process: pid (4457) (python)
##[section]Finishing: Finalize Job

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 @rust-lang/infra. (Feature Requests)

@RalfJung
Copy link
Member Author

r? @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 Jun 16, 2020
tmandry added a commit to tmandry/rust that referenced this pull request Jun 17, 2020
… r=matthewjasper

Specialization is unsound

As discussed in rust-lang#31844 (comment), it might be a good idea to warn users of specialization that the feature they are using is unsound.

I also expanded the "incomplete feature" warning to link the user to the tracking issue.
RalfJung added a commit to RalfJung/rust that referenced this pull request Jun 18, 2020
… r=matthewjasper

Specialization is unsound

As discussed in rust-lang#31844 (comment), it might be a good idea to warn users of specialization that the feature they are using is unsound.

I also expanded the "incomplete feature" warning to link the user to the tracking issue.
@bors
Copy link
Contributor

bors commented Jun 19, 2020

⌛ Testing commit d1265e7 with merge 25af17199dba15a4206d9f81567b0c6f800d2b04...

@bors
Copy link
Contributor

bors commented Jun 19, 2020

💔 Test failed - checks-azure

@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 Jun 19, 2020
@RalfJung
Copy link
Member Author

@rust-lang/infra there's not even a build log at https://dev.azure.com/rust-lang/rust/_build/results?buildId=32326&view=logs&j=9121965e-722b-5094-4a33-f061e41958d8&t=64b964f2-043e-5afc-0fc3-c41881c5cd34 ? Is there any way to figure out what went wrong here?

@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 Jun 19, 2020
@pietroalbini
Copy link
Member

##[error]We stopped hearing from agent Azure Pipelines 53. Verify the agent machine is running and has a healthy network connection. Anything that terminates an agent process, starves it for CPU, or blocks its network access can cause this error. For more information, see: https://go.microsoft.com/fwlink/?linkid=846610

I guess the underlying VM crashed. Please ping infra again if it starts to happen consistently.

@bors
Copy link
Contributor

bors commented Jun 19, 2020

⌛ Testing commit d1265e7 with merge 6696efcd9f5e10f076cec28c7322f765345d8c24...

@RalfJung
Copy link
Member Author

Yield.
@bors retry

bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 20, 2020
…arth

Rollup of 16 pull requests

Successful merges:

 - rust-lang#71420 (Specialization is unsound)
 - rust-lang#71899 (Refactor `try_find` a little)
 - rust-lang#72689 (add str to common types)
 - rust-lang#72791 (update coerce docs and unify relevant tests)
 - rust-lang#72934 (forbid mutable references in all constant contexts except for const-fns)
 - rust-lang#73027 (Make `need_type_info_err` more conservative)
 - rust-lang#73347 (Diagnose use of incompatible sanitizers)
 - rust-lang#73359 (shim.rs: avoid creating `Call` terminators calling `Self`)
 - rust-lang#73399 (Clean up E0668 explanation)
 - rust-lang#73436 (Clean up E0670 explanation)
 - rust-lang#73440 (Add src/librustdoc as an alias for src/tools/rustdoc)
 - rust-lang#73442 (pretty/mir: const value enums with no variants)
 - rust-lang#73452 (Unify region variables when projecting associated types)
 - rust-lang#73458 (Use alloc::Layout in DroplessArena API)
 - rust-lang#73484 (Update the doc for std::prelude to the correct behavior)
 - rust-lang#73506 (Bump Rustfmt and RLS)

Failed merges:

r? @ghost
@bors bors merged commit 203305d into rust-lang:master Jun 20, 2020
@RalfJung RalfJung deleted the specialization-incomplete branch June 20, 2020 10:07
lambda-fairy added a commit to lambda-fairy/maud that referenced this pull request Jun 22, 2020
See rust-lang/rust#71420.

This doesn't need a release (or a changelog entry) as it just fixes a warning.
lambda-fairy added a commit to lambda-fairy/maud that referenced this pull request Jun 22, 2020
See rust-lang/rust#71420.

This doesn't need a release (or a changelog entry) as it just fixes a warning.
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.

7 participants