bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Auto merge of #120393 - Urgau:rfc3373-non-local-defs, r=WaffleLapkin

Browse source 
Implement RFC 3373: Avoid non-local definitions in functions

This PR implements [RFC 3373: Avoid non-local definitions in functions](https://github.com/rust-lang/rust/issues/120363).
This commit is contained in:
bors 2024-02-25 19:11:06 +00:00
commit b0d3e04ca9
59 changed files with 1518 additions and 92 deletions
Download patch file Download diff file Expand all files Collapse all files

1
compiler/rustc_lint/Cargo.toml
View file

@ -23,6 +23,7 @@ rustc_span = { path = "../rustc_span" }
rustc_target = { path = "../rustc_target" }
rustc_trait_selection = { path = "../rustc_trait_selection" }
rustc_type_ir = { path = "../rustc_type_ir" }
smallvec = { version = "1.8.1", features = ["union", "may_dangle"] }
tracing = "0.1"
unicode-security = "0.1.0"
# tidy-alphabetical-end

23
compiler/rustc_lint/messages.ftl
View file

@ -414,6 +414,29 @@ lint_non_fmt_panic_unused =
}
.add_fmt_suggestion = or add a "{"{"}{"}"}" format string to use the message literally
lint_non_local_definitions_cargo_update = the {$macro_kind} `{$macro_name}` may come from an old version of the `{$crate_name}` crate, try updating your dependency with `cargo update -p {$crate_name}`
lint_non_local_definitions_deprecation = this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363>
lint_non_local_definitions_impl = non-local `impl` definition, they should be avoided as they go against expectation
.help =
move this `impl` block outside the of the current {$body_kind_descr} {$depth ->
[one] `{$body_name}`
*[other] `{$body_name}` and up {$depth} bodies
}
.non_local = an `impl` definition is non-local if it is nested inside an item and neither the type nor the trait are at the same nesting level as the `impl` block
.exception = one exception to the rule are anon-const (`const _: () = {"{"} ... {"}"}`) at top-level module and anon-const at the same nesting as the trait or type
.const_anon = use a const-anon item to suppress this lint
lint_non_local_definitions_macro_rules = non-local `macro_rules!` definition, they should be avoided as they go against expectation
.help =
remove the `#[macro_export]` or move this `macro_rules!` outside the of the current {$body_kind_descr} {$depth ->
[one] `{$body_name}`
*[other] `{$body_name}` and up {$depth} bodies
}
.non_local = a `macro_rules!` definition is non-local if it is nested inside an item and has a `#[macro_export]` attribute
.exception = one exception to the rule are anon-const (`const _: () = {"{"} ... {"}"}`) at top-level module
lint_non_snake_case = {$sort} `{$name}` should have a snake case name
.rename_or_convert_suggestion = rename the identifier or convert it to a snake case raw identifier
.cannot_convert_note = `{$sc}` cannot be used as a raw identifier

3
compiler/rustc_lint/src/lib.rs
View file

@ -70,6 +70,7 @@ mod methods;
mod multiple_supertrait_upcastable;
mod non_ascii_idents;
mod non_fmt_panic;
mod non_local_def;
mod nonstandard_style;
mod noop_method_call;
mod opaque_hidden_inferred_bound;
@ -105,6 +106,7 @@ use methods::*;
use multiple_supertrait_upcastable::*;
use non_ascii_idents::*;
use non_fmt_panic::NonPanicFmt;
use non_local_def::*;
use nonstandard_style::*;
use noop_method_call::*;
use opaque_hidden_inferred_bound::*;
@ -229,6 +231,7 @@ late_lint_methods!(
MissingDebugImplementations: MissingDebugImplementations,
MissingDoc: MissingDoc,
AsyncFnInTrait: AsyncFnInTrait,
NonLocalDefinitions: NonLocalDefinitions::default(),
]
]
);

39
compiler/rustc_lint/src/lints.rs
View file

