Skip to content

Reject unconstrained lifetimes in type_of(assoc_ty) instead of during wfcheck of the impl item #127973

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

Closed
wants to merge 4 commits into from
Closed
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
1 change: 1 addition & 0 deletions compiler/rustc_hir_analysis/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,7 @@ hir_analysis_unconstrained_generic_parameter = the {$param_def_kind} `{$param_na
.label = unconstrained {$param_def_kind}
.const_param_note = expressions using a const parameter must map each value to a distinct output value
.const_param_note2 = proving the result of expressions other than the parameter are unique is not supported
.help = this use of an otherwise unconstrained lifetime is not allowed

hir_analysis_unconstrained_opaque_type = unconstrained opaque type
.note = `{$name}` must be used in combination with a concrete type within the same {$what}
Expand Down
95 changes: 87 additions & 8 deletions compiler/rustc_hir_analysis/src/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,18 @@
//! At present, however, we do run collection across all items in the
//! crate as a kind of pass. This should eventually be factored away.

use std::cell::Cell;
use std::cell::{Cell, RefCell};
use std::iter;
use std::ops::Bound;

use rustc_abi::ExternAbi;
use rustc_ast::Recovered;
use rustc_data_structures::captures::Captures;
use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap};
use rustc_data_structures::unord::UnordMap;
use rustc_errors::{
Applicability, Diag, DiagCtxtHandle, E0228, ErrorGuaranteed, StashKey, struct_span_code_err,
Applicability, Diag, DiagCtxtHandle, E0207, E0228, ErrorGuaranteed, StashKey,
struct_span_code_err,
};
use rustc_hir::def::DefKind;
use rustc_hir::def_id::{DefId, LocalDefId};
Expand All @@ -36,7 +37,9 @@ use rustc_middle::hir::nested_filter;
use rustc_middle::query::Providers;
use rustc_middle::ty::fold::fold_regions;
use rustc_middle::ty::util::{Discr, IntTypeExt};
use rustc_middle::ty::{self, AdtKind, Const, IsSuggestable, Ty, TyCtxt, TypingMode};
use rustc_middle::ty::{
self, AdtKind, Const, IsSuggestable, Ty, TyCtxt, TypeVisitableExt as _, TypingMode,
};
use rustc_middle::{bug, span_bug};
use rustc_span::symbol::{Ident, Symbol, kw, sym};
use rustc_span::{DUMMY_SP, Span};
Expand All @@ -46,7 +49,8 @@ use rustc_trait_selection::traits::ObligationCtxt;
use tracing::{debug, instrument};

use crate::check::intrinsic::intrinsic_operation_unsafety;
use crate::errors;
use crate::constrained_generic_params::{self as cgp, Parameter};
use crate::errors::{self, UnconstrainedGenericParameter};
use crate::hir_ty_lowering::{FeedConstTy, HirTyLowerer, RegionInferReason};

pub(crate) mod dump;
Expand Down Expand Up @@ -127,6 +131,7 @@ pub struct ItemCtxt<'tcx> {
tcx: TyCtxt<'tcx>,
item_def_id: LocalDefId,
tainted_by_errors: Cell<Option<ErrorGuaranteed>>,
pub(crate) forbidden_params: RefCell<FxHashMap<Parameter, ty::GenericParamDef>>,
}

///////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -375,7 +380,12 @@ fn bad_placeholder<'cx, 'tcx>(

impl<'tcx> ItemCtxt<'tcx> {
pub fn new(tcx: TyCtxt<'tcx>, item_def_id: LocalDefId) -> ItemCtxt<'tcx> {
ItemCtxt { tcx, item_def_id, tainted_by_errors: Cell::new(None) }
ItemCtxt {
tcx,
item_def_id,
tainted_by_errors: Cell::new(None),
forbidden_params: Default::default(),
}
}

pub fn lower_ty(&self, hir_ty: &hir::Ty<'tcx>) -> Ty<'tcx> {
Expand All @@ -396,6 +406,58 @@ impl<'tcx> ItemCtxt<'tcx> {
None => Ok(()),
}
}

fn forbid_unconstrained_lifetime_params_from_parent_impl(&self) -> Result<(), ErrorGuaranteed> {
let tcx = self.tcx;
let impl_def_id = tcx.hir().get_parent_item(self.hir_id());
let impl_self_ty = tcx.type_of(impl_def_id).instantiate_identity();
impl_self_ty.error_reported()?;
let impl_generics = tcx.generics_of(impl_def_id);
let impl_predicates = tcx.predicates_of(impl_def_id);
let impl_trait_ref =
tcx.impl_trait_ref(impl_def_id).map(ty::EarlyBinder::instantiate_identity);

impl_trait_ref.error_reported()?;

let mut input_parameters = cgp::parameters_for_impl(tcx, impl_self_ty, impl_trait_ref);

cgp::identify_constrained_generic_params(
tcx,
impl_predicates,
impl_trait_ref,
&mut input_parameters,
);

for param in &impl_generics.own_params {
let p = match param.kind {
// This is a horrible concession to reality. It'd be
// better to just ban unconstrained lifetimes outright, but in
// practice people do non-hygienic macros like:
//
// ```
// macro_rules! __impl_slice_eq1 {
// ($Lhs: ty, $Rhs: ty, $Bound: ident) => {
// impl<'a, 'b, A: $Bound, B> PartialEq<$Rhs> for $Lhs where A: PartialEq<B> {
// ....
// }
// }
// }
// ```
//
// In a concession to backwards compatibility, we continue to
// permit those, so long as the lifetimes aren't used in
// associated types. This is sound, because lifetimes
// used elsewhere are not projected back out.
ty::GenericParamDefKind::Type { .. } => continue,
ty::GenericParamDefKind::Lifetime => param.to_early_bound_region_data().into(),
ty::GenericParamDefKind::Const { .. } => continue,
};
if !input_parameters.contains(&p) {
self.forbidden_params.borrow_mut().insert(p, param.clone());
}
}
Ok(())
}
}

