Skip to content

Commit 4c3e330

Browse files
mdibaieemdibaiee
committed
feat: pass_by_value lint attribute
Useful for thin wrapper attributes that are best passed as value instead of reference.
1 parent 66f64a4 commit 4c3e330

File tree

12 files changed

+165
-56
lines changed

12 files changed

+165
-56
lines changed

‎compiler/rustc_feature/src/builtin_attrs.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -623,6 +623,11 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
623623
lang, Normal, template!(NameValueStr: "name"), DuplicatesOk, lang_items,
624624
"language items are subject to change",
625625
),
626+
rustc_attr!(
627+
rustc_pass_by_value, Normal,
628+
template!(Word, NameValueStr: "reason"), WarnFollowing,
629+
"#[rustc_pass_by_value] is used to mark types that must be passed by value instead of reference."
630+
),
626631
BuiltinAttribute {
627632
name: sym::rustc_diagnostic_item,
628633
type_: Normal,

‎compiler/rustc_lint/src/internal.rs

Lines changed: 1 addition & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,7 @@ use crate::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext}
55
use rustc_ast as ast;
66
use rustc_errors::Applicability;
77
use rustc_hir::def::Res;
8-
use rustc_hir::{
9-
GenericArg, HirId, Item, ItemKind, MutTy, Mutability, Node, Path, PathSegment, QPath, Ty,
10-
TyKind,
11-
};
8+
use rustc_hir::{GenericArg, HirId, Item, ItemKind, Node, Path, PathSegment, QPath, Ty, TyKind};
129
use rustc_middle::ty;
1310
use rustc_session::{declare_lint_pass, declare_tool_lint};
1411
use rustc_span::hygiene::{ExpnKind, MacroKind};
@@ -58,13 +55,6 @@ declare_tool_lint! {
5855
report_in_external_macro: true
5956
}
6057

61-
declare_tool_lint! {
62-
pub rustc::TY_PASS_BY_REFERENCE,
63-
Allow,
64-
"passing `Ty` or `TyCtxt` by reference",
65-
report_in_external_macro: true
66-
}
67-
6858
declare_tool_lint! {
6959
pub rustc::USAGE_OF_QUALIFIED_TY,
7060
Allow,
@@ -74,7 +64,6 @@ declare_tool_lint! {
7464

7565
declare_lint_pass!(TyTyKind => [
7666
USAGE_OF_TY_TYKIND,
77-
TY_PASS_BY_REFERENCE,
7867
USAGE_OF_QUALIFIED_TY,
7968
]);
8069

@@ -131,26 +120,6 @@ impl<'tcx> LateLintPass<'tcx> for TyTyKind {
131120
}
132121
}
133122
}
134-
TyKind::Rptr(_, MutTy { ty: inner_ty, mutbl: Mutability::Not }) => {
135-
if let Some(impl_did) = cx.tcx.impl_of_method(ty.hir_id.owner.to_def_id()) {
136-
if cx.tcx.impl_trait_ref(impl_did).is_some() {
137-
return;
138-
}
139-
}
140-
if let Some(t) = is_ty_or_ty_ctxt(cx, &inner_ty) {
141-
cx.struct_span_lint(TY_PASS_BY_REFERENCE, ty.span, |lint| {
142-
lint.build(&format!("passing `{}` by reference", t))
143-
.span_suggestion(
144-
ty.span,
145-
"try passing by value",
146-
t,
147-
// Changing type of function argument
148-
Applicability::MaybeIncorrect,
149-
)
150-
.emit();
151-
})
152-
}
153-
}
154123
_ => {}
155124
}
156125
}

