Skip to content

Commit 572377a

Browse files
oli-obkoli-obk
committed
Reject unconstrained lifetimes in type_of(assoc_ty) instead of during wfcheck of the impl item
1 parent 9896de4 commit 572377a

31 files changed

+310
-213
lines changed

‎compiler/rustc_hir_analysis/messages.ftl

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

‎compiler/rustc_hir_analysis/src/collect.rs

+87-8
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,17 @@
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_ast::Recovered;
2222
use rustc_data_structures::captures::Captures;
23-
use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
23+
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap};
2424
use rustc_data_structures::unord::UnordMap;
2525
use rustc_errors::{
26-
struct_span_code_err, Applicability, Diag, DiagCtxtHandle, ErrorGuaranteed, StashKey, E0228,
26+
struct_span_code_err, Applicability, Diag, DiagCtxtHandle, ErrorGuaranteed, StashKey, E0207,
27+
E0228,
2728
};
2829
use rustc_hir::def::DefKind;
2930
use rustc_hir::def_id::{DefId, LocalDefId};
@@ -34,7 +35,9 @@ use rustc_infer::traits::ObligationCause;
3435
use rustc_middle::hir::nested_filter;
3536
use rustc_middle::query::Providers;
3637
use rustc_middle::ty::util::{Discr, IntTypeExt};
37-
use rustc_middle::ty::{self, AdtKind, Const, IsSuggestable, Ty, TyCtxt, Upcast};
38+
use rustc_middle::ty::{
39+
self, AdtKind, Const, IsSuggestable, Ty, TyCtxt, TypeVisitableExt as _, Upcast,
40+
};
3841
use rustc_middle::{bug, span_bug};
3942
use rustc_span::symbol::{kw, sym, Ident, Symbol};
4043
use rustc_span::{Span, DUMMY_SP};
@@ -44,7 +47,8 @@ use rustc_trait_selection::infer::InferCtxtExt;
4447
use rustc_trait_selection::traits::ObligationCtxt;
4548

4649
use crate::check::intrinsic::intrinsic_operation_unsafety;
47-
use crate::errors;
50+
use crate::constrained_generic_params::{self as cgp, Parameter};
51+
use crate::errors::{self, UnconstrainedGenericParameter};
4852
use crate::hir_ty_lowering::{HirTyLowerer, RegionInferReason};
4953

5054
pub(crate) mod dump;
@@ -122,6 +126,7 @@ pub struct ItemCtxt<'tcx> {
122126
tcx: TyCtxt<'tcx>,
123127
item_def_id: LocalDefId,
124128
tainted_by_errors: Cell<Option<ErrorGuaranteed>>,
129+
pub forbidden_params: RefCell<FxHashMap<Parameter, ty::GenericParamDef>>,
125130
}
126131

127132
///////////////////////////////////////////////////////////////////////////
@@ -354,7 +359,12 @@ fn bad_placeholder<'cx, 'tcx>(
354359

355360
impl<'tcx> ItemCtxt<'tcx> {
356361
pub fn new(tcx: TyCtxt<'tcx>, item_def_id: LocalDefId) -> ItemCtxt<'tcx> {
357-
ItemCtxt { tcx, item_def_id, tainted_by_errors: Cell::new(None) }
362+
ItemCtxt {
363+
tcx,
364+
item_def_id,
365+
tainted_by_errors: Cell::new(None),
366+
forbidden_params: Default::default(),
367+
}
358368
}
359369

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

380442
impl<'tcx> HirTyLowerer<'tcx> for ItemCtxt<'tcx> {
@@ -516,8 +578,25 @@ impl<'tcx> HirTyLowerer<'tcx> for ItemCtxt<'tcx> {
516578
ty.ty_adt_def()
517579
}
518580

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

523602
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
@@ -340,6 +340,10 @@ pub(super) fn type_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::EarlyBinder<'_
340340
use rustc_hir::*;
341341
use rustc_middle::ty::Ty;
342342

343+
let hir_id = tcx.local_def_id_to_hir_id(def_id);
344+
345+
let icx = ItemCtxt::new(tcx, def_id);
346+
343347
// If we are computing `type_of` the synthesized associated type for an RPITIT in the impl
344348
// side, use `collect_return_position_impl_trait_in_trait_tys` to infer the value of the
345349
// associated type in the impl.
@@ -348,7 +352,15 @@ pub(super) fn type_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::EarlyBinder<'_
348352
match tcx.collect_return_position_impl_trait_in_trait_tys(fn_def_id) {
349353
Ok(map) => {
350354
let assoc_item = tcx.associated_item(def_id);
351-
return map[&assoc_item.trait_item_def_id.unwrap()];
355+
let ty = map[&assoc_item.trait_item_def_id.unwrap()];
356+
match icx.forbid_unconstrained_lifetime_params_from_parent_impl() {
357+
Ok(()) => {
358+
// Ensure no unconstrained lifetimes are used in these impls.
359+
icx.record_ty(hir_id, ty.instantiate_identity(), tcx.def_span(def_id));
360+
}
361+
Err(guar) => return ty::EarlyBinder::bind(Ty::new_error(tcx, guar)),
362+
}
363+
return ty;
352364
}
353365
Err(_) => {
354366
return ty::EarlyBinder::bind(Ty::new_error_with_message(
@@ -370,10 +382,6 @@ pub(super) fn type_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::EarlyBinder<'_
370382
None => {}
371383
}
372384

373-
let hir_id = tcx.local_def_id_to_hir_id(def_id);
374-
375-
let icx = ItemCtxt::new(tcx, def_id);
376-
377385
let output = match tcx.hir_node(hir_id) {
378386
Node::TraitItem(item) => match item.kind {
379387
TraitItemKind::Fn(..) => {
@@ -424,7 +432,10 @@ pub(super) fn type_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::EarlyBinder<'_
424432
check_feature_inherent_assoc_ty(tcx, item.span);
425433
}
426434

427-
icx.lower_ty(ty)
435+
match icx.forbid_unconstrained_lifetime_params_from_parent_impl() {
436+
Err(guar) => Ty::new_error(tcx, guar),
437+
Ok(()) => icx.lower_ty(ty),
438+
}
428439
}
429440
},
430441

‎compiler/rustc_hir_analysis/src/errors.rs

+2
Original file line numberDiff line numberDiff line change
@@ -1646,6 +1646,8 @@ pub(crate) struct UnconstrainedGenericParameter {
16461646
pub const_param_note: Option<()>,
16471647
#[note(hir_analysis_const_param_note2)]
16481648
pub const_param_note2: Option<()>,
1649+
#[help]
1650+
pub lifetime_help: Option<Span>,
16491651
}
16501652

16511653
#[derive(Diagnostic)]

‎compiler/rustc_hir_analysis/src/impl_wf_check.rs

+2-44
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
//! fixed, but for the moment it's easier to do these checks early.
1010
1111
use min_specialization::check_min_specialization;
12-
use rustc_data_structures::fx::FxHashSet;
1312
use rustc_errors::codes::*;
1413
use rustc_hir::def::DefKind;
1514
use rustc_hir::def_id::LocalDefId;
@@ -84,25 +83,6 @@ fn enforce_impl_params_are_constrained(
8483
&mut input_parameters,
8584
);
8685

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

‎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)