diff --git a/compiler/rustc_feature/src/builtin_attrs.rs b/compiler/rustc_feature/src/builtin_attrs.rs index f25b2d8f566..88bf81864b2 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 c64a67b6b9f..7353cd6b876 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 c7823032b0c..3b95a2487ba 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 00000000000..0bfa2a673c2 --- /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 dd571e29bf6..ab85f104ce3 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 78ccfbd5e8c..365d4c4aaba 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 d7b00699491..2febb2e56ec 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 84cf8878af8..b1d868fbb88 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 e0fdbaeac30..783019d8945 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 2751a37f741..5fbde938789 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 48b140d9174..8877148bb56 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 15a06e721dd..f86aea95aa7 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>`