‎compiler/rustc_lint/src/lib.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ mod non_ascii_idents;
5656
mod non_fmt_panic;
5757
mod nonstandard_style;
5858
mod noop_method_call;
59+
mod pass_by_value;
5960
mod passes;
6061
mod redundant_semicolon;
6162
mod traits;
@@ -85,6 +86,7 @@ use non_ascii_idents::*;
8586
use non_fmt_panic::NonPanicFmt;
8687
use nonstandard_style::*;
8788
use noop_method_call::*;
89+
use pass_by_value::*;
8890
use redundant_semicolon::*;
8991
use traits::*;
9092
use types::*;
@@ -489,15 +491,17 @@ fn register_internals(store: &mut LintStore) {
489491
store.register_late_pass(|| Box::new(ExistingDocKeyword));
490492
store.register_lints(&TyTyKind::get_lints());
491493
store.register_late_pass(|| Box::new(TyTyKind));
494+
store.register_lints(&PassByValue::get_lints());
495+
store.register_late_pass(|| Box::new(PassByValue));
492496
store.register_group(
493497
false,
494498
"rustc::internal",
495499
None,
496500
vec![
497501
LintId::of(DEFAULT_HASH_TYPES),
498502
LintId::of(USAGE_OF_TY_TYKIND),
503+
LintId::of(PASS_BY_VALUE),
499504
LintId::of(LINT_PASS_IMPL_WITHOUT_MACRO),
500-
LintId::of(TY_PASS_BY_REFERENCE),
501505
LintId::of(USAGE_OF_QUALIFIED_TY),
502506
LintId::of(EXISTING_DOC_KEYWORD),
503507
],

‎compiler/rustc_lint/src/pass_by_value.rs

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
use crate::{LateContext, LateLintPass, LintContext};
2+
use rustc_errors::Applicability;
3+
use rustc_hir as hir;
4+
use rustc_hir::def::Res;
5+
use rustc_hir::def_id::DefId;
6+
use rustc_hir::{GenericArg, PathSegment, QPath, TyKind};
7+
use rustc_middle::ty;
8+
use rustc_span::symbol::sym;
9+
10+
declare_tool_lint! {
11+
/// The `rustc_pass_by_value` lint marks a type with `#[rustc_pass_by_value]` requiring it to always be passed by value.
12+
/// This is usually used for types that are thin wrappers around references, so there is no benefit to an extra
13+
/// layer of indirection. (Example: `Ty` which is a reference to a `TyS`)
14+
pub rustc::PASS_BY_VALUE,
15+
Warn,
16+
"pass by reference of a type flagged as `#[rustc_pass_by_value]`",
17+
report_in_external_macro: true
18+
}
19+
20+
declare_lint_pass!(PassByValue => [PASS_BY_VALUE]);
21+
22+
impl<'tcx> LateLintPass<'tcx> for PassByValue {
23+
fn check_ty(&mut self, cx: &LateContext<'_>, ty: &'tcx hir::Ty<'tcx>) {
24+
match &ty.kind {
25+
TyKind::Rptr(_, hir::MutTy { ty: inner_ty, mutbl: hir::Mutability::Not }) => {
26+
if let Some(impl_did) = cx.tcx.impl_of_method(ty.hir_id.owner.to_def_id()) {
27+
if cx.tcx.impl_trait_ref(impl_did).is_some() {
28+
return;
29+
}
30+
}
31+
if let Some(t) = path_for_pass_by_value(cx, &inner_ty) {
32+
cx.struct_span_lint(PASS_BY_VALUE, ty.span, |lint| {
33+
lint.build(&format!("passing `{}` by reference", t))
34+
.span_suggestion(
35+
ty.span,
36+
"try passing by value",
37+
t,
38+
// Changing type of function argument
39+
Applicability::MaybeIncorrect,
40+
)
41+
.emit();
42+
})
43+
}
44+
}
45+
_ => {}
46+
}
47+
}
48+
}
49+
50+
fn path_for_pass_by_value(cx: &LateContext<'_>, ty: &hir::Ty<'_>) -> Option<String> {
51+
if let TyKind::Path(QPath::Resolved(_, path)) = &ty.kind {
52+
match path.res {
53+
Res::Def(_, def_id) if has_pass_by_value_attr(cx, def_id) => {
54+
if let Some(name) = cx.tcx.get_diagnostic_name(def_id) {
55+
return Some(format!("{}{}", name, gen_args(path.segments.last().unwrap())));
56+
}
57+
}
58+
Res::SelfTy(None, Some((did, _))) => {
59+
if let ty::Adt(adt, substs) = cx.tcx.type_of(did).kind() {
60+
if has_pass_by_value_attr(cx, adt.did) {
61+
if let Some(name) = cx.tcx.get_diagnostic_name(adt.did) {
62+
return Some(format!("{}<{}>", name, substs[0]));
63+
}
64+
}
65+
}
66+
}
67+
_ => (),
68+
}
69+
}
70+
71+
None
72+
}
73+
74+
fn has_pass_by_value_attr(cx: &LateContext<'_>, def_id: DefId) -> bool {
75+
for attr in cx.tcx.get_attrs(def_id).iter() {
76+
if attr.has_name(sym::rustc_pass_by_value) {
77+
return true;
78+
}
79+
}
80+
false
81+
}
82+
83+
fn gen_args(segment: &PathSegment<'_>) -> String {
84+
if let Some(args) = &segment.args {
85+
let lifetimes = args
86+
.args
87+
.iter()
88+
.filter_map(|arg| {
89+
if let GenericArg::Lifetime(lt) = arg {
90+
Some(lt.name.ident().to_string())
91+
} else {
92+
None
93+
}
94+
})
95+
.collect::<Vec<_>>();
96+
97+
if !lifetimes.is_empty() {
98+
return format!("<{}>", lifetimes.join(", "));
99+
}
100+
}
101+
102+
String::new()
103+
}

‎compiler/rustc_middle/src/ty/context.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -961,6 +961,7 @@ pub struct FreeRegionInfo {
961961
/// [rustc dev guide]: https://rustc-dev-guide.rust-lang.org/ty.html
962962
#[derive(Copy, Clone)]
963963
#[rustc_diagnostic_item = "TyCtxt"]
964+
#[cfg_attr(not(bootstrap), rustc_pass_by_value)]
964965
pub struct TyCtxt<'tcx> {
965966
gcx: &'tcx GlobalCtxt<'tcx>,
966967
}

‎compiler/rustc_middle/src/ty/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -462,6 +462,7 @@ impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for TyS<'tcx> {
462462
}
463463

464464
#[rustc_diagnostic_item = "Ty"]
465+
#[cfg_attr(not(bootstrap), rustc_pass_by_value)]
465466
pub type Ty<'tcx> = &'tcx TyS<'tcx>;
466467

467468
impl ty::EarlyBoundRegion {

‎compiler/rustc_passes/src/check_attr.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ impl CheckAttrVisitor<'_> {
114114
}
115115
sym::must_not_suspend => self.check_must_not_suspend(&attr, span, target),
116116
sym::must_use => self.check_must_use(hir_id, &attr, span, target),
117+
sym::rustc_pass_by_value => self.check_pass_by_value(&attr, span, target),
117118
sym::rustc_const_unstable
118119
| sym::rustc_const_stable
119120
| sym::unstable
@@ -1066,6 +1067,29 @@ impl CheckAttrVisitor<'_> {
10661067
is_valid
10671068
}
10681069

