Skip to content

Commit b0d3e04

Browse files
committed
Auto merge of #120393 - Urgau:rfc3373-non-local-defs, r=WaffleLapkin
Implement RFC 3373: Avoid non-local definitions in functions This PR implements [RFC 3373: Avoid non-local definitions in functions](#120363).
2 parents 8c0b1fc + 63469ab commit b0d3e04

59 files changed

Lines changed: 1518 additions & 92 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4101,6 +4101,7 @@ dependencies = [
41014101
"rustc_target",
41024102
"rustc_trait_selection",
41034103
"rustc_type_ir",
4104+
"smallvec",
41044105
"tracing",
41054106
"unicode-security",
41064107
]

‎compiler/rustc_hir_analysis/src/coherence/inherent_impls_overlap.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,11 @@ struct InherentOverlapChecker<'tcx> {
2424
tcx:TyCtxt<'tcx>,
2525
}
2626

27+
rustc_index::newtype_index! {
28+
#[orderable]
29+
pubstructRegionId{}
30+
}
31+
2732
impl<'tcx>InherentOverlapChecker<'tcx>{
2833
/// Checks whether any associated items in impls 1 and 2 share the same identifier and
2934
/// namespace.
@@ -205,11 +210,6 @@ impl<'tcx> InherentOverlapChecker<'tcx> {
205210
// This is advantageous to running the algorithm over the
206211
// entire graph when there are many connected regions.
207212

208-
rustc_index::newtype_index! {
209-
#[orderable]
210-
pubstructRegionId{}
211-
}
212-
213213
structConnectedRegion{
214214
idents:SmallVec<[Symbol;8]>,
215215
impl_blocks:FxHashSet<usize>,

‎compiler/rustc_lint/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ rustc_span = { path = "../rustc_span" }
2323
rustc_target = { path = "../rustc_target" }
2424
rustc_trait_selection = { path = "../rustc_trait_selection" }
2525
rustc_type_ir = { path = "../rustc_type_ir" }
26+
smallvec = { version = "1.8.1", features = ["union", "may_dangle"] }
2627
tracing = "0.1"
2728
unicode-security = "0.1.0"
2829
# tidy-alphabetical-end

‎compiler/rustc_lint/messages.ftl‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -414,6 +414,29 @@ lint_non_fmt_panic_unused =
414414
}
415415
.add_fmt_suggestion = or add a "{"{"}{"}"}" format string to use the message literally
416416
417+
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}`
418+
419+
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>
420+
421+
lint_non_local_definitions_impl = non-local `impl` definition, they should be avoided as they go against expectation
422+
.help =
423+
move this `impl` block outside the of the current {$body_kind_descr}{$depth->
424+
[one] `{$body_name}`
425+
*[other] `{$body_name}` and up {$depth} bodies
426+
}
427+
.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
428+
.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
429+
.const_anon = use a const-anon item to suppress this lint
430+
431+
lint_non_local_definitions_macro_rules = non-local `macro_rules!` definition, they should be avoided as they go against expectation
432+
.help =
433+
remove the `#[macro_export]` or move this `macro_rules!` outside the of the current {$body_kind_descr}{$depth->
434+
[one] `{$body_name}`
435+
*[other] `{$body_name}` and up {$depth} bodies
436+
}
437+
.non_local = a `macro_rules!` definition is non-local if it is nested inside an item and has a `#[macro_export]` attribute
438+
.exception = one exception to the rule are anon-const (`const _: () = {"{"} ... {"}"}`) at top-level module
439+
417440
lint_non_snake_case = {$sort} `{$name}` should have a snake case name
418441
.rename_or_convert_suggestion = rename the identifier or convert it to a snake case raw identifier
419442
.cannot_convert_note = `{$sc}` cannot be used as a raw identifier

‎compiler/rustc_lint/src/lib.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ mod methods;
7070
mod multiple_supertrait_upcastable;
7171
mod non_ascii_idents;
7272
mod non_fmt_panic;
73+
mod non_local_def;
7374
mod nonstandard_style;
7475
mod noop_method_call;
7576
mod opaque_hidden_inferred_bound;
@@ -105,6 +106,7 @@ use methods::*;
105106
use multiple_supertrait_upcastable::*;
106107
use non_ascii_idents::*;
107108
use non_fmt_panic::NonPanicFmt;
109+
use non_local_def::*;
108110
use nonstandard_style::*;
109111
use noop_method_call::*;
110112
use opaque_hidden_inferred_bound::*;
@@ -229,6 +231,7 @@ late_lint_methods!(
229231
MissingDebugImplementations:MissingDebugImplementations,
230232
MissingDoc:MissingDoc,
231233
AsyncFnInTrait:AsyncFnInTrait,
234+
NonLocalDefinitions:NonLocalDefinitions::default(),
232235
]
233236
]
234237
);

