Skip to content

Commit d93ed60

Browse files
authored
Unrolled build for rust-lang#127220
Rollup merge of rust-lang#127220 - BoxyUwU:dropck_handle_extra_impl_params, r=compiler-errors Graciously handle `Drop` impls introducing more generic parameters than the ADT Follow up to rust-lang#110577 Fixes rust-lang#126378 Fixes rust-lang#126889 ## Motivation A current issue with the way we check drop impls do not specialize any of their generic parameters is that when the `Drop` impl introduces *more* generic parameters than are present on the ADT, we fail to prove any bounds involving those parameters. This can be demonstrated with the following [code on stable](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=139b65e4294634d7286a3282bc61e628) which fails due to the fact that `<T as Trait>::Assoc == U` is not present in `Foo`s `ParamEnv` even though arguably there is no reason it cannot compiler: ```rust struct Foo<T: Trait>(T); trait Trait { type Assoc; } impl<T: Trait<Assoc = U>, U: ?Sized> Drop for Foo<T> { //~^ ERROR: `Drop` impl requires `<T as Trait>::Assoc == U` but the struct ... fn drop(&mut self) {} } fn main() {} ``` I think the motivation for supporting this code is somewhat lacking, it might be useful in practice for deeply nested associated types where you might want to be able to write: `where T: Trait<Assoc: Other<AnotherAssoc: MoreTrait<YetAnotherAssoc: InnerTrait<Final = U>>>>` in order to be able to just use `U` in the function body instead of writing out the whole nested associated type. Regardless I don't think there is really any reason to *not* support this code and it is relatively easy to support it. What I find slightly more compelling is the fact that when defining a const parameter `const N: u8` we desugar that to having a where clause requiring the constant `N` is typed as `u8` (`ClauseKind::ConstArgHasType`). As we *always* desugar const parameters to have these bounds, if we attempt to prove that some const parameter `N` is of type `u8` and there is no bound on `N` in the enviroment that generally indicates usage of an incorrect `ParamEnv` (this has caught a bug already). Given that, if we write the following code: ```rust #![feature(associated_const_equality)] struct Foo<T: Trait>(T); trait Trait { const ASSOC: usize; } impl<T: Trait<ASSOC = N>, const N: usize> Drop for Foo<T> { fn drop(&mut self) {} } fn main() {} ``` The `Drop` impl would have this desugared where clause about `N` being of type `usize`, and if we were to try to prove that where clause in `Foo`'s `ParamEnv` we would ICE as there would not be any `ConstArgHasType` in the environment (which generally indicates improper `ParamEnv` usage. As this is otherwise well formed code (the `T: Trait<ASSOC = N>` causes `N` to be constrained) we have to handle this *somehow* and I believe the only principled way to support this is the changes I have made to `dropck.rs` that would cause these code examples to compiler (Perhaps we could just throw out all `ConstArgHasType` where clauses from the predicates we prove but that makes me nervous even if it might actually be okay). ## The changes Currently the way `dropck.rs` works is that take the `ParamEnv` of the ADT and instantiate it with the generic arguments used on the self ty of the `impl`. We then instantiate the predicates of the drop impl with the identity params to the impl, e.g. in the original example `<T as Trait>::Assoc == U` stays as `<T as Trait>::Assoc == U`. We then attempt to prove all the where clauses in the instantiated env of the self type ADT. This PR changes us to first instantiate the impl with infer vars, then we equate the self type (with infer vars as its generic arguments) with the self type as written by the user. This causes all generic parameters on the impl that are constrained via associated type/const equality bounds to be left as inference variables while all other parameters are still `Ty`/`Const`/`Region` Finally when instantiating the predicates on the impl, instead of using the identity arguments, we use the list of inference variables of which some have been inferred to the impl parameters. In practice this means that we wind up proving `<T as Trait>::Assoc == ?x` which can succeed just fine. In the const generics example we would wind up trying to prove `ConstArgHasType(?x: usize)` instead of `ConstArgHasType(N: usize)` which avoids the ICE as it is expected to encounter goals of the form `?x: usize`. At a higher level the way I justify/think about this is that as we are proving goals in the environment of the ADT (`Foo` in the above examples), we do not expect to encounter generic parameters from a different environment so we must "deal with them" somehow. In this PR we handle them by replacing them with inference variables as they should either *actually* be unconstrained (and we will error later) or they are constrained to be equal to some associated type/const. To go along with this it would be nice if we were not instantiating the adt's env with the generic arguments to the ADT in the `Drop` impl as it would make it clearer we are proving bounds in the adt's env instead of the `Drop` impl's. Instead we would map the predicates on the drop impl to be valid in the environment of the adt. In practice this causes diagnostic regressions as all of the generic parameters in errors refer to the ones defined on the adt; attempting to map these back to the ones on the impl, while possible, is involved as writing a `TypeFolder` over `FulfillmentError` is non trivial. ## Edge cases There are some subtle interactions here: One is that we should not allow `<T as Trait>::Assoc == U` to be present on the `Drop` if `U` is constrained by the self type of the impl and the bound is not present in the ADT's environment. demonstrated with the [following code](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=af839e2c3e43e03a624825c58af84dff): ```rust 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() {} ``` This is tested at `tests/ui/dropck/constrained_by_assoc_type_equality_and_self_ty.rs`. Another weirdness is that we permit the following code to compile now: ```rust struct Foo<T>(T); impl<'a, T: 'a> Drop for Foo<T> { fn drop(&mut self) {} } ``` This is caused by the fact that we permit unconstrained lifetime parameters in trait implementations as long as they are not used in associated types (so we do not wind up erroring on this code like we perhaps ought to), combined with the fact that as we are now proving `T: '?x` instead of `T: 'a` which allows proving the bound via `'?x= 'empty` wheras previously it would have failed. This is tested as part of `tests/ui/dropck/reject-specialized-drops-8142.rs`. --- r? `@compiler-errors`
2 parents 2f26b2a + 6a00008 commit d93ed60