1070+
/// Warns against some misuses of `#[pass_by_value]`
1071+
fn check_pass_by_value(&self, attr: &Attribute, span: &Span, target: Target) -> bool {
1072+
match target {
1073+
Target::Struct
1074+
| Target::Enum
1075+
| Target::Union
1076+
| Target::Trait
1077+
| Target::TraitAlias
1078+
| Target::TyAlias => true,
1079+
_ => {
1080+
self.tcx
1081+
.sess
1082+
.struct_span_err(
1083+
attr.span,
1084+
"`pass_by_value` attribute should be applied to a struct, enum, trait or type alias.",
1085+
)
1086+
.span_label(*span, "is not a struct, enum, trait or type alias")
1087+
.emit();
1088+
false
1089+
}
1090+
}
1091+
}
1092+
10691093
/// Warns against some misuses of `#[must_use]`
10701094
fn check_must_use(
10711095
&self,

‎compiler/rustc_span/src/symbol.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1143,6 +1143,7 @@ symbols! {
11431143
rustc_paren_sugar,
11441144
rustc_partition_codegened,
11451145
rustc_partition_reused,
1146+
rustc_pass_by_value,
11461147
rustc_peek,
11471148
rustc_peek_definite_init,
11481149
rustc_peek_liveness,

‎src/test/ui-fulldeps/internal-lints/pass_ty_by_ref.rs renamed to ‎src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// compile-flags: -Z unstable-options
22

33
#![feature(rustc_private)]
4-
#![deny(rustc::ty_pass_by_reference)]
4+
#![deny(rustc::pass_by_value)]
55
#![allow(unused)]
66

77
extern crate rustc_middle;

‎src/test/ui-fulldeps/internal-lints/pass_ty_by_ref.stderr renamed to ‎src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,77 +1,77 @@
11
error: passing `Ty<'_>` by reference
2-
--> $DIR/pass_ty_by_ref.rs:13:13
2+
--> $DIR/rustc_pass_by_value.rs:13:13
33
|
44
LL | ty_ref: &Ty<'_>,
55
| ^^^^^^^ help: try passing by value: `Ty<'_>`
66
|
77
note: the lint level is defined here
8-
--> $DIR/pass_ty_by_ref.rs:4:9
8+
--> $DIR/rustc_pass_by_value.rs:4:9
99
|
10-
LL | #![deny(rustc::ty_pass_by_reference)]
11-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
10+
LL | #![deny(rustc::pass_by_value)]
11+
| ^^^^^^^^^^^^^^^^^^^^
1212

1313
error: passing `TyCtxt<'_>` by reference
14-
--> $DIR/pass_ty_by_ref.rs:15:18
14+
--> $DIR/rustc_pass_by_value.rs:15:18
1515
|
1616
LL | ty_ctxt_ref: &TyCtxt<'_>,
1717
| ^^^^^^^^^^^ help: try passing by value: `TyCtxt<'_>`
1818

1919
error: passing `Ty<'_>` by reference
20-
--> $DIR/pass_ty_by_ref.rs:19:28
20+
--> $DIR/rustc_pass_by_value.rs:19:28
2121
|
2222
LL | fn ty_multi_ref(ty_multi: &&Ty<'_>, ty_ctxt_multi: &&&&TyCtxt<'_>) {}
2323
| ^^^^^^^ help: try passing by value: `Ty<'_>`
2424

2525
error: passing `TyCtxt<'_>` by reference
26-
--> $DIR/pass_ty_by_ref.rs:19:55
26+
--> $DIR/rustc_pass_by_value.rs:19:55
2727
|
2828
LL | fn ty_multi_ref(ty_multi: &&Ty<'_>, ty_ctxt_multi: &&&&TyCtxt<'_>) {}
2929
| ^^^^^^^^^^^ help: try passing by value: `TyCtxt<'_>`
3030

3131
error: passing `Ty<'_>` by reference
32-
--> $DIR/pass_ty_by_ref.rs:26:17
32+
--> $DIR/rustc_pass_by_value.rs:26:17
3333
|
3434
LL | ty_ref: &Ty<'_>,
3535
| ^^^^^^^ help: try passing by value: `Ty<'_>`
3636

3737
error: passing `TyCtxt<'_>` by reference
38-
--> $DIR/pass_ty_by_ref.rs:28:22
38+
--> $DIR/rustc_pass_by_value.rs:28:22
3939
|
4040
LL | ty_ctxt_ref: &TyCtxt<'_>,
4141
| ^^^^^^^^^^^ help: try passing by value: `TyCtxt<'_>`
4242

4343
error: passing `Ty<'_>` by reference
44-
--> $DIR/pass_ty_by_ref.rs:31:41
44+
--> $DIR/rustc_pass_by_value.rs:31:41
4545
|
4646
LL | fn ty_multi_ref_in_trait(ty_multi: &&Ty<'_>, ty_ctxt_multi: &&&&TyCtxt<'_>);
4747
| ^^^^^^^ help: try passing by value: `Ty<'_>`
4848

4949
error: passing `TyCtxt<'_>` by reference
50-
--> $DIR/pass_ty_by_ref.rs:31:68
50+
--> $DIR/rustc_pass_by_value.rs:31:68
5151
|
5252
LL | fn ty_multi_ref_in_trait(ty_multi: &&Ty<'_>, ty_ctxt_multi: &&&&TyCtxt<'_>);
5353
| ^^^^^^^^^^^ help: try passing by value: `TyCtxt<'_>`
5454

5555
error: passing `Ty<'_>` by reference
56-
--> $DIR/pass_ty_by_ref.rs:53:17
56+
--> $DIR/rustc_pass_by_value.rs:53:17
5757
|
5858
LL | ty_ref: &Ty<'_>,
5959
| ^^^^^^^ help: try passing by value: `Ty<'_>`
6060

6161
error: passing `TyCtxt<'_>` by reference
62-
--> $DIR/pass_ty_by_ref.rs:55:22
62+
--> $DIR/rustc_pass_by_value.rs:55:22
6363
|
6464
LL | ty_ctxt_ref: &TyCtxt<'_>,
6565
| ^^^^^^^^^^^ help: try passing by value: `TyCtxt<'_>`
6666

6767
error: passing `Ty<'_>` by reference
68-
--> $DIR/pass_ty_by_ref.rs:59:38
68+
--> $DIR/rustc_pass_by_value.rs:59:38
6969
|
7070
LL | fn ty_multi_ref_assoc(ty_multi: &&Ty<'_>, ty_ctxt_multi: &&&&TyCtxt<'_>) {}
7171
| ^^^^^^^ help: try passing by value: `Ty<'_>`
7272

7373
error: passing `TyCtxt<'_>` by reference
74-
--> $DIR/pass_ty_by_ref.rs:59:65
74+
--> $DIR/rustc_pass_by_value.rs:59:65
7575
|
7676
LL | fn ty_multi_ref_assoc(ty_multi: &&Ty<'_>, ty_ctxt_multi: &&&&TyCtxt<'_>) {}
7777
| ^^^^^^^^^^^ help: try passing by value: `TyCtxt<'_>`

0 commit comments

Comments
 (0)