From 4c3e330a8c023af20beb23a3a46b5d6d77a62839 Mon Sep 17 00:00:00 2001 From: Mahdi Dibaiee Date: Fri, 7 Jan 2022 11:38:16 +0000 Subject: [PATCH 1/6] feat: pass_by_value lint attribute Useful for thin wrapper attributes that are best passed as value instead of reference. --- compiler/rustc_feature/src/builtin_attrs.rs | 5 + compiler/rustc_lint/src/internal.rs | 33 +----- compiler/rustc_lint/src/lib.rs | 6 +- compiler/rustc_lint/src/pass_by_value.rs | 103 ++++++++++++++++++ compiler/rustc_middle/src/ty/context.rs | 1 + compiler/rustc_middle/src/ty/mod.rs | 1 + compiler/rustc_passes/src/check_attr.rs | 24 ++++ compiler/rustc_span/src/symbol.rs | 1 + ...ss_ty_by_ref.rs => rustc_pass_by_value.rs} | 2 +- ..._ref.stderr => rustc_pass_by_value.stderr} | 30 ++--- ...ef_self.rs => rustc_pass_by_value_self.rs} | 5 +- ...stderr => rustc_pass_by_value_self.stderr} | 10 +- 12 files changed, 165 insertions(+), 56 deletions(-) create mode 100644 compiler/rustc_lint/src/pass_by_value.rs rename src/test/ui-fulldeps/internal-lints/{pass_ty_by_ref.rs => rustc_pass_by_value.rs} (97%) rename src/test/ui-fulldeps/internal-lints/{pass_ty_by_ref.stderr => rustc_pass_by_value.stderr} (80%) rename src/test/ui-fulldeps/internal-lints/{pass_ty_by_ref_self.rs => rustc_pass_by_value_self.rs} (90%) rename src/test/ui-fulldeps/internal-lints/{pass_ty_by_ref_self.stderr => rustc_pass_by_value_self.stderr} (65%) diff --git a/compiler/rustc_feature/src/builtin_attrs.rs b/compiler/rustc_feature/src/builtin_attrs.rs index f25b2d8f566c0..88bf81864b23f 100644 --- a/compiler/rustc_feature/src/builtin_attrs.rs +++ b/compiler/rustc_feature/src/builtin_attrs.rs @@ -623,6 +623,11 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ lang, Normal, template!(NameValueStr: "name"), DuplicatesOk, lang_items, "language items are subject to change", ), + rustc_attr!( + rustc_pass_by_value, Normal, + template!(Word, NameValueStr: "reason"), WarnFollowing, + "#[rustc_pass_by_value] is used to mark types that must be passed by value instead of reference." + ), BuiltinAttribute { name: sym::rustc_diagnostic_item, type_: Normal, diff --git a/compiler/rustc_lint/src/internal.rs b/compiler/rustc_lint/src/internal.rs index c64a67b6b9f1b..7353cd6b876b9 100644 --- a/compiler/rustc_lint/src/internal.rs +++ b/compiler/rustc_lint/src/internal.rs @@ -5,10 +5,7 @@ use crate::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext} use rustc_ast as ast; use rustc_errors::Applicability; use rustc_hir::def::Res; -use rustc_hir::{ - GenericArg, HirId, Item, ItemKind, MutTy, Mutability, Node, Path, PathSegment, QPath, Ty, - TyKind, -}; +use rustc_hir::{GenericArg, HirId, Item, ItemKind, Node, Path, PathSegment, QPath, Ty, TyKind}; use rustc_middle::ty; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::hygiene::{ExpnKind, MacroKind}; @@ -58,13 +55,6 @@ declare_tool_lint! { report_in_external_macro: true } -declare_tool_lint! { - pub rustc::TY_PASS_BY_REFERENCE, - Allow, - "passing `Ty` or `TyCtxt` by reference", - report_in_external_macro: true -} - declare_tool_lint! { pub rustc::USAGE_OF_QUALIFIED_TY, Allow, @@ -74,7 +64,6 @@ declare_tool_lint! { declare_lint_pass!(TyTyKind => [ USAGE_OF_TY_TYKIND, - TY_PASS_BY_REFERENCE, USAGE_OF_QUALIFIED_TY, ]); @@ -131,26 +120,6 @@ impl<'tcx> LateLintPass<'tcx> for TyTyKind { } } } - TyKind::Rptr(_, MutTy { ty: inner_ty, mutbl: Mutability::Not }) => { - if let Some(impl_did) = cx.tcx.impl_of_method(ty.hir_id.owner.to_def_id()) { - if cx.tcx.impl_trait_ref(impl_did).is_some() { - return; - } - } - if let Some(t) = is_ty_or_ty_ctxt(cx, &inner_ty) { - cx.struct_span_lint(TY_PASS_BY_REFERENCE, ty.span, |lint| { - lint.build(&format!("passing `{}` by reference", t)) - .span_suggestion( - ty.span, - "try passing by value", - t, - // Changing type of function argument - Applicability::MaybeIncorrect, - ) - .emit(); - }) - } - } _ => {} } } diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index c7823032b0c23..3b95a2487baed 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -56,6 +56,7 @@ mod non_ascii_idents; mod non_fmt_panic; mod nonstandard_style; mod noop_method_call; +mod pass_by_value; mod passes; mod redundant_semicolon; mod traits; @@ -85,6 +86,7 @@ use non_ascii_idents::*; use non_fmt_panic::NonPanicFmt; use nonstandard_style::*; use noop_method_call::*; +use pass_by_value::*; use redundant_semicolon::*; use traits::*; use types::*; @@ -489,6 +491,8 @@ fn register_internals(store: &mut LintStore) { store.register_late_pass(|| Box::new(ExistingDocKeyword)); store.register_lints(&TyTyKind::get_lints()); store.register_late_pass(|| Box::new(TyTyKind)); + store.register_lints(&PassByValue::get_lints()); + store.register_late_pass(|| Box::new(PassByValue)); store.register_group( false, "rustc::internal", @@ -496,8 +500,8 @@ fn register_internals(store: &mut LintStore) { vec![ LintId::of(DEFAULT_HASH_TYPES), LintId::of(USAGE_OF_TY_TYKIND), + LintId::of(PASS_BY_VALUE), LintId::of(LINT_PASS_IMPL_WITHOUT_MACRO), - LintId::of(TY_PASS_BY_REFERENCE), LintId::of(USAGE_OF_QUALIFIED_TY), LintId::of(EXISTING_DOC_KEYWORD), ], diff --git a/compiler/rustc_lint/src/pass_by_value.rs b/compiler/rustc_lint/src/pass_by_value.rs new file mode 100644 index 0000000000000..0bfa2a673c2c4 --- /dev/null +++ b/compiler/rustc_lint/src/pass_by_value.rs @@ -0,0 +1,103 @@ +use crate::{LateContext, LateLintPass, LintContext}; +use rustc_errors::Applicability; +use rustc_hir as hir; +use rustc_hir::def::Res; +use rustc_hir::def_id::DefId; +use rustc_hir::{GenericArg, PathSegment, QPath, TyKind}; +use rustc_middle::ty; +use rustc_span::symbol::sym; + +declare_tool_lint! { + /// The `rustc_pass_by_value` lint marks a type with `#[rustc_pass_by_value]` requiring it to always be passed by value. + /// This is usually used for types that are thin wrappers around references, so there is no benefit to an extra + /// layer of indirection. (Example: `Ty` which is a reference to a `TyS`) + pub rustc::PASS_BY_VALUE, + Warn, + "pass by reference of a type flagged as `#[rustc_pass_by_value]`", + report_in_external_macro: true +} + +declare_lint_pass!(PassByValue => [PASS_BY_VALUE]); + +impl<'tcx> LateLintPass<'tcx> for PassByValue { + fn check_ty(&mut self, cx: &LateContext<'_>, ty: &'tcx hir::Ty<'tcx>) { + match &ty.kind { + TyKind::Rptr(_, hir::MutTy { ty: inner_ty, mutbl: hir::Mutability::Not }) => { + if let Some(impl_did) = cx.tcx.impl_of_method(ty.hir_id.owner.to_def_id()) { + if cx.tcx.impl_trait_ref(impl_did).is_some() { + return; + } + } + if let Some(t) = path_for_pass_by_value(cx, &inner_ty) { + cx.struct_span_lint(PASS_BY_VALUE, ty.span, |lint| { + lint.build(&format!("passing `{}` by reference", t)) + .span_suggestion( + ty.span, + "try passing by value", + t, + // Changing type of function argument + Applicability::MaybeIncorrect, + ) + .emit(); + }) + } + } + _ => {} + } + } +} + +fn path_for_pass_by_value(cx: &LateContext<'_>, ty: &hir::Ty<'_>) -> Option { + if let TyKind::Path(QPath::Resolved(_, path)) = &ty.kind { + match path.res { + Res::Def(_, def_id) if has_pass_by_value_attr(cx, def_id) => { + if let Some(name) = cx.tcx.get_diagnostic_name(def_id) { + return Some(format!("{}{}", name, gen_args(path.segments.last().unwrap()))); + } + } + Res::SelfTy(None, Some((did, _))) => { + if let ty::Adt(adt, substs) = cx.tcx.type_of(did).kind() { + if has_pass_by_value_attr(cx, adt.did) { + if let Some(name) = cx.tcx.get_diagnostic_name(adt.did) { + return Some(format!("{}<{}>", name, substs[0])); + } + } + } + } + _ => (), + } + } + + None +} + +fn has_pass_by_value_attr(cx: &LateContext<'_>, def_id: DefId) -> bool { + for attr in cx.tcx.get_attrs(def_id).iter() { + if attr.has_name(sym::rustc_pass_by_value) { + return true; + } + } + false +} + +fn gen_args(segment: &PathSegment<'_>) -> String { + if let Some(args) = &segment.args { + let lifetimes = args + .args + .iter() + .filter_map(|arg| { + if let GenericArg::Lifetime(lt) = arg { + Some(lt.name.ident().to_string()) + } else { + None + } + }) + .collect::>(); + + if !lifetimes.is_empty() { + return format!("<{}>", lifetimes.join(", ")); + } + } + + String::new() +} diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index dd571e29bf695..ab85f104ce398 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -961,6 +961,7 @@ pub struct FreeRegionInfo { /// [rustc dev guide]: https://rustc-dev-guide.rust-lang.org/ty.html #[derive(Copy, Clone)] #[rustc_diagnostic_item = "TyCtxt"] +#[cfg_attr(not(bootstrap), rustc_pass_by_value)] pub struct TyCtxt<'tcx> { gcx: &'tcx GlobalCtxt<'tcx>, } diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index 78ccfbd5e8cdc..365d4c4aabaad 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -462,6 +462,7 @@ impl<'a, 'tcx> HashStable> for TyS<'tcx> { } #[rustc_diagnostic_item = "Ty"] +#[cfg_attr(not(bootstrap), rustc_pass_by_value)] pub type Ty<'tcx> = &'tcx TyS<'tcx>; impl ty::EarlyBoundRegion { diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs index d7b00699491d4..2febb2e56ecf8 100644 --- a/compiler/rustc_passes/src/check_attr.rs +++ b/compiler/rustc_passes/src/check_attr.rs @@ -114,6 +114,7 @@ impl CheckAttrVisitor<'_> { } sym::must_not_suspend => self.check_must_not_suspend(&attr, span, target), sym::must_use => self.check_must_use(hir_id, &attr, span, target), + sym::rustc_pass_by_value => self.check_pass_by_value(&attr, span, target), sym::rustc_const_unstable | sym::rustc_const_stable | sym::unstable @@ -1066,6 +1067,29 @@ impl CheckAttrVisitor<'_> { is_valid } + /// Warns against some misuses of `#[pass_by_value]` + fn check_pass_by_value(&self, attr: &Attribute, span: &Span, target: Target) -> bool { + match target { + Target::Struct + | Target::Enum + | Target::Union + | Target::Trait + | Target::TraitAlias + | Target::TyAlias => true, + _ => { + self.tcx + .sess + .struct_span_err( + attr.span, + "`pass_by_value` attribute should be applied to a struct, enum, trait or type alias.", + ) + .span_label(*span, "is not a struct, enum, trait or type alias") + .emit(); + false + } + } + } + /// Warns against some misuses of `#[must_use]` fn check_must_use( &self, diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 84cf8878af809..b1d868fbb88f6 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1143,6 +1143,7 @@ symbols! { rustc_paren_sugar, rustc_partition_codegened, rustc_partition_reused, + rustc_pass_by_value, rustc_peek, rustc_peek_definite_init, rustc_peek_liveness, diff --git a/src/test/ui-fulldeps/internal-lints/pass_ty_by_ref.rs b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs similarity index 97% rename from src/test/ui-fulldeps/internal-lints/pass_ty_by_ref.rs rename to src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs index e0fdbaeac3069..783019d894513 100644 --- a/src/test/ui-fulldeps/internal-lints/pass_ty_by_ref.rs +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs @@ -1,7 +1,7 @@ // compile-flags: -Z unstable-options #![feature(rustc_private)] -#![deny(rustc::ty_pass_by_reference)] +#![deny(rustc::pass_by_value)] #![allow(unused)] extern crate rustc_middle; diff --git a/src/test/ui-fulldeps/internal-lints/pass_ty_by_ref.stderr b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr similarity index 80% rename from src/test/ui-fulldeps/internal-lints/pass_ty_by_ref.stderr rename to src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr index 2751a37f7419d..5fbde93878931 100644 --- a/src/test/ui-fulldeps/internal-lints/pass_ty_by_ref.stderr +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr @@ -1,77 +1,77 @@ error: passing `Ty<'_>` by reference - --> $DIR/pass_ty_by_ref.rs:13:13 + --> $DIR/rustc_pass_by_value.rs:13:13 | LL | ty_ref: &Ty<'_>, | ^^^^^^^ help: try passing by value: `Ty<'_>` | note: the lint level is defined here - --> $DIR/pass_ty_by_ref.rs:4:9 + --> $DIR/rustc_pass_by_value.rs:4:9 | -LL | #![deny(rustc::ty_pass_by_reference)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | #![deny(rustc::pass_by_value)] + | ^^^^^^^^^^^^^^^^^^^^ error: passing `TyCtxt<'_>` by reference - --> $DIR/pass_ty_by_ref.rs:15:18 + --> $DIR/rustc_pass_by_value.rs:15:18 | LL | ty_ctxt_ref: &TyCtxt<'_>, | ^^^^^^^^^^^ help: try passing by value: `TyCtxt<'_>` error: passing `Ty<'_>` by reference - --> $DIR/pass_ty_by_ref.rs:19:28 + --> $DIR/rustc_pass_by_value.rs:19:28 | LL | fn ty_multi_ref(ty_multi: &&Ty<'_>, ty_ctxt_multi: &&&&TyCtxt<'_>) {} | ^^^^^^^ help: try passing by value: `Ty<'_>` error: passing `TyCtxt<'_>` by reference - --> $DIR/pass_ty_by_ref.rs:19:55 + --> $DIR/rustc_pass_by_value.rs:19:55 | LL | fn ty_multi_ref(ty_multi: &&Ty<'_>, ty_ctxt_multi: &&&&TyCtxt<'_>) {} | ^^^^^^^^^^^ help: try passing by value: `TyCtxt<'_>` error: passing `Ty<'_>` by reference - --> $DIR/pass_ty_by_ref.rs:26:17 + --> $DIR/rustc_pass_by_value.rs:26:17 | LL | ty_ref: &Ty<'_>, | ^^^^^^^ help: try passing by value: `Ty<'_>` error: passing `TyCtxt<'_>` by reference - --> $DIR/pass_ty_by_ref.rs:28:22 + --> $DIR/rustc_pass_by_value.rs:28:22 | LL | ty_ctxt_ref: &TyCtxt<'_>, | ^^^^^^^^^^^ help: try passing by value: `TyCtxt<'_>` error: passing `Ty<'_>` by reference - --> $DIR/pass_ty_by_ref.rs:31:41 + --> $DIR/rustc_pass_by_value.rs:31:41 | LL | fn ty_multi_ref_in_trait(ty_multi: &&Ty<'_>, ty_ctxt_multi: &&&&TyCtxt<'_>); | ^^^^^^^ help: try passing by value: `Ty<'_>` error: passing `TyCtxt<'_>` by reference - --> $DIR/pass_ty_by_ref.rs:31:68 + --> $DIR/rustc_pass_by_value.rs:31:68 | LL | fn ty_multi_ref_in_trait(ty_multi: &&Ty<'_>, ty_ctxt_multi: &&&&TyCtxt<'_>); | ^^^^^^^^^^^ help: try passing by value: `TyCtxt<'_>` error: passing `Ty<'_>` by reference - --> $DIR/pass_ty_by_ref.rs:53:17 + --> $DIR/rustc_pass_by_value.rs:53:17 | LL | ty_ref: &Ty<'_>, | ^^^^^^^ help: try passing by value: `Ty<'_>` error: passing `TyCtxt<'_>` by reference - --> $DIR/pass_ty_by_ref.rs:55:22 + --> $DIR/rustc_pass_by_value.rs:55:22 | LL | ty_ctxt_ref: &TyCtxt<'_>, | ^^^^^^^^^^^ help: try passing by value: `TyCtxt<'_>` error: passing `Ty<'_>` by reference - --> $DIR/pass_ty_by_ref.rs:59:38 + --> $DIR/rustc_pass_by_value.rs:59:38 | LL | fn ty_multi_ref_assoc(ty_multi: &&Ty<'_>, ty_ctxt_multi: &&&&TyCtxt<'_>) {} | ^^^^^^^ help: try passing by value: `Ty<'_>` error: passing `TyCtxt<'_>` by reference - --> $DIR/pass_ty_by_ref.rs:59:65 + --> $DIR/rustc_pass_by_value.rs:59:65 | LL | fn ty_multi_ref_assoc(ty_multi: &&Ty<'_>, ty_ctxt_multi: &&&&TyCtxt<'_>) {} | ^^^^^^^^^^^ help: try passing by value: `TyCtxt<'_>` diff --git a/src/test/ui-fulldeps/internal-lints/pass_ty_by_ref_self.rs b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.rs similarity index 90% rename from src/test/ui-fulldeps/internal-lints/pass_ty_by_ref_self.rs rename to src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.rs index 48b140d91744c..8877148bb56bd 100644 --- a/src/test/ui-fulldeps/internal-lints/pass_ty_by_ref_self.rs +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.rs @@ -5,10 +5,11 @@ // Considering that all other `internal-lints` are tested here // this seems like the cleaner solution though. #![feature(rustc_attrs)] -#![deny(rustc::ty_pass_by_reference)] +#![deny(rustc::pass_by_value)] #![allow(unused)] #[rustc_diagnostic_item = "TyCtxt"] +#[rustc_pass_by_value] struct TyCtxt<'tcx> { inner: &'tcx (), } @@ -18,12 +19,12 @@ impl<'tcx> TyCtxt<'tcx> { fn by_ref(&self) {} //~ ERROR passing `TyCtxt<'tcx>` by reference } - struct TyS<'tcx> { inner: &'tcx (), } #[rustc_diagnostic_item = "Ty"] +#[rustc_pass_by_value] type Ty<'tcx> = &'tcx TyS<'tcx>; impl<'tcx> TyS<'tcx> { diff --git a/src/test/ui-fulldeps/internal-lints/pass_ty_by_ref_self.stderr b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.stderr similarity index 65% rename from src/test/ui-fulldeps/internal-lints/pass_ty_by_ref_self.stderr rename to src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.stderr index 15a06e721ddcb..f86aea95aa7c6 100644 --- a/src/test/ui-fulldeps/internal-lints/pass_ty_by_ref_self.stderr +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.stderr @@ -1,17 +1,17 @@ error: passing `TyCtxt<'tcx>` by reference - --> $DIR/pass_ty_by_ref_self.rs:18:15 + --> $DIR/rustc_pass_by_value_self.rs:19:15 | LL | fn by_ref(&self) {} | ^^^^^ help: try passing by value: `TyCtxt<'tcx>` | note: the lint level is defined here - --> $DIR/pass_ty_by_ref_self.rs:8:9 + --> $DIR/rustc_pass_by_value_self.rs:8:9 | -LL | #![deny(rustc::ty_pass_by_reference)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | #![deny(rustc::pass_by_value)] + | ^^^^^^^^^^^^^^^^^^^^ error: passing `Ty<'tcx>` by reference - --> $DIR/pass_ty_by_ref_self.rs:31:21 + --> $DIR/rustc_pass_by_value_self.rs:32:21 | LL | fn by_ref(self: &Ty<'tcx>) {} | ^^^^^^^^^ help: try passing by value: `Ty<'tcx>` From 91ed6892f7ec60fb76eeaaa024919f293a58d733 Mon Sep 17 00:00:00 2001 From: Mahdi Dibaiee Date: Mon, 10 Jan 2022 08:54:42 +0000 Subject: [PATCH 2/6] rustc_pass_by_value lint: add test on custom types --- compiler/rustc_lint/src/pass_by_value.rs | 1 + compiler/rustc_passes/src/check_attr.rs | 11 ++-- .../internal-lints/rustc_pass_by_value.rs | 40 ++++++++++++++ .../internal-lints/rustc_pass_by_value.stderr | 52 ++++++++++++++----- 4 files changed, 82 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_lint/src/pass_by_value.rs b/compiler/rustc_lint/src/pass_by_value.rs index 0bfa2a673c2c4..0847f600f9d14 100644 --- a/compiler/rustc_lint/src/pass_by_value.rs +++ b/compiler/rustc_lint/src/pass_by_value.rs @@ -11,6 +11,7 @@ declare_tool_lint! { /// The `rustc_pass_by_value` lint marks a type with `#[rustc_pass_by_value]` requiring it to always be passed by value. /// This is usually used for types that are thin wrappers around references, so there is no benefit to an extra /// layer of indirection. (Example: `Ty` which is a reference to a `TyS`) + /// This lint relies on `#[rustc_diagnostic_item]` being available for the target. pub rustc::PASS_BY_VALUE, Warn, "pass by reference of a type flagged as `#[rustc_pass_by_value]`", diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs index 2febb2e56ecf8..e700a61ce48ab 100644 --- a/compiler/rustc_passes/src/check_attr.rs +++ b/compiler/rustc_passes/src/check_attr.rs @@ -1070,20 +1070,15 @@ impl CheckAttrVisitor<'_> { /// Warns against some misuses of `#[pass_by_value]` fn check_pass_by_value(&self, attr: &Attribute, span: &Span, target: Target) -> bool { match target { - Target::Struct - | Target::Enum - | Target::Union - | Target::Trait - | Target::TraitAlias - | Target::TyAlias => true, + Target::Struct | Target::Enum | Target::TyAlias => true, _ => { self.tcx .sess .struct_span_err( attr.span, - "`pass_by_value` attribute should be applied to a struct, enum, trait or type alias.", + "`pass_by_value` attribute should be applied to a struct, enum or type alias.", ) - .span_label(*span, "is not a struct, enum, trait or type alias") + .span_label(*span, "is not a struct, enum or type alias") .emit(); false } diff --git a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs index 783019d894513..bf2b1fbaf45fc 100644 --- a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs @@ -1,5 +1,6 @@ // compile-flags: -Z unstable-options +#![feature(rustc_attrs)] #![feature(rustc_private)] #![deny(rustc::pass_by_value)] #![allow(unused)] @@ -61,4 +62,43 @@ impl Foo { //~^^ ERROR passing `TyCtxt<'_>` by reference } +#[rustc_diagnostic_item = "CustomEnum"] +#[rustc_pass_by_value] +enum CustomEnum { + A, + B, +} + +impl CustomEnum { + fn test( + value: CustomEnum, + reference: &CustomEnum, //~ ERROR passing `CustomEnum` by reference + ) { + } +} + +#[rustc_diagnostic_item = "CustomStruct"] +#[rustc_pass_by_value] +struct CustomStruct { + s: u8, +} + +#[rustc_diagnostic_item = "CustomAlias"] +#[rustc_pass_by_value] +type CustomAlias<'a> = &'a CustomStruct; //~ ERROR passing `CustomStruct` by reference + +impl CustomStruct { + fn test( + value: CustomStruct, + reference: &CustomStruct, //~ ERROR passing `CustomStruct` by reference + ) { + } + + fn test_alias( + value: CustomAlias, + reference: &CustomAlias, //~ ERROR passing `CustomAlias<>` by reference + ) { + } +} + fn main() {} diff --git a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr index 5fbde93878931..c59c1adf8997d 100644 --- a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr @@ -1,80 +1,104 @@ error: passing `Ty<'_>` by reference - --> $DIR/rustc_pass_by_value.rs:13:13 + --> $DIR/rustc_pass_by_value.rs:14:13 | LL | ty_ref: &Ty<'_>, | ^^^^^^^ help: try passing by value: `Ty<'_>` | note: the lint level is defined here - --> $DIR/rustc_pass_by_value.rs:4:9 + --> $DIR/rustc_pass_by_value.rs:5:9 | LL | #![deny(rustc::pass_by_value)] | ^^^^^^^^^^^^^^^^^^^^ error: passing `TyCtxt<'_>` by reference - --> $DIR/rustc_pass_by_value.rs:15:18 + --> $DIR/rustc_pass_by_value.rs:16:18 | LL | ty_ctxt_ref: &TyCtxt<'_>, | ^^^^^^^^^^^ help: try passing by value: `TyCtxt<'_>` error: passing `Ty<'_>` by reference - --> $DIR/rustc_pass_by_value.rs:19:28 + --> $DIR/rustc_pass_by_value.rs:20:28 | LL | fn ty_multi_ref(ty_multi: &&Ty<'_>, ty_ctxt_multi: &&&&TyCtxt<'_>) {} | ^^^^^^^ help: try passing by value: `Ty<'_>` error: passing `TyCtxt<'_>` by reference - --> $DIR/rustc_pass_by_value.rs:19:55 + --> $DIR/rustc_pass_by_value.rs:20:55 | LL | fn ty_multi_ref(ty_multi: &&Ty<'_>, ty_ctxt_multi: &&&&TyCtxt<'_>) {} | ^^^^^^^^^^^ help: try passing by value: `TyCtxt<'_>` error: passing `Ty<'_>` by reference - --> $DIR/rustc_pass_by_value.rs:26:17 + --> $DIR/rustc_pass_by_value.rs:27:17 | LL | ty_ref: &Ty<'_>, | ^^^^^^^ help: try passing by value: `Ty<'_>` error: passing `TyCtxt<'_>` by reference - --> $DIR/rustc_pass_by_value.rs:28:22 + --> $DIR/rustc_pass_by_value.rs:29:22 | LL | ty_ctxt_ref: &TyCtxt<'_>, | ^^^^^^^^^^^ help: try passing by value: `TyCtxt<'_>` error: passing `Ty<'_>` by reference - --> $DIR/rustc_pass_by_value.rs:31:41 + --> $DIR/rustc_pass_by_value.rs:32:41 | LL | fn ty_multi_ref_in_trait(ty_multi: &&Ty<'_>, ty_ctxt_multi: &&&&TyCtxt<'_>); | ^^^^^^^ help: try passing by value: `Ty<'_>` error: passing `TyCtxt<'_>` by reference - --> $DIR/rustc_pass_by_value.rs:31:68 + --> $DIR/rustc_pass_by_value.rs:32:68 | LL | fn ty_multi_ref_in_trait(ty_multi: &&Ty<'_>, ty_ctxt_multi: &&&&TyCtxt<'_>); | ^^^^^^^^^^^ help: try passing by value: `TyCtxt<'_>` error: passing `Ty<'_>` by reference - --> $DIR/rustc_pass_by_value.rs:53:17 + --> $DIR/rustc_pass_by_value.rs:54:17 | LL | ty_ref: &Ty<'_>, | ^^^^^^^ help: try passing by value: `Ty<'_>` error: passing `TyCtxt<'_>` by reference - --> $DIR/rustc_pass_by_value.rs:55:22 + --> $DIR/rustc_pass_by_value.rs:56:22 | LL | ty_ctxt_ref: &TyCtxt<'_>, | ^^^^^^^^^^^ help: try passing by value: `TyCtxt<'_>` error: passing `Ty<'_>` by reference - --> $DIR/rustc_pass_by_value.rs:59:38 + --> $DIR/rustc_pass_by_value.rs:60:38 | LL | fn ty_multi_ref_assoc(ty_multi: &&Ty<'_>, ty_ctxt_multi: &&&&TyCtxt<'_>) {} | ^^^^^^^ help: try passing by value: `Ty<'_>` error: passing `TyCtxt<'_>` by reference - --> $DIR/rustc_pass_by_value.rs:59:65 + --> $DIR/rustc_pass_by_value.rs:60:65 | LL | fn ty_multi_ref_assoc(ty_multi: &&Ty<'_>, ty_ctxt_multi: &&&&TyCtxt<'_>) {} | ^^^^^^^^^^^ help: try passing by value: `TyCtxt<'_>` -error: aborting due to 12 previous errors +error: passing `CustomEnum` by reference + --> $DIR/rustc_pass_by_value.rs:75:20 + | +LL | reference: &CustomEnum, + | ^^^^^^^^^^^ help: try passing by value: `CustomEnum` + +error: passing `CustomStruct` by reference + --> $DIR/rustc_pass_by_value.rs:88:24 + | +LL | type CustomAlias<'a> = &'a CustomStruct; + | ^^^^^^^^^^^^^^^^ help: try passing by value: `CustomStruct` + +error: passing `CustomStruct` by reference + --> $DIR/rustc_pass_by_value.rs:93:20 + | +LL | reference: &CustomStruct, + | ^^^^^^^^^^^^^ help: try passing by value: `CustomStruct` + +error: passing `CustomAlias<>` by reference + --> $DIR/rustc_pass_by_value.rs:99:20 + | +LL | reference: &CustomAlias, + | ^^^^^^^^^^^^ help: try passing by value: `CustomAlias<>` + +error: aborting due to 16 previous errors From 71e33146734362984258df415ac8308618968ed4 Mon Sep 17 00:00:00 2001 From: Mahdi Dibaiee Date: Mon, 10 Jan 2022 18:12:28 +0000 Subject: [PATCH 3/6] rustc_pass_by_value remove dependency on rustc_diagnostic_item --- compiler/rustc_lint/src/pass_by_value.rs | 11 ++++------- .../ui-fulldeps/internal-lints/rustc_pass_by_value.rs | 3 --- .../internal-lints/rustc_pass_by_value.stderr | 8 ++++---- .../internal-lints/rustc_pass_by_value_self.rs | 2 -- .../internal-lints/rustc_pass_by_value_self.stderr | 4 ++-- 5 files changed, 10 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_lint/src/pass_by_value.rs b/compiler/rustc_lint/src/pass_by_value.rs index 0847f600f9d14..c689f34d9e3b4 100644 --- a/compiler/rustc_lint/src/pass_by_value.rs +++ b/compiler/rustc_lint/src/pass_by_value.rs @@ -11,7 +11,6 @@ declare_tool_lint! { /// The `rustc_pass_by_value` lint marks a type with `#[rustc_pass_by_value]` requiring it to always be passed by value. /// This is usually used for types that are thin wrappers around references, so there is no benefit to an extra /// layer of indirection. (Example: `Ty` which is a reference to a `TyS`) - /// This lint relies on `#[rustc_diagnostic_item]` being available for the target. pub rustc::PASS_BY_VALUE, Warn, "pass by reference of a type flagged as `#[rustc_pass_by_value]`", @@ -52,16 +51,14 @@ fn path_for_pass_by_value(cx: &LateContext<'_>, ty: &hir::Ty<'_>) -> Option { - if let Some(name) = cx.tcx.get_diagnostic_name(def_id) { - return Some(format!("{}{}", name, gen_args(path.segments.last().unwrap()))); - } + let name = cx.tcx.item_name(def_id).to_ident_string(); + return Some(format!("{}{}", name, gen_args(path.segments.last().unwrap()))); } Res::SelfTy(None, Some((did, _))) => { if let ty::Adt(adt, substs) = cx.tcx.type_of(did).kind() { if has_pass_by_value_attr(cx, adt.did) { - if let Some(name) = cx.tcx.get_diagnostic_name(adt.did) { - return Some(format!("{}<{}>", name, substs[0])); - } + let name = cx.tcx.item_name(adt.did).to_ident_string(); + return Some(format!("{}<{}>", name, substs[0])); } } } diff --git a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs index bf2b1fbaf45fc..293464c07ef78 100644 --- a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs @@ -62,7 +62,6 @@ impl Foo { //~^^ ERROR passing `TyCtxt<'_>` by reference } -#[rustc_diagnostic_item = "CustomEnum"] #[rustc_pass_by_value] enum CustomEnum { A, @@ -77,13 +76,11 @@ impl CustomEnum { } } -#[rustc_diagnostic_item = "CustomStruct"] #[rustc_pass_by_value] struct CustomStruct { s: u8, } -#[rustc_diagnostic_item = "CustomAlias"] #[rustc_pass_by_value] type CustomAlias<'a> = &'a CustomStruct; //~ ERROR passing `CustomStruct` by reference diff --git a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr index c59c1adf8997d..dbb9180ed7d2f 100644 --- a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr @@ -77,25 +77,25 @@ LL | fn ty_multi_ref_assoc(ty_multi: &&Ty<'_>, ty_ctxt_multi: &&&&TyCtxt<'_> | ^^^^^^^^^^^ help: try passing by value: `TyCtxt<'_>` error: passing `CustomEnum` by reference - --> $DIR/rustc_pass_by_value.rs:75:20 + --> $DIR/rustc_pass_by_value.rs:74:20 | LL | reference: &CustomEnum, | ^^^^^^^^^^^ help: try passing by value: `CustomEnum` error: passing `CustomStruct` by reference - --> $DIR/rustc_pass_by_value.rs:88:24 + --> $DIR/rustc_pass_by_value.rs:85:24 | LL | type CustomAlias<'a> = &'a CustomStruct; | ^^^^^^^^^^^^^^^^ help: try passing by value: `CustomStruct` error: passing `CustomStruct` by reference - --> $DIR/rustc_pass_by_value.rs:93:20 + --> $DIR/rustc_pass_by_value.rs:90:20 | LL | reference: &CustomStruct, | ^^^^^^^^^^^^^ help: try passing by value: `CustomStruct` error: passing `CustomAlias<>` by reference - --> $DIR/rustc_pass_by_value.rs:99:20 + --> $DIR/rustc_pass_by_value.rs:96:20 | LL | reference: &CustomAlias, | ^^^^^^^^^^^^ help: try passing by value: `CustomAlias<>` diff --git a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.rs b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.rs index 8877148bb56bd..a4529f9856368 100644 --- a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.rs +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.rs @@ -8,7 +8,6 @@ #![deny(rustc::pass_by_value)] #![allow(unused)] -#[rustc_diagnostic_item = "TyCtxt"] #[rustc_pass_by_value] struct TyCtxt<'tcx> { inner: &'tcx (), @@ -23,7 +22,6 @@ struct TyS<'tcx> { inner: &'tcx (), } -#[rustc_diagnostic_item = "Ty"] #[rustc_pass_by_value] type Ty<'tcx> = &'tcx TyS<'tcx>; diff --git a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.stderr b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.stderr index f86aea95aa7c6..ca47babd13d8a 100644 --- a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.stderr +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.stderr @@ -1,5 +1,5 @@ error: passing `TyCtxt<'tcx>` by reference - --> $DIR/rustc_pass_by_value_self.rs:19:15 + --> $DIR/rustc_pass_by_value_self.rs:18:15 | LL | fn by_ref(&self) {} | ^^^^^ help: try passing by value: `TyCtxt<'tcx>` @@ -11,7 +11,7 @@ LL | #![deny(rustc::pass_by_value)] | ^^^^^^^^^^^^^^^^^^^^ error: passing `Ty<'tcx>` by reference - --> $DIR/rustc_pass_by_value_self.rs:32:21 + --> $DIR/rustc_pass_by_value_self.rs:30:21 | LL | fn by_ref(self: &Ty<'tcx>) {} | ^^^^^^^^^ help: try passing by value: `Ty<'tcx>` From a6762e962e50d5b6f864e33ebb27878e90651f22 Mon Sep 17 00:00:00 2001 From: Mahdi Dibaiee Date: Tue, 11 Jan 2022 09:28:13 +0000 Subject: [PATCH 4/6] rustc_pass_by_value: allow types with no parameters on self includes minor refactorings --- compiler/rustc_feature/src/builtin_attrs.rs | 2 +- compiler/rustc_lint/src/pass_by_value.rs | 18 +++++------------- .../internal-lints/rustc_pass_by_value_self.rs | 7 +++++++ .../rustc_pass_by_value_self.stderr | 8 +++++++- 4 files changed, 20 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_feature/src/builtin_attrs.rs b/compiler/rustc_feature/src/builtin_attrs.rs index 88bf81864b23f..46817bc9c3f08 100644 --- a/compiler/rustc_feature/src/builtin_attrs.rs +++ b/compiler/rustc_feature/src/builtin_attrs.rs @@ -625,7 +625,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ ), rustc_attr!( rustc_pass_by_value, Normal, - template!(Word, NameValueStr: "reason"), WarnFollowing, + template!(Word), WarnFollowing, "#[rustc_pass_by_value] is used to mark types that must be passed by value instead of reference." ), BuiltinAttribute { diff --git a/compiler/rustc_lint/src/pass_by_value.rs b/compiler/rustc_lint/src/pass_by_value.rs index c689f34d9e3b4..00b023c26f3f3 100644 --- a/compiler/rustc_lint/src/pass_by_value.rs +++ b/compiler/rustc_lint/src/pass_by_value.rs @@ -2,7 +2,6 @@ use crate::{LateContext, LateLintPass, LintContext}; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_hir::def::Res; -use rustc_hir::def_id::DefId; use rustc_hir::{GenericArg, PathSegment, QPath, TyKind}; use rustc_middle::ty; use rustc_span::symbol::sym; @@ -50,15 +49,17 @@ impl<'tcx> LateLintPass<'tcx> for PassByValue { fn path_for_pass_by_value(cx: &LateContext<'_>, ty: &hir::Ty<'_>) -> Option { if let TyKind::Path(QPath::Resolved(_, path)) = &ty.kind { match path.res { - Res::Def(_, def_id) if has_pass_by_value_attr(cx, def_id) => { + Res::Def(_, def_id) if cx.tcx.has_attr(def_id, sym::rustc_pass_by_value) => { let name = cx.tcx.item_name(def_id).to_ident_string(); return Some(format!("{}{}", name, gen_args(path.segments.last().unwrap()))); } Res::SelfTy(None, Some((did, _))) => { if let ty::Adt(adt, substs) = cx.tcx.type_of(did).kind() { - if has_pass_by_value_attr(cx, adt.did) { + if cx.tcx.has_attr(adt.did, sym::rustc_pass_by_value) { let name = cx.tcx.item_name(adt.did).to_ident_string(); - return Some(format!("{}<{}>", name, substs[0])); + let param = + substs.first().map(|s| format!("<{}>", s)).unwrap_or("".to_string()); + return Some(format!("{}{}", name, param)); } } } @@ -69,15 +70,6 @@ fn path_for_pass_by_value(cx: &LateContext<'_>, ty: &hir::Ty<'_>) -> Option, def_id: DefId) -> bool { - for attr in cx.tcx.get_attrs(def_id).iter() { - if attr.has_name(sym::rustc_pass_by_value) { - return true; - } - } - false -} - fn gen_args(segment: &PathSegment<'_>) -> String { if let Some(args) = &segment.args { let lifetimes = args diff --git a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.rs b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.rs index a4529f9856368..1be01e21bd5f1 100644 --- a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.rs +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.rs @@ -30,4 +30,11 @@ impl<'tcx> TyS<'tcx> { fn by_ref(self: &Ty<'tcx>) {} //~ ERROR passing `Ty<'tcx>` by reference } +#[rustc_pass_by_value] +struct Foo; + +impl Foo { + fn with_ref(&self) {} //~ ERROR passing `Foo` by reference +} + fn main() {} diff --git a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.stderr b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.stderr index ca47babd13d8a..965e79d962cdf 100644 --- a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.stderr +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.stderr @@ -16,5 +16,11 @@ error: passing `Ty<'tcx>` by reference LL | fn by_ref(self: &Ty<'tcx>) {} | ^^^^^^^^^ help: try passing by value: `Ty<'tcx>` -error: aborting due to 2 previous errors +error: passing `Foo` by reference + --> $DIR/rustc_pass_by_value_self.rs:37:17 + | +LL | fn with_ref(&self) {} + | ^^^^^ help: try passing by value: `Foo` + +error: aborting due to 3 previous errors From 959bf2bc2e79defd0fe7d3c9987a6023eb8503cd Mon Sep 17 00:00:00 2001 From: Mahdi Dibaiee Date: Tue, 11 Jan 2022 19:59:06 +0000 Subject: [PATCH 5/6] rustc_pass_by_value: handle generic and const type parameters --- compiler/rustc_lint/src/pass_by_value.rs | 33 +++++++++++-------- .../internal-lints/rustc_pass_by_value.rs | 15 +++++++++ .../internal-lints/rustc_pass_by_value.stderr | 14 +++++++- .../rustc_pass_by_value_self.rs | 14 ++++++++ .../rustc_pass_by_value_self.stderr | 14 +++++++- 5 files changed, 74 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_lint/src/pass_by_value.rs b/compiler/rustc_lint/src/pass_by_value.rs index 00b023c26f3f3..3435a5a6c82db 100644 --- a/compiler/rustc_lint/src/pass_by_value.rs +++ b/compiler/rustc_lint/src/pass_by_value.rs @@ -51,15 +51,13 @@ fn path_for_pass_by_value(cx: &LateContext<'_>, ty: &hir::Ty<'_>) -> Option { let name = cx.tcx.item_name(def_id).to_ident_string(); - return Some(format!("{}{}", name, gen_args(path.segments.last().unwrap()))); + let path_segment = path.segments.last().unwrap(); + return Some(format!("{}{}", name, gen_args(cx, path_segment))); } Res::SelfTy(None, Some((did, _))) => { if let ty::Adt(adt, substs) = cx.tcx.type_of(did).kind() { if cx.tcx.has_attr(adt.did, sym::rustc_pass_by_value) { - let name = cx.tcx.item_name(adt.did).to_ident_string(); - let param = - substs.first().map(|s| format!("<{}>", s)).unwrap_or("".to_string()); - return Some(format!("{}{}", name, param)); + return Some(cx.tcx.def_path_str_with_substs(adt.did, substs)); } } } @@ -70,22 +68,29 @@ fn path_for_pass_by_value(cx: &LateContext<'_>, ty: &hir::Ty<'_>) -> Option) -> String { +fn gen_args(cx: &LateContext<'_>, segment: &PathSegment<'_>) -> String { if let Some(args) = &segment.args { - let lifetimes = args + let params = args .args .iter() - .filter_map(|arg| { - if let GenericArg::Lifetime(lt) = arg { - Some(lt.name.ident().to_string()) - } else { - None + .filter_map(|arg| match arg { + GenericArg::Lifetime(lt) => Some(lt.name.ident().to_string()), + GenericArg::Type(ty) => { + let snippet = + cx.tcx.sess.source_map().span_to_snippet(ty.span).unwrap_or_default(); + Some(snippet) } + GenericArg::Const(c) => { + let snippet = + cx.tcx.sess.source_map().span_to_snippet(c.span).unwrap_or_default(); + Some(snippet) + } + _ => None, }) .collect::>(); - if !lifetimes.is_empty() { - return format!("<{}>", lifetimes.join(", ")); + if !params.is_empty() { + return format!("<{}>", params.join(", ")); } } diff --git a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs index 293464c07ef78..f8ab0f056d79f 100644 --- a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs @@ -98,4 +98,19 @@ impl CustomStruct { } } +#[rustc_pass_by_value] +struct WithParameters { + slice: [T; N], + m: M, +} + +impl WithParameters { + fn test( + value: WithParameters, + reference: &WithParameters, //~ ERROR passing `WithParameters` by reference + reference_with_m: &WithParameters, //~ ERROR passing `WithParameters` by reference + ) { + } +} + fn main() {} diff --git a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr index dbb9180ed7d2f..c5307f0f67d9b 100644 --- a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr @@ -100,5 +100,17 @@ error: passing `CustomAlias<>` by reference LL | reference: &CustomAlias, | ^^^^^^^^^^^^ help: try passing by value: `CustomAlias<>` -error: aborting due to 16 previous errors +error: passing `WithParameters` by reference + --> $DIR/rustc_pass_by_value.rs:110:20 + | +LL | reference: &WithParameters, + | ^^^^^^^^^^^^^^^^^^^^^ help: try passing by value: `WithParameters` + +error: passing `WithParameters` by reference + --> $DIR/rustc_pass_by_value.rs:111:27 + | +LL | reference_with_m: &WithParameters, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try passing by value: `WithParameters` + +error: aborting due to 18 previous errors diff --git a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.rs b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.rs index 1be01e21bd5f1..2868517774d46 100644 --- a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.rs +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.rs @@ -37,4 +37,18 @@ impl Foo { fn with_ref(&self) {} //~ ERROR passing `Foo` by reference } +#[rustc_pass_by_value] +struct WithParameters { + slice: [T; N], + m: M, +} + +impl WithParameters { + fn with_ref(&self) {} //~ ERROR passing `WithParameters` by reference +} + +impl WithParameters { + fn with_ref(&self) {} //~ ERROR passing `WithParameters` by reference +} + fn main() {} diff --git a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.stderr b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.stderr index 965e79d962cdf..54a7cf7cab757 100644 --- a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.stderr +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.stderr @@ -22,5 +22,17 @@ error: passing `Foo` by reference LL | fn with_ref(&self) {} | ^^^^^ help: try passing by value: `Foo` -error: aborting due to 3 previous errors +error: passing `WithParameters` by reference + --> $DIR/rustc_pass_by_value_self.rs:47:17 + | +LL | fn with_ref(&self) {} + | ^^^^^ help: try passing by value: `WithParameters` + +error: passing `WithParameters` by reference + --> $DIR/rustc_pass_by_value_self.rs:51:17 + | +LL | fn with_ref(&self) {} + | ^^^^^ help: try passing by value: `WithParameters` + +error: aborting due to 5 previous errors From 2728af7bc02ab48bf4dd861cb69b5b786ecb261d Mon Sep 17 00:00:00 2001 From: Mahdi Dibaiee Date: Tue, 11 Jan 2022 21:28:04 +0000 Subject: [PATCH 6/6] rustc_pass_by_value: handle inferred generic types (with _) --- compiler/rustc_lint/src/pass_by_value.rs | 14 +++++--------- .../internal-lints/rustc_pass_by_value.rs | 8 +++++--- .../internal-lints/rustc_pass_by_value.stderr | 18 +++++++++++++++--- 3 files changed, 25 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_lint/src/pass_by_value.rs b/compiler/rustc_lint/src/pass_by_value.rs index 3435a5a6c82db..26d0560bf89bb 100644 --- a/compiler/rustc_lint/src/pass_by_value.rs +++ b/compiler/rustc_lint/src/pass_by_value.rs @@ -73,19 +73,15 @@ fn gen_args(cx: &LateContext<'_>, segment: &PathSegment<'_>) -> String { let params = args .args .iter() - .filter_map(|arg| match arg { - GenericArg::Lifetime(lt) => Some(lt.name.ident().to_string()), + .map(|arg| match arg { + GenericArg::Lifetime(lt) => lt.name.ident().to_string(), GenericArg::Type(ty) => { - let snippet = - cx.tcx.sess.source_map().span_to_snippet(ty.span).unwrap_or_default(); - Some(snippet) + cx.tcx.sess.source_map().span_to_snippet(ty.span).unwrap_or_default() } GenericArg::Const(c) => { - let snippet = - cx.tcx.sess.source_map().span_to_snippet(c.span).unwrap_or_default(); - Some(snippet) + cx.tcx.sess.source_map().span_to_snippet(c.span).unwrap_or_default() } - _ => None, + GenericArg::Infer(_) => String::from("_"), }) .collect::>(); diff --git a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs index f8ab0f056d79f..402c41f376602 100644 --- a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs @@ -105,11 +105,13 @@ struct WithParameters { } impl WithParameters { - fn test( + fn test<'a>( value: WithParameters, - reference: &WithParameters, //~ ERROR passing `WithParameters` by reference + reference: &'a WithParameters, //~ ERROR passing `WithParameters` by reference reference_with_m: &WithParameters, //~ ERROR passing `WithParameters` by reference - ) { + ) -> &'a WithParameters { + //~^ ERROR passing `WithParameters` by reference + reference as &WithParameters<_, 1> //~ ERROR passing `WithParameters<_, 1>` by reference } } diff --git a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr index c5307f0f67d9b..7f6e57b38f38d 100644 --- a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr @@ -103,8 +103,8 @@ LL | reference: &CustomAlias, error: passing `WithParameters` by reference --> $DIR/rustc_pass_by_value.rs:110:20 | -LL | reference: &WithParameters, - | ^^^^^^^^^^^^^^^^^^^^^ help: try passing by value: `WithParameters` +LL | reference: &'a WithParameters, + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try passing by value: `WithParameters` error: passing `WithParameters` by reference --> $DIR/rustc_pass_by_value.rs:111:27 @@ -112,5 +112,17 @@ error: passing `WithParameters` by reference LL | reference_with_m: &WithParameters, | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try passing by value: `WithParameters` -error: aborting due to 18 previous errors +error: passing `WithParameters` by reference + --> $DIR/rustc_pass_by_value.rs:112:10 + | +LL | ) -> &'a WithParameters { + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try passing by value: `WithParameters` + +error: passing `WithParameters<_, 1>` by reference + --> $DIR/rustc_pass_by_value.rs:114:22 + | +LL | reference as &WithParameters<_, 1> + | ^^^^^^^^^^^^^^^^^^^^^ help: try passing by value: `WithParameters<_, 1>` + +error: aborting due to 20 previous errors