Deny specializing items not in the parent impl #64564

jonas-schievink · 2019-09-17T22:12:58Z

Part of #29661 (rust-lang/rfcs#2532). At least sort of?

This was discussed in #61812 (comment) and is needed for that PR to make progress (fixing an unsoundness).

One annoyance with doing this is that it sometimes requires users to copy-paste a provided trait method into an impl just to mark it default (ie. there is no syntax to forward this impl method to the provided trait method).

cc @Centril and @arielb1

rust-highfive · 2019-09-17T22:13:02Z

r? @zackmdavis

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

src/liballoc/boxed.rs

src/librustc/traits/specialize/specialization_graph.rs

src/test/ui/specialization/specialization-default-methods.rs

Centril · 2019-09-17T22:56:41Z

r? @nikomatsakis

src/test/ui/specialization/specialization-default-methods-fail.rs

jonas-schievink · 2019-09-19T18:40:01Z

Ah I forgot that we wanted to crater this.

@bors try

bors · 2019-09-19T18:40:12Z

⌛ Trying commit 88f3075 with merge 8d67a66...

@Centril

Deny specializing items not in the parent impl Part of #29661 (rust-lang/rfcs#2532). At least sort of? This was discussed in #61812 (comment) and is needed for that PR to make progress (fixing an unsoundness). One annoyance with doing this is that it sometimes requires users to copy-paste a provided trait method into an impl just to mark it `default` (ie. there is no syntax to forward this impl method to the provided trait method). cc @Centril and @arielb1

bors · 2019-09-19T21:41:55Z

☀️ Try build successful - checks-azure
Build commit: 8d67a66

Centril · 2019-09-19T21:43:44Z

@craterbot run mode=check-only

craterbot · 2019-09-19T21:43:46Z

👌 Experiment pr-64564 created and queued.
🤖 Automatically detected try build 8d67a66
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

nikomatsakis

r=me once crater run is complete etc

craterbot · 2019-09-30T06:43:08Z

🚧 Experiment pr-64564 is now running on agent aws-1.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2019-10-05T12:54:57Z

