Skip to content

Commit

Permalink
Rollup merge of #104742 - WaffleLapkin:forbidden-SUPER-deref, r=compi…
Browse files Browse the repository at this point in the history
…ler-errors

Make `deref_into_dyn_supertrait` lint the impl and not the usage

Proposed by ``@compiler-errors`` in #89460 (comment)
r? ``@crlf0710``
  • Loading branch information
matthiaskrgr authored Nov 24, 2022
2 parents b43c2e7 + 20f3de5 commit c08c57e
Show file tree
Hide file tree
Showing 11 changed files with 146 additions and 95 deletions.
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 @@ -809,16 +807,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

0 comments on commit c08c57e

Please sign in to comment.