‎compiler/rustc_lint/src/lints.rs‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1334,6 +1334,45 @@ pub struct SuspiciousDoubleRefCloneDiag<'a> {
13341334
pubty:Ty<'a>,
13351335
}
13361336

1337+
// non_local_defs.rs
1338+
#[derive(LintDiagnostic)]
1339+
pubenumNonLocalDefinitionsDiag{
1340+
#[diag(lint_non_local_definitions_impl)]
1341+
#[help]
1342+
#[note(lint_non_local)]
1343+
#[note(lint_exception)]
1344+
#[note(lint_non_local_definitions_deprecation)]
1345+
Impl{
1346+
depth:u32,
1347+
body_kind_descr:&'staticstr,
1348+
body_name:String,
1349+
#[subdiagnostic]
1350+
cargo_update:Option<NonLocalDefinitionsCargoUpdateNote>,
1351+
#[suggestion(lint_const_anon, code = "_", applicability = "machine-applicable")]
1352+
const_anon:Option<Span>,
1353+
},
1354+
#[diag(lint_non_local_definitions_macro_rules)]
1355+
#[help]
1356+
#[note(lint_non_local)]
1357+
#[note(lint_exception)]
1358+
#[note(lint_non_local_definitions_deprecation)]
1359+
MacroRules{
1360+
depth:u32,
1361+
body_kind_descr:&'staticstr,
1362+
body_name:String,
1363+
#[subdiagnostic]
1364+
cargo_update:Option<NonLocalDefinitionsCargoUpdateNote>,
1365+
},
1366+
}
1367+
1368+
#[derive(Subdiagnostic)]
1369+
#[note(lint_non_local_definitions_cargo_update)]
1370+
pubstructNonLocalDefinitionsCargoUpdateNote{
1371+
pubmacro_kind:&'staticstr,
1372+
pubmacro_name:Symbol,
1373+
pubcrate_name:Symbol,
1374+
}
1375+
13371376
// pass_by_value.rs
13381377
#[derive(LintDiagnostic)]
13391378
#[diag(lint_pass_by_value)]
Lines changed: 222 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,222 @@
1+
use rustc_hir::{def::DefKind,Body,Item,ItemKind,Node,Path,QPath,TyKind};
2+
use rustc_span::def_id::{DefId,LOCAL_CRATE};
3+
use rustc_span::{sym, symbol::kw,ExpnKind,MacroKind};
4+
5+
use smallvec::{smallvec,SmallVec};
6+
7+
usecrate::lints::{NonLocalDefinitionsCargoUpdateNote,NonLocalDefinitionsDiag};
8+
usecrate::{LateContext,LateLintPass,LintContext};
9+
10+
declare_lint!{
11+
/// The `non_local_definitions` lint checks for `impl` blocks and `#[macro_export]`
12+
/// macro inside bodies (functions, enum discriminant, ...).
13+
///
14+
/// ### Example
15+
///
16+
/// ```rust
17+
/// trait MyTrait {}
18+
/// struct MyStruct;
19+
///
20+
/// fn foo() {
21+
/// impl MyTrait for MyStruct {}
22+
/// }
23+
/// ```
24+
///
25+
/// {{produces}}
26+
///
27+
/// ### Explanation
28+
///
29+
/// Creating non-local definitions go against expectation and can create discrepancies
30+
/// in tooling. It should be avoided. It may become deny-by-default in edition 2024
31+
/// and higher, see see the tracking issue <https://github.com/rust-lang/rust/issues/120363>.
32+
///
33+
/// An `impl` definition is non-local if it is nested inside an item and neither
34+
/// the type nor the trait are at the same nesting level as the `impl` block.
35+
///
36+
/// All nested bodies (functions, enum discriminant, array length, consts) (expect for
37+
/// `const _: Ty = { ... }` in top-level module, which is still undecided) are checked.
38+
pubNON_LOCAL_DEFINITIONS,
39+
Warn,
40+
"checks for non-local definitions",
41+
report_in_external_macro
42+
}
43+
44+
#[derive(Default)]
45+
pubstructNonLocalDefinitions{
46+
body_depth:u32,
47+
}
48+
49+
impl_lint_pass!(NonLocalDefinitions => [NON_LOCAL_DEFINITIONS]);
50+
51+
// FIXME(Urgau): Figure out how to handle modules nested in bodies.
52+
// It's currently not handled by the current logic because modules are not bodies.
53+
// They don't even follow the correct order (check_body -> check_mod -> check_body_post)
54+
// instead check_mod is called after every body has been handled.
55+
56+
impl<'tcx>LateLintPass<'tcx>forNonLocalDefinitions{
57+
fncheck_body(&mutself,_cx:&LateContext<'tcx>,_body:&'tcxBody<'tcx>){
58+
self.body_depth += 1;
59+
}
60+
61+
fncheck_body_post(&mutself,_cx:&LateContext<'tcx>,_body:&'tcxBody<'tcx>){
62+
self.body_depth -= 1;
63+
}
64+
65+
fncheck_item(&mutself,cx:&LateContext<'tcx>,item:&'tcxItem<'tcx>){
66+
ifself.body_depth == 0{
67+
return;
68+
}
69+
70+
let parent = cx.tcx.parent(item.owner_id.def_id.into());
71+
let parent_def_kind = cx.tcx.def_kind(parent);
72+
let parent_opt_item_name = cx.tcx.opt_item_name(parent);
73+
74+
// Per RFC we (currently) ignore anon-const (`const _: Ty = ...`) in top-level module.
75+
ifself.body_depth == 1
76+
&& parent_def_kind == DefKind::Const
77+
&& parent_opt_item_name == Some(kw::Underscore)
78+
{
79+
return;
80+
}
81+
82+
let cargo_update = || {
83+
let oexpn = item.span.ctxt().outer_expn_data();
84+
ifletSome(def_id) = oexpn.macro_def_id
85+
&& letExpnKind::Macro(macro_kind, macro_name) = oexpn.kind
86+
&& def_id.krate != LOCAL_CRATE
87+
&& std::env::var_os("CARGO").is_some()
88+
{
89+
Some(NonLocalDefinitionsCargoUpdateNote{
90+
macro_kind: macro_kind.descr(),
91+
macro_name,
92+
crate_name: cx.tcx.crate_name(def_id.krate),
93+
})
94+
}else{
95+
None
96+
}
97+
};
98+
99+
match item.kind{
100+
ItemKind::Impl(impl_) => {
101+
// The RFC states:
102+
//
103+
// > An item nested inside an expression-containing item (through any
104+
// > level of nesting) may not define an impl Trait for Type unless
105+
// > either the **Trait** or the **Type** is also nested inside the
106+
// > same expression-containing item.
107+
//
108+
// To achieve this we get try to get the paths of the _Trait_ and
109+
// _Type_, and we look inside thoses paths to try a find in one
110+
// of them a type whose parent is the same as the impl definition.
111+
//
112+
// If that's the case this means that this impl block declaration
113+
// is using local items and so we don't lint on it.
114+
115+
// We also ignore anon-const in item by including the anon-const
116+
// parent as well; and since it's quite uncommon, we use smallvec
117+
// to avoid unnecessary heap allocations.
118+
let local_parents:SmallVec<[DefId;1]> = if parent_def_kind == DefKind::Const
119+
&& parent_opt_item_name == Some(kw::Underscore)
120+
{
121+
smallvec![parent, cx.tcx.parent(parent)]
122+
}else{
123+
smallvec![parent]
124+
};
125+
126+
let self_ty_has_local_parent = match impl_.self_ty.kind{
127+
TyKind::Path(QPath::Resolved(_, ty_path)) => {
128+
path_has_local_parent(ty_path, cx,&*local_parents)
129+
}
130+
TyKind::TraitObject([principle_poly_trait_ref, ..], _, _) => {
131+
path_has_local_parent(
132+
principle_poly_trait_ref.trait_ref.path,
133+
cx,
134+
&*local_parents,
135+
)
136+
}
137+
TyKind::TraitObject([], _, _)
138+
| TyKind::InferDelegation(_, _)
139+
| TyKind::Slice(_)
140+
| TyKind::Array(_, _)
141+
| TyKind::Ptr(_)
142+
| TyKind::Ref(_, _)
143+
| TyKind::BareFn(_)
144+
| TyKind::Never
145+
| TyKind::Tup(_)
146+
| TyKind::Path(_)
147+
| TyKind::AnonAdt(_)
148+
| TyKind::OpaqueDef(_, _, _)
149+
| TyKind::Typeof(_)
150+
| TyKind::Infer
151+
| TyKind::Err(_) => false,
152+
};
153+
154+
let of_trait_has_local_parent = impl_
155+
.of_trait
156+
.map(|of_trait| path_has_local_parent(of_trait.path, cx,&*local_parents))
157+
.unwrap_or(false);
158+
159+
// If none of them have a local parent (LOGICAL NOR) this means that
160+
// this impl definition is a non-local definition and so we lint on it.
161+
if !(self_ty_has_local_parent || of_trait_has_local_parent){
162+
let const_anon = ifself.body_depth == 1
163+
&& parent_def_kind == DefKind::Const
164+
&& parent_opt_item_name != Some(kw::Underscore)
165+
&& letSome(parent) = parent.as_local()
166+
&& letNode::Item(item) = cx.tcx.hir_node_by_def_id(parent)
167+
&& letItemKind::Const(ty, _, _) = item.kind
168+
&& letTyKind::Tup(&[]) = ty.kind
169+
{
170+
Some(item.ident.span)
171+
}else{
172+
None
173+
};
174+
175+
cx.emit_span_lint(
176+
NON_LOCAL_DEFINITIONS,
177+
item.span,
178+
NonLocalDefinitionsDiag::Impl{
179+
depth:self.body_depth,
180+
body_kind_descr: cx.tcx.def_kind_descr(parent_def_kind, parent),
181+
body_name: parent_opt_item_name
182+
.map(|s| s.to_ident_string())
183+
.unwrap_or_else(|| "<unnameable>".to_string()),
184+
cargo_update:cargo_update(),
185+
const_anon,
186+
},
187+
)
188+
}
189+
}
190+
ItemKind::Macro(_macro,MacroKind::Bang)
191+
if cx.tcx.has_attr(item.owner_id.def_id, sym::macro_export) =>
192+
{
193+
cx.emit_span_lint(
194+
NON_LOCAL_DEFINITIONS,
195+
item.span,
196+
NonLocalDefinitionsDiag::MacroRules{
197+
depth:self.body_depth,
198+
body_kind_descr: cx.tcx.def_kind_descr(parent_def_kind, parent),
199+
body_name: parent_opt_item_name
200+
.map(|s| s.to_ident_string())
201+
.unwrap_or_else(|| "<unnameable>".to_string()),
202+
cargo_update:cargo_update(),
203+
},
204+
)
205+
}
206+
_ => {}
207+
}
208+
}
209+
}
210+
211+
/// Given a path and a parent impl def id, this checks if the if parent resolution
212+
/// def id correspond to the def id of the parent impl definition.
213+
///
214+
/// Given this path, we will look at the path (and ignore any generic args):
215+
///
216+
/// ```text
217+
/// std::convert::PartialEq<Foo<Bar>>
218+
/// ^^^^^^^^^^^^^^^^^^^^^^^
219+
/// ```
220+
fnpath_has_local_parent(path:&Path<'_>,cx:&LateContext<'_>,local_parents:&[DefId]) -> bool{
221+
path.res.opt_def_id().is_some_and(|did| local_parents.contains(&cx.tcx.parent(did)))
222+
}