🎉 Experiment pr-64564 is completed!
📊 49 regressed and 3 fixed (74234 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

jonas-schievink · 2019-10-05T13:18:59Z

Summary

There are 7 root regressions: pyo3, tao-of-rust, paperclip-core, kompact, serde_traitobject, mockiato and incrust.

There is one seemingly unrelated build failure in moxie.

Full Analysis

pyo3

Axect/freeze_out
JellyWX/md-html
RuneBlazer/kmer_vec
StephanHeijl/global-supertrees
a-hacker/PyRounders
aleozlx/sturdy-engine
ckaran/pyo3_issue_report
cualbondi/pyosmptparser
cybermatt/fast-luhn
dante-signal31/steganer
deaz/pyo3-test
gavrie/trial_division
hhatto/fcsv
hhatto/fpath
kdar/rust-python-example
mozilla/pyo3-parsepatch
nikhilmitrax/python-oxidation
omerbenamram/pyevtx-rs
omerbenamram/pymft-rs
omerbenamram/pyo3-file
pattonw/rust-pyn5
petervaro/rusty-python-fibonacci
rdeaton/pyskim
sbdchd/codeowners
dict_derive
inline-python-macros
inline-python
mbf_gtf
numpy
orkhon
pyo3-built
pyo3-file
pyrus-nn
retworkx
spellcheck-rs

tao-of-rust

paperclip-core:

paperclip-actix

kompact:

Max-Meldrum/kompact_template

serde_traitobject

alecmocatta/deploy

mockiato

jeremystucki/mvr
myelin-ai/engine
myelin-engine

incrust

moxie

jonas-schievink · 2019-10-06T15:43:55Z

Okay, given that there are only 7 root regressions (meaning that only 7 projects need to be fixed after landing this), and this is an unstable feature anyways, I'm going to go ahead and:

@bors r=nikomatsakis

bors · 2019-10-06T15:43:56Z

📌 Commit 47f89e7 has been approved by nikomatsakis

bors · 2019-10-06T16:32:51Z

⌛ Testing commit 47f89e7 with merge 421bd77...

@Centril

Deny specializing items not in the parent impl Part of #29661 (rust-lang/rfcs#2532). At least sort of? This was discussed in #61812 (comment) and is needed for that PR to make progress (fixing an unsoundness). One annoyance with doing this is that it sometimes requires users to copy-paste a provided trait method into an impl just to mark it `default` (ie. there is no syntax to forward this impl method to the provided trait method). cc @Centril and @arielb1

bors · 2019-10-06T20:24:32Z

☀️ Test successful - checks-azure
Approved by: nikomatsakis
Pushing 421bd77 to master...

…40.0 [1]: PyO3/pyo3#614 [2]: rust-lang/rust#64564

Implement RFC 2532 – Associated Type Defaults This is a partial implementation that is still missing the changes to object types, since I ran into some trouble while implementing that. I'm opening this part already to get feedback on the implementation and the unexpected test fallout (see my comments below). The remaining changes can be done in a later PR. Blockers before this can land: * [x] Resolve unsoundness around interaction with specialization (#61812 (comment)) - #64564 cc #29661 Fixes #53907 Fixes #54182 Fixes #62211 Fixes #41868 Fixes #63593 Fixes #47385 Fixes #43924 Fixes #32350 Fixes #26681 Fixes #67187

rust-highfive assigned zackmdavis Sep 17, 2019

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

jonas-schievink added A-specialization Area: Trait impl specialization F-specialization `#![feature(specialization)]` labels Sep 17, 2019

Centril added the F-associated_type_defaults `#![feature(associated_type_defaults)]` label Sep 17, 2019

This comment has been minimized.

Sign in to view

Centril reviewed Sep 17, 2019

View reviewed changes

rust-highfive assigned nikomatsakis and unassigned zackmdavis Sep 17, 2019

Centril reviewed Sep 18, 2019

View reviewed changes

src/test/ui/specialization/specialization-default-methods-fail.rs Outdated Show resolved Hide resolved

craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 19, 2019

nikomatsakis approved these changes Sep 23, 2019

View reviewed changes

jonas-schievink mentioned this pull request Oct 1, 2019

Implement RFC 2532 – Associated Type Defaults #61812

Merged

1 task

craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Oct 5, 2019

jonas-schievink added 2 commits October 5, 2019 15:33

Deny specializing items not in the parent impl

2cd5030

Add comment to Type and OpaqueTy match

55277d4

jonas-schievink added 6 commits October 5, 2019 15:33

Hide the Iterator specialization behind a trait

02f36e5

Fix the bootstrap

9aaef06

Test that we get the proper errors

a0cf531

Pacify tidy

98f02b2

Extend test and fix nits

33d23cd

Rename the test

47f89e7

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 6, 2019

bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 6, 2019

bors merged commit 47f89e7 into rust-lang:master Oct 6, 2019

konstin mentioned this pull request Oct 7, 2019

Rust 1.40.0 nightly breaks specialization PyO3/pyo3#614

Closed

jonas-schievink deleted the cowardly-default branch October 7, 2019 13:43

luxrck added a commit to luxrck/distances that referenced this pull request Nov 8, 2019

Update pyo3 to version 0.8.2 due to rust specialization changes in 1.…

9a0b46f

…40.0 [1]: PyO3/pyo3#614 [2]: rust-lang/rust#64564

konstin mentioned this pull request Nov 15, 2019

Get rid of specialization PyO3/pyo3#210

Closed

Centril mentioned this pull request Jan 17, 2020

Specialization should allow for delegation to default trait method implementation #68309

Open

jonas-schievink mentioned this pull request Mar 27, 2020

blank specializing impls don't "lock in" defaults #70442

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deny specializing items not in the parent impl #64564

Deny specializing items not in the parent impl #64564

jonas-schievink commented Sep 17, 2019 •

edited

Loading

rust-highfive commented Sep 17, 2019

This comment has been minimized.

Centril commented Sep 17, 2019

jonas-schievink commented Sep 19, 2019

bors commented Sep 19, 2019

bors commented Sep 19, 2019

Centril commented Sep 19, 2019

craterbot commented Sep 19, 2019

nikomatsakis left a comment

craterbot commented Sep 30, 2019

craterbot commented Oct 5, 2019

jonas-schievink commented Oct 5, 2019

jonas-schievink commented Oct 6, 2019

bors commented Oct 6, 2019

bors commented Oct 6, 2019

bors commented Oct 6, 2019

Deny specializing items not in the parent impl #64564

Deny specializing items not in the parent impl #64564

Conversation

jonas-schievink commented Sep 17, 2019 • edited Loading

rust-highfive commented Sep 17, 2019

This comment has been minimized.

Centril commented Sep 17, 2019

jonas-schievink commented Sep 19, 2019

bors commented Sep 19, 2019

bors commented Sep 19, 2019

Centril commented Sep 19, 2019

craterbot commented Sep 19, 2019

nikomatsakis left a comment

Choose a reason for hiding this comment

craterbot commented Sep 30, 2019

craterbot commented Oct 5, 2019

jonas-schievink commented Oct 5, 2019

Summary

Full Analysis

jonas-schievink commented Oct 6, 2019

bors commented Oct 6, 2019

bors commented Oct 6, 2019

bors commented Oct 6, 2019

jonas-schievink commented Sep 17, 2019 •

edited

Loading