@ -1334,6 +1334,45 @@ pub struct SuspiciousDoubleRefCloneDiag<'a> {
pub ty: Ty<'a>,
}
// non_local_defs.rs
#[derive(LintDiagnostic)]
pub enum NonLocalDefinitionsDiag {
#[diag(lint_non_local_definitions_impl)]
#[help]
#[note(lint_non_local)]
#[note(lint_exception)]
#[note(lint_non_local_definitions_deprecation)]
Impl {
depth: u32,
body_kind_descr: &'static str,
body_name: String,
#[subdiagnostic]
cargo_update: Option<NonLocalDefinitionsCargoUpdateNote>,
#[suggestion(lint_const_anon, code = "_", applicability = "machine-applicable")]
const_anon: Option<Span>,
},
#[diag(lint_non_local_definitions_macro_rules)]
#[help]
#[note(lint_non_local)]
#[note(lint_exception)]
#[note(lint_non_local_definitions_deprecation)]
MacroRules {
depth: u32,
body_kind_descr: &'static str,
body_name: String,
#[subdiagnostic]
cargo_update: Option<NonLocalDefinitionsCargoUpdateNote>,
},
}
#[derive(Subdiagnostic)]
#[note(lint_non_local_definitions_cargo_update)]
pub struct NonLocalDefinitionsCargoUpdateNote {
pub macro_kind: &'static str,
pub macro_name: Symbol,
pub crate_name: Symbol,
}
// pass_by_value.rs
#[derive(LintDiagnostic)]
#[diag(lint_pass_by_value)]

222
compiler/rustc_lint/src/non_local_def.rs Normal file
View file

