Allow opaque types in trait impl headers and rely on coherence to reject unsound cases #103488

oli-obk · 2022-10-24T15:48:41Z

lcnr

this is still unsound when equating two opaque types with the same DefId during coherence, similar to #102048. With the current setup that might be difficult to fix, though I might be wrong.

We can land this PR and then I can try to write an example and open an issue.

compiler/rustc_middle/src/ty/fast_reject.rs

lcnr · 2022-10-24T16:47:37Z

src/test/ui/impl-trait/recursive-type-alias-impl-trait-declaration-too-subtle-2.rs

    fn eq(&self, _other: &(Foo, i32)) -> bool {
        true
    }
 }

 fn foo() -> Foo {
+    //~^ ERROR can't compare `Bar` with `(Bar, i32)`


expected, still weird 😁

oli-obk · 2022-10-27T12:02:34Z

src/test/ui/type-alias-impl-trait/impl_trait_for_same_tait.stderr

+error[E0119]: conflicting implementations of trait `Bop` for type `Bar<()>`
+  --> $DIR/impl_trait_for_same_tait.rs:27:1
+   |
+LL | impl Bop for Bar<()> {}
+   | -------------------- first implementation here
+...
+LL | impl Bop for Barr {
+   | ^^^^^^^^^^^^^^^^^ conflicting implementation for `Bar<()>`


I'm not sure yet how to improve this diagnostic beyond looking at the entries in take_opaque_types() and doing some guessing for common cases.

bors · 2022-10-29T17:48:07Z

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

compiler/rustc_middle/src/ty/relate.rs

src/test/ui/type-alias-impl-trait/impl_trait_for_same_tait.rs

rustbot · 2022-11-03T07:55:15Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

bors · 2022-11-04T04:21:29Z

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

compiler/rustc_infer/src/infer/mod.rs

src/test/ui/type-alias-impl-trait/impl_trait_for_same_tait.rs

compiler/rustc_infer/src/infer/combine.rs

lcnr · 2022-11-14T12:54:39Z

compiler/rustc_infer/src/infer/combine.rs

@@ -809,6 +831,10 @@ impl<'tcx> TypeRelation<'tcx> for ConstInferUnifier<'_, 'tcx> {
        true
    }

+    fn mark_ambiguous(&mut self) {
+        bug!()


that should be reachable when using generic const exprs when relating a const inference var (from a const param) with an unevaluated constant during coherence

compiler/rustc_infer/src/infer/nll_relate/mod.rs

compiler/rustc_infer/src/infer/error_reporting/mod.rs

compiler/rustc_infer/src/infer/glb.rs

compiler/rustc_infer/src/infer/lub.rs

compiler/rustc_infer/src/infer/combine.rs

…ect unsound cases

bors · 2022-11-21T16:05:08Z

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

…ence

…lally allow trait resolution to prove false things during coherence

lcnr

r=me

lcnr · 2022-11-22T13:45:00Z

src/test/ui/type-alias-impl-trait/issue-76202-trait-impl-for-tait.rs

+
+// Ok because `i32` does not implement `Dummy`,
+// so it can't possibly be the hidden type of `F`.
+impl Test for i32 {


how does that work? 🤔

when coherence is checking whether F conflicts with i32 we register i32 as the hidden type of F and that fails because i32: !Dummy but F = impl Dummy.

oli-obk · 2022-11-22T13:54:32Z

@bors r=lcnr

bors · 2022-11-22T13:54:34Z

📌 Commit c16a90f has been approved by lcnr

It is now in the queue for this repository.

Allow opaque types in trait impl headers and rely on coherence to reject unsound cases r? `@lcnr` fixes rust-lang#99840

Allow opaque types in trait impl headers and rely on coherence to reject unsound cases r? ``@lcnr`` fixes rust-lang#99840

Allow opaque types in trait impl headers and rely on coherence to reject unsound cases r? ```@lcnr``` fixes rust-lang#99840

…earth Rollup of 6 pull requests Successful merges: - rust-lang#103488 (Allow opaque types in trait impl headers and rely on coherence to reject unsound cases) - rust-lang#104359 (Refactor must_use lint into two parts) - rust-lang#104612 (Lower return type outside async block creation) - rust-lang#104621 (Fix --extern library finding errors) - rust-lang#104647 (enable fuzzy_provenance_casts lint in liballoc and libstd) - rust-lang#104750 (Bump `fd-lock` in `bootstrap` again) Failed merges: - rust-lang#104732 (Refactor `ty::ClosureKind` related stuff) r? `@ghost` `@rustbot` modify labels: rollup

Mark-Simulacrum · 2022-11-29T13:16:55Z

This PR was a perf regression (see the report here #104758 (comment)). It's relatively small and mostly limited to secondary workloads so I'm marking as triaged.

Allow opaque types in trait impl headers and rely on coherence to reject unsound cases r? ````@lcnr```` fixes rust-lang#99840

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 24, 2022

rust-highfive assigned lcnr Oct 24, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 24, 2022

lcnr reviewed Oct 26, 2022

View reviewed changes

oli-obk force-pushed the impl_trait_for_tait branch from 87bc33e to a41f93a Compare October 27, 2022 12:00

oli-obk commented Oct 27, 2022

View reviewed changes

oli-obk force-pushed the impl_trait_for_tait branch from a41f93a to 86dc770 Compare October 27, 2022 15:18

lcnr reviewed Oct 31, 2022

View reviewed changes

compiler/rustc_middle/src/ty/relate.rs Show resolved Hide resolved

src/test/ui/type-alias-impl-trait/impl_trait_for_same_tait.rs Outdated Show resolved Hide resolved

oli-obk force-pushed the impl_trait_for_tait branch from 86dc770 to 4d7c724 Compare November 2, 2022 15:20

This comment has been minimized.

Sign in to view

oli-obk force-pushed the impl_trait_for_tait branch from 4d7c724 to b8a45f6 Compare November 3, 2022 07:55

oli-obk force-pushed the impl_trait_for_tait branch from b8a45f6 to c67ec50 Compare November 7, 2022 15:21

lcnr reviewed Nov 14, 2022

View reviewed changes

lcnr reviewed Nov 15, 2022

View reviewed changes

compiler/rustc_infer/src/infer/nll_relate/mod.rs Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

oli-obk force-pushed the impl_trait_for_tait branch from 5b9d06f to 1e768de Compare November 15, 2022 16:40

lcnr reviewed Nov 17, 2022

View reviewed changes

compiler/rustc_infer/src/infer/combine.rs Outdated Show resolved Hide resolved

oli-obk force-pushed the impl_trait_for_tait branch from 200226d to 8774fa8 Compare November 17, 2022 10:08

Allow opaque types in trait impl headers and rely on coherence to rej…

2752e32

…ect unsound cases

oli-obk added 6 commits November 21, 2022 16:06

Treat different opaque types of the same def id as equal during coher…

94fe30f

…ence

Add an always-ambiguous predicate to make sure that we don't accident…

ae80c76

…lally allow trait resolution to prove false things during coherence

nit treat different opaque types

9cd44f8

Move a field around

9a8e1ee

Register obligations from type relation

f42e490

Remove a function that doesn't actually do anything

11ae334

oli-obk added 2 commits November 21, 2022 16:38

Type generalization should not look at opaque type in coherence

7301cd7

Add some more assertions for type relations not used during coherence

11adf03

oli-obk force-pushed the impl_trait_for_tait branch from 8774fa8 to 603a83b Compare November 21, 2022 16:44

Test generalization during coherence

c16a90f

oli-obk force-pushed the impl_trait_for_tait branch from 603a83b to c16a90f Compare November 21, 2022 16:47

lcnr reviewed Nov 22, 2022

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 Nov 22, 2022

Manishearth added a commit to Manishearth/rust that referenced this pull request Nov 22, 2022

Rollup merge of rust-lang#103488 - oli-obk:impl_trait_for_tait, r=lcnr

43eb708

Allow opaque types in trait impl headers and rely on coherence to reject unsound cases r? `@lcnr` fixes rust-lang#99840

Manishearth mentioned this pull request Nov 22, 2022

Rollup of 11 pull requests #104738

Closed

Manishearth mentioned this pull request Nov 22, 2022

Rollup of 10 pull requests #104749

Closed

This was referenced Nov 23, 2022

Rollup of 7 pull requests #104756

Closed

Rollup of 6 pull requests #104758

Merged

bors merged commit 53eab24 into rust-lang:master Nov 23, 2022

rustbot added this to the 1.67.0 milestone Nov 23, 2022

oli-obk deleted the impl_trait_for_tait branch November 23, 2022 10:25

compiler-errors mentioned this pull request Nov 24, 2022

ICE: ty_is_local invoked on closure or generator #104817

Closed

Mark-Simulacrum added the perf-regression Performance regression. label Nov 29, 2022

Mark-Simulacrum added the perf-regression-triaged The performance regression has been triaged. label Nov 29, 2022

flip1995 pushed a commit to flip1995/rust that referenced this pull request Dec 1, 2022

Rollup merge of rust-lang#103488 - oli-obk:impl_trait_for_tait, r=lcnr

91d2ce3

Allow opaque types in trait impl headers and rely on coherence to reject unsound cases r? ````@lcnr```` fixes rust-lang#99840

aliemjay mentioned this pull request Apr 2, 2023

remove TypeRelation::{intercrate,mark_ambiguous} #109863

Closed

matthiaskrgr mentioned this pull request Apr 22, 2023

ICE failed to resolve instance for #110696

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow opaque types in trait impl headers and rely on coherence to reject unsound cases #103488

Allow opaque types in trait impl headers and rely on coherence to reject unsound cases #103488

oli-obk commented Oct 24, 2022

lcnr left a comment

lcnr Oct 24, 2022

oli-obk Oct 27, 2022

bors commented Oct 29, 2022

This comment has been minimized.

rustbot commented Nov 3, 2022

bors commented Nov 4, 2022

lcnr Nov 14, 2022

This comment has been minimized.

bors commented Nov 21, 2022

lcnr left a comment

lcnr Nov 22, 2022

oli-obk Nov 22, 2022

oli-obk commented Nov 22, 2022

bors commented Nov 22, 2022

Mark-Simulacrum commented Nov 29, 2022

Allow opaque types in trait impl headers and rely on coherence to reject unsound cases #103488

Allow opaque types in trait impl headers and rely on coherence to reject unsound cases #103488

Conversation

oli-obk commented Oct 24, 2022

lcnr left a comment

Choose a reason for hiding this comment

lcnr Oct 24, 2022

Choose a reason for hiding this comment

oli-obk Oct 27, 2022

Choose a reason for hiding this comment

bors commented Oct 29, 2022

This comment has been minimized.

rustbot commented Nov 3, 2022

bors commented Nov 4, 2022

lcnr Nov 14, 2022

Choose a reason for hiding this comment

This comment has been minimized.

bors commented Nov 21, 2022

lcnr left a comment

Choose a reason for hiding this comment

lcnr Nov 22, 2022

Choose a reason for hiding this comment

oli-obk Nov 22, 2022

Choose a reason for hiding this comment

oli-obk commented Nov 22, 2022

bors commented Nov 22, 2022

Mark-Simulacrum commented Nov 29, 2022