Take supertraits into account when calculating associated types #55687

alexreg · 2018-11-05T03:37:49Z

Fixes #24010 and #23856. Applies to trait aliases too.

As a by-product, this PR also makes repeated bindings of the same associated item in the same definition a hard error. This was previously a warning with a note about it becoming a hard error in the future. See #50589 for more info.

I talked about this with @nikomatsakis recently, but only very superficially, so this shouldn't stop anyone from assigning it to themself to review and r+.

N.B. The "WIP" commits represent imperfect attempts to solve the problem just for trait objects, but I've left them in for reference for the sake of whomever is reviewing this.

CC @carllerche @theemathas @durka @mbrubeck

alexreg · 2018-11-05T03:38:27Z

r? @nikomatsakis

(If someone else would rather take it, feel free to r? yourself, per above.)

pietroalbini · 2018-11-05T19:04:52Z

@craterbot run start=6cfc6033955dd2685dfa7baeec6f6bc3bfdfe2f1 end=6cfc6033955dd2685dfa7baeec6f6bc3bfdfe2f1+rustflags=-Dduplicate_associated_type_bindings mode=check-only

craterbot · 2018-11-05T19:04:55Z

👌 Experiment pr-55687 created and queued.
🔍 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

craterbot · 2018-11-05T19:04:58Z

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

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

oli-obk

First pass, will review in detail when back on a pc

src/test/run-pass/traits/trait-alias-objects.rs

src/librustc_typeck/astconv.rs

pietroalbini · 2018-11-06T10:00:03Z

Uh, woops.

@craterbot abort

craterbot · 2018-11-06T10:00:06Z

🗑️ Experiment pr-55687 deleted!

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

pietroalbini · 2018-11-06T10:01:05Z

Wrong commit hash...

@craterbot run start=24e66c28980442a48d9458f1a4f9b76cc722dc8a end=24e66c28980442a48d9458f1a4f9b76cc722dc8a+rustflags=-Dduplicate_associated_type_bindings mode=check-only

craterbot · 2018-11-06T10:01:06Z

👌 Experiment pr-55687 created and queued.
🔍 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

craterbot · 2018-11-06T10:01:09Z

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

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

pietroalbini · 2018-11-06T10:02:08Z

@craterbot abort

craterbot · 2018-11-06T10:02:09Z

🗑️ Experiment pr-55687 deleted!

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

pietroalbini · 2018-11-06T10:02:36Z

...forgot this is not my local branch...

@craterbot run start=master#24e66c28980442a48d9458f1a4f9b76cc722dc8a end=master#24e66c28980442a48d9458f1a4f9b76cc722dc8a+rustflags=-Dduplicate_associated_type_bindings mode=check-only

craterbot · 2018-11-06T10:02:37Z

👌 Experiment pr-55687 created and queued.
🔍 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 · 2018-11-07T15:57:47Z

Seems like @scalexm is on this? =)

craterbot · 2018-11-07T19:01:31Z

🎉 Experiment pr-55687 is completed!
📰 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

This has been replaced by the hard error E0719.

scalexm · 2018-11-07T22:24:25Z

@bors r+

bors · 2018-11-07T22:24:26Z

📌 Commit 90a1438 has been approved by scalexm

@nikomatsakis

Take supertraits into account when calculating associated types Fixes rust-lang#24010 and rust-lang#23856. Applies to trait aliases too. As a by-product, this PR also makes repeated bindings of the same associated item in the same definition a hard error. This was previously a warning with a note about it becoming a hard error in the future. See rust-lang#50589 for more info. I talked about this with @nikomatsakis recently, but only very superficially, so this shouldn't stop anyone from assigning it to themself to review and r+. N.B. The "WIP" commits represent imperfect attempts to solve the problem just for trait objects, but I've left them in for reference for the sake of whomever is reviewing this. CC @carllerche @theemathas @durka @mbrubeck

Rollup of 17 pull requests Successful merges: - #55630 (resolve: Filter away macro prelude in modules with `#[no_implicit_prelude]` on 2018 edition) - #55687 (Take supertraits into account when calculating associated types) - #55745 (Convert `outlives_components`' return value to a `SmallVec` outparam.) - #55764 (Fix Rc/Arc allocation layout) - #55792 (Prevent ICE in const-prop array oob check) - #55799 (Removed unneeded instance of `// revisions` from a lint test) - #55800 (Fix ICE in `return_type_impl_trait`) - #55801 (NLL: Update box insensitivity test) - #55802 (Don't inline virtual calls (take 2)) - #55816 (Use `SmallVec` to avoid allocations in `from_decimal_string`.) - #55819 (Typecheck patterns of all match arms first, so we get types for bindings) - #55822 (ICE with #![feature(nll)] and elided lifetimes) - #55828 (Add missing `rustc_promotable` attribute to unsigned `min_value` and `max_value`) - #55839 (Fix docstring spelling mistakes) - #55844 (Fix documentation typos.) - #55845 (Set BINARYEN_TRAP_MODE=clamp) - #55856 (rustdoc: refactor: move all static-file include!s into a single module)

