Skip to content
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

feat: rustc_pass_by_value lint attribute #92646

Merged
merged 6 commits into from
Jan 16, 2022
Merged
Show file tree
Hide file tree
Changes from 3 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, NameValueStr: "reason"), WarnFollowing,
mdibaiee marked this conversation as resolved.
Show resolved Hide resolved
"#[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
101 changes: 101 additions & 0 deletions compiler/rustc_lint/src/pass_by_value.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
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) => {
let name = cx.tcx.item_name(def_id).to_ident_string();
return Some(format!("{}{}", name, gen_args(path.segments.last().unwrap())));
mdibaiee marked this conversation as resolved.
Show resolved Hide resolved
}
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) {
let name = cx.tcx.item_name(adt.did).to_ident_string();
return Some(format!("{}<{}>", name, substs[0]));
mdibaiee marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
_ => (),
}
}

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
mdibaiee marked this conversation as resolved.
Show resolved Hide resolved
}

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()
}
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)]
mdibaiee marked this conversation as resolved.
Show resolved Hide resolved
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
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,40 @@ 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
) {
}
}

fn main() {}
Loading