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

Prevent opaque types in impl headers #87382

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 21 additions & 8 deletions compiler/rustc_typeck/src/coherence/orphan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use rustc_errors::struct_span_err;
use rustc_hir as hir;
use rustc_hir::itemlikevisit::ItemLikeVisitor;
use rustc_infer::infer::TyCtxtInferExt;
use rustc_middle::ty::{self, TyCtxt};
use rustc_middle::ty::{self, fold::BottomUpFolder, TyCtxt, TypeFoldable};
use rustc_trait_selection::traits;

pub fn check(tcx: TyCtxt<'_>) {
Expand Down Expand Up @@ -233,13 +233,26 @@ impl ItemLikeVisitor<'v> for OrphanChecker<'tcx> {
}
}

if let ty::Opaque(def_id, _) = *trait_ref.self_ty().kind() {
self.tcx
.sess
.struct_span_err(sp, "cannot implement trait on type alias impl trait")
.span_note(self.tcx.def_span(def_id), "type alias impl trait defined here")
.emit();
}
// Ensure no opaque types are present in this impl header. See issues #76202 and #86411 for examples,
// and #84660 where it would otherwise allow unsoundness.
trait_ref.substs.fold_with(&mut BottomUpFolder {
tcx: self.tcx,
ty_op: |ty| {
if let ty::Opaque(def_id, _) = *ty.kind() {
self.tcx
.sess
.struct_span_err(sp, "cannot implement trait on type alias impl trait")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have an idea that could help with this span, not sure if it pans out. Create a new visitor struct. Implement intravisit::Visitor for it, most notably implement the Visitor::visit_qpath method and use qpath_res to get the DefId. If it is equal to the opaque type's use that span. If no span is found during visiting, fall back to the one you have right now.

First visit the self_ty variable, and if no span is found, visit the tr variable.

Copy link
Member Author

@lqd lqd Jul 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC qpath_res doesn't work here since we're not in an Expr or Pat node, but the qpath should be resolved so we can extract its res anyway. However, when visiting the self_ty, the qpath's def_id is not the same as the opaque type's here. (It's almost as if the visitor is seeing the type alias node, while the TypeFolder sees the opaque type node)

.span_note(
self.tcx.def_span(def_id),
"type alias impl trait defined here",
)
.emit();
}
ty
},
lt_op: |lt| lt,
ct_op: |ct| ct,
});
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
14 changes: 13 additions & 1 deletion src/test/ui/impl-trait/auto-trait.full_tait.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,18 @@ LL | impl<T: Send> AnotherTrait for T {}
LL | impl AnotherTrait for D<OpaqueType> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `D<impl OpaqueTrait>`

error: aborting due to previous error; 1 warning emitted
error: cannot implement trait on type alias impl trait
--> $DIR/auto-trait.rs:24:1
|
LL | impl AnotherTrait for D<OpaqueType> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: type alias impl trait defined here
--> $DIR/auto-trait.rs:10:19
|
LL | type OpaqueType = impl OpaqueTrait;
| ^^^^^^^^^^^^^^^^

error: aborting due to 2 previous errors; 1 warning emitted

For more information about this error, try `rustc --explain E0119`.
14 changes: 13 additions & 1 deletion src/test/ui/impl-trait/auto-trait.min_tait.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,18 @@ LL | impl<T: Send> AnotherTrait for T {}
LL | impl AnotherTrait for D<OpaqueType> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `D<impl OpaqueTrait>`

error: aborting due to previous error
error: cannot implement trait on type alias impl trait
--> $DIR/auto-trait.rs:24:1
|
LL | impl AnotherTrait for D<OpaqueType> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: type alias impl trait defined here
--> $DIR/auto-trait.rs:10:19
|
LL | type OpaqueType = impl OpaqueTrait;
| ^^^^^^^^^^^^^^^^

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0119`.
2 changes: 1 addition & 1 deletion src/test/ui/impl-trait/auto-trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ impl<T: Send> AnotherTrait for T {}
// This is in error, because we cannot assume that `OpaqueType: !Send`.
// (We treat opaque types as "foreign types" that could grow more impls
// in the future.)
impl AnotherTrait for D<OpaqueType> {
impl AnotherTrait for D<OpaqueType> { //~ ERROR cannot implement trait
//~^ ERROR conflicting implementations of trait `AnotherTrait` for type `D<impl OpaqueTrait>`
}

Expand Down
14 changes: 13 additions & 1 deletion src/test/ui/impl-trait/negative-reasoning.full_tait.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,18 @@ LL | impl AnotherTrait for D<OpaqueType> {
|
= note: upstream crates may add a new impl of trait `std::fmt::Debug` for type `impl OpaqueTrait` in future versions

error: aborting due to previous error; 1 warning emitted
error: cannot implement trait on type alias impl trait
--> $DIR/negative-reasoning.rs:22:1
|
LL | impl AnotherTrait for D<OpaqueType> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: type alias impl trait defined here
--> $DIR/negative-reasoning.rs:10:19
|
LL | type OpaqueType = impl OpaqueTrait;
| ^^^^^^^^^^^^^^^^

error: aborting due to 2 previous errors; 1 warning emitted

For more information about this error, try `rustc --explain E0119`.
14 changes: 13 additions & 1 deletion src/test/ui/impl-trait/negative-reasoning.min_tait.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,18 @@ LL | impl AnotherTrait for D<OpaqueType> {
|
= note: upstream crates may add a new impl of trait `std::fmt::Debug` for type `impl OpaqueTrait` in future versions

error: aborting due to previous error
error: cannot implement trait on type alias impl trait
--> $DIR/negative-reasoning.rs:22:1
|
LL | impl AnotherTrait for D<OpaqueType> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: type alias impl trait defined here
--> $DIR/negative-reasoning.rs:10:19
|
LL | type OpaqueType = impl OpaqueTrait;
| ^^^^^^^^^^^^^^^^

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0119`.
2 changes: 1 addition & 1 deletion src/test/ui/impl-trait/negative-reasoning.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ trait AnotherTrait {}
impl<T: std::fmt::Debug> AnotherTrait for T {}

// This is in error, because we cannot assume that `OpaqueType: !Debug`
impl AnotherTrait for D<OpaqueType> {
impl AnotherTrait for D<OpaqueType> { //~ ERROR cannot implement trait
//~^ ERROR conflicting implementations of trait `AnotherTrait` for type `D<impl OpaqueTrait>`
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Regression test for issues #84660 and #86411: both are variations on #76202.
// Tests that we don't ICE when we have an opaque type appearing anywhere in an impl header.

#![feature(min_type_alias_impl_trait)]

trait Foo {}
impl Foo for () {}
type Bar = impl Foo;
fn _defining_use() -> Bar {}

trait TraitArg<T> {
fn f();
}

impl TraitArg<Bar> for () { //~ ERROR cannot implement trait
fn f() {
println!("ho");
}
}

fn main() {
<() as TraitArg<Bar>>::f();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
error: cannot implement trait on type alias impl trait
--> $DIR/issue-84660-trait-impl-for-tait.rs:15:1
|
LL | impl TraitArg<Bar> for () {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: type alias impl trait defined here
--> $DIR/issue-84660-trait-impl-for-tait.rs:8:12
|
LL | type Bar = impl Foo;
| ^^^^^^^^

error: aborting due to previous error

41 changes: 41 additions & 0 deletions src/test/ui/type-alias-impl-trait/issue-84660-unsoundness.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Another example from issue #84660, this time weaponized as a safe transmut: an opaque type in an
// impl header being accepted was used to create unsoundness.

#![feature(min_type_alias_impl_trait)]

trait Foo {}
impl Foo for () {}
type Bar = impl Foo;
fn _defining_use() -> Bar {}

trait Trait<T, In> {
type Out;
fn convert(i: In) -> Self::Out;
}

impl<In, Out> Trait<Bar, In> for Out { //~ ERROR cannot implement trait
type Out = Out;
fn convert(_i: In) -> Self::Out {
unreachable!();
}
}

impl<In, Out> Trait<(), In> for Out {
type Out = In;
fn convert(i: In) -> Self::Out {
i
}
}

fn transmute<In, Out>(i: In) -> Out {
<Out as Trait<Bar, In>>::convert(i)
}

fn main() {
let d;
{
let x = "Hello World".to_string();
d = transmute::<&String, &String>(&x);
}
println!("{}", d);
}
14 changes: 14 additions & 0 deletions src/test/ui/type-alias-impl-trait/issue-84660-unsoundness.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
error: cannot implement trait on type alias impl trait
--> $DIR/issue-84660-unsoundness.rs:16:1
|
LL | impl<In, Out> Trait<Bar, In> for Out {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: type alias impl trait defined here
--> $DIR/issue-84660-unsoundness.rs:8:12
|
LL | type Bar = impl Foo;
| ^^^^^^^^

error: aborting due to previous error