Skip to content

Commit d8b0864

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

31 files changed

+308
-208
lines changed

‎compiler/rustc_hir_analysis/messages.ftl‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -552,6 +552,7 @@ hir_analysis_unconstrained_generic_parameter = the {$param_def_kind} `{$param_na
552552
.label = unconstrained {$param_def_kind}
553553
.const_param_note = expressions using a const parameter must map each value to a distinct output value
554554
.const_param_note2 = proving the result of expressions other than the parameter are unique is not supported
555+
.help = this use of an otherwise unconstrained lifetime is not allowed
555556
556557
hir_analysis_unconstrained_opaque_type = unconstrained opaque type
557558
.note = `{$name}` must be used in combination with a concrete type within the same {$what}

‎compiler/rustc_hir_analysis/src/collect.rs‎

Lines changed: 87 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,18 @@
1414
//! At present, however, we do run collection across all items in the
1515
//! crate as a kind of pass. This should eventually be factored away.
1616
17-
use std::cell::Cell;
17+
use std::cell::{Cell, RefCell};
1818
use std::iter;
1919
use std::ops::Bound;
2020

2121
use rustc_abi::ExternAbi;
2222
use rustc_ast::Recovered;
2323
use rustc_data_structures::captures::Captures;
24-
use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
24+
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap};
2525
use rustc_data_structures::unord::UnordMap;
2626
use rustc_errors::{
27-
Applicability, Diag, DiagCtxtHandle, E0228, ErrorGuaranteed, StashKey, struct_span_code_err,
27+
Applicability, Diag, DiagCtxtHandle, E0207, E0228, ErrorGuaranteed, StashKey,
28+
struct_span_code_err,
2829
};
2930
use rustc_hir::def::DefKind;
3031
use rustc_hir::def_id::{DefId, LocalDefId};
@@ -36,7 +37,9 @@ use rustc_middle::hir::nested_filter;
3637
use rustc_middle::query::Providers;
3738
use rustc_middle::ty::fold::fold_regions;
3839
use rustc_middle::ty::util::{Discr, IntTypeExt};
39-
use rustc_middle::ty::{self, AdtKind, Const, IsSuggestable, Ty, TyCtxt, TypingMode};
40+
use rustc_middle::ty::{
41+
self, AdtKind, Const, IsSuggestable, Ty, TyCtxt, TypeVisitableExt as _, TypingMode,
42+
};
4043
use rustc_middle::{bug, span_bug};
4144
use rustc_span::symbol::{Ident, Symbol, kw, sym};
4245
use rustc_span::{DUMMY_SP, Span};
@@ -46,7 +49,8 @@ use rustc_trait_selection::traits::ObligationCtxt;
4649
use tracing::{debug, instrument};
4750

4851
use crate::check::intrinsic::intrinsic_operation_unsafety;
49-
use crate::errors;
52+
use crate::constrained_generic_params::{self as cgp, Parameter};
53+
use crate::errors::{self, UnconstrainedGenericParameter};
5054
use crate::hir_ty_lowering::{FeedConstTy, HirTyLowerer, RegionInferReason};
5155

5256
pub(crate) mod dump;
@@ -127,6 +131,7 @@ pub struct ItemCtxt<'tcx> {
127131
tcx: TyCtxt<'tcx>,
128132
item_def_id: LocalDefId,
129133
tainted_by_errors: Cell<Option<ErrorGuaranteed>>,
134+
pub(crate) forbidden_params: RefCell<FxHashMap<Parameter, ty::GenericParamDef>>,
130135
}
131136

132137
///////////////////////////////////////////////////////////////////////////
@@ -375,7 +380,12 @@ fn bad_placeholder<'cx, 'tcx>(
375380

376381
impl<'tcx> ItemCtxt<'tcx> {
377382
pub fn new(tcx: TyCtxt<'tcx>, item_def_id: LocalDefId) -> ItemCtxt<'tcx> {
378-
ItemCtxt { tcx, item_def_id, tainted_by_errors: Cell::new(None) }
383+
ItemCtxt {
384+
tcx,
385+
item_def_id,
386+
tainted_by_errors: Cell::new(None),
387+
forbidden_params: Default::default(),
388+
}
379389
}
380390

381391
pub fn lower_ty(&self, hir_ty: &hir::Ty<'tcx>) -> Ty<'tcx> {
@@ -396,6 +406,58 @@ impl<'tcx> ItemCtxt<'tcx> {
396406
None => Ok(()),
397407
}
398408
}
409+
410+
fn forbid_unconstrained_lifetime_params_from_parent_impl(&self) -> Result<(), ErrorGuaranteed> {
411+
let tcx = self.tcx;
412+
let impl_def_id = tcx.hir().get_parent_item(self.hir_id());
413+
let impl_self_ty = tcx.type_of(impl_def_id).instantiate_identity();
414+
impl_self_ty.error_reported()?;
415+
let impl_generics = tcx.generics_of(impl_def_id);
416+
let impl_predicates = tcx.predicates_of(impl_def_id);
417+
let impl_trait_ref =
418+
tcx.impl_trait_ref(impl_def_id).map(ty::EarlyBinder::instantiate_identity);
419+
420+
impl_trait_ref.error_reported()?;
421+
422+
let mut input_parameters = cgp::parameters_for_impl(tcx, impl_self_ty, impl_trait_ref);
423+
424+
cgp::identify_constrained_generic_params(
425+
tcx,
426+
impl_predicates,
427+
impl_trait_ref,
428+
&mut input_parameters,
429+
);
430+
431+
for param in &impl_generics.own_params {
432+
let p = match param.kind {
433+
// This is a horrible concession to reality. It'd be
434+
// better to just ban unconstrained lifetimes outright, but in
435+
// practice people do non-hygienic macros like:
436+
//
437+
// ```
438+
// macro_rules! __impl_slice_eq1 {
439+
// ($Lhs: ty, $Rhs: ty, $Bound: ident) => {
440+
// impl<'a, 'b, A: $Bound, B> PartialEq<$Rhs> for $Lhs where A: PartialEq<B> {
441+
// ....
442+
// }
443+
// }
444+
// }
445+
// ```
446+
//
447+
// In a concession to backwards compatibility, we continue to
448+
// permit those, so long as the lifetimes aren't used in
449+
// associated types. This is sound, because lifetimes
450+
// used elsewhere are not projected back out.
451+
ty::GenericParamDefKind::Type { .. } => continue,
452+
ty::GenericParamDefKind::Lifetime => param.to_early_bound_region_data().into(),
453+
ty::GenericParamDefKind::Const { .. } => continue,
454+
};
455+
if !input_parameters.contains(&p) {
456+
self.forbidden_params.borrow_mut().insert(p, param.clone());
457+
}
458+
}
459+
Ok(())
460+
}
399461
}
400462

