Rollup merge of #92646 - mdibaiee:76935/pass-by-value, r=lcnr

feat: rustc_pass_by_value lint attribute

Useful for thin wrapper attributes that are best passed as value instead
of reference.

Fixes #76935

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), 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,

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();
- })
- }
- }
  _ => {}
  }
  }

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::*;
@@ -490,15 +492,17 @@ 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",
  None,
  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),
  ],

compiler/rustc_lint/src/pass_by_value.rs

@@ -0,0 +1,94 @@
+use crate::{LateContext, LateLintPass, LintContext};
+use rustc_errors::Applicability;
+use rustc_hir as hir;
+use rustc_hir::def::Res;
+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<String> {
+ if let TyKind::Path(QPath::Resolved(_, path)) = &ty.kind {
+ match path.res {
+ 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();
+ 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) {
+ return Some(cx.tcx.def_path_str_with_substs(adt.did, substs));
+ }
+ }
+ }
+ _ => (),
+ }
+ }
+
+ None
+}
+
+fn gen_args(cx: &LateContext<'_>, segment: &PathSegment<'_>) -> String {
+ if let Some(args) = &segment.args {
+ let params = args
+ .args
+ .iter()
+ .map(|arg| match arg {
+ GenericArg::Lifetime(lt) => lt.name.ident().to_string(),
+ GenericArg::Type(ty) => {
+ cx.tcx.sess.source_map().span_to_snippet(ty.span).unwrap_or_default()
+ }
+ GenericArg::Const(c) => {
+ cx.tcx.sess.source_map().span_to_snippet(c.span).unwrap_or_default()
+ }
+ GenericArg::Infer(_) => String::from("_"),
+ })
+ .collect::<Vec<_>>();
+
+ if !params.is_empty() {
+ return format!("<{}>", params.join(", "));
+ }
+ }
+
+ String::new()
+}

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>,
 }

compiler/rustc_middle/src/ty/mod.rs

@@ -464,6 +464,7 @@ impl<'a, 'tcx> HashStable<StableHashingContext<'a>> 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 {

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,24 @@ 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::TyAlias => true,
+ _ => {
+ self.tcx
+ .sess
+ .struct_span_err(
+ attr.span,
+ "`pass_by_value` attribute should be applied to a struct, enum or type alias.",
+ )
+ .span_label(*span, "is not a struct, enum or type alias")
+ .emit();
+ false
+ }
+ }
+ }
+
  /// Warns against some misuses of `#[must_use]`
  fn check_must_use(
  &self,

compiler/rustc_span/src/symbol.rs

@@ -1170,6 +1170,7 @@ symbols! {
  rustc_paren_sugar,
  rustc_partition_codegened,
  rustc_partition_reused,
+ rustc_pass_by_value,
  rustc_peek,
  rustc_peek_definite_init,
  rustc_peek_liveness,