bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Rollup merge of #120716 - spastorino:change-some-lint-msgs, r=lcnr

Browse source 
Change leak check and suspicious auto trait lint warning messages

The leak check lint message "this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!" is misleading as some cases may not be phased out and could end being accepted. This is under discussion still.

The suspicious auto trait lint the change in behavior already happened, so the new message is probably more accurate.

r? `@lcnr`

Closes #93367
This commit is contained in:
Nilstrieb 2024-02-20 07:35:45 +01:00 • committed by GitHub
commit 5b8b435d5d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
35 changed files with 104 additions and 563 deletions
Download patch file Download diff file Expand all files Collapse all files

188
compiler/rustc_hir_analysis/src/coherence/orphan.rs
View file

@ -1,20 +1,12 @@
//! Orphan checker: every impl either implements a trait defined in this
//! crate or pertains to a type defined in this crate.
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::{DelayDm, ErrorGuaranteed};
use rustc_errors::ErrorGuaranteed;
use rustc_hir as hir;
use rustc_middle::ty::util::CheckRegions;
use rustc_middle::ty::GenericArgs;
use rustc_middle::ty::{
self, AliasKind, ImplPolarity, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitableExt,
TypeVisitor,
};
use rustc_session::lint;
use rustc_span::def_id::{DefId, LocalDefId};
use rustc_middle::ty::{self, AliasKind, Ty, TyCtxt, TypeVisitableExt};
use rustc_span::def_id::LocalDefId;
use rustc_span::Span;
use rustc_trait_selection::traits;
use std::ops::ControlFlow;
use crate::errors;
@ -26,30 +18,17 @@ pub(crate) fn orphan_check_impl(
let trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap().instantiate_identity();
trait_ref.error_reported()?;
let ret = do_orphan_check_impl(tcx, trait_ref, impl_def_id);
if tcx.trait_is_auto(trait_ref.def_id) {
lint_auto_trait_impl(tcx, trait_ref, impl_def_id);
}
ret
}
fn do_orphan_check_impl<'tcx>(
tcx: TyCtxt<'tcx>,
trait_ref: ty::TraitRef<'tcx>,
def_id: LocalDefId,
) -> Result<(), ErrorGuaranteed> {
let trait_def_id = trait_ref.def_id;
match traits::orphan_check(tcx, def_id.to_def_id()) {
match traits::orphan_check(tcx, impl_def_id.to_def_id()) {
Ok(()) => {}
Err(err) => {
let item = tcx.hir().expect_item(def_id);
let item = tcx.hir().expect_item(impl_def_id);
let hir::ItemKind::Impl(impl_) = item.kind else {
bug!("{:?} is not an impl: {:?}", def_id, item);
bug!("{:?} is not an impl: {:?}", impl_def_id, item);
};
let tr = impl_.of_trait.as_ref().unwrap();
let sp = tcx.def_span(def_id);
let sp = tcx.def_span(impl_def_id);
emit_orphan_check_error(
tcx,
@ -193,7 +172,7 @@ fn do_orphan_check_impl<'tcx>(
// impl<T> AutoTrait for T {}
// impl<T: ?Sized> AutoTrait for T {}
ty::Param(..) => (
if self_ty.is_sized(tcx, tcx.param_env(def_id)) {
if self_ty.is_sized(tcx, tcx.param_env(impl_def_id)) {
LocalImpl::Allow
} else {
LocalImpl::Disallow { problematic_kind: "generic type" }
@ -250,7 +229,7 @@ fn do_orphan_check_impl<'tcx>(
| ty::Bound(..)
| ty::Placeholder(..)
| ty::Infer(..) => {
let sp = tcx.def_span(def_id);
let sp = tcx.def_span(impl_def_id);
span_bug!(sp, "weird self type for autotrait impl")
}
@ -262,7 +241,7 @@ fn do_orphan_check_impl<'tcx>(
LocalImpl::Allow => {}
LocalImpl::Disallow { problematic_kind } => {
return Err(tcx.dcx().emit_err(errors::TraitsWithDefaultImpl {
span: tcx.def_span(def_id),
span: tcx.def_span(impl_def_id),
traits: tcx.def_path_str(trait_def_id),
problematic_kind,
self_ty,
@ -274,13 +253,13 @@ fn do_orphan_check_impl<'tcx>(
NonlocalImpl::Allow => {}
NonlocalImpl::DisallowBecauseNonlocal => {
return Err(tcx.dcx().emit_err(errors::CrossCrateTraitsDefined {
span: tcx.def_span(def_id),
span: tcx.def_span(impl_def_id),
traits: tcx.def_path_str(trait_def_id),
}));
}
NonlocalImpl::DisallowOther => {
return Err(tcx.dcx().emit_err(errors::CrossCrateTraits {
span: tcx.def_span(def_id),
span: tcx.def_span(impl_def_id),
traits: tcx.def_path_str(trait_def_id),
self_ty,
}));
@ -445,146 +424,3 @@ fn emit_orphan_check_error<'tcx>(
}
})
}
/// Lint impls of auto traits if they are likely to have
/// unsound or surprising effects on auto impls.
fn lint_auto_trait_impl<'tcx>(
tcx: TyCtxt<'tcx>,
trait_ref: ty::TraitRef<'tcx>,
impl_def_id: LocalDefId,
) {
if trait_ref.args.len() != 1 {
tcx.dcx().span_delayed_bug(
tcx.def_span(impl_def_id),
"auto traits cannot have generic parameters",
);
return;
}
let self_ty = trait_ref.self_ty();
let (self_type_did, args) = match self_ty.kind() {
ty::Adt(def, args) => (def.did(), args),
_ => {
// FIXME: should also lint for stuff like `&i32` but
// considering that auto traits are unstable, that
// isn't too important for now as this only affects
// crates using `nightly`, and std.
return;
}
};
// Impls which completely cover a given root type are fine as they
// disable auto impls entirely. So only lint if the args
// are not a permutation of the identity args.
let Err(arg) = tcx.uses_unique_generic_params(args, CheckRegions::No) else {
// ok
return;
};
// Ideally:
//
// - compute the requirements for the auto impl candidate
// - check whether these are implied by the non covering impls
// - if not, emit the lint
//
// What we do here is a bit simpler:
//
// - badly check if an auto impl candidate definitely does not apply
// for the given simplified type
// - if so, do not lint
if fast_reject_auto_impl(tcx, trait_ref.def_id, self_ty) {
// ok
return;
}
tcx.node_span_lint(
lint::builtin::SUSPICIOUS_AUTO_TRAIT_IMPLS,
tcx.local_def_id_to_hir_id(impl_def_id),
tcx.def_span(impl_def_id),
DelayDm(|| {
format!(
"cross-crate traits with a default impl, like `{}`, \
should not be specialized",
tcx.def_path_str(trait_ref.def_id),
)
}),
|lint| {
let item_span = tcx.def_span(self_type_did);
let self_descr = tcx.def_descr(self_type_did);
match arg {
ty::util::NotUniqueParam::DuplicateParam(arg) => {
lint.note(format!("`{arg}` is mentioned multiple times"));
}
ty::util::NotUniqueParam::NotParam(arg) => {
lint.note(format!("`{arg}` is not a generic parameter"));
}
}
lint.span_note(
item_span,
format!(
"try using the same sequence of generic parameters as the {self_descr} definition",
),
);
},
);
}
fn fast_reject_auto_impl<'tcx>(tcx: TyCtxt<'tcx>, trait_def_id: DefId, self_ty: Ty<'tcx>) -> bool {
struct DisableAutoTraitVisitor<'tcx> {
tcx: TyCtxt<'tcx>,
trait_def_id: DefId,
self_ty_root: Ty<'tcx>,
seen: FxHashSet<DefId>,
}
impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for DisableAutoTraitVisitor<'tcx> {
type BreakTy = ();
fn visit_ty(&mut self, ty: Ty<'tcx>) -> ControlFlow<Self::BreakTy> {
let tcx = self.tcx;
if ty != self.self_ty_root {
for impl_def_id in tcx.non_blanket_impls_for_ty(self.trait_def_id, ty) {
match tcx.impl_polarity(impl_def_id) {
ImplPolarity::Negative => return ControlFlow::Break(()),
ImplPolarity::Reservation => {}
// FIXME(@lcnr): That's probably not good enough, idk
//
// We might just want to take the rustdoc code and somehow avoid
// explicit impls for `Self`.
ImplPolarity::Positive => return ControlFlow::Continue(()),
}
}
}
match ty.kind() {
ty::Adt(def, args) if def.is_phantom_data() => args.visit_with(self),
ty::Adt(def, args) => {
// @lcnr: This is the only place where cycles can happen. We avoid this
// by only visiting each `DefId` once.
//
// This will be is incorrect in subtle cases, but I don't care :)
if self.seen.insert(def.did()) {
for ty in def.all_fields().map(|field| field.ty(tcx, args)) {
ty.visit_with(self)?;
}
}
ControlFlow::Continue(())
}
_ => ty.super_visit_with(self),
}
}
}
let self_ty_root = match self_ty.kind() {
ty::Adt(def, _) => Ty::new_adt(tcx, *def, GenericArgs::identity_for_item(tcx, def.did())),
_ => unimplemented!("unexpected self ty {:?}", self_ty),
};
self_ty_root
.visit_with(&mut DisableAutoTraitVisitor {
tcx,
self_ty_root,
trait_def_id,
seen: FxHashSet::default(),
})
.is_break()
}