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

Graciously handle Drop impls introducing more generic parameters than the ADT #127220

Merged
merged 2 commits into from
Jul 26, 2024
Merged
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
34 changes: 25 additions & 9 deletions compiler/rustc_hir_analysis/src/check/dropck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ use rustc_data_structures::fx::FxHashSet;
use rustc_errors::{codes::*, struct_span_code_err, ErrorGuaranteed};
use rustc_infer::infer::outlives::env::OutlivesEnvironment;
use rustc_infer::infer::{RegionResolutionError, TyCtxtInferExt};
use rustc_infer::traits::ObligationCauseCode;
use rustc_infer::traits::{ObligationCause, ObligationCauseCode};
use rustc_middle::ty::util::CheckRegions;
use rustc_middle::ty::GenericArgsRef;
use rustc_middle::ty::{self, TyCtxt};
use rustc_middle::ty::{GenericArgsRef, Ty};
use rustc_trait_selection::regions::InferCtxtRegionExt;
use rustc_trait_selection::traits::{self, ObligationCtxt};

Expand Down Expand Up @@ -115,8 +115,9 @@ fn ensure_drop_params_and_item_params_correspond<'tcx>(
Err(err.emit())
}

/// Confirms that every predicate imposed by dtor_predicates is
/// implied by assuming the predicates attached to self_type_did.
/// Confirms that all predicates defined on the `Drop` impl (`drop_impl_def_id`) are able to be
/// proven from within `adt_def_id`'s environment. I.e. all the predicates on the impl are
/// implied by the ADT being well formed.
fn ensure_drop_predicates_are_implied_by_item_defn<'tcx>(
tcx: TyCtxt<'tcx>,
drop_impl_def_id: LocalDefId,
Expand All @@ -126,6 +127,8 @@ fn ensure_drop_predicates_are_implied_by_item_defn<'tcx>(
let infcx = tcx.infer_ctxt().build();
let ocx = ObligationCtxt::new_with_diagnostics(&infcx);

let impl_span = tcx.def_span(drop_impl_def_id.to_def_id());

// Take the param-env of the adt and instantiate the args that show up in
// the implementation's self type. This gives us the assumptions that the
// self ty of the implementation is allowed to know just from it being a
Expand All @@ -135,14 +138,27 @@ fn ensure_drop_predicates_are_implied_by_item_defn<'tcx>(
// We don't need to normalize this param-env or anything, since we're only
// instantiating it with free params, so no additional param-env normalization
// can occur on top of what has been done in the param_env query itself.
let param_env =
//
// Note: Ideally instead of instantiating the `ParamEnv` with the arguments from the impl ty we
// could instead use identity args for the adt. Unfortunately this would cause any errors to
// reference the params from the ADT instead of from the impl which is bad UX. To resolve
// this we "rename" the ADT's params to be the impl's params which should not affect behaviour.
let impl_adt_ty = Ty::new_adt(tcx, tcx.adt_def(adt_def_id), adt_to_impl_args);
let adt_env =
ty::EarlyBinder::bind(tcx.param_env(adt_def_id)).instantiate(tcx, adt_to_impl_args);

for (pred, span) in tcx.predicates_of(drop_impl_def_id).instantiate_identity(tcx) {
let fresh_impl_args = infcx.fresh_args_for_item(impl_span, drop_impl_def_id.to_def_id());
let fresh_adt_ty =
tcx.impl_trait_ref(drop_impl_def_id).unwrap().instantiate(tcx, fresh_impl_args).self_ty();

ocx.eq(&ObligationCause::dummy_with_span(impl_span), adt_env, fresh_adt_ty, impl_adt_ty)
.unwrap();

for (clause, span) in tcx.predicates_of(drop_impl_def_id).instantiate(tcx, fresh_impl_args) {
let normalize_cause = traits::ObligationCause::misc(span, adt_def_id);
let pred = ocx.normalize(&normalize_cause, param_env, pred);
let pred = ocx.normalize(&normalize_cause, adt_env, clause);
let cause = traits::ObligationCause::new(span, adt_def_id, ObligationCauseCode::DropImpl);
ocx.register_obligation(traits::Obligation::new(tcx, cause, param_env, pred));
ocx.register_obligation(traits::Obligation::new(tcx, cause, adt_env, pred));
}

// All of the custom error reporting logic is to preserve parity with the old
Expand Down Expand Up @@ -176,7 +192,7 @@ fn ensure_drop_predicates_are_implied_by_item_defn<'tcx>(
return Err(guar.unwrap());
}