12 files changed

+358
-150
lines changed

compiler/rustc_hir_analysis/src/check/dropck.rs

+25-9
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@ use rustc_data_structures::fx::FxHashSet;
66
use rustc_errors::{codes::*, struct_span_code_err, ErrorGuaranteed};
77
use rustc_infer::infer::outlives::env::OutlivesEnvironment;
88
use rustc_infer::infer::{RegionResolutionError, TyCtxtInferExt};
9-
use rustc_infer::traits::ObligationCauseCode;
9+
use rustc_infer::traits::{ObligationCause, ObligationCauseCode};
1010
use rustc_middle::ty::util::CheckRegions;
11-
use rustc_middle::ty::GenericArgsRef;
1211
use rustc_middle::ty::{self, TyCtxt};
12+
use rustc_middle::ty::{GenericArgsRef, Ty};
1313
use rustc_trait_selection::regions::InferCtxtRegionExt;
1414
use rustc_trait_selection::traits::{self, ObligationCtxt};
1515

@@ -115,8 +115,9 @@ fn ensure_drop_params_and_item_params_correspond<'tcx>(
115115
Err(err.emit())
116116
}
117117

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

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

141-
for (pred, span) in tcx.predicates_of(drop_impl_def_id).instantiate_identity(tcx) {
150+
let fresh_impl_args = infcx.fresh_args_for_item(impl_span, drop_impl_def_id.to_def_id());
151+
let fresh_adt_ty =
152+
tcx.impl_trait_ref(drop_impl_def_id).unwrap().instantiate(tcx, fresh_impl_args).self_ty();
153+
154+
ocx.eq(&ObligationCause::dummy_with_span(impl_span), adt_env, fresh_adt_ty, impl_adt_ty)
155+
.unwrap();
156+
157+
for (clause, span) in tcx.predicates_of(drop_impl_def_id).instantiate(tcx, fresh_impl_args) {
142158
let normalize_cause = traits::ObligationCause::misc(span, adt_def_id);
143-
let pred = ocx.normalize(&normalize_cause, param_env, pred);
159+
let pred = ocx.normalize(&normalize_cause, adt_env, clause);
144160
let cause = traits::ObligationCause::new(span, adt_def_id, ObligationCauseCode::DropImpl);
145-
ocx.register_obligation(traits::Obligation::new(tcx, cause, param_env, pred));
161+
ocx.register_obligation(traits::Obligation::new(tcx, cause, adt_env, pred));
146162
}
147163

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

179-
let errors = ocx.infcx.resolve_regions(&OutlivesEnvironment::new(param_env));
195+
let errors = ocx.infcx.resolve_regions(&OutlivesEnvironment::new(adt_env));
180196
if !errors.is_empty() {
181197
let mut guar = None;
182198
for error in errors {
+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
#![feature(effects)]
2+
//~^ WARN: the feature `effects` is incomplete
3+
4+
struct A();
5+
6+
impl const Drop for A {}
7+
//~^ ERROR: const trait impls are experimental
8+
//~| const `impl` for trait `Drop` which is not marked with `#[const_trait]`
9+
//~| not all trait items implemented, missing: `drop`
10+
11+
fn main() {}
+45
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
error[E0658]: const trait impls are experimental
2+
--> $DIR/const_drop_is_valid.rs:6:6
3+
|
4+
LL | impl const Drop for A {}
5+
| ^^^^^
6+
|
7+
= note: see issue #67792 <https://github.com/rust-lang/rust/issues/67792> for more information
8+
= help: add `#![feature(const_trait_impl)]` to the crate attributes to enable
9+
= note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date
10+
11+
warning: the feature `effects` is incomplete and may not be safe to use and/or cause compiler crashes
12+
--> $DIR/const_drop_is_valid.rs:1:12
13+
|
14+
LL | #![feature(effects)]
15+
| ^^^^^^^
16+
|
17+
= note: see issue #102090 <https://github.com/rust-lang/rust/issues/102090> for more information
18+
= note: `#[warn(incomplete_features)]` on by default
19+
20+
error: using `#![feature(effects)]` without enabling next trait solver globally
21+
|
22+
= note: the next trait solver must be enabled globally for the effects feature to work correctly
23+
= help: use `-Znext-solver` to enable
24+
25+
error: const `impl` for trait `Drop` which is not marked with `#[const_trait]`
26+
--> $DIR/const_drop_is_valid.rs:6:12
27+
|
28+
LL | impl const Drop for A {}
29+
| ^^^^
30+
|
31+
= note: marking a trait with `#[const_trait]` ensures all default method bodies are `const`
32+
= note: adding a non-const method body in the future would be a breaking change
33+
34+
error[E0046]: not all trait items implemented, missing: `drop`
35+
--> $DIR/const_drop_is_valid.rs:6:1
36+
|
37+
LL | impl const Drop for A {}
38+
| ^^^^^^^^^^^^^^^^^^^^^ missing `drop` in implementation
39+
|
40+
= help: implement the missing item: `fn drop(&mut self) { todo!() }`
41+
42+
error: aborting due to 4 previous errors; 1 warning emitted
43+
44+
Some errors have detailed explanations: E0046, E0658.
45+
For more information about an error, try `rustc --explain E0046`.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
//@ check-pass
2+
3+
struct Foo<T: Trait>(T);
4+
5+
trait Trait {
6+
type Assoc;
7+
}
8+
9+
impl<T: Trait<Assoc = U>, U: ?Sized> Drop for Foo<T> {
10+
fn drop(&mut self) {}
11+
}
12+
13+
fn main() {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
trait Trait {
2+
type Assoc;
3+
}
4+
5+
struct Foo<T: Trait, U: ?Sized>(T, U);
6+
7+
impl<T: Trait<Assoc = U>, U: ?Sized> Drop for Foo<T, U> {
8+
//~^ ERROR: `Drop` impl requires `<T as Trait>::Assoc == U`
9+
fn drop(&mut self) {}
10+
}
11+
12+
fn main() {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
error[E0367]: `Drop` impl requires `<T as Trait>::Assoc == U` but the struct it is implemented for does not
2+
--> $DIR/constrained_by_assoc_type_equality_and_self_ty.rs:7:15
3+
|
4+
LL | impl<T: Trait<Assoc = U>, U: ?Sized> Drop for Foo<T, U> {
5+
| ^^^^^^^^^
6+
|
7+
note: the implementor must specify the same requirement
8+
--> $DIR/constrained_by_assoc_type_equality_and_self_ty.rs:5:1
9+
|
10+
LL | struct Foo<T: Trait, U: ?Sized>(T, U);
11+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
12+
13+
error: aborting due to 1 previous error
14+
15+
For more information about this error, try `rustc --explain E0367`.
+136-66
Original file line numberDiff line numberDiff line change
@@ -1,75 +1,145 @@
11
// Issue 8142: Test that Drop impls cannot be specialized beyond the
22
// predicates attached to the type definition itself.
3-
trait Bound { fn foo(&self) { } }
4-
struct K<'l1,'l2> { x: &'l1 i8, y: &'l2 u8 }
5-
struct L<'l1,'l2> { x: &'l1 i8, y: &'l2 u8 }
6-
struct M<'m> { x: &'m i8 }
7-
struct N<'n> { x: &'n i8 }
8-
struct O<To> { x: *const To }
9-
struct P<Tp> { x: *const Tp }
10-
struct Q<Tq> { x: *const Tq }
11-
struct R<Tr> { x: *const Tr }
12-
struct S<Ts:Bound> { x: *const Ts }
13-
struct T<'t,Ts:'t> { x: &'t Ts }
3+
trait Bound {
4+
fn foo(&self) {}
5+
}
6+
struct K<'l1, 'l2> {
7+
x: &'l1 i8,
8+
y: &'l2 u8,
9+
}
10+
struct L<'l1, 'l2> {
11+
x: &'l1 i8,
12+
y: &'l2 u8,
13+
}
14+
struct M<'m> {
15+
x: &'m i8,
16+
}
17+
struct N<'n> {
18+
x: &'n i8,
19+
}
20+
struct O<To> {
21+
x: *const To,
22+
}
23+
struct P<Tp> {
24+
x: *const Tp,
25+
}
26+
struct Q<Tq> {
27+
x: *const Tq,
28+
}
29+
struct R<Tr> {
30+
x: *const Tr,
31+
}
32+
struct S<Ts: Bound> {
33+
x: *const Ts,
34+
}
35+
struct T<'t, Ts: 't> {
36+
x: &'t Ts,
37+
}
1438
struct U;
15-
struct V<Tva, Tvb> { x: *const Tva, y: *const Tvb }
16-
struct W<'l1, 'l2> { x: &'l1 i8, y: &'l2 u8 }
39+
struct V<Tva, Tvb> {
40+
x: *const Tva,
41+
y: *const Tvb,
42+
}
43+
struct W<'l1, 'l2> {
44+
x: &'l1 i8,
45+
y: &'l2 u8,
46+
}
1747
struct X<const Ca: usize>;
1848
struct Y<const Ca: usize, const Cb: usize>;
1949

20-
enum Enum<T> { Variant(T) }
50+
enum Enum<T> {
51+
Variant(T),
52+
}
2153
struct TupleStruct<T>(T);
22-
union Union<T: Copy> { f: T }
54+
union Union<T: Copy> {
55+
f: T,
56+
}
2357

24-
impl<'al,'adds_bnd:'al> Drop for K<'al,'adds_bnd> { // REJECT
58+
impl<'al, 'adds_bnd: 'al> Drop for K<'al, 'adds_bnd> {
2559
//~^ ERROR `Drop` impl requires `'adds_bnd: 'al`
26-
fn drop(&mut self) { } }
27-
28-
impl<'al,'adds_bnd> Drop for L<'al,'adds_bnd> where 'adds_bnd:'al { // REJECT
29-
//~^ ERROR `Drop` impl requires `'adds_bnd: 'al`
30-
fn drop(&mut self) { } }
31-
32-
impl<'ml> Drop for M<'ml> { fn drop(&mut self) { } } // ACCEPT
33-
34-
impl Drop for N<'static> { fn drop(&mut self) { } } // REJECT
35-
//~^ ERROR `Drop` impls cannot be specialized
36-
37-
impl<COkNoBound> Drop for O<COkNoBound> { fn drop(&mut self) { } } // ACCEPT
38-
39-
impl Drop for P<i8> { fn drop(&mut self) { } } // REJECT
40-
//~^ ERROR `Drop` impls cannot be specialized
41-
42-
impl<AddsBnd:Bound> Drop for Q<AddsBnd> { fn drop(&mut self) { } } // REJECT
43-
//~^ ERROR `Drop` impl requires `AddsBnd: Bound`
44-
45-
impl<'rbnd,AddsRBnd:'rbnd> Drop for R<AddsRBnd> { fn drop(&mut self) { } } // REJECT
46-
//~^ ERROR `Drop` impl requires `AddsRBnd: 'rbnd`
47-
48-
impl<Bs:Bound> Drop for S<Bs> { fn drop(&mut self) { } } // ACCEPT
49-
50-
impl<'t,Bt:'t> Drop for T<'t,Bt> { fn drop(&mut self) { } } // ACCEPT
51-
52-
impl Drop for U { fn drop(&mut self) { } } // ACCEPT
53-
54-
impl<One> Drop for V<One,One> { fn drop(&mut self) { } } // REJECT
55-
//~^ ERROR `Drop` impls cannot be specialized
56-
57-
impl<'lw> Drop for W<'lw,'lw> { fn drop(&mut self) { } } // REJECT
58-
//~^ ERROR `Drop` impls cannot be specialized
59-
60-
impl Drop for X<3> { fn drop(&mut self) { } } // REJECT
61-
//~^ ERROR `Drop` impls cannot be specialized
62-
63-
impl<const Ca: usize> Drop for Y<Ca, Ca> { fn drop(&mut self) { } } // REJECT
64-
//~^ ERROR `Drop` impls cannot be specialized
65-
66-
impl<AddsBnd:Bound> Drop for Enum<AddsBnd> { fn drop(&mut self) { } } // REJECT
67-
//~^ ERROR `Drop` impl requires `AddsBnd: Bound`
68-
69-
impl<AddsBnd:Bound> Drop for TupleStruct<AddsBnd> { fn drop(&mut self) { } } // REJECT
70-
//~^ ERROR `Drop` impl requires `AddsBnd: Bound`
71-
72-
impl<AddsBnd:Copy + Bound> Drop for Union<AddsBnd> { fn drop(&mut self) { } } // REJECT
73-
//~^ ERROR `Drop` impl requires `AddsBnd: Bound`
74-
75-
pub fn main() { }
60+
fn drop(&mut self) {}
61+
}
62+
63+
impl<'al, 'adds_bnd> Drop for L<'al, 'adds_bnd>
64+
//~^ ERROR `Drop` impl requires `'adds_bnd: 'al`
65+
where
66+
'adds_bnd: 'al,
67+
{
68+
fn drop(&mut self) {}
69+
}
70+
71+
impl<'ml> Drop for M<'ml> {
72+
fn drop(&mut self) {}
73+
}
74+
75+
impl Drop for N<'static> {
76+
//~^ ERROR `Drop` impls cannot be specialized
77+
fn drop(&mut self) {}
78+
}
79+
80+
impl<COkNoBound> Drop for O<COkNoBound> {
81+
fn drop(&mut self) {}
82+
}
83+
84+
impl Drop for P<i8> {
85+
//~^ ERROR `Drop` impls cannot be specialized
86+
fn drop(&mut self) {}
87+
}
88+
89+
impl<AddsBnd: Bound> Drop for Q<AddsBnd> {
90+
//~^ ERROR `Drop` impl requires `AddsBnd: Bound`
91+
fn drop(&mut self) {}
92+
}
93+
94+
impl<'rbnd, AddsRBnd: 'rbnd> Drop for R<AddsRBnd> {
95+
fn drop(&mut self) {}
96+
}
97+
98+
impl<Bs: Bound> Drop for S<Bs> {
99+
fn drop(&mut self) {}
100+
}
101+
102+
impl<'t, Bt: 't> Drop for T<'t, Bt> {
103+
fn drop(&mut self) {}
104+
}
105+
106+
impl Drop for U {
107+
fn drop(&mut self) {}
108+
}
109+
110+
impl<One> Drop for V<One, One> {
111+
//~^ ERROR `Drop` impls cannot be specialized
112+
fn drop(&mut self) {}
113+
}
114+
115+
impl<'lw> Drop for W<'lw, 'lw> {
116+
//~^ ERROR `Drop` impls cannot be specialized
117+
fn drop(&mut self) {}
118+
}
119+
120+
impl Drop for X<3> {
121+
//~^ ERROR `Drop` impls cannot be specialized
122+
fn drop(&mut self) {}
123+
}
124+
125+
impl<const Ca: usize> Drop for Y<Ca, Ca> {
126+
//~^ ERROR `Drop` impls cannot be specialized
127+
fn drop(&mut self) {}
128+
}
129+
130+
impl<AddsBnd: Bound> Drop for Enum<AddsBnd> {
131+
//~^ ERROR `Drop` impl requires `AddsBnd: Bound`
132+
fn drop(&mut self) {}
133+
}
134+
135+
impl<AddsBnd: Bound> Drop for TupleStruct<AddsBnd> {
136+
//~^ ERROR `Drop` impl requires `AddsBnd: Bound`
137+
fn drop(&mut self) {}
138+
}
139+
140+
impl<AddsBnd: Copy + Bound> Drop for Union<AddsBnd> {
141+
//~^ ERROR `Drop` impl requires `AddsBnd: Bound`
142+
fn drop(&mut self) {}
143+
}
144+
145+
pub fn main() {}

0 commit comments

Comments
 (0)