‎compiler/rustc_macros/src/diagnostics/mod.rs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,8 @@ use synstructure::Structure;
5555
///
5656
/// See rustc dev guide for more examples on using the `#[derive(Diagnostic)]`:
5757
/// <https://rustc-dev-guide.rust-lang.org/diagnostics/diagnostic-structs.html>
58-
pubfnsession_diagnostic_derive(s:Structure<'_>) -> TokenStream{
58+
pubfnsession_diagnostic_derive(muts:Structure<'_>) -> TokenStream{
59+
s.underscore_const(true);
5960
DiagnosticDerive::new(s).into_tokens()
6061
}
6162

@@ -101,7 +102,8 @@ pub fn session_diagnostic_derive(s: Structure<'_>) -> TokenStream {
101102
///
102103
/// See rustc dev guide for more examples on using the `#[derive(LintDiagnostic)]`:
103104
/// <https://rustc-dev-guide.rust-lang.org/diagnostics/diagnostic-structs.html#reference>
104-
pubfnlint_diagnostic_derive(s:Structure<'_>) -> TokenStream{
105+
pubfnlint_diagnostic_derive(muts:Structure<'_>) -> TokenStream{
106+
s.underscore_const(true);
105107
LintDiagnosticDerive::new(s).into_tokens()
106108
}
107109

@@ -151,6 +153,7 @@ pub fn lint_diagnostic_derive(s: Structure<'_>) -> TokenStream {
151153
///
152154
/// diag.subdiagnostic(RawIdentifierSuggestion { span, applicability, ident });
153155
/// ```
154-
pubfnsession_subdiagnostic_derive(s:Structure<'_>) -> TokenStream{
156+
pubfnsession_subdiagnostic_derive(muts:Structure<'_>) -> TokenStream{
157+
s.underscore_const(true);
155158
SubdiagnosticDeriveBuilder::new().into_tokens(s)
156159
}

0 commit comments

Comments
 (0)