401463
impl<'tcx> HirTyLowerer<'tcx> for ItemCtxt<'tcx> {
@@ -538,8 +600,25 @@ impl<'tcx> HirTyLowerer<'tcx> for ItemCtxt<'tcx> {
538600
ty.ty_adt_def()
539601
}
540602

541-
fn record_ty(&self, _hir_id: hir::HirId, _ty: Ty<'tcx>, _span: Span) {
542-
// There's no place to record types from signatures?
603+
fn record_ty(&self, _hir_id: hir::HirId, ty: Ty<'tcx>, span: Span) {
604+
// There's no place to record types from signatures
605+
606+
if !self.forbidden_params.borrow().is_empty() {
607+
for param in cgp::parameters_for(self.tcx, ty, true) {
608+
if let Some(param) = self.forbidden_params.borrow_mut().remove(&param) {
609+
let mut diag = self.dcx().create_err(UnconstrainedGenericParameter {
610+
span: self.tcx.def_span(param.def_id),
611+
param_name: param.name,
612+
param_def_kind: self.tcx.def_descr(param.def_id),
613+
const_param_note: false,
614+
const_param_note2: false,
615+
lifetime_help: Some(span),
616+
});
617+
diag.code(E0207);
618+
diag.emit();
619+
}
620+
}
621+
}
543622
}
544623

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

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

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,10 @@ pub(super) fn type_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::EarlyBinder<'_
388388
use rustc_hir::*;
389389
use rustc_middle::ty::Ty;
390390

391+
let hir_id = tcx.local_def_id_to_hir_id(def_id);
392+
393+
let icx = ItemCtxt::new(tcx, def_id);
394+
391395
// If we are computing `type_of` the synthesized associated type for an RPITIT in the impl
392396
// side, use `collect_return_position_impl_trait_in_trait_tys` to infer the value of the
393397
// associated type in the impl.
@@ -396,7 +400,15 @@ pub(super) fn type_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::EarlyBinder<'_
396400
match tcx.collect_return_position_impl_trait_in_trait_tys(fn_def_id) {
397401
Ok(map) => {
398402
let assoc_item = tcx.associated_item(def_id);
399-
return map[&assoc_item.trait_item_def_id.unwrap()];
403+
let ty = map[&assoc_item.trait_item_def_id.unwrap()];
404+
match icx.forbid_unconstrained_lifetime_params_from_parent_impl() {
405+
Ok(()) => {
406+
// Ensure no unconstrained lifetimes are used in these impls.
407+
icx.record_ty(hir_id, ty.instantiate_identity(), tcx.def_span(def_id));
408+
}
409+
Err(guar) => return ty::EarlyBinder::bind(Ty::new_error(tcx, guar)),
410+
}
411+
return ty;
400412
}
401413
Err(_) => {
402414
return ty::EarlyBinder::bind(Ty::new_error_with_message(
@@ -418,10 +430,6 @@ pub(super) fn type_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::EarlyBinder<'_
418430
None => {}
419431
}
420432

421-
let hir_id = tcx.local_def_id_to_hir_id(def_id);
422-
423-
let icx = ItemCtxt::new(tcx, def_id);
424-
425433
let output = match tcx.hir_node(hir_id) {
426434
Node::TraitItem(item) => match item.kind {
427435
TraitItemKind::Fn(..) => {
@@ -472,7 +480,10 @@ pub(super) fn type_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::EarlyBinder<'_
472480
check_feature_inherent_assoc_ty(tcx, item.span);
473481
}
474482

475-
icx.lower_ty(ty)
483+
match icx.forbid_unconstrained_lifetime_params_from_parent_impl() {
484+
Err(guar) => Ty::new_error(tcx, guar),
485+
Ok(()) => icx.lower_ty(ty),
486+
}
476487
}
477488
},
478489

‎compiler/rustc_hir_analysis/src/errors.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1617,6 +1617,8 @@ pub(crate) struct UnconstrainedGenericParameter {
16171617
pub const_param_note: bool,
16181618
#[note(hir_analysis_const_param_note2)]
16191619
pub const_param_note2: bool,
1620+
#[help]
1621+
pub lifetime_help: Option<Span>,
16201622
}
16211623

16221624
#[derive(Diagnostic)]

‎compiler/rustc_hir_analysis/src/impl_wf_check.rs‎

Lines changed: 2 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
use std::assert_matches::debug_assert_matches;
1212

1313
use min_specialization::check_min_specialization;
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;
@@ -89,25 +88,6 @@ fn enforce_impl_params_are_constrained(
8988
&mut input_parameters,
9089
);
9190

92-
// Disallow unconstrained lifetimes, but only if they appear in assoc types.
93-
let lifetimes_in_associated_types: FxHashSet<_> = tcx
94-
.associated_item_def_ids(impl_def_id)
95-
.iter()
96-
.flat_map(|def_id| {
97-
let item = tcx.associated_item(def_id);
98-
match item.kind {
99-
ty::AssocKind::Type => {
100-
if item.defaultness(tcx).has_value() {
101-
cgp::parameters_for(tcx, tcx.type_of(def_id).instantiate_identity(), true)
102-
} else {
103-
vec![]
104-
}
105-
}
106-
ty::AssocKind::Fn | ty::AssocKind::Const => vec![],
107-
}
108-
})
109-
.collect();
110-
11191
let mut res = Ok(());
11292
for param in &impl_generics.own_params {
11393
let err = match param.kind {
@@ -116,11 +96,7 @@ fn enforce_impl_params_are_constrained(
11696
let param_ty = ty::ParamTy::for_def(param);
11797
!input_parameters.contains(&cgp::Parameter::from(param_ty))
11898
}
119-
ty::GenericParamDefKind::Lifetime => {
120-
let param_lt = cgp::Parameter::from(param.to_early_bound_region_data());
121-
lifetimes_in_associated_types.contains(&param_lt) && // (*)
122-
!input_parameters.contains(&param_lt)
123-
}
99+
ty::GenericParamDefKind::Lifetime => false,
124100
ty::GenericParamDefKind::Const { .. } => {
125101
let param_ct = ty::ParamConst::for_def(param);
126102
!input_parameters.contains(&cgp::Parameter::from(param_ct))
@@ -134,29 +110,11 @@ fn enforce_impl_params_are_constrained(
134110
param_def_kind: tcx.def_descr(param.def_id),
135111
const_param_note,
136112
const_param_note2: const_param_note,
113+
lifetime_help: None,
137114
});
138115
diag.code(E0207);
139116
res = Err(diag.emit());
140117
}
141118
}
142119
res
143-
144-
// (*) This is a horrible concession to reality. I think it'd be
145-
// better to just ban unconstrained lifetimes outright, but in
146-
// practice people do non-hygienic macros like:
147-
//
148-
// ```
149-
// macro_rules! __impl_slice_eq1 {
150-
// ($Lhs: ty, $Rhs: ty, $Bound: ident) => {
151-
// impl<'a, 'b, A: $Bound, B> PartialEq<$Rhs> for $Lhs where A: PartialEq<B> {
152-
// ....
153-
// }
154-
// }
155-
// }
156-
// ```
157-
//
158-
// In a concession to backwards compatibility, we continue to
159-
// permit those, so long as the lifetimes aren't used in
160-
// associated types. I believe this is sound, because lifetimes
161-
// used elsewhere are not projected back out.
162120
}

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

Lines changed: 6 additions & 0 deletions
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‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ 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
15+
//~| ERROR the lifetime parameter `'a` is not constrained by the impl trait, self type, or predicates
1516
type Message = &'a ();
1617
async fn on_mount(self, _: impl Inbox<&'a ()>) {}
1718
}

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

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,25 @@ error[E0207]: the lifetime parameter `'a` is not constrained by the impl trait,
33
|
44
LL | impl<'a> Actor for () {
55
| ^^ unconstrained lifetime parameter
6+
|
7+
help: this use of an otherwise unconstrained lifetime is not allowed
8+
--> $DIR/unconstrained-impl-region.rs:16:20
9+
|
10+
LL | type Message = &'a ();
11+
| ^^^^^^
12+
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
15+
|
16+
LL | impl<'a> Actor for () {
17+
| ^^ unconstrained lifetime parameter
18+
|
19+
help: this use of an otherwise unconstrained lifetime is not allowed
20+
--> $DIR/unconstrained-impl-region.rs:17:5
21+
|
22+
LL | async fn on_mount(self, _: impl Inbox<&'a ()>) {}
23+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
624

7-
error: aborting due to 1 previous error
25+
error: aborting due to 2 previous errors
826

927
For more information about this error, try `rustc --explain E0207`.

‎tests/ui/delegation/unsupported.stderr‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,11 @@ note: ...which requires comparing an impl and trait method signature, inferring
1010
LL | reuse to_reuse::opaque_ret;
1111
| ^^^^^^^^^^
1212
= note: ...which again requires computing type of `opaque::<impl at $DIR/unsupported.rs:21:5: 21:24>::{synthetic#0}`, completing the cycle
13-
note: cycle used when checking that `opaque::<impl at $DIR/unsupported.rs:21:5: 21:24>` is well-formed
14-
--> $DIR/unsupported.rs:21:5
13+
note: cycle used when checking assoc item `opaque::<impl at $DIR/unsupported.rs:21:5: 21:24>::opaque_ret` is compatible with trait definition
14+
--> $DIR/unsupported.rs:22:25
1515
|
16-
LL | impl ToReuse for u8 {
17-
| ^^^^^^^^^^^^^^^^^^^
16+
LL | reuse to_reuse::opaque_ret;
17+
| ^^^^^^^^^^
1818
= note: see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information
1919

2020
error[E0391]: cycle detected when computing type of `opaque::<impl at $DIR/unsupported.rs:24:5: 24:25>::{synthetic#0}`
@@ -29,11 +29,11 @@ note: ...which requires comparing an impl and trait method signature, inferring
2929
LL | reuse ToReuse::opaque_ret;
3030
| ^^^^^^^^^^
3131
= note: ...which again requires computing type of `opaque::<impl at $DIR/unsupported.rs:24:5: 24:25>::{synthetic#0}`, completing the cycle
32-
note: cycle used when checking that `opaque::<impl at $DIR/unsupported.rs:24:5: 24:25>` is well-formed
33-
--> $DIR/unsupported.rs:24:5
32+
note: cycle used when checking assoc item `opaque::<impl at $DIR/unsupported.rs:24:5: 24:25>::opaque_ret` is compatible with trait definition
33+
--> $DIR/unsupported.rs:25:24
3434
|
35-
LL | impl ToReuse for u16 {
36-
| ^^^^^^^^^^^^^^^^^^^^
35+
LL | reuse ToReuse::opaque_ret;
36+
| ^^^^^^^^^^
3737
= note: see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information
3838

3939
error: recursive delegation is not supported yet

0 commit comments

Comments
 (0)