Skip to content

Commit 3373a0e

Browse files
authored
Rollup merge of rust-lang#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 rust-lang#76935
2 parents 29adca6 + 2728af7 commit 3373a0e

File tree

14 files changed

+372
-138
lines changed

14 files changed

+372
-138
lines changed

compiler/rustc_feature/src/builtin_attrs.rs

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

+1-32
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

+5-1
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::*;
@@ -490,15 +492,17 @@ fn register_internals(store: &mut LintStore) {
490492
store.register_late_pass(|| Box::new(ExistingDocKeyword));
491493
store.register_lints(&TyTyKind::get_lints());
492494
store.register_late_pass(|| Box::new(TyTyKind));
495+
store.register_lints(&PassByValue::get_lints());
496+
store.register_late_pass(|| Box::new(PassByValue));
493497
store.register_group(
494498
false,
495499
"rustc::internal",
496500
None,
497501
vec![
498502
LintId::of(DEFAULT_HASH_TYPES),
499503
LintId::of(USAGE_OF_TY_TYKIND),
504+
LintId::of(PASS_BY_VALUE),
500505
LintId::of(LINT_PASS_IMPL_WITHOUT_MACRO),
501-
LintId::of(TY_PASS_BY_REFERENCE),
502506
LintId::of(USAGE_OF_QUALIFIED_TY),
503507
LintId::of(EXISTING_DOC_KEYWORD),
504508
],
+94
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
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::{GenericArg, PathSegment, QPath, TyKind};
6+
use rustc_middle::ty;
7+
use rustc_span::symbol::sym;
8+
9+
declare_tool_lint! {
10+
/// The `rustc_pass_by_value` lint marks a type with `#[rustc_pass_by_value]` requiring it to always be passed by value.
11+
/// This is usually used for types that are thin wrappers around references, so there is no benefit to an extra
12+
/// layer of indirection. (Example: `Ty` which is a reference to a `TyS`)
13+
pub rustc::PASS_BY_VALUE,
14+
Warn,
15+
"pass by reference of a type flagged as `#[rustc_pass_by_value]`",
16+
report_in_external_macro: true
17+
}
18+
19+
declare_lint_pass!(PassByValue => [PASS_BY_VALUE]);
20+
21+
impl<'tcx> LateLintPass<'tcx> for PassByValue {
22+
fn check_ty(&mut self, cx: &LateContext<'_>, ty: &'tcx hir::Ty<'tcx>) {
23+
match &ty.kind {
24+
TyKind::Rptr(_, hir::MutTy { ty: inner_ty, mutbl: hir::Mutability::Not }) => {
25+
if let Some(impl_did) = cx.tcx.impl_of_method(ty.hir_id.owner.to_def_id()) {
26+
if cx.tcx.impl_trait_ref(impl_did).is_some() {
27+
return;
28+
}
29+
}
30+
if let Some(t) = path_for_pass_by_value(cx, &inner_ty) {
31+
cx.struct_span_lint(PASS_BY_VALUE, ty.span, |lint| {
32+
lint.build(&format!("passing `{}` by reference", t))
33+
.span_suggestion(
34+
ty.span,
35+
"try passing by value",
36+
t,
37+
// Changing type of function argument
38+
Applicability::MaybeIncorrect,
39+
)
40+
.emit();
41+
})
42+
}
43+
}
44+
_ => {}
45+
}
46+
}
47+
}
48+
49+
fn path_for_pass_by_value(cx: &LateContext<'_>, ty: &hir::Ty<'_>) -> Option<String> {
50+
if let TyKind::Path(QPath::Resolved(_, path)) = &ty.kind {
51+
match path.res {
52+
Res::Def(_, def_id) if cx.tcx.has_attr(def_id, sym::rustc_pass_by_value) => {
53+
let name = cx.tcx.item_name(def_id).to_ident_string();
54+
let path_segment = path.segments.last().unwrap();
55+
return Some(format!("{}{}", name, gen_args(cx, path_segment)));
56+
}
57+
Res::SelfTy(None, Some((did, _))) => {
58+
if let ty::Adt(adt, substs) = cx.tcx.type_of(did).kind() {
59+
if cx.tcx.has_attr(adt.did, sym::rustc_pass_by_value) {
60+
return Some(cx.tcx.def_path_str_with_substs(adt.did, substs));
61+
}
62+
}
63+
}
64+
_ => (),
65+
}
66+
}
67+
68+
None
69+
}
70+
71+
fn gen_args(cx: &LateContext<'_>, segment: &PathSegment<'_>) -> String {
72+
if let Some(args) = &segment.args {
73+
let params = args
74+
.args
75+
.iter()
76+
.map(|arg| match arg {
77+
GenericArg::Lifetime(lt) => lt.name.ident().to_string(),
78+
GenericArg::Type(ty) => {
79+
cx.tcx.sess.source_map().span_to_snippet(ty.span).unwrap_or_default()
80+
}
81+
GenericArg::Const(c) => {
82+
cx.tcx.sess.source_map().span_to_snippet(c.span).unwrap_or_default()
83+
}
84+
GenericArg::Infer(_) => String::from("_"),
85+
})
86+
.collect::<Vec<_>>();
87+
88+
if !params.is_empty() {
89+
return format!("<{}>", params.join(", "));
90+
}
91+
}
92+
93+
String::new()
94+
}

compiler/rustc_middle/src/ty/context.rs

+1
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

+1
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,7 @@ impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for TyS<'tcx> {
464464
}
465465

466466
#[rustc_diagnostic_item = "Ty"]
467+
#[cfg_attr(not(bootstrap), rustc_pass_by_value)]
467468
pub type Ty<'tcx> = &'tcx TyS<'tcx>;
468469

469470
impl ty::EarlyBoundRegion {

compiler/rustc_passes/src/check_attr.rs

+19
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,24 @@ 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 | Target::Enum | Target::TyAlias => true,
1074+
_ => {
1075+
self.tcx
1076+
.sess
1077+
.struct_span_err(
1078+
attr.span,
1079+
"`pass_by_value` attribute should be applied to a struct, enum or type alias.",
1080+
)
1081+
.span_label(*span, "is not a struct, enum or type alias")
1082+
.emit();
1083+
false
1084+
}
1085+
}
1086+
}
1087+
10691088
/// Warns against some misuses of `#[must_use]`
10701089
fn check_must_use(
10711090
&self,

compiler/rustc_span/src/symbol.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1147,6 +1147,7 @@ symbols! {
11471147
rustc_paren_sugar,
11481148
rustc_partition_codegened,
11491149
rustc_partition_reused,
1150+
rustc_pass_by_value,
11501151
rustc_peek,
11511152
rustc_peek_definite_init,
11521153
rustc_peek_liveness,

src/test/ui-fulldeps/internal-lints/pass_ty_by_ref.stderr

-80
This file was deleted.

src/test/ui-fulldeps/internal-lints/pass_ty_by_ref_self.stderr

-20
This file was deleted.

0 commit comments

Comments
 (0)