-
Notifications
You must be signed in to change notification settings - Fork 13.7k
Fortify generic param default checks #144977
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -223,60 +223,27 @@ pub(super) fn generics_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Generics { | |
"synthetic HIR should have its `generics_of` explicitly fed" | ||
), | ||
|
||
_ => span_bug!(tcx.def_span(def_id), "unhandled node {node:?}"), | ||
_ => span_bug!(tcx.def_span(def_id), "generics_of: unexpected node kind {node:?}"), | ||
}; | ||
|
||
enum Defaults { | ||
Allowed, | ||
// See #36887 | ||
FutureCompatDisallowed, | ||
Deny, | ||
} | ||
|
||
let hir_generics = node.generics().unwrap_or(hir::Generics::empty()); | ||
let (opt_self, allow_defaults) = match node { | ||
Node::Item(item) => { | ||
match item.kind { | ||
ItemKind::Trait(..) | ItemKind::TraitAlias(..) => { | ||
// Add in the self type parameter. | ||
// | ||
// Something of a hack: use the node id for the trait, also as | ||
// the node id for the Self type parameter. | ||
let opt_self = Some(ty::GenericParamDef { | ||
index: 0, | ||
name: kw::SelfUpper, | ||
def_id: def_id.to_def_id(), | ||
pure_wrt_drop: false, | ||
kind: ty::GenericParamDefKind::Type { | ||
has_default: false, | ||
synthetic: false, | ||
}, | ||
}); | ||
|
||
(opt_self, Defaults::Allowed) | ||
} | ||
ItemKind::TyAlias(..) | ||
| ItemKind::Enum(..) | ||
| ItemKind::Struct(..) | ||
| ItemKind::Union(..) => (None, Defaults::Allowed), | ||
ItemKind::Const(..) => (None, Defaults::Deny), | ||
_ => (None, Defaults::FutureCompatDisallowed), | ||
} | ||
} | ||
|
||
Node::OpaqueTy(..) => (None, Defaults::Allowed), | ||
|
||
// GATs | ||
Node::TraitItem(item) if matches!(item.kind, TraitItemKind::Type(..)) => { | ||
(None, Defaults::Deny) | ||
} | ||
Node::ImplItem(item) if matches!(item.kind, ImplItemKind::Type(..)) => { | ||
(None, Defaults::Deny) | ||
} | ||
|
||
_ => (None, Defaults::FutureCompatDisallowed), | ||
// Add in the self type parameter. | ||
let opt_self = if let Node::Item(item) = node | ||
&& let ItemKind::Trait(..) | ItemKind::TraitAlias(..) = item.kind | ||
{ | ||
// Something of a hack: We reuse the node ID of the trait for the self type parameter. | ||
Some(ty::GenericParamDef { | ||
index: 0, | ||
name: kw::SelfUpper, | ||
def_id: def_id.to_def_id(), | ||
pure_wrt_drop: false, | ||
kind: ty::GenericParamDefKind::Type { has_default: false, synthetic: false }, | ||
}) | ||
} else { | ||
None | ||
}; | ||
|
||
let param_default_policy = param_default_policy(node); | ||
let hir_generics = node.generics().unwrap_or(hir::Generics::empty()); | ||
let has_self = opt_self.is_some(); | ||
let mut parent_has_self = false; | ||
let mut own_start = has_self as u32; | ||
|
@@ -312,60 +279,53 @@ pub(super) fn generics_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Generics { | |
prev + type_start | ||
}; | ||
|
||
const TYPE_DEFAULT_NOT_ALLOWED: &'static str = "defaults for type parameters are only allowed in \ | ||
`struct`, `enum`, `type`, or `trait` definitions"; | ||
|
||
own_params.extend(hir_generics.params.iter().filter_map(|param| match param.kind { | ||
GenericParamKind::Lifetime { .. } => None, | ||
GenericParamKind::Type { default, synthetic, .. } => { | ||
if default.is_some() { | ||
match allow_defaults { | ||
Defaults::Allowed => {} | ||
Defaults::FutureCompatDisallowed => { | ||
tcx.node_span_lint( | ||
lint::builtin::INVALID_TYPE_PARAM_DEFAULT, | ||
param.hir_id, | ||
param.span, | ||
|lint| { | ||
lint.primary_message(TYPE_DEFAULT_NOT_ALLOWED); | ||
}, | ||
); | ||
} | ||
Defaults::Deny => { | ||
tcx.dcx().span_err(param.span, TYPE_DEFAULT_NOT_ALLOWED); | ||
own_params.extend(hir_generics.params.iter().filter_map(|param| { | ||
const MESSAGE: &str = "defaults for generic parameters are not allowed here"; | ||
let kind = match param.kind { | ||
GenericParamKind::Lifetime { .. } => return None, | ||
GenericParamKind::Type { default, synthetic } => { | ||
if default.is_some() { | ||
match param_default_policy.expect("no policy for generic param default") { | ||
ParamDefaultPolicy::Allowed => {} | ||
ParamDefaultPolicy::FutureCompatForbidden => { | ||
tcx.node_span_lint( | ||
lint::builtin::INVALID_TYPE_PARAM_DEFAULT, | ||
param.hir_id, | ||
param.span, | ||
|lint| { | ||
lint.primary_message(MESSAGE); | ||
}, | ||
); | ||
} | ||
ParamDefaultPolicy::Forbidden => { | ||
tcx.dcx().span_err(param.span, MESSAGE); | ||
} | ||
} | ||
} | ||
} | ||
|
||
let kind = ty::GenericParamDefKind::Type { has_default: default.is_some(), synthetic }; | ||
|
||
Some(ty::GenericParamDef { | ||
index: next_index(), | ||
name: param.name.ident().name, | ||
def_id: param.def_id.to_def_id(), | ||
pure_wrt_drop: param.pure_wrt_drop, | ||
kind, | ||
}) | ||
} | ||
GenericParamKind::Const { ty: _, default, synthetic } => { | ||
if !matches!(allow_defaults, Defaults::Allowed) && default.is_some() { | ||
tcx.dcx().span_err( | ||
param.span, | ||
"defaults for const parameters are only allowed in \ | ||
`struct`, `enum`, `type`, or `trait` definitions", | ||
); | ||
ty::GenericParamDefKind::Type { has_default: default.is_some(), synthetic } | ||
} | ||
GenericParamKind::Const { ty: _, default, synthetic } => { | ||
if default.is_some() { | ||
match param_default_policy.expect("no policy for generic param default") { | ||
ParamDefaultPolicy::Allowed => {} | ||
ParamDefaultPolicy::FutureCompatForbidden | ||
| ParamDefaultPolicy::Forbidden => { | ||
tcx.dcx().span_err(param.span, MESSAGE); | ||
} | ||
} | ||
} | ||
|
||
let index = next_index(); | ||
|
||
Some(ty::GenericParamDef { | ||
index, | ||
name: param.name.ident().name, | ||
def_id: param.def_id.to_def_id(), | ||
pure_wrt_drop: param.pure_wrt_drop, | ||
kind: ty::GenericParamDefKind::Const { has_default: default.is_some(), synthetic }, | ||
}) | ||
} | ||
ty::GenericParamDefKind::Const { has_default: default.is_some(), synthetic } | ||
} | ||
}; | ||
Some(ty::GenericParamDef { | ||
index: next_index(), | ||
name: param.name.ident().name, | ||
def_id: param.def_id.to_def_id(), | ||
pure_wrt_drop: param.pure_wrt_drop, | ||
kind, | ||
}) | ||
})); | ||
|
||
// 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 { | |
} | ||
} | ||
|
||
#[derive(Clone, Copy)] | ||
enum ParamDefaultPolicy { | ||
Allowed, | ||
/// Tracked in <https://github.com/rust-lang/rust/issues/36887>. | ||
FutureCompatForbidden, | ||
Forbidden, | ||
} | ||
|
||
fn param_default_policy(node: Node<'_>) -> Option<ParamDefaultPolicy> { | ||
use rustc_hir::*; | ||
|
||
Some(match node { | ||
Node::Item(item) => match item.kind { | ||
ItemKind::Trait(..) | ||
| ItemKind::TraitAlias(..) | ||
| ItemKind::TyAlias(..) | ||
| ItemKind::Enum(..) | ||
| ItemKind::Struct(..) | ||
| ItemKind::Union(..) => ParamDefaultPolicy::Allowed, | ||
ItemKind::Fn { .. } | ItemKind::Impl(_) => ParamDefaultPolicy::FutureCompatForbidden, | ||
// Re. GCI, we're not bound by backward compatibility. | ||
ItemKind::Const(..) => ParamDefaultPolicy::Forbidden, | ||
_ => return None, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I could exhaustively match on |
||
}, | ||
Node::TraitItem(item) => match item.kind { | ||
// Re. GATs and GACs (generic_const_items), we're not bound by backward compatibility. | ||
TraitItemKind::Const(..) | TraitItemKind::Type(..) => ParamDefaultPolicy::Forbidden, | ||
TraitItemKind::Fn(..) => ParamDefaultPolicy::FutureCompatForbidden, | ||
}, | ||
Node::ImplItem(item) => match item.kind { | ||
// Re. GATs and GACs (generic_const_items), we're not bound by backward compatibility. | ||
ImplItemKind::Const(..) | ImplItemKind::Type(..) => ParamDefaultPolicy::Forbidden, | ||
ImplItemKind::Fn(..) => ParamDefaultPolicy::FutureCompatForbidden, | ||
}, | ||
// Generic params are (semantically) invalid on foreign items. Still, for maximum forward | ||
// compatibility, let's hard-reject defaults on them. | ||
Node::ForeignItem(_) => ParamDefaultPolicy::Forbidden, | ||
Node::OpaqueTy(..) => ParamDefaultPolicy::Allowed, | ||
_ => return None, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Exhaustively matching on |
||
}) | ||
} | ||
|
||
fn has_late_bound_regions<'tcx>(tcx: TyCtxt<'tcx>, node: Node<'tcx>) -> Option<Span> { | ||
struct LateBoundRegionsDetector<'tcx> { | ||
tcx: TyCtxt<'tcx>, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
struct Foo<const N: usize>; | ||
|
||
impl<const N: usize = 1> Foo<N> {} | ||
//~^ ERROR defaults for const parameters are only allowed | ||
//~^ ERROR defaults for generic parameters are not allowed here | ||
|
||
fn main() {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,10 @@ | ||
#![feature(generic_const_exprs)] | ||
#![allow(incomplete_features)] | ||
#![expect(incomplete_features)] | ||
|
||
trait Trait<T> { | ||
fn fnc<const N: usize = "">(&self) {} //~ERROR defaults for const parameters are only allowed in `struct`, `enum`, `type`, or `trait` definitions | ||
fn fnc<const N: usize = "">(&self) {} //~ERROR defaults for generic parameters are not allowed here | ||
//~^ ERROR: mismatched types | ||
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 | ||
fn foo<const N: usize = { std::mem::size_of::<T>() }>(&self) {} //~ERROR defaults for generic parameters are not allowed here | ||
} | ||
|
||
fn main() {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
#![crate_type = "lib"] | ||
|
||
fn foo<const SIZE: usize = 5usize>() {} | ||
//~^ ERROR defaults for const parameters are | ||
//~^ ERROR defaults for generic parameters are not allowed here |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,20 +1,37 @@ | ||
error: defaults for type parameters are only allowed in `struct`, `enum`, `type`, or `trait` definitions | ||
error: defaults for generic parameters are not allowed here | ||
--> $DIR/parameter-defaults.rs:10:12 | ||
| | ||
LL | const NONE<T = ()>: Option<T> = None::<T>; | ||
| ^^^^^^ | ||
|
||
error: defaults for generic parameters are not allowed here | ||
--> $DIR/parameter-defaults.rs:14:16 | ||
| | ||
LL | const NADA<T = ()>: Option<T> = None::<T>; | ||
| ^^^^^^ | ||
|
||
error[E0282]: type annotations needed for `Option<_>` | ||
--> $DIR/parameter-defaults.rs:20:18 | ||
| | ||
LL | fn body0() { let _ = NONE; } | ||
| ^ ---- type must be known at this point | ||
| | ||
help: consider giving this pattern a type, where the type for type parameter `T` is specified | ||
| | ||
LL | fn body0() { let _: Option<T> = NONE; } | ||
| +++++++++++ | ||
|
||
error[E0282]: type annotations needed for `Option<_>` | ||
--> $DIR/parameter-defaults.rs:13:9 | ||
--> $DIR/parameter-defaults.rs:21:18 | ||
| | ||
LL | let _ = NONE; | ||
| ^ ---- type must be known at this point | ||
LL | fn body1() { let _ = Host::NADA; } | ||
| ^ ---------- type must be known at this point | ||
| | ||
help: consider giving this pattern a type, where the type for type parameter `T` is specified | ||
| | ||
LL | let _: Option<T> = NONE; | ||
| +++++++++++ | ||
LL | fn body1() { let _: Option<T> = Host::NADA; } | ||
| +++++++++++ | ||
|
||
error: aborting due to 2 previous errors | ||
error: aborting due to 4 previous errors | ||
|
||
For more information about this error, try `rustc --explain E0282`. |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course, instead of panicking we could simply default to
Forbidden
(on master we default toFutureCompatForbidden
which I heavily dislike). Just LMK :)