impl<'tcx> HirTyLowerer<'tcx> for ItemCtxt<'tcx> {
Expand Down Expand Up @@ -538,8 +600,25 @@ impl<'tcx> HirTyLowerer<'tcx> for ItemCtxt<'tcx> {
ty.ty_adt_def()
}

fn record_ty(&self, _hir_id: hir::HirId, _ty: Ty<'tcx>, _span: Span) {
// There's no place to record types from signatures?
fn record_ty(&self, _hir_id: hir::HirId, ty: Ty<'tcx>, span: Span) {
// There's no place to record types from signatures

if !self.forbidden_params.borrow().is_empty() {
for param in cgp::parameters_for(self.tcx, ty, true) {
if let Some(param) = self.forbidden_params.borrow_mut().remove(&param) {
let mut diag = self.dcx().create_err(UnconstrainedGenericParameter {
span: self.tcx.def_span(param.def_id),
param_name: param.name,
param_def_kind: self.tcx.def_descr(param.def_id),
const_param_note: false,
const_param_note2: false,
lifetime_help: Some(span),
});
diag.code(E0207);
diag.emit();
}
}
}
}

fn infcx(&self) -> Option<&InferCtxt<'tcx>> {
Expand Down
27 changes: 21 additions & 6 deletions compiler/rustc_hir_analysis/src/collect/predicates_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ use rustc_hir as hir;
use rustc_hir::def::DefKind;
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_middle::ty::{
self, GenericPredicates, ImplTraitInTraitData, Ty, TyCtxt, TypeVisitable, TypeVisitor, Upcast,
self, GenericPredicates, ImplTraitInTraitData, Ty, TyCtxt, TypeVisitable, TypeVisitableExt,
TypeVisitor, Upcast,
};
use rustc_middle::{bug, span_bug};
use rustc_span::symbol::Ident;
Expand Down Expand Up @@ -107,6 +108,7 @@ fn gather_explicit_predicates_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Gen
return ty::GenericPredicates {
parent: Some(tcx.parent(def_id.to_def_id())),
predicates: tcx.arena.alloc_from_iter(predicates),
errored_due_to_unconstrained_params: Ok(()),
};
}

Expand All @@ -128,6 +130,8 @@ fn gather_explicit_predicates_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Gen
return ty::GenericPredicates {
parent: Some(impl_def_id),
predicates: tcx.arena.alloc_from_iter(impl_predicates),
errored_due_to_unconstrained_params: trait_assoc_predicates
.errored_due_to_unconstrained_params,
};
}

