Skip to content

feat: rustc_pass_by_value lint attribute #92646

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jan 16, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
33 changes: 1 addition & 32 deletions compiler/rustc_lint/src/internal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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,
Expand All @@ -74,7 +64,6 @@ declare_tool_lint! {

declare_lint_pass!(TyTyKind => [
USAGE_OF_TY_TYKIND,
TY_PASS_BY_REFERENCE,
USAGE_OF_QUALIFIED_TY,
]);

Expand Down Expand Up @@ -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();
})
}
}
_ => {}
}
}
Expand Down
6 changes: 5 additions & 1 deletion compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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::*;
Expand Down Expand Up @@ -489,15 +491,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),
],
Expand Down
98 changes: 98 additions & 0 deletions compiler/rustc_lint/src/pass_by_value.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
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()
.filter_map(|arg| match arg {
GenericArg::Lifetime(lt) => Some(lt.name.ident().to_string()),
GenericArg::Type(ty) => {
let snippet =
cx.tcx.sess.source_map().span_to_snippet(ty.span).unwrap_or_default();
Some(snippet)
}
GenericArg::Const(c) => {
let snippet =
cx.tcx.sess.source_map().span_to_snippet(c.span).unwrap_or_default();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
cx.tcx.sess.source_map().span_to_snippet(c.span).unwrap_or_default();
cx.tcx.sess.source_map().span_to_snippet(c.span).unwrap_or_else(|_| String::from("_"));

🤔 don't want empty strings in case the span is broken. Don't know how to even get broken strings here, so i don't think this matters too much :p

Some(snippet)
}
_ => None,
})
.collect::<Vec<_>>();

if !params.is_empty() {
return format!("<{}>", params.join(", "));
}
}

String::new()
}
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
19 changes: 19 additions & 0 deletions compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
20 changes: 0 additions & 20 deletions src/test/ui-fulldeps/internal-lints/pass_ty_by_ref_self.stderr

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
// compile-flags: -Z unstable-options

#![feature(rustc_attrs)]
#![feature(rustc_private)]
#![deny(rustc::ty_pass_by_reference)]
#![deny(rustc::pass_by_value)]
#![allow(unused)]

extern crate rustc_middle;
Expand Down Expand Up @@ -61,4 +62,55 @@ impl Foo {
//~^^ ERROR passing `TyCtxt<'_>` by reference
}

#[rustc_pass_by_value]
enum CustomEnum {
A,
B,
}

impl CustomEnum {
fn test(
value: CustomEnum,
reference: &CustomEnum, //~ ERROR passing `CustomEnum` by reference
) {
}
}

#[rustc_pass_by_value]
struct CustomStruct {
s: u8,
}

#[rustc_pass_by_value]
type CustomAlias<'a> = &'a CustomStruct; //~ ERROR passing `CustomStruct` by reference

impl CustomStruct {
fn test(
value: CustomStruct,
reference: &CustomStruct, //~ ERROR passing `CustomStruct` by reference
) {
}

fn test_alias(
value: CustomAlias,
reference: &CustomAlias, //~ ERROR passing `CustomAlias<>` by reference
) {
}
}

#[rustc_pass_by_value]
struct WithParameters<T, const N: usize, M = u32> {
slice: [T; N],
m: M,
}

impl<T> WithParameters<T, 1> {
fn test(
value: WithParameters<T, 1>,
reference: &WithParameters<T, 1>, //~ ERROR passing `WithParameters<T, 1>` by reference
reference_with_m: &WithParameters<T, 1, u32>, //~ ERROR passing `WithParameters<T, 1, u32>` by reference
) {
}
}

fn main() {}
Loading