Skip to content

Commit aaaed98

Browse files
oli-obkoli-obk
committed
Reject unconstrained lifetimes in type_of(assoc_ty) instead of during wfcheck of the impl item
1 parent 931f6d2 commit aaaed98

31 files changed

+310
-212
lines changed

‎compiler/rustc_hir_analysis/messages.ftl

+1
Original file line numberDiff line numberDiff line change
@@ -523,6 +523,7 @@ hir_analysis_unconstrained_generic_parameter = the {$param_def_kind} `{$param_na
523523
.label = unconstrained {$param_def_kind}
524524
.const_param_note = expressions using a const parameter must map each value to a distinct output value
525525
.const_param_note2 = proving the result of expressions other than the parameter are unique is not supported
526+
.help = this use of an otherwise unconstrained lifetime is not allowed
526527
527528
hir_analysis_unconstrained_opaque_type = unconstrained opaque type
528529
.note = `{$name}` must be used in combination with a concrete type within the same {$what}

‎compiler/rustc_hir_analysis/src/collect.rs

+87-7
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,11 @@
1616
1717
use rustc_ast::Recovered;
1818
use rustc_data_structures::captures::Captures;
19-
use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
19+
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap};
2020
use rustc_data_structures::unord::UnordMap;
2121
use rustc_errors::{
22-
struct_span_code_err, Applicability, Diag, DiagCtxtHandle, ErrorGuaranteed, StashKey, E0228,
22+
struct_span_code_err, Applicability, Diag, DiagCtxtHandle, ErrorGuaranteed, StashKey, E0207,
23+
E0228,
2324
};
2425
use rustc_hir::def::DefKind;
2526
use rustc_hir::def_id::{DefId, LocalDefId};
@@ -31,7 +32,9 @@ use rustc_infer::traits::ObligationCause;
3132
use rustc_middle::hir::nested_filter;
3233
use rustc_middle::query::Providers;
3334
use rustc_middle::ty::util::{Discr, IntTypeExt};
34-
use rustc_middle::ty::{self, AdtKind, Const, IsSuggestable, Ty, TyCtxt, Upcast};
35+
use rustc_middle::ty::{
36+
self, AdtKind, Const, IsSuggestable, Ty, TyCtxt, TypeVisitableExt as _, Upcast,
37+
};
3538
use rustc_middle::{bug, span_bug};
3639
use rustc_span::symbol::{kw, sym, Ident, Symbol};
3740
use rustc_span::{Span, DUMMY_SP};
@@ -40,11 +43,13 @@ use rustc_trait_selection::error_reporting::traits::suggestions::NextTypeParamNa
4043
use rustc_trait_selection::infer::InferCtxtExt;
4144
use rustc_trait_selection::traits::ObligationCtxt;
4245
use std::cell::Cell;
46+
use std::cell::RefCell;
4347
use std::iter;
4448
use std::ops::Bound;
4549

4650
use crate::check::intrinsic::intrinsic_operation_unsafety;
47-
use crate::errors;
51+
use crate::constrained_generic_params::{self as cgp, Parameter};
52+
use crate::errors::{self, UnconstrainedGenericParameter};
4853
use crate::hir_ty_lowering::{HirTyLowerer, RegionInferReason};
4954

5055
pub(crate) mod dump;
@@ -121,6 +126,7 @@ pub struct ItemCtxt<'tcx> {
121126
tcx: TyCtxt<'tcx>,
122127
item_def_id: LocalDefId,
123128
tainted_by_errors: Cell<Option<ErrorGuaranteed>>,
129+
pub forbidden_params: RefCell<FxHashMap<Parameter, ty::GenericParamDef>>,
124130
}
125131

126132
///////////////////////////////////////////////////////////////////////////
@@ -351,7 +357,12 @@ fn bad_placeholder<'cx, 'tcx>(
351357

352358
impl<'tcx> ItemCtxt<'tcx> {
353359
pub fn new(tcx: TyCtxt<'tcx>, item_def_id: LocalDefId) -> ItemCtxt<'tcx> {
354-
ItemCtxt { tcx, item_def_id, tainted_by_errors: Cell::new(None) }
360+
ItemCtxt {
361+
tcx,
362+
item_def_id,
363+
tainted_by_errors: Cell::new(None),
364+
forbidden_params: Default::default(),
365+
}
355366
}
356367

357368
pub fn lower_ty(&self, hir_ty: &hir::Ty<'tcx>) -> Ty<'tcx> {
@@ -372,6 +383,58 @@ impl<'tcx> ItemCtxt<'tcx> {
372383
None => Ok(()),
373384
}
374385
}
386+
387+
fn forbid_unconstrained_lifetime_params_from_parent_impl(&self) -> Result<(), ErrorGuaranteed> {
388+
let tcx = self.tcx;
389+
let impl_def_id = tcx.hir().get_parent_item(self.hir_id());
390+
let impl_self_ty = tcx.type_of(impl_def_id).instantiate_identity();
391+
impl_self_ty.error_reported()?;
392+
let impl_generics = tcx.generics_of(impl_def_id);
393+
let impl_predicates = tcx.predicates_of(impl_def_id);
394+
let impl_trait_ref =
395+
tcx.impl_trait_ref(impl_def_id).map(ty::EarlyBinder::instantiate_identity);
396+
397+
impl_trait_ref.error_reported()?;
398+
399+
let mut input_parameters = cgp::parameters_for_impl(tcx, impl_self_ty, impl_trait_ref);
400+
401+
cgp::identify_constrained_generic_params(
402+
tcx,
403+
impl_predicates,
404+
impl_trait_ref,
405+
&mut input_parameters,
406+
);
407+
408+
for param in &impl_generics.own_params {
409+
let p = match param.kind {
410+
// This is a horrible concession to reality. It'd be
411+
// better to just ban unconstrained lifetimes outright, but in
412+
// practice people do non-hygienic macros like:
413+
//
414+
// ```
415+
// macro_rules! __impl_slice_eq1 {
416+
// ($Lhs: ty, $Rhs: ty, $Bound: ident) => {
417+
// impl<'a, 'b, A: $Bound, B> PartialEq<$Rhs> for $Lhs where A: PartialEq<B> {
418+
// ....
419+
// }
420+
// }
421+
// }
422+
// ```
423+
//
424+
// In a concession to backwards compatibility, we continue to
425+
// permit those, so long as the lifetimes aren't used in
426+
// associated types. This is sound, because lifetimes
427+
// used elsewhere are not projected back out.
428+
ty::GenericParamDefKind::Type { .. } => continue,
429+
ty::GenericParamDefKind::Lifetime => param.to_early_bound_region_data().into(),
430+
ty::GenericParamDefKind::Const { .. } => continue,
431+
};
432+
if !input_parameters.contains(&p) {
433+
self.forbidden_params.borrow_mut().insert(p, param.clone());
434+
}
435+
}
436+
Ok(())
437+
}
375438
}
376439