Expand All @@ -153,6 +157,7 @@ fn gather_explicit_predicates_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Gen
// We use an `IndexSet` to preserve order of insertion.
// Preserving the order of insertion is important here so as not to break UI tests.
let mut predicates: FxIndexSet<(ty::Clause<'_>, Span)> = FxIndexSet::default();
let mut errored_due_to_unconstrained_params = Ok(());

let hir_generics = node.generics().unwrap_or(NO_GENERICS);
if let Node::Item(item) = node {
Expand Down Expand Up @@ -319,11 +324,16 @@ fn gather_explicit_predicates_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Gen
if let Node::Item(&Item { kind: ItemKind::Impl { .. }, .. }) = node {
let self_ty = tcx.type_of(def_id).instantiate_identity();
let trait_ref = tcx.impl_trait_ref(def_id).map(ty::EarlyBinder::instantiate_identity);
cgp::setup_constraining_predicates(
tcx,
&mut predicates,
trait_ref,
&mut cgp::parameters_for_impl(tcx, self_ty, trait_ref),
let mut input_parameters = cgp::parameters_for_impl(tcx, self_ty, trait_ref);
cgp::setup_constraining_predicates(tcx, &mut predicates, trait_ref, &mut input_parameters);
errored_due_to_unconstrained_params = errored_due_to_unconstrained_params.and(
self_ty.error_reported().and(trait_ref.error_reported()).and_then(|()| {
crate::impl_wf_check::enforce_impl_params_are_constrained(
tcx,
def_id,
input_parameters,
)
}),
);
}

Expand All @@ -338,6 +348,7 @@ fn gather_explicit_predicates_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Gen
ty::GenericPredicates {
parent: generics.parent,
predicates: tcx.arena.alloc_from_iter(predicates),
errored_due_to_unconstrained_params,
}
}

Expand Down Expand Up @@ -507,6 +518,8 @@ pub(super) fn explicit_predicates_of<'tcx>(
ty::GenericPredicates {
parent: predicates_and_bounds.parent,
predicates: tcx.arena.alloc_slice(&predicates),
errored_due_to_unconstrained_params: predicates_and_bounds
.errored_due_to_unconstrained_params,
}
}
} else {
Expand Down Expand Up @@ -558,6 +571,8 @@ pub(super) fn explicit_predicates_of<'tcx>(
return GenericPredicates {
parent: parent_preds.parent,
predicates: { tcx.arena.alloc_from_iter(filtered_predicates) },
errored_due_to_unconstrained_params: parent_preds
.errored_due_to_unconstrained_params,
};
}
gather_explicit_predicates_of(tcx, def_id)
Expand Down
23 changes: 17 additions & 6 deletions compiler/rustc_hir_analysis/src/collect/type_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,10 @@ pub(super) fn type_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::EarlyBinder<'_
use rustc_hir::*;
use rustc_middle::ty::Ty;

let hir_id = tcx.local_def_id_to_hir_id(def_id);

let icx = ItemCtxt::new(tcx, def_id);

// If we are computing `type_of` the synthesized associated type for an RPITIT in the impl
// side, use `collect_return_position_impl_trait_in_trait_tys` to infer the value of the
// associated type in the impl.
Expand All @@ -396,7 +400,15 @@ pub(super) fn type_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::EarlyBinder<'_
match tcx.collect_return_position_impl_trait_in_trait_tys(fn_def_id) {
Ok(map) => {
let assoc_item = tcx.associated_item(def_id);
return map[&assoc_item.trait_item_def_id.unwrap()];
let ty = map[&assoc_item.trait_item_def_id.unwrap()];
match icx.forbid_unconstrained_lifetime_params_from_parent_impl() {
Ok(()) => {
// Ensure no unconstrained lifetimes are used in these impls.
icx.record_ty(hir_id, ty.instantiate_identity(), tcx.def_span(def_id));
}
Err(guar) => return ty::EarlyBinder::bind(Ty::new_error(tcx, guar)),
}
return ty;
}
Err(_) => {
return ty::EarlyBinder::bind(Ty::new_error_with_message(
Expand All @@ -418,10 +430,6 @@ pub(super) fn type_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::EarlyBinder<'_
None => {}
}

let hir_id = tcx.local_def_id_to_hir_id(def_id);

let icx = ItemCtxt::new(tcx, def_id);

let output = match tcx.hir_node(hir_id) {
Node::TraitItem(item) => match item.kind {
TraitItemKind::Fn(..) => {
Expand Down Expand Up @@ -472,7 +480,10 @@ pub(super) fn type_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::EarlyBinder<'_
check_feature_inherent_assoc_ty(tcx, item.span);
}

icx.lower_ty(ty)
match icx.forbid_unconstrained_lifetime_params_from_parent_impl() {
Err(guar) => Ty::new_error(tcx, guar),
Ok(()) => icx.lower_ty(ty),
}
}
},

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir_analysis/src/delegation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,7 @@ impl<'tcx> PredicatesBuilder<'tcx> {
ty::GenericPredicates {
parent: self.parent,
predicates: self.tcx.arena.alloc_from_iter(preds),
errored_due_to_unconstrained_params: Ok(()),
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_hir_analysis/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1617,6 +1617,8 @@ pub(crate) struct UnconstrainedGenericParameter {
pub const_param_note: bool,
#[note(hir_analysis_const_param_note2)]
pub const_param_note2: bool,
#[help]
pub lifetime_help: Option<Span>,
}

#[derive(Diagnostic)]
Expand Down
Loading