rust-highfive assigned eddyb Nov 5, 2018

This comment has been minimized.

Sign in to view

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

rust-highfive assigned nikomatsakis and unassigned eddyb Nov 5, 2018

This comment has been minimized.

Sign in to view

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 Nov 5, 2018

alexreg force-pushed the fix-24010 branch from 9b0d8a1 to 5297665 Compare November 6, 2018 03:12

This comment has been minimized.

Sign in to view

oli-obk reviewed Nov 6, 2018

View reviewed changes

src/test/run-pass/traits/trait-alias-objects.rs Outdated Show resolved Hide resolved

src/librustc_typeck/astconv.rs Outdated Show resolved Hide resolved

src/librustc_typeck/astconv.rs Outdated Show resolved Hide resolved

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 Nov 6, 2018

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 Nov 6, 2018

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 Nov 6, 2018

rust-highfive assigned scalexm and unassigned nikomatsakis Nov 7, 2018

alexreg force-pushed the fix-24010 branch 2 times, most recently from 92ac02a to 19d71f2 Compare November 7, 2018 18:06

This comment has been minimized.

Sign in to view

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 Nov 7, 2018

alexreg force-pushed the fix-24010 branch 2 times, most recently from 5825653 to 4c1d24a Compare November 7, 2018 21:51

scalexm approved these changes Nov 7, 2018

View reviewed changes

Alexander Regueiro added 5 commits November 7, 2018 21:56

Look at projections from supertraits when constructing trait objects.

d08a42b

Added tests.

0e89f57

Added error for duplicate bindings of associated type.

6d3ee41

Only do check for trait objects, not trait or trait alias definitions.

7d9ee03

Removed DUPLICATE_ASSOCIATED_TYPE_BINDINGS lint.

90a1438

This has been replaced by the hard error E0719.

alexreg force-pushed the fix-24010 branch from 4c1d24a to 90a1438 Compare November 7, 2018 21:57

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 Nov 7, 2018

pietroalbini mentioned this pull request Nov 10, 2018

Rollup of 17 pull requests #55859

Merged

bors merged commit 90a1438 into rust-lang:master Nov 11, 2018

arielb1 mentioned this pull request Dec 2, 2018

problems with trait objects with a trait projection value that contains Self #56288

Closed

coolreader18 mentioned this pull request Dec 27, 2018

Give wasm a proper API, add a small example of how to use that library, and add codemirror to the demo site. RustPython/RustPython#238

Closed

Imxset21 mentioned this pull request Mar 20, 2019

Incorrect "associated type must be specified" for trait objects #23856

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Take supertraits into account when calculating associated types #55687

Take supertraits into account when calculating associated types #55687

alexreg commented Nov 5, 2018

This comment has been minimized.

alexreg commented Nov 5, 2018 •

edited

Loading

This comment has been minimized.

pietroalbini commented Nov 5, 2018

craterbot commented Nov 5, 2018

craterbot commented Nov 5, 2018

This comment has been minimized.

oli-obk left a comment

pietroalbini commented Nov 6, 2018

craterbot commented Nov 6, 2018

pietroalbini commented Nov 6, 2018

craterbot commented Nov 6, 2018

craterbot commented Nov 6, 2018

pietroalbini commented Nov 6, 2018

craterbot commented Nov 6, 2018

pietroalbini commented Nov 6, 2018

craterbot commented Nov 6, 2018

nikomatsakis commented Nov 7, 2018

This comment has been minimized.

craterbot commented Nov 7, 2018

scalexm commented Nov 7, 2018

bors commented Nov 7, 2018

Take supertraits into account when calculating associated types #55687

Take supertraits into account when calculating associated types #55687

Conversation

alexreg commented Nov 5, 2018

This comment has been minimized.

alexreg commented Nov 5, 2018 • edited Loading

This comment has been minimized.

pietroalbini commented Nov 5, 2018

craterbot commented Nov 5, 2018

craterbot commented Nov 5, 2018

This comment has been minimized.

oli-obk left a comment

Choose a reason for hiding this comment

pietroalbini commented Nov 6, 2018

craterbot commented Nov 6, 2018

pietroalbini commented Nov 6, 2018

craterbot commented Nov 6, 2018

craterbot commented Nov 6, 2018

pietroalbini commented Nov 6, 2018

craterbot commented Nov 6, 2018

pietroalbini commented Nov 6, 2018

craterbot commented Nov 6, 2018

nikomatsakis commented Nov 7, 2018

This comment has been minimized.

craterbot commented Nov 7, 2018

scalexm commented Nov 7, 2018

bors commented Nov 7, 2018

alexreg commented Nov 5, 2018 •

edited

Loading