let errors = ocx.infcx.resolve_regions(&OutlivesEnvironment::new(param_env));
let errors = ocx.infcx.resolve_regions(&OutlivesEnvironment::new(adt_env));
if !errors.is_empty() {
let mut guar = None;
for error in errors {
Expand Down
11 changes: 11 additions & 0 deletions tests/ui/dropck/const_drop_is_valid.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#![feature(effects)]
//~^ WARN: the feature `effects` is incomplete

struct A();

impl const Drop for A {}
//~^ ERROR: const trait impls are experimental
//~| const `impl` for trait `Drop` which is not marked with `#[const_trait]`
//~| not all trait items implemented, missing: `drop`

fn main() {}
45 changes: 45 additions & 0 deletions tests/ui/dropck/const_drop_is_valid.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
error[E0658]: const trait impls are experimental
--> $DIR/const_drop_is_valid.rs:6:6
|
LL | impl const Drop for A {}
| ^^^^^
|
= note: see issue #67792 <https://github.com/rust-lang/rust/issues/67792> for more information
= help: add `#![feature(const_trait_impl)]` to the crate attributes to enable
= note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date

warning: the feature `effects` is incomplete and may not be safe to use and/or cause compiler crashes
--> $DIR/const_drop_is_valid.rs:1:12
|
LL | #![feature(effects)]
| ^^^^^^^
|
= note: see issue #102090 <https://github.com/rust-lang/rust/issues/102090> for more information
= note: `#[warn(incomplete_features)]` on by default

error: using `#![feature(effects)]` without enabling next trait solver globally
|
= note: the next trait solver must be enabled globally for the effects feature to work correctly
= help: use `-Znext-solver` to enable

error: const `impl` for trait `Drop` which is not marked with `#[const_trait]`
--> $DIR/const_drop_is_valid.rs:6:12
|
LL | impl const Drop for A {}
| ^^^^
|
= note: marking a trait with `#[const_trait]` ensures all default method bodies are `const`
= note: adding a non-const method body in the future would be a breaking change

error[E0046]: not all trait items implemented, missing: `drop`
--> $DIR/const_drop_is_valid.rs:6:1
|
LL | impl const Drop for A {}
| ^^^^^^^^^^^^^^^^^^^^^ missing `drop` in implementation
|
= help: implement the missing item: `fn drop(&mut self) { todo!() }`

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

Some errors have detailed explanations: E0046, E0658.
For more information about an error, try `rustc --explain E0046`.
13 changes: 13 additions & 0 deletions tests/ui/dropck/constrained_by_assoc_type_equality.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
//@ check-pass

struct Foo<T: Trait>(T);

trait Trait {
type Assoc;
}

impl<T: Trait<Assoc = U>, U: ?Sized> Drop for Foo<T> {
fn drop(&mut self) {}
}

fn main() {}
12 changes: 12 additions & 0 deletions tests/ui/dropck/constrained_by_assoc_type_equality_and_self_ty.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
trait Trait {
type Assoc;
}

struct Foo<T: Trait, U: ?Sized>(T, U);

impl<T: Trait<Assoc = U>, U: ?Sized> Drop for Foo<T, U> {
//~^ ERROR: `Drop` impl requires `<T as Trait>::Assoc == U`
fn drop(&mut self) {}
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
error[E0367]: `Drop` impl requires `<T as Trait>::Assoc == U` but the struct it is implemented for does not
--> $DIR/constrained_by_assoc_type_equality_and_self_ty.rs:7:15
|
LL | impl<T: Trait<Assoc = U>, U: ?Sized> Drop for Foo<T, U> {
| ^^^^^^^^^
|
note: the implementor must specify the same requirement
--> $DIR/constrained_by_assoc_type_equality_and_self_ty.rs:5:1
|
LL | struct Foo<T: Trait, U: ?Sized>(T, U);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0367`.
202 changes: 136 additions & 66 deletions tests/ui/dropck/reject-specialized-drops-8142.rs
Original file line number Diff line number Diff line change
@@ -1,75 +1,145 @@
// Issue 8142: Test that Drop impls cannot be specialized beyond the
// predicates attached to the type definition itself.
trait Bound { fn foo(&self) { } }
struct K<'l1,'l2> { x: &'l1 i8, y: &'l2 u8 }
struct L<'l1,'l2> { x: &'l1 i8, y: &'l2 u8 }
struct M<'m> { x: &'m i8 }
struct N<'n> { x: &'n i8 }
struct O<To> { x: *const To }
struct P<Tp> { x: *const Tp }
struct Q<Tq> { x: *const Tq }
struct R<Tr> { x: *const Tr }
struct S<Ts:Bound> { x: *const Ts }
struct T<'t,Ts:'t> { x: &'t Ts }
trait Bound {
fn foo(&self) {}
}
struct K<'l1, 'l2> {
x: &'l1 i8,
y: &'l2 u8,
}
struct L<'l1, 'l2> {
x: &'l1 i8,
y: &'l2 u8,
}
struct M<'m> {
x: &'m i8,
}
struct N<'n> {
x: &'n i8,
}
struct O<To> {
x: *const To,
}
struct P<Tp> {
x: *const Tp,
}
struct Q<Tq> {
x: *const Tq,
}
struct R<Tr> {
x: *const Tr,
}
struct S<Ts: Bound> {
x: *const Ts,
}
struct T<'t, Ts: 't> {
x: &'t Ts,
}
struct U;
struct V<Tva, Tvb> { x: *const Tva, y: *const Tvb }
struct W<'l1, 'l2> { x: &'l1 i8, y: &'l2 u8 }
struct V<Tva, Tvb> {
x: *const Tva,
y: *const Tvb,
}
struct W<'l1, 'l2> {
x: &'l1 i8,
y: &'l2 u8,
}
struct X<const Ca: usize>;
struct Y<const Ca: usize, const Cb: usize>;

enum Enum<T> { Variant(T) }
enum Enum<T> {
Variant(T),
}
struct TupleStruct<T>(T);
union Union<T: Copy> { f: T }
union Union<T: Copy> {
f: T,
}

impl<'al,'adds_bnd:'al> Drop for K<'al,'adds_bnd> { // REJECT
impl<'al, 'adds_bnd: 'al> Drop for K<'al, 'adds_bnd> {
//~^ ERROR `Drop` impl requires `'adds_bnd: 'al`
fn drop(&mut self) { } }

impl<'al,'adds_bnd> Drop for L<'al,'adds_bnd> where 'adds_bnd:'al { // REJECT
//~^ ERROR `Drop` impl requires `'adds_bnd: 'al`
fn drop(&mut self) { } }

impl<'ml> Drop for M<'ml> { fn drop(&mut self) { } } // ACCEPT

impl Drop for N<'static> { fn drop(&mut self) { } } // REJECT
//~^ ERROR `Drop` impls cannot be specialized

impl<COkNoBound> Drop for O<COkNoBound> { fn drop(&mut self) { } } // ACCEPT

impl Drop for P<i8> { fn drop(&mut self) { } } // REJECT
//~^ ERROR `Drop` impls cannot be specialized

impl<AddsBnd:Bound> Drop for Q<AddsBnd> { fn drop(&mut self) { } } // REJECT
//~^ ERROR `Drop` impl requires `AddsBnd: Bound`

impl<'rbnd,AddsRBnd:'rbnd> Drop for R<AddsRBnd> { fn drop(&mut self) { } } // REJECT
//~^ ERROR `Drop` impl requires `AddsRBnd: 'rbnd`

impl<Bs:Bound> Drop for S<Bs> { fn drop(&mut self) { } } // ACCEPT

impl<'t,Bt:'t> Drop for T<'t,Bt> { fn drop(&mut self) { } } // ACCEPT

impl Drop for U { fn drop(&mut self) { } } // ACCEPT

impl<One> Drop for V<One,One> { fn drop(&mut self) { } } // REJECT
//~^ ERROR `Drop` impls cannot be specialized

impl<'lw> Drop for W<'lw,'lw> { fn drop(&mut self) { } } // REJECT
//~^ ERROR `Drop` impls cannot be specialized

impl Drop for X<3> { fn drop(&mut self) { } } // REJECT
//~^ ERROR `Drop` impls cannot be specialized

impl<const Ca: usize> Drop for Y<Ca, Ca> { fn drop(&mut self) { } } // REJECT
//~^ ERROR `Drop` impls cannot be specialized

impl<AddsBnd:Bound> Drop for Enum<AddsBnd> { fn drop(&mut self) { } } // REJECT
//~^ ERROR `Drop` impl requires `AddsBnd: Bound`

impl<AddsBnd:Bound> Drop for TupleStruct<AddsBnd> { fn drop(&mut self) { } } // REJECT
//~^ ERROR `Drop` impl requires `AddsBnd: Bound`

impl<AddsBnd:Copy + Bound> Drop for Union<AddsBnd> { fn drop(&mut self) { } } // REJECT
//~^ ERROR `Drop` impl requires `AddsBnd: Bound`

pub fn main() { }
fn drop(&mut self) {}
}

impl<'al, 'adds_bnd> Drop for L<'al, 'adds_bnd>
//~^ ERROR `Drop` impl requires `'adds_bnd: 'al`
where
'adds_bnd: 'al,
{
fn drop(&mut self) {}
}

impl<'ml> Drop for M<'ml> {
fn drop(&mut self) {}
}

impl Drop for N<'static> {
//~^ ERROR `Drop` impls cannot be specialized
fn drop(&mut self) {}
}

impl<COkNoBound> Drop for O<COkNoBound> {
fn drop(&mut self) {}
}

impl Drop for P<i8> {
//~^ ERROR `Drop` impls cannot be specialized
fn drop(&mut self) {}
}

impl<AddsBnd: Bound> Drop for Q<AddsBnd> {
//~^ ERROR `Drop` impl requires `AddsBnd: Bound`
fn drop(&mut self) {}
}

impl<'rbnd, AddsRBnd: 'rbnd> Drop for R<AddsRBnd> {
fn drop(&mut self) {}
}

impl<Bs: Bound> Drop for S<Bs> {
fn drop(&mut self) {}
}

impl<'t, Bt: 't> Drop for T<'t, Bt> {
fn drop(&mut self) {}
}

impl Drop for U {
fn drop(&mut self) {}
}

impl<One> Drop for V<One, One> {
//~^ ERROR `Drop` impls cannot be specialized
fn drop(&mut self) {}
}

impl<'lw> Drop for W<'lw, 'lw> {
//~^ ERROR `Drop` impls cannot be specialized
fn drop(&mut self) {}
}

impl Drop for X<3> {
//~^ ERROR `Drop` impls cannot be specialized
fn drop(&mut self) {}
}

impl<const Ca: usize> Drop for Y<Ca, Ca> {
//~^ ERROR `Drop` impls cannot be specialized
fn drop(&mut self) {}
}

impl<AddsBnd: Bound> Drop for Enum<AddsBnd> {
//~^ ERROR `Drop` impl requires `AddsBnd: Bound`
fn drop(&mut self) {}
}

impl<AddsBnd: Bound> Drop for TupleStruct<AddsBnd> {
//~^ ERROR `Drop` impl requires `AddsBnd: Bound`
fn drop(&mut self) {}
}

impl<AddsBnd: Copy + Bound> Drop for Union<AddsBnd> {
//~^ ERROR `Drop` impl requires `AddsBnd: Bound`
fn drop(&mut self) {}
}

pub fn main() {}
Loading
Loading