Skip to content

Commit 12d1b17

Browse files
GuillaumeGomezGuillaumeGomez
authored
Rollup merge of rust-lang#144977 - fmease:fortify-param-default-checks, r=compiler-errors
Fortify generic param default checks * Hard-reject instead of lint-reject type param defaults in generic assoc consts (GACs) (feature: `generic_const_items`). * In rust-lang#113522, I explicitly handled the free const item case and forgot about the assoc const one. * This led rustc to assume the default of emitting the deny-by-default lint `invalid_type_param_default`. * GCIs are unstable, thus we're not bound by backward compat * Hard-reject instead of lint-reject type param defaults in foreign items. * We already hard-reject generic params on foreign items, so this isn't a breaking change. * There's no reason why we need to lint-reject. * Refactor the way we determine where generic param defaults are allowed: * Don't default to emitting lint `invalid_type_param_defaults` for nodes that aren't explicitly handled but instead panic. * This would've caught my GAC oversight from above much earlier via fuzzing * Prevents us from accidentally stabilizing more invalid type param defaults in the future * Streamline the phrasing of the diagnostic
2 parents 094b893 + 02ea38c commit 12d1b17

30 files changed

+297
-285
lines changed

‎compiler/rustc_hir_analysis/src/collect/generics_of.rs‎

Lines changed: 101 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -223,60 +223,27 @@ pub(super) fn generics_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Generics {
223223
"synthetic HIR should have its `generics_of` explicitly fed"
224224
),
225225

226-
_ => span_bug!(tcx.def_span(def_id), "unhandled node {node:?}"),
226+
_ => span_bug!(tcx.def_span(def_id), "generics_of: unexpected node kind {node:?}"),
227227
};
228228

