Skip to content
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

Make deref_into_dyn_supertrait lint the impl and not the usage #104742

Merged
merged 3 commits into from
Nov 24, 2022
Merged
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
17 changes: 17 additions & 0 deletions compiler/rustc_lint/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1245,6 +1245,23 @@ impl<'tcx> LateContext<'tcx> {

AbsolutePathPrinter { tcx: self.tcx }.print_def_path(def_id, &[]).unwrap()
}

/// Returns the associated type `name` for `self_ty` as an implementation of `trait_id`.
/// Do not invoke without first verifying that the type implements the trait.
pub fn get_associated_type(
&self,
self_ty: Ty<'tcx>,
trait_id: DefId,
name: &str,
) -> Option<Ty<'tcx>> {
let tcx = self.tcx;
tcx.associated_items(trait_id)
.find_by_name_and_kind(tcx, Ident::from_str(name), ty::AssocKind::Type, trait_id)
.and_then(|assoc| {
let proj = tcx.mk_projection(assoc.def_id, tcx.mk_substs_trait(self_ty, []));
tcx.try_normalize_erasing_regions(self.param_env, proj).ok()
})
}
}

impl<'tcx> abi::HasDataLayout for LateContext<'tcx> {
Expand Down
92 changes: 92 additions & 0 deletions compiler/rustc_lint/src/deref_into_dyn_supertrait.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
use crate::{LateContext, LateLintPass, LintContext};

use rustc_errors::DelayDm;
use rustc_hir as hir;
use rustc_middle::{traits::util::supertraits, ty};
use rustc_span::sym;

declare_lint! {
/// The `deref_into_dyn_supertrait` lint is output whenever there is a use of the
/// `Deref` implementation with a `dyn SuperTrait` type as `Output`.
///
/// These implementations will become shadowed when the `trait_upcasting` feature is stabilized.
/// The `deref` functions will no longer be called implicitly, so there might be behavior change.
///
/// ### Example
///
/// ```rust,compile_fail
/// #![deny(deref_into_dyn_supertrait)]
/// #![allow(dead_code)]
///
/// use core::ops::Deref;
///
/// trait A {}
/// trait B: A {}
/// impl<'a> Deref for dyn 'a + B {
/// type Target = dyn A;
/// fn deref(&self) -> &Self::Target {
/// todo!()
/// }
/// }
///
/// fn take_a(_: &dyn A) { }
///
/// fn take_b(b: &dyn B) {
/// take_a(b);
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// The dyn upcasting coercion feature adds new coercion rules, taking priority
/// over certain other coercion rules, which will cause some behavior change.
pub DEREF_INTO_DYN_SUPERTRAIT,
Warn,
"`Deref` implementation usage with a supertrait trait object for output might be shadowed in the future",
@future_incompatible = FutureIncompatibleInfo {
reference: "issue #89460 <https://github.com/rust-lang/rust/issues/89460>",
};
}

declare_lint_pass!(DerefIntoDynSupertrait => [DEREF_INTO_DYN_SUPERTRAIT]);

impl<'tcx> LateLintPass<'tcx> for DerefIntoDynSupertrait {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'tcx>) {
// `Deref` is being implemented for `t`
if let hir::ItemKind::Impl(impl_) = item.kind
&& let Some(trait_) = &impl_.of_trait
&& let t = cx.tcx.type_of(item.owner_id)
&& let opt_did @ Some(did) = trait_.trait_def_id()
&& opt_did == cx.tcx.lang_items().deref_trait()
// `t` is `dyn t_principal`
&& let ty::Dynamic(data, _, ty::Dyn) = t.kind()
&& let Some(t_principal) = data.principal()
// `<T as Deref>::Target` is `dyn target_principal`
&& let Some(target) = cx.get_associated_type(t, did, "Target")
&& let ty::Dynamic(data, _, ty::Dyn) = target.kind()
&& let Some(target_principal) = data.principal()
// `target_principal` is a supertrait of `t_principal`
&& supertraits(cx.tcx, t_principal.with_self_ty(cx.tcx, cx.tcx.types.trait_object_dummy_self))
.any(|sup| sup.map_bound(|x| ty::ExistentialTraitRef::erase_self_ty(cx.tcx, x)) == target_principal)
{
cx.struct_span_lint(
DEREF_INTO_DYN_SUPERTRAIT,
cx.tcx.def_span(item.owner_id.def_id),
DelayDm(|| {
format!(
"`{t}` implements `Deref` with supertrait `{target_principal}` as target"
)
}),
|lint| {
if let Some(target_span) = impl_.items.iter().find_map(|i| (i.ident.name == sym::Target).then_some(i.span)) {
lint.span_label(target_span, "target type is set here");
}

lint
},
)
}
}
}
3 changes: 3 additions & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ extern crate tracing;
mod array_into_iter;
pub mod builtin;
mod context;
mod deref_into_dyn_supertrait;
mod early;
mod enum_intrinsics_non_enums;
mod errors;
Expand Down Expand Up @@ -87,6 +88,7 @@ use rustc_span::Span;

