about summary refs log tree commit diff
diff options
context:
space:
mode:
authorMahdi Dibaiee <mdibaiee@pm.me>2022-01-07 11:38:16 +0000
committerMahdi Dibaiee <mdibaiee@pm.me>2022-01-09 13:05:51 +0000
commit4c3e330a8c023af20beb23a3a46b5d6d77a62839 (patch)
tree71d1e8e05f28116e9a1da65d7f0acc8ec29ee6c2
parent66f64a441a05cee8d5d701477b43ed851f778f3a (diff)
downloadrust-4c3e330a8c023af20beb23a3a46b5d6d77a62839.tar.gz
rust-4c3e330a8c023af20beb23a3a46b5d6d77a62839.zip
feat: pass_by_value lint attribute
Useful for thin wrapper attributes that are best passed as value instead
of reference.
-rw-r--r--compiler/rustc_feature/src/builtin_attrs.rs5
-rw-r--r--compiler/rustc_lint/src/internal.rs33
-rw-r--r--compiler/rustc_lint/src/lib.rs6
-rw-r--r--compiler/rustc_lint/src/pass_by_value.rs103
-rw-r--r--compiler/rustc_middle/src/ty/context.rs1
-rw-r--r--compiler/rustc_middle/src/ty/mod.rs1
-rw-r--r--compiler/rustc_passes/src/check_attr.rs24
-rw-r--r--compiler/rustc_span/src/symbol.rs1
-rw-r--r--src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs (renamed from src/test/ui-fulldeps/internal-lints/pass_ty_by_ref.rs)2
-rw-r--r--src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr (renamed from src/test/ui-fulldeps/internal-lints/pass_ty_by_ref.stderr)30
-rw-r--r--src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.rs (renamed from src/test/ui-fulldeps/internal-lints/pass_ty_by_ref_self.rs)5
-rw-r--r--src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.stderr (renamed from src/test/ui-fulldeps/internal-lints/pass_ty_by_ref_self.stderr)10
12 files changed, 165 insertions, 56 deletions
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};
@@ -59,13 +56,6 @@ declare_tool_lint! {
 }
 
 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,
     "using `ty::{Ty,TyCtxt}` instead of importing it",
@@ -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<String> {
+    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::<Vec<_>>();
+
+        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<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 {
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
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
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
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
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>`