229-
enum Defaults {
230-
Allowed,
231-
// See #36887
232-
FutureCompatDisallowed,
233-
Deny,
234-
}
235-
236-
let hir_generics = node.generics().unwrap_or(hir::Generics::empty());
237-
let (opt_self, allow_defaults) = match node {
238-
Node::Item(item) => {
239-
match item.kind {
240-
ItemKind::Trait(..) | ItemKind::TraitAlias(..) => {
241-
// Add in the self type parameter.
242-
//
243-
// Something of a hack: use the node id for the trait, also as
244-
// the node id for the Self type parameter.
245-
let opt_self = Some(ty::GenericParamDef {
246-
index: 0,
247-
name: kw::SelfUpper,
248-
def_id: def_id.to_def_id(),
249-
pure_wrt_drop: false,
250-
kind: ty::GenericParamDefKind::Type {
251-
has_default: false,
252-
synthetic: false,
253-
},
254-
});
255-
256-
(opt_self, Defaults::Allowed)
257-
}
258-
ItemKind::TyAlias(..)
259-
| ItemKind::Enum(..)
260-
| ItemKind::Struct(..)
261-
| ItemKind::Union(..) => (None, Defaults::Allowed),
262-
ItemKind::Const(..) => (None, Defaults::Deny),
263-
_ => (None, Defaults::FutureCompatDisallowed),
264-
}
265-
}
266-
267-
Node::OpaqueTy(..) => (None, Defaults::Allowed),
268-
269-
// GATs
270-
Node::TraitItem(item) if matches!(item.kind, TraitItemKind::Type(..)) => {
271-
(None, Defaults::Deny)
272-
}
273-
Node::ImplItem(item) if matches!(item.kind, ImplItemKind::Type(..)) => {
274-
(None, Defaults::Deny)
275-
}
276-
277-
_ => (None, Defaults::FutureCompatDisallowed),
229+
// Add in the self type parameter.
230+
let opt_self = if let Node::Item(item) = node
231+
&& let ItemKind::Trait(..) | ItemKind::TraitAlias(..) = item.kind
232+
{
233+
// Something of a hack: We reuse the node ID of the trait for the self type parameter.
234+
Some(ty::GenericParamDef {
235+
index: 0,
236+
name: kw::SelfUpper,
237+
def_id: def_id.to_def_id(),
238+
pure_wrt_drop: false,
239+
kind: ty::GenericParamDefKind::Type { has_default: false, synthetic: false },
240+
})
241+
} else {
242+
None
278243
};
279244

245+
let param_default_policy = param_default_policy(node);
246+
let hir_generics = node.generics().unwrap_or(hir::Generics::empty());
280247
let has_self = opt_self.is_some();
281248
let mut parent_has_self = false;
282249
let mut own_start = has_self as u32;
@@ -312,60 +279,53 @@ pub(super) fn generics_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Generics {
312279
prev + type_start
313280
};
314281

315-
const TYPE_DEFAULT_NOT_ALLOWED: &'static str = "defaults for type parameters are only allowed in \
316-
`struct`, `enum`, `type`, or `trait` definitions";
317-
318-
own_params.extend(hir_generics.params.iter().filter_map(|param| match param.kind {
319-
GenericParamKind::Lifetime { .. } => None,
320-
GenericParamKind::Type { default, synthetic, .. } => {
321-
if default.is_some() {
322-
match allow_defaults {
323-
Defaults::Allowed => {}
324-
Defaults::FutureCompatDisallowed => {
325-
tcx.node_span_lint(
326-
lint::builtin::INVALID_TYPE_PARAM_DEFAULT,
327-
param.hir_id,
328-
param.span,
329-
|lint| {
330-
lint.primary_message(TYPE_DEFAULT_NOT_ALLOWED);
331-
},
332-
);
333-
}
334-
Defaults::Deny => {
335-
tcx.dcx().span_err(param.span, TYPE_DEFAULT_NOT_ALLOWED);
282+
own_params.extend(hir_generics.params.iter().filter_map(|param| {
283+
const MESSAGE: &str = "defaults for generic parameters are not allowed here";
284+
let kind = match param.kind {
285+
GenericParamKind::Lifetime { .. } => return None,
286+
GenericParamKind::Type { default, synthetic } => {
287+
if default.is_some() {
288+
match param_default_policy.expect("no policy for generic param default") {
289+
ParamDefaultPolicy::Allowed => {}
290+
ParamDefaultPolicy::FutureCompatForbidden => {
291+
tcx.node_span_lint(
292+
lint::builtin::INVALID_TYPE_PARAM_DEFAULT,
293+
param.hir_id,
294+
param.span,
295+
|lint| {
296+
lint.primary_message(MESSAGE);
297+
},
298+
);
299+
}
300+
ParamDefaultPolicy::Forbidden => {
301+
tcx.dcx().span_err(param.span, MESSAGE);
302+
}
336303
}
337304
}
338-
}
339-
340-
let kind = ty::GenericParamDefKind::Type { has_default: default.is_some(), synthetic };
341305

342-
Some(ty::GenericParamDef {
343-
index: next_index(),
344-
name: param.name.ident().name,
345-
def_id: param.def_id.to_def_id(),
346-
pure_wrt_drop: param.pure_wrt_drop,
347-
kind,
348-
})
349-
}
350-
GenericParamKind::Const { ty: _, default, synthetic } => {
351-
if !matches!(allow_defaults, Defaults::Allowed) && default.is_some() {
352-
tcx.dcx().span_err(
353-
param.span,
354-
"defaults for const parameters are only allowed in \
355-
`struct`, `enum`, `type`, or `trait` definitions",
356-
);
306+
ty::GenericParamDefKind::Type { has_default: default.is_some(), synthetic }
357307
}
308+
GenericParamKind::Const { ty: _, default, synthetic } => {
309+
if default.is_some() {
310+
match param_default_policy.expect("no policy for generic param default") {
311+
ParamDefaultPolicy::Allowed => {}
312+
ParamDefaultPolicy::FutureCompatForbidden
313+
| ParamDefaultPolicy::Forbidden => {
314+
tcx.dcx().span_err(param.span, MESSAGE);
315+
}
316+
}
317+
}
358318

359-
let index = next_index();
360-
361-
Some(ty::GenericParamDef {
362-
index,
363-
name: param.name.ident().name,
364-
def_id: param.def_id.to_def_id(),
365-
pure_wrt_drop: param.pure_wrt_drop,
366-
kind: ty::GenericParamDefKind::Const { has_default: default.is_some(), synthetic },
367-
})
368-
}
319+
ty::GenericParamDefKind::Const { has_default: default.is_some(), synthetic }
320+
}
321+
};
322+
Some(ty::GenericParamDef {
323+
index: next_index(),
324+
name: param.name.ident().name,
325+
def_id: param.def_id.to_def_id(),
326+
pure_wrt_drop: param.pure_wrt_drop,
327+
kind,
328+
})
369329
}));
370330

371331
// provide junk type parameter defs - the only place that
@@ -438,6 +398,48 @@ pub(super) fn generics_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Generics {
438398
}
439399
}
440400

401+
#[derive(Clone, Copy)]
402+
enum ParamDefaultPolicy {
403+
Allowed,
404+
/// Tracked in <https://github.com/rust-lang/rust/issues/36887>.
405+
FutureCompatForbidden,
406+
Forbidden,
407+
}
408+
409+
fn param_default_policy(node: Node<'_>) -> Option<ParamDefaultPolicy> {
410+
use rustc_hir::*;
411+
412+
Some(match node {
413+
Node::Item(item) => match item.kind {
414+
ItemKind::Trait(..)
415+
| ItemKind::TraitAlias(..)
416+
| ItemKind::TyAlias(..)
417+
| ItemKind::Enum(..)
418+
| ItemKind::Struct(..)
419+
| ItemKind::Union(..) => ParamDefaultPolicy::Allowed,
420+
ItemKind::Fn { .. } | ItemKind::Impl(_) => ParamDefaultPolicy::FutureCompatForbidden,
421+
// Re. GCI, we're not bound by backward compatibility.
422+
ItemKind::Const(..) => ParamDefaultPolicy::Forbidden,
423+
_ => return None,
424+
},
425+
Node::TraitItem(item) => match item.kind {
426+
// Re. GATs and GACs (generic_const_items), we're not bound by backward compatibility.
427+
TraitItemKind::Const(..) | TraitItemKind::Type(..) => ParamDefaultPolicy::Forbidden,
428+
TraitItemKind::Fn(..) => ParamDefaultPolicy::FutureCompatForbidden,
429+
},
430+
Node::ImplItem(item) => match item.kind {
431+
// Re. GATs and GACs (generic_const_items), we're not bound by backward compatibility.
432+
ImplItemKind::Const(..) | ImplItemKind::Type(..) => ParamDefaultPolicy::Forbidden,
433+
ImplItemKind::Fn(..) => ParamDefaultPolicy::FutureCompatForbidden,
434+
},
435+
// Generic params are (semantically) invalid on foreign items. Still, for maximum forward
436+
// compatibility, let's hard-reject defaults on them.
437+
Node::ForeignItem(_) => ParamDefaultPolicy::Forbidden,
438+
Node::OpaqueTy(..) => ParamDefaultPolicy::Allowed,
439+
_ => return None,
440+
})
441+
}
442+
441443
fn has_late_bound_regions<'tcx>(tcx: TyCtxt<'tcx>, node: Node<'tcx>) -> Option<Span> {
442444
struct LateBoundRegionsDetector<'tcx> {
443445
tcx: TyCtxt<'tcx>,

‎tests/ui/const-generics/defaults/default-on-impl.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
struct Foo<const N: usize>;
22

33
impl<const N: usize = 1> Foo<N> {}
4-
//~^ ERROR defaults for const parameters are only allowed
4+
//~^ ERROR defaults for generic parameters are not allowed here
55

66
fn main() {}

‎tests/ui/const-generics/defaults/default-on-impl.stderr‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
error: defaults for const parameters are only allowed in `struct`, `enum`, `type`, or `trait` definitions
1+
error: defaults for generic parameters are not allowed here
22
--> $DIR/default-on-impl.rs:3:6
33
|
44
LL | impl<const N: usize = 1> Foo<N> {}

‎tests/ui/const-generics/generic_const_exprs/issue-105257.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
#![feature(generic_const_exprs)]
2-
#![allow(incomplete_features)]
2+
#![expect(incomplete_features)]
33

44
trait Trait<T> {
5-
fn fnc<const N: usize = "">(&self) {} //~ERROR defaults for const parameters are only allowed in `struct`, `enum`, `type`, or `trait` definitions
5+
fn fnc<const N: usize = "">(&self) {} //~ERROR defaults for generic parameters are not allowed here
66
//~^ ERROR: mismatched types
7-
fn foo<const N: usize = { std::mem::size_of::<T>() }>(&self) {} //~ERROR defaults for const parameters are only allowed in `struct`, `enum`, `type`, or `trait` definitions
7+
fn foo<const N: usize = { std::mem::size_of::<T>() }>(&self) {} //~ERROR defaults for generic parameters are not allowed here
88
}
99

1010
fn main() {}

‎tests/ui/const-generics/generic_const_exprs/issue-105257.stderr‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
error: defaults for const parameters are only allowed in `struct`, `enum`, `type`, or `trait` definitions
1+
error: defaults for generic parameters are not allowed here
22
--> $DIR/issue-105257.rs:5:12
33
|
44
LL | fn fnc<const N: usize = "">(&self) {}
55
| ^^^^^^^^^^^^^^^^^^^
66

7-
error: defaults for const parameters are only allowed in `struct`, `enum`, `type`, or `trait` definitions
7+
error: defaults for generic parameters are not allowed here
88
--> $DIR/issue-105257.rs:7:12
99
|
1010
LL | fn foo<const N: usize = { std::mem::size_of::<T>() }>(&self) {}

‎tests/ui/const-generics/min_const_generics/default_function_param.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
#![crate_type = "lib"]
22

33
fn foo<const SIZE: usize = 5usize>() {}
4-
//~^ ERROR defaults for const parameters are
4+
//~^ ERROR defaults for generic parameters are not allowed here

‎tests/ui/const-generics/min_const_generics/default_function_param.stderr‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
error: defaults for const parameters are only allowed in `struct`, `enum`, `type`, or `trait` definitions
1+
error: defaults for generic parameters are not allowed here
22
--> $DIR/default_function_param.rs:3:8
33
|
44
LL | fn foo<const SIZE: usize = 5usize>() {}

‎tests/ui/generic-associated-types/type-param-defaults.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,17 @@
44

55
trait Trait {
66
type Assoc<T = u32>;
7-
//~^ ERROR defaults for type parameters are only allowed
7+
//~^ ERROR defaults for generic parameters are not allowed here
88
}
99

1010
impl Trait for () {
1111
type Assoc<T = u32> = u64;
12-
//~^ ERROR defaults for type parameters are only allowed
12+
//~^ ERROR defaults for generic parameters are not allowed here
1313
}
1414

1515
impl Trait for u32 {
1616
type Assoc<T = u32> = T;
17-
//~^ ERROR defaults for type parameters are only allowed
17+
//~^ ERROR defaults for generic parameters are not allowed here
1818
}
1919

2020
trait Other {}

‎tests/ui/generic-associated-types/type-param-defaults.stderr‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,16 @@
1-
error: defaults for type parameters are only allowed in `struct`, `enum`, `type`, or `trait` definitions
1+
error: defaults for generic parameters are not allowed here
22
--> $DIR/type-param-defaults.rs:6:16
33
|
44
LL | type Assoc<T = u32>;
55
| ^^^^^^^
66

7-
error: defaults for type parameters are only allowed in `struct`, `enum`, `type`, or `trait` definitions
7+
error: defaults for generic parameters are not allowed here
88
--> $DIR/type-param-defaults.rs:11:16
99
|
1010
LL | type Assoc<T = u32> = u64;
1111
| ^^^^^^^
1212

13-
error: defaults for type parameters are only allowed in `struct`, `enum`, `type`, or `trait` definitions
13+
error: defaults for generic parameters are not allowed here
1414
--> $DIR/type-param-defaults.rs:16:16
1515
|
1616
LL | type Assoc<T = u32> = T;

‎tests/ui/generic-const-items/parameter-defaults.rs‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,17 @@
77

88
// FIXME(default_type_parameter_fallback): Consider reallowing them once they work properly.
99

10-
const NONE<T = ()>: Option<T> = None::<T>; //~ ERROR defaults for type parameters are only allowed
10+
const NONE<T = ()>: Option<T> = None::<T>;
11+
//~^ ERROR defaults for generic parameters are not allowed here
1112

12-
fn main() {
13-
let _ = NONE;
14-
//~^ ERROR type annotations needed
13+
impl Host {
14+
const NADA<T = ()>: Option<T> = None::<T>;
15+
//~^ ERROR defaults for generic parameters are not allowed here
1516
}
17+
18+
enum Host {}
19+
20+
fn body0() { let _ = NONE; } //~ ERROR type annotations needed
21+
fn body1() { let _ = Host::NADA; } //~ ERROR type annotations needed
22+
23+
fn main() {}

0 commit comments

Comments
 (0)