@ -0,0 +1,222 @@
use rustc_hir::{def::DefKind, Body, Item, ItemKind, Node, Path, QPath, TyKind};
use rustc_span::def_id::{DefId, LOCAL_CRATE};
use rustc_span::{sym, symbol::kw, ExpnKind, MacroKind};
use smallvec::{smallvec, SmallVec};
use crate::lints::{NonLocalDefinitionsCargoUpdateNote, NonLocalDefinitionsDiag};
use crate::{LateContext, LateLintPass, LintContext};
declare_lint! {
/// The `non_local_definitions` lint checks for `impl` blocks and `#[macro_export]`
/// macro inside bodies (functions, enum discriminant, ...).
///
/// ### Example
///
/// ```rust
/// trait MyTrait {}
/// struct MyStruct;
///
/// fn foo() {
/// impl MyTrait for MyStruct {}
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// Creating non-local definitions go against expectation and can create discrepancies
/// in tooling. It should be avoided. It may become deny-by-default in edition 2024
/// and higher, see see the tracking issue <https://github.com/rust-lang/rust/issues/120363>.
///
/// An `impl` definition is non-local if it is nested inside an item and neither
/// the type nor the trait are at the same nesting level as the `impl` block.
///
/// All nested bodies (functions, enum discriminant, array length, consts) (expect for
/// `const _: Ty = { ... }` in top-level module, which is still undecided) are checked.
pub NON_LOCAL_DEFINITIONS,
Warn,
"checks for non-local definitions",
report_in_external_macro
}
#[derive(Default)]
pub struct NonLocalDefinitions {
body_depth: u32,
}
impl_lint_pass!(NonLocalDefinitions => [NON_LOCAL_DEFINITIONS]);
// FIXME(Urgau): Figure out how to handle modules nested in bodies.
// It's currently not handled by the current logic because modules are not bodies.
// They don't even follow the correct order (check_body -> check_mod -> check_body_post)
// instead check_mod is called after every body has been handled.
impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
fn check_body(&mut self, _cx: &LateContext<'tcx>, _body: &'tcx Body<'tcx>) {
self.body_depth += 1;
}
fn check_body_post(&mut self, _cx: &LateContext<'tcx>, _body: &'tcx Body<'tcx>) {
self.body_depth -= 1;
}
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
if self.body_depth == 0 {
return;
}
let parent = cx.tcx.parent(item.owner_id.def_id.into());
let parent_def_kind = cx.tcx.def_kind(parent);
let parent_opt_item_name = cx.tcx.opt_item_name(parent);
// Per RFC we (currently) ignore anon-const (`const _: Ty = ...`) in top-level module.
if self.body_depth == 1
&& parent_def_kind == DefKind::Const
&& parent_opt_item_name == Some(kw::Underscore)
{
return;
}
let cargo_update = || {
let oexpn = item.span.ctxt().outer_expn_data();
if let Some(def_id) = oexpn.macro_def_id
&& let ExpnKind::Macro(macro_kind, macro_name) = oexpn.kind
&& def_id.krate != LOCAL_CRATE
&& std::env::var_os("CARGO").is_some()
{
Some(NonLocalDefinitionsCargoUpdateNote {
macro_kind: macro_kind.descr(),
macro_name,
crate_name: cx.tcx.crate_name(def_id.krate),
})
} else {
None
}
};
match item.kind {
ItemKind::Impl(impl_) => {
// The RFC states:
//
// > An item nested inside an expression-containing item (through any
// > level of nesting) may not define an impl Trait for Type unless
// > either the **Trait** or the **Type** is also nested inside the
// > same expression-containing item.
//
// To achieve this we get try to get the paths of the _Trait_ and
// _Type_, and we look inside thoses paths to try a find in one
// of them a type whose parent is the same as the impl definition.
//
// If that's the case this means that this impl block declaration
// is using local items and so we don't lint on it.
// We also ignore anon-const in item by including the anon-const
// parent as well; and since it's quite uncommon, we use smallvec
// to avoid unnecessary heap allocations.
let local_parents: SmallVec<[DefId; 1]> = if parent_def_kind == DefKind::Const
&& parent_opt_item_name == Some(kw::Underscore)
{
smallvec![parent, cx.tcx.parent(parent)]
} else {
smallvec![parent]
};
let self_ty_has_local_parent = match impl_.self_ty.kind {
TyKind::Path(QPath::Resolved(_, ty_path)) => {
path_has_local_parent(ty_path, cx, &*local_parents)
}
TyKind::TraitObject([principle_poly_trait_ref, ..], _, _) => {
path_has_local_parent(
principle_poly_trait_ref.trait_ref.path,
cx,
&*local_parents,
)
}
TyKind::TraitObject([], _, _)
| TyKind::InferDelegation(_, _)
| TyKind::Slice(_)
| TyKind::Array(_, _)
| TyKind::Ptr(_)
| TyKind::Ref(_, _)
| TyKind::BareFn(_)
| TyKind::Never
| TyKind::Tup(_)
| TyKind::Path(_)
| TyKind::AnonAdt(_)
| TyKind::OpaqueDef(_, _, _)
| TyKind::Typeof(_)
| TyKind::Infer
| TyKind::Err(_) => false,
};
let of_trait_has_local_parent = impl_
.of_trait
.map(|of_trait| path_has_local_parent(of_trait.path, cx, &*local_parents))
.unwrap_or(false);
// If none of them have a local parent (LOGICAL NOR) this means that
// this impl definition is a non-local definition and so we lint on it.
if !(self_ty_has_local_parent || of_trait_has_local_parent) {
let const_anon = if self.body_depth == 1
&& parent_def_kind == DefKind::Const
&& parent_opt_item_name != Some(kw::Underscore)
&& let Some(parent) = parent.as_local()
&& let Node::Item(item) = cx.tcx.hir_node_by_def_id(parent)
&& let ItemKind::Const(ty, _, _) = item.kind
&& let TyKind::Tup(&[]) = ty.kind
{
Some(item.ident.span)
} else {
None
};
cx.emit_span_lint(
NON_LOCAL_DEFINITIONS,
item.span,
NonLocalDefinitionsDiag::Impl {
depth: self.body_depth,
body_kind_descr: cx.tcx.def_kind_descr(parent_def_kind, parent),
body_name: parent_opt_item_name
.map(|s| s.to_ident_string())
.unwrap_or_else(|| "<unnameable>".to_string()),
cargo_update: cargo_update(),
const_anon,
},
)
}
}
ItemKind::Macro(_macro, MacroKind::Bang)
if cx.tcx.has_attr(item.owner_id.def_id, sym::macro_export) =>
{
cx.emit_span_lint(
NON_LOCAL_DEFINITIONS,
item.span,
NonLocalDefinitionsDiag::MacroRules {
depth: self.body_depth,
body_kind_descr: cx.tcx.def_kind_descr(parent_def_kind, parent),
body_name: parent_opt_item_name
.map(|s| s.to_ident_string())
.unwrap_or_else(|| "<unnameable>".to_string()),
cargo_update: cargo_update(),
},
)
}
_ => {}
}
}
}
/// Given a path and a parent impl def id, this checks if the if parent resolution
/// def id correspond to the def id of the parent impl definition.
///
/// Given this path, we will look at the path (and ignore any generic args):
///
/// ```text
/// std::convert::PartialEq<Foo<Bar>>
/// ^^^^^^^^^^^^^^^^^^^^^^^
/// ```
fn path_has_local_parent(path: &Path<'_>, cx: &LateContext<'_>, local_parents: &[DefId]) -> bool {
path.res.opt_def_id().is_some_and(|did| local_parents.contains(&cx.tcx.parent(did)))
}