377440
impl<'tcx> HirTyLowerer<'tcx> for ItemCtxt<'tcx> {
@@ -513,8 +576,25 @@ impl<'tcx> HirTyLowerer<'tcx> for ItemCtxt<'tcx> {
513576
ty.ty_adt_def()
514577
}
515578

516-
fn record_ty(&self, _hir_id: hir::HirId, _ty: Ty<'tcx>, _span: Span) {
517-
// There's no place to record types from signatures?
579+
fn record_ty(&self, _hir_id: hir::HirId, ty: Ty<'tcx>, span: Span) {
580+
// There's no place to record types from signatures
581+
582+
if !self.forbidden_params.borrow().is_empty() {
583+
for param in cgp::parameters_for(self.tcx, ty, true) {
584+
if let Some(param) = self.forbidden_params.borrow_mut().remove(&param) {
585+
let mut diag = self.dcx().create_err(UnconstrainedGenericParameter {
586+
span: self.tcx.def_span(param.def_id),
587+
param_name: param.name,
588+
param_def_kind: self.tcx.def_descr(param.def_id),
589+
const_param_note: None,
590+
const_param_note2: None,
591+
lifetime_help: Some(span),
592+
});
593+
diag.code(E0207);
594+
diag.emit();
595+
}
596+
}
597+
}
518598
}
519599

520600
fn infcx(&self) -> Option<&InferCtxt<'tcx>> {

‎compiler/rustc_hir_analysis/src/collect/type_of.rs

+17-6
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,10 @@ pub(super) fn type_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::EarlyBinder<'_
314314
use rustc_hir::*;
315315
use rustc_middle::ty::Ty;
316316

317+
let hir_id = tcx.local_def_id_to_hir_id(def_id);
318+
319+
let icx = ItemCtxt::new(tcx, def_id);
320+
317321
// If we are computing `type_of` the synthesized associated type for an RPITIT in the impl
318322
// side, use `collect_return_position_impl_trait_in_trait_tys` to infer the value of the
319323
// associated type in the impl.
@@ -322,7 +326,15 @@ pub(super) fn type_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::EarlyBinder<'_
322326
match tcx.collect_return_position_impl_trait_in_trait_tys(fn_def_id) {
323327
Ok(map) => {
324328
let assoc_item = tcx.associated_item(def_id);
325-
return map[&assoc_item.trait_item_def_id.unwrap()];
329+
let ty = map[&assoc_item.trait_item_def_id.unwrap()];
330+
match icx.forbid_unconstrained_lifetime_params_from_parent_impl() {
331+
Ok(()) => {
332+
// Ensure no unconstrained lifetimes are used in these impls.
333+
icx.record_ty(hir_id, ty.instantiate_identity(), tcx.def_span(def_id));
334+
}
335+
Err(guar) => return ty::EarlyBinder::bind(Ty::new_error(tcx, guar)),
336+
}
337+
return ty;
326338
}
327339
Err(_) => {
328340
return ty::EarlyBinder::bind(Ty::new_error_with_message(
@@ -344,10 +356,6 @@ pub(super) fn type_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::EarlyBinder<'_
344356
None => {}
345357
}
346358

347-
let hir_id = tcx.local_def_id_to_hir_id(def_id);
348-
349-
let icx = ItemCtxt::new(tcx, def_id);
350-
351359
let output = match tcx.hir_node(hir_id) {
352360
Node::TraitItem(item) => match item.kind {
353361
TraitItemKind::Fn(..) => {
@@ -398,7 +406,10 @@ pub(super) fn type_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::EarlyBinder<'_
398406
check_feature_inherent_assoc_ty(tcx, item.span);
399407
}
400408

401-
icx.lower_ty(ty)
409+
match icx.forbid_unconstrained_lifetime_params_from_parent_impl() {
410+
Err(guar) => Ty::new_error(tcx, guar),
411+
Ok(()) => icx.lower_ty(ty),
412+
}
402413
}
403414
},
404415

‎compiler/rustc_hir_analysis/src/errors.rs

+2
Original file line numberDiff line numberDiff line change
@@ -1616,6 +1616,8 @@ pub(crate) struct UnconstrainedGenericParameter {
16161616
pub const_param_note: Option<()>,
16171617
#[note(hir_analysis_const_param_note2)]
16181618
pub const_param_note2: Option<()>,
1619+
#[help]
1620+
pub lifetime_help: Option<Span>,
16191621
}
16201622

16211623
#[derive(Diagnostic)]

‎compiler/rustc_hir_analysis/src/impl_wf_check.rs

+2-44
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
use crate::{constrained_generic_params as cgp, errors::UnconstrainedGenericParameter};
1212
use min_specialization::check_min_specialization;
1313

14-
use rustc_data_structures::fx::FxHashSet;
1514
use rustc_errors::codes::*;
1615
use rustc_hir::def::DefKind;
1716
use rustc_hir::def_id::LocalDefId;
@@ -83,25 +82,6 @@ fn enforce_impl_params_are_constrained(
8382
&mut input_parameters,
8483
);
8584

86-
// Disallow unconstrained lifetimes, but only if they appear in assoc types.
87-
let lifetimes_in_associated_types: FxHashSet<_> = tcx
88-
.associated_item_def_ids(impl_def_id)
89-
.iter()
90-
.flat_map(|def_id| {
91-
let item = tcx.associated_item(def_id);
92-
match item.kind {
93-
ty::AssocKind::Type => {
94-
if item.defaultness(tcx).has_value() {
95-
cgp::parameters_for(tcx, tcx.type_of(def_id).instantiate_identity(), true)
96-
} else {
97-
vec![]
98-
}
99-
}
100-
ty::AssocKind::Fn | ty::AssocKind::Const => vec![],
101-
}
102-
})
103-
.collect();
104-
10585
let mut res = Ok(());
10686
for param in &impl_generics.own_params {
10787
let err = match param.kind {
@@ -110,11 +90,7 @@ fn enforce_impl_params_are_constrained(
11090
let param_ty = ty::ParamTy::for_def(param);
11191
!input_parameters.contains(&cgp::Parameter::from(param_ty))
11292
}
113-
ty::GenericParamDefKind::Lifetime => {
114-
let param_lt = cgp::Parameter::from(param.to_early_bound_region_data());
115-
lifetimes_in_associated_types.contains(&param_lt) && // (*)
116-
!input_parameters.contains(&param_lt)
117-
}
93+
ty::GenericParamDefKind::Lifetime => false,
11894
ty::GenericParamDefKind::Const { .. } => {
11995
let param_ct = ty::ParamConst::for_def(param);
12096
!input_parameters.contains(&cgp::Parameter::from(param_ct))
@@ -129,29 +105,11 @@ fn enforce_impl_params_are_constrained(
129105
param_def_kind: tcx.def_descr(param.def_id),
130106
const_param_note,
131107
const_param_note2: const_param_note,
108+
lifetime_help: None,
132109
});
133110
diag.code(E0207);
134111
res = Err(diag.emit());
135112
}
136113
}
137114
res
138-
139-
// (*) This is a horrible concession to reality. I think it'd be
140-
// better to just ban unconstrained lifetimes outright, but in
141-
// practice people do non-hygienic macros like:
142-
//
143-
// ```
144-
// macro_rules! __impl_slice_eq1 {
145-
// ($Lhs: ty, $Rhs: ty, $Bound: ident) => {
146-
// impl<'a, 'b, A: $Bound, B> PartialEq<$Rhs> for $Lhs where A: PartialEq<B> {
147-
// ....
148-
// }
149-
// }
150-
// }
151-
// ```
152-
//
153-
// In a concession to backwards compatibility, we continue to
154-
// permit those, so long as the lifetimes aren't used in
155-
// associated types. I believe this is sound, because lifetimes
156-
// used elsewhere are not projected back out.
157115
}

‎tests/ui/associated-types/issue-26262.stderr

+6
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,12 @@ error[E0207]: the lifetime parameter `'a` is not constrained by the impl trait,
99
|
1010
LL | impl<'a,T: Trait2<'a>> Trait1<<T as Trait2<'a>>::Foo> for T {
1111
| ^^ unconstrained lifetime parameter
12+
|
13+
help: this use of an otherwise unconstrained lifetime is not allowed
14+
--> $DIR/issue-26262.rs:19:16
15+
|
16+
LL | type Bar = &'a ();
17+
| ^^^^^^
1218

1319
error: aborting due to 2 previous errors
1420

‎tests/ui/async-await/in-trait/unconstrained-impl-region.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,11 @@ pub(crate) trait Actor: Sized {
1111
}
1212

1313
impl<'a> Actor for () {
14-
//~^ ERROR the lifetime parameter `'a` is not constrained by the impl trait, self type, or predicates
14+
//~^ ERROR the lifetime parameter `'a` is not constrained by the impl trait, self type, or predicates
1515
type Message = &'a ();
1616
async fn on_mount(self, _: impl Inbox<&'a ()>) {}
1717
//~^ ERROR the trait bound `impl Inbox<&'a ()>: Inbox<&'a ()>` is not satisfied
18+
//~| ERROR impl has stricter requirements than trait
1819
}
1920

2021
fn main() {}

‎tests/ui/async-await/in-trait/unconstrained-impl-region.stderr

+21-6
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,15 @@
1+
error[E0207]: the lifetime parameter `'a` is not constrained by the impl trait, self type, or predicates
2+
--> $DIR/unconstrained-impl-region.rs:13:6
3+
|
4+
LL | impl<'a> Actor for () {
5+
| ^^ unconstrained lifetime parameter
6+
|
7+
help: this use of an otherwise unconstrained lifetime is not allowed
8+
--> $DIR/unconstrained-impl-region.rs:15:20
9+
|
10+
LL | type Message = &'a ();
11+
| ^^^^^^
12+
113
error[E0277]: the trait bound `impl Inbox<&'a ()>: Inbox<&'a ()>` is not satisfied
214
--> $DIR/unconstrained-impl-region.rs:16:5
315
|
@@ -10,13 +22,16 @@ note: required by a bound in `<() as Actor>::on_mount`
1022
LL | async fn on_mount(self, _: impl Inbox<&'a ()>) {}
1123
| ^^^^^^^^^^^^^ required by this bound in `<() as Actor>::on_mount`
1224

13-
error[E0207]: the lifetime parameter `'a` is not constrained by the impl trait, self type, or predicates
14-
--> $DIR/unconstrained-impl-region.rs:13:6
25+
error[E0276]: impl has stricter requirements than trait
26+
--> $DIR/unconstrained-impl-region.rs:16:37
1527
|
16-
LL | impl<'a> Actor for () {
17-
| ^^ unconstrained lifetime parameter
28+
LL | async fn on_mount(self, _: impl Inbox<Self::Message>);
29+
| ------------------------------------------------------ definition of `on_mount` from trait
30+
...
31+
LL | async fn on_mount(self, _: impl Inbox<&'a ()>) {}
32+
| ^^^^^^^^^^^^^ impl has extra requirement `impl Inbox<&'a ()>: Inbox<&'a ()>`
1833

19-
error: aborting due to 2 previous errors
34+
error: aborting due to 3 previous errors
2035

21-
Some errors have detailed explanations: E0207, E0277.
36+
Some errors have detailed explanations: E0207, E0276, E0277.
2237
For more information about an error, try `rustc --explain E0207`.

‎tests/ui/feature-gates/feature-gate-impl_trait_in_assoc_type.stderr

+8-8
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,14 @@ LL | type Bar = impl std::fmt::Debug;
2626
|
2727
= note: `Bar` must be used in combination with a concrete type within the same impl
2828

29+
error: unconstrained opaque type
30+
--> $DIR/feature-gate-impl_trait_in_assoc_type.rs:14:16
31+
|
32+
LL | type Bop = impl std::fmt::Debug;
33+
| ^^^^^^^^^^^^^^^^^^^^
34+
|
35+
= note: `Bop` must be used in combination with a concrete type within the same impl
36+
2937
error[E0658]: inherent associated types are unstable
3038
--> $DIR/feature-gate-impl_trait_in_assoc_type.rs:14:5
3139
|
@@ -36,14 +44,6 @@ LL | type Bop = impl std::fmt::Debug;
3644
= help: add `#![feature(inherent_associated_types)]` to the crate attributes to enable
3745
= note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date
3846

39-
error: unconstrained opaque type
40-
--> $DIR/feature-gate-impl_trait_in_assoc_type.rs:14:16
41-
|
42-
LL | type Bop = impl std::fmt::Debug;
43-
| ^^^^^^^^^^^^^^^^^^^^
44-
|
45-
= note: `Bop` must be used in combination with a concrete type within the same impl
46-
4747
error: aborting due to 5 previous errors
4848

4949
For more information about this error, try `rustc --explain E0658`.

0 commit comments

Comments
 (0)