use array_into_iter::ArrayIntoIter;
use builtin::*;
use deref_into_dyn_supertrait::*;
use enum_intrinsics_non_enums::EnumIntrinsicsNonEnums;
use for_loops_over_fallibles::*;
use hidden_unicode_codepoints::*;
Expand Down Expand Up @@ -192,6 +194,7 @@ macro_rules! late_lint_mod_passes {
$args,
[
ForLoopsOverFallibles: ForLoopsOverFallibles,
DerefIntoDynSupertrait: DerefIntoDynSupertrait,
HardwiredLints: HardwiredLints,
ImproperCTypesDeclarations: ImproperCTypesDeclarations,
ImproperCTypesDefinitions: ImproperCTypesDefinitions,
Expand Down
46 changes: 0 additions & 46 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3262,7 +3262,6 @@ declare_lint_pass! {
UNUSED_TUPLE_STRUCT_FIELDS,
NON_EXHAUSTIVE_OMITTED_PATTERNS,
TEXT_DIRECTION_CODEPOINT_IN_COMMENT,
DEREF_INTO_DYN_SUPERTRAIT,
DEPRECATED_CFG_ATTR_CRATE_TYPE_NAME,
DUPLICATE_MACRO_ATTRIBUTES,
SUSPICIOUS_AUTO_TRAIT_IMPLS,
Expand Down Expand Up @@ -3764,51 +3763,6 @@ declare_lint! {
"invisible directionality-changing codepoints in comment"
}

declare_lint! {
/// The `deref_into_dyn_supertrait` lint is output whenever there is a use of the
/// `Deref` implementation with a `dyn SuperTrait` type as `Output`.
///
/// These implementations will become shadowed when the `trait_upcasting` feature is stabilized.
/// The `deref` functions will no longer be called implicitly, so there might be behavior change.
///
/// ### Example
///
/// ```rust,compile_fail
/// #![deny(deref_into_dyn_supertrait)]
/// #![allow(dead_code)]
///
/// use core::ops::Deref;
///
/// trait A {}
/// trait B: A {}
/// impl<'a> Deref for dyn 'a + B {
/// type Target = dyn A;
/// fn deref(&self) -> &Self::Target {
/// todo!()
/// }
/// }
///
/// fn take_a(_: &dyn A) { }
///
/// fn take_b(b: &dyn B) {
/// take_a(b);
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// The dyn upcasting coercion feature adds new coercion rules, taking priority
/// over certain other coercion rules, which will cause some behavior change.
pub DEREF_INTO_DYN_SUPERTRAIT,
Warn,
"`Deref` implementation usage with a supertrait trait object for output might be shadowed in the future",
@future_incompatible = FutureIncompatibleInfo {
reference: "issue #89460 <https://github.com/rust-lang/rust/issues/89460>",
};
}

declare_lint! {
/// The `duplicate_macro_attributes` lint detects when a `#[test]`-like built-in macro
/// attribute is duplicated on an item. This lint may trigger on `bench`, `cfg_eval`, `test`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,9 @@
//!
//! [rustc dev guide]:https://rustc-dev-guide.rust-lang.org/traits/resolution.html#candidate-assembly
use hir::LangItem;
use rustc_errors::DelayDm;
use rustc_hir as hir;
use rustc_infer::traits::ObligationCause;
use rustc_infer::traits::{Obligation, SelectionError, TraitObligation};
use rustc_lint_defs::builtin::DEREF_INTO_DYN_SUPERTRAIT;
use rustc_middle::ty::print::with_no_trimmed_paths;
use rustc_middle::ty::{self, Ty, TypeVisitable};
use rustc_target::spec::abi::Abi;
Expand Down Expand Up @@ -811,16 +809,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
&obligation.cause,
) {
if deref_trait_ref.def_id() == target_trait_did {
self.tcx().struct_span_lint_hir(
DEREF_INTO_DYN_SUPERTRAIT,
obligation.cause.body_id,
obligation.cause.span,
DelayDm(|| format!(
"`{}` implements `Deref` with supertrait `{}` as output",
source, deref_trait_ref
)),
|lint| lint,
);
return;
}
}
Expand Down
6 changes: 4 additions & 2 deletions src/test/ui/traits/trait-upcasting/migrate-lint-deny.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@ use core::ops::Deref;
// issue 89190
trait A {}
trait B: A {}

impl<'a> Deref for dyn 'a + B {
//~^ ERROR `(dyn B + 'a)` implements `Deref` with supertrait `A` as target
//~| WARN this was previously accepted by the compiler but is being phased out;

type Target = dyn A;
fn deref(&self) -> &Self::Target {
todo!()
Expand All @@ -18,8 +22,6 @@ fn take_a(_: &dyn A) {}

fn whoops(b: &dyn B) {
take_a(b)
//~^ ERROR `dyn B` implements `Deref` with supertrait `A` as output
//~^^ WARN this was previously accepted by the compiler but is being phased out;
}

fn main() {}
11 changes: 7 additions & 4 deletions src/test/ui/traits/trait-upcasting/migrate-lint-deny.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
error: `dyn B` implements `Deref` with supertrait `A` as output
--> $DIR/migrate-lint-deny.rs:20:12
error: `(dyn B + 'a)` implements `Deref` with supertrait `A` as target
--> $DIR/migrate-lint-deny.rs:11:1
|
LL | take_a(b)
| ^
LL | impl<'a> Deref for dyn 'a + B {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
LL | type Target = dyn A;
| -------------------- target type is set here
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #89460 <https://github.com/rust-lang/rust/issues/89460>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::snippet_opt;
use clippy_utils::ty::{get_associated_type, implements_trait, is_copy};
use clippy_utils::ty::{implements_trait, is_copy};
use rustc_errors::Applicability;
use rustc_hir::Expr;
use rustc_lint::LateContext;
Expand All @@ -25,7 +25,7 @@ pub(super) fn check<'tcx>(
&& let Some(method_id) = typeck.type_dependent_def_id(cloned_call.hir_id)
&& cx.tcx.trait_of_item(method_id) == Some(iter_id)
&& let cloned_recv_ty = typeck.expr_ty_adjusted(cloned_recv)
&& let Some(iter_assoc_ty) = get_associated_type(cx, cloned_recv_ty, iter_id, "Item")
&& let Some(iter_assoc_ty) = cx.get_associated_type(cloned_recv_ty, iter_id, "Item")
&& matches!(*iter_assoc_ty.kind(), ty::Ref(_, ty, _) if !is_copy(cx, ty))
{
if needs_into_iter
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use super::utils::clone_or_copy_needed;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::higher::ForLoop;
use clippy_utils::source::snippet_opt;
use clippy_utils::ty::{get_associated_type, get_iterator_item_ty, implements_trait};
use clippy_utils::ty::{get_iterator_item_ty, implements_trait};
use clippy_utils::{fn_def_id, get_parent_expr};
use rustc_errors::Applicability;
use rustc_hir::{def_id::DefId, Expr, ExprKind};
Expand Down Expand Up @@ -54,7 +54,7 @@ pub fn check_for_loop_iter(
if let Some(into_iterator_trait_id) = cx.tcx.get_diagnostic_item(sym::IntoIterator);
let collection_ty = cx.typeck_results().expr_ty(collection);
if implements_trait(cx, collection_ty, into_iterator_trait_id, &[]);
if let Some(into_iter_item_ty) = get_associated_type(cx, collection_ty, into_iterator_trait_id, "Item");
if let Some(into_iter_item_ty) = cx.get_associated_type(collection_ty, into_iterator_trait_id, "Item");

if iter_item_ty == into_iter_item_ty;
if let Some(collection_snippet) = snippet_opt(cx, collection.span);
Expand Down
27 changes: 18 additions & 9 deletions src/tools/clippy/clippy_lints/src/methods/unnecessary_to_owned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ use super::implicit_clone::is_clone_like;
use super::unnecessary_iter_cloned::{self, is_into_iter};
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_opt;
use clippy_utils::ty::{get_associated_type, get_iterator_item_ty, implements_trait, is_copy, peel_mid_ty_refs};
use clippy_utils::ty::{get_iterator_item_ty, implements_trait, is_copy, peel_mid_ty_refs};
use clippy_utils::visitors::find_all_ret_expressions;
use clippy_utils::{fn_def_id, get_parent_expr, is_diag_item_method, is_diag_trait_item, return_ty};
use clippy_utils::{
fn_def_id, get_parent_expr, is_diag_item_method, is_diag_trait_item, return_ty,
};
use clippy_utils::{meets_msrv, msrvs};
use rustc_errors::Applicability;
use rustc_hir::{def_id::DefId, BorrowKind, Expr, ExprKind, ItemKind, Node};
Expand All @@ -18,7 +20,9 @@ use rustc_middle::ty::EarlyBinder;
use rustc_middle::ty::{self, ParamTy, PredicateKind, ProjectionPredicate, TraitPredicate, Ty};
use rustc_semver::RustcVersion;
use rustc_span::{sym, Symbol};
use rustc_trait_selection::traits::{query::evaluate_obligation::InferCtxtExt as _, Obligation, ObligationCause};
use rustc_trait_selection::traits::{
query::evaluate_obligation::InferCtxtExt as _, Obligation, ObligationCause,
};
use std::cmp::max;

use super::UNNECESSARY_TO_OWNED;
Expand Down Expand Up @@ -146,7 +150,7 @@ fn check_addr_of_expr(
if_chain! {
if let Some(deref_trait_id) = cx.tcx.get_diagnostic_item(sym::Deref);
if implements_trait(cx, receiver_ty, deref_trait_id, &[]);
if get_associated_type(cx, receiver_ty, deref_trait_id, "Target") == Some(target_ty);
if cx.get_associated_type(receiver_ty, deref_trait_id, "Target") == Some(target_ty);
then {
if n_receiver_refs > 0 {
span_lint_and_sugg(
Expand Down Expand Up @@ -341,13 +345,13 @@ fn get_input_traits_and_projections<'tcx>(
if trait_predicate.trait_ref.self_ty() == input {
trait_predicates.push(trait_predicate);
}
},
}
PredicateKind::Projection(projection_predicate) => {
if projection_predicate.projection_ty.self_ty() == input {
projection_predicates.push(projection_predicate);
}
},
_ => {},
}
_ => {}
}
}
(trait_predicates, projection_predicates)
Expand Down Expand Up @@ -462,7 +466,12 @@ fn is_cloned_or_copied(cx: &LateContext<'_>, method_name: Symbol, method_def_id:

/// Returns true if the named method can be used to convert the receiver to its "owned"
/// representation.
fn is_to_owned_like<'a>(cx: &LateContext<'a>, call_expr: &Expr<'a>, method_name: Symbol, method_def_id: DefId) -> bool {
fn is_to_owned_like<'a>(
cx: &LateContext<'a>,
call_expr: &Expr<'a>,
method_name: Symbol,
method_def_id: DefId,
) -> bool {
is_clone_like(cx, method_name.as_str(), method_def_id)
|| is_cow_into_owned(cx, method_name, method_def_id)
|| is_to_string_on_string_like(cx, call_expr, method_name, method_def_id)
Expand Down Expand Up @@ -490,7 +499,7 @@ fn is_to_string_on_string_like<'a>(
&& let GenericArgKind::Type(ty) = generic_arg.unpack()
&& let Some(deref_trait_id) = cx.tcx.get_diagnostic_item(sym::Deref)
&& let Some(as_ref_trait_id) = cx.tcx.get_diagnostic_item(sym::AsRef)
&& (get_associated_type(cx, ty, deref_trait_id, "Target") == Some(cx.tcx.types.str_) ||
&& (cx.get_associated_type(ty, deref_trait_id, "Target") == Some(cx.tcx.types.str_) ||
implements_trait(cx, ty, as_ref_trait_id, &[cx.tcx.types.str_.into()])) {
true
} else {
Expand Down
Loading