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 all 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
94 changes: 94 additions & 0 deletions compiler/rustc_lint/src/pass_by_value.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
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()
.map(|arg| match arg {
GenericArg::Lifetime(lt) => lt.name.ident().to_string(),
GenericArg::Type(ty) => {
cx.tcx.sess.source_map().span_to_snippet(ty.span).unwrap_or_default()
}
GenericArg::Const(c) => {
cx.tcx.sess.source_map().span_to_snippet(c.span).unwrap_or_default()
}
GenericArg::Infer(_) => String::from("_"),
})
.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)]
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
80 changes: 0 additions & 80 deletions src/test/ui-fulldeps/internal-lints/pass_ty_by_ref.stderr

This file was deleted.

20 changes: 0 additions & 20 deletions src/test/ui-fulldeps/internal-lints/pass_ty_by_ref_self.stderr

This file was deleted.

Loading