Skip to content

Commit

Permalink
Auto merge of #127722 - BoxyUwU:new_adt_const_params_limitations, r=c…
Browse files Browse the repository at this point in the history
…ompiler-errors

Forbid borrows and unsized types from being used as the type of a const generic under `adt_const_params`

Fixes #112219
Fixes #112124
Fixes #112125

### Motivation

Currently the `adt_const_params` feature allows writing `Foo<const N: [u8]>` this is entirely useless as it is not possible to write an expression which evaluates to a type that is not `Sized`. In order to actually use unsized types in const generics they are typically written as `const N: &[u8]` which *is* possible to provide a value of.

Unfortunately allowing the types of const parameters to contain references is non trivial (#120961) as it introduces a number of difficult questions about how equality of references in the type system should behave. References in the types of const generics is largely only useful for using unsized types in const generics.

This PR introduces a new feature gate `unsized_const_parameters` and moves support for `const N: [u8]` and `const N: &...` from `adt_const_params` into it. The goal here hopefully is to experiment with allowing `const N: [u8]` to work without references and then eventually completely forbid references in const generics.

Splitting this out into a new feature gate means that stabilization of `adt_const_params` does not have to resolve #120961 which is the only remaining "big" blocker for the feature. Remaining issues after this are a few ICEs and naming bikeshed for `ConstParamTy`.

### Implementation

The implementation is slightly subtle here as we would like to ensure that a stabilization of `adt_const_params` is forwards compatible with any outcome of `unsized_const_parameters`. This is inherently tricky as we do not support unstable trait implementations and we determine whether a type is valid as the type of a const parameter via a trait bound.

There are a few constraints here:
- We would like to *allow for the possibility* of adding a `Sized` supertrait to `ConstParamTy` in the event that we wind up opting to not support unsized types and instead requiring people to write the 'sized version', e.g. `const N: [u8; M]` instead of `const N: [u8]`.
- Crates should be able to enable `unsized_const_parameters` and write trait implementations of `ConstParamTy` for `!Sized` types without downstream crates that only enable `adt_const_params` being able to observe this (required for std to be able to `impl<T> ConstParamTy for [T]`

Ultimately the way this is accomplished is via having two traits (sad), `ConstParamTy` and `UnsizedConstParamTy`. Depending on whether `unsized_const_parameters` is enabled or not we change which trait is used to check whether a type is allowed to be a const parameter.

Long term (when stabilizing `UnsizedConstParamTy`) it should be possible to completely merge these traits (and derive macros), only having a single `trait ConstParamTy` and `macro ConstParamTy`.

Under `adt_const_params` it is now illegal to directly refer to `ConstParamTy` it is only used as an internal impl detail by `derive(ConstParamTy)` and checking const parameters are well formed. This is necessary in order to ensure forwards compatibility with all possible future directions for `feature(unsized_const_parameters)`.

Generally the intuition here should be that `ConstParamTy` is the stable trait that everything uses, and `UnsizedConstParamTy` is that plus unstable implementations (well, I suppose `ConstParamTy` isn't stable yet :P).
  • Loading branch information
bors committed Jul 21, 2024
2 parents a62ac15 + d0c11bf commit 9629b90
Show file tree
Hide file tree
Showing 141 changed files with 1,242 additions and 543 deletions.
39 changes: 38 additions & 1 deletion compiler/rustc_builtin_macros/src/deriving/bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,44 @@ pub(crate) fn expand_deriving_const_param_ty(
) {
let trait_def = TraitDef {
span,
path: path_std!(marker::ConstParamTy),
path: path_std!(marker::ConstParamTy_),
skip_path_as_bound: false,
needs_copy_as_bound_if_packed: false,
additional_bounds: vec![ty::Ty::Path(path_std!(cmp::Eq))],
supports_unions: false,
methods: Vec::new(),
associated_types: Vec::new(),
is_const,
};

trait_def.expand(cx, mitem, item, push);

let trait_def = TraitDef {
span,
path: path_std!(marker::UnsizedConstParamTy),
skip_path_as_bound: false,
needs_copy_as_bound_if_packed: false,
additional_bounds: vec![ty::Ty::Path(path_std!(cmp::Eq))],
supports_unions: false,
methods: Vec::new(),
associated_types: Vec::new(),
is_const,
};

trait_def.expand(cx, mitem, item, push);
}

pub(crate) fn expand_deriving_unsized_const_param_ty(
cx: &ExtCtxt<'_>,
span: Span,
mitem: &MetaItem,
item: &Annotatable,
push: &mut dyn FnMut(Annotatable),
is_const: bool,
) {
let trait_def = TraitDef {
span,
path: path_std!(marker::UnsizedConstParamTy),
skip_path_as_bound: false,
needs_copy_as_bound_if_packed: false,
additional_bounds: vec![ty::Ty::Path(path_std!(cmp::Eq))],
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_builtin_macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ pub fn register_builtin_macros(resolver: &mut dyn ResolverExpand) {
Clone: clone::expand_deriving_clone,
Copy: bounds::expand_deriving_copy,
ConstParamTy: bounds::expand_deriving_const_param_ty,
UnsizedConstParamTy: bounds::expand_deriving_unsized_const_param_ty,
Debug: debug::expand_deriving_debug,
Default: default::expand_deriving_default,
Eq: eq::expand_deriving_eq,
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_error_codes/src/error_codes/E0771.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ allowed.
Erroneous code example:

```compile_fail,E0770
#![feature(adt_const_params)]
#![feature(adt_const_params, unsized_const_params)]
fn function_with_str<'a, const STRING: &'a str>() {} // error!
```
Expand All @@ -15,7 +15,7 @@ To fix this issue, the lifetime in the const generic need to be changed to
`'static`:

```
#![feature(adt_const_params)]
#![feature(adt_const_params, unsized_const_params)]
fn function_with_str<const STRING: &'static str>() {} // ok!
```
Expand Down
7 changes: 5 additions & 2 deletions compiler/rustc_feature/src/unstable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -339,8 +339,8 @@ declare_features! (
(unstable, abi_riscv_interrupt, "1.73.0", Some(111889)),
/// Allows `extern "x86-interrupt" fn()`.
(unstable, abi_x86_interrupt, "1.17.0", Some(40180)),
/// Allows additional const parameter types, such as `&'static str` or user defined types
(incomplete, adt_const_params, "1.56.0", Some(95174)),
/// Allows additional const parameter types, such as `[u8; 10]` or user defined types
(unstable, adt_const_params, "1.56.0", Some(95174)),
/// Allows defining an `#[alloc_error_handler]`.
(unstable, alloc_error_handler, "1.29.0", Some(51540)),
/// Allows trait methods with arbitrary self types.
Expand Down Expand Up @@ -630,6 +630,9 @@ declare_features! (
(unstable, unsafe_attributes, "1.80.0", Some(123757)),
/// Allows unsafe on extern declarations and safety qualifiers over internal items.
(unstable, unsafe_extern_blocks, "1.80.0", Some(123743)),
/// Allows const generic parameters to be defined with types that
/// are not `Sized`, e.g. `fn foo<const N: [u8]>() {`.
(incomplete, unsized_const_params, "CURRENT_RUSTC_VERSION", Some(95174)),
/// Allows unsized fn parameters.
(internal, unsized_fn_params, "1.49.0", Some(48055)),
/// Allows unsized rvalues at arguments and parameters.
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir/src/lang_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,7 @@ language_item_table! {
PointerLike, sym::pointer_like, pointer_like, Target::Trait, GenericRequirement::Exact(0);

ConstParamTy, sym::const_param_ty, const_param_ty_trait, Target::Trait, GenericRequirement::Exact(0);
UnsizedConstParamTy, sym::unsized_const_param_ty, unsized_const_param_ty_trait, Target::Trait, GenericRequirement::Exact(0);

Poll, sym::Poll, poll, Target::Enum, GenericRequirement::None;
PollReady, sym::Ready, poll_ready_variant, Target::Variant, GenericRequirement::None;
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_hir_analysis/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@ hir_analysis_const_param_ty_impl_on_non_adt =
the trait `ConstParamTy` may not be implemented for this type
.label = type is not a structure or enumeration
hir_analysis_const_param_ty_impl_on_unsized =
the trait `ConstParamTy` may not be implemented for this type
.label = type is not `Sized`
hir_analysis_const_specialize = cannot specialize on const impl with non-const impl
hir_analysis_copy_impl_on_non_adt =
Expand Down
55 changes: 39 additions & 16 deletions compiler/rustc_hir_analysis/src/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -922,10 +922,8 @@ fn check_param_wf(tcx: TyCtxt<'_>, param: &hir::GenericParam<'_>) -> Result<(),
} => {
let ty = tcx.type_of(param.def_id).instantiate_identity();

if tcx.features().adt_const_params {
if tcx.features().unsized_const_params {
enter_wf_checking_ctxt(tcx, hir_ty.span, param.def_id, |wfcx| {
let trait_def_id =
tcx.require_lang_item(LangItem::ConstParamTy, Some(hir_ty.span));
wfcx.register_bound(
ObligationCause::new(
hir_ty.span,
Expand All @@ -934,7 +932,21 @@ fn check_param_wf(tcx: TyCtxt<'_>, param: &hir::GenericParam<'_>) -> Result<(),
),
wfcx.param_env,
ty,
trait_def_id,
tcx.require_lang_item(LangItem::UnsizedConstParamTy, Some(hir_ty.span)),
);
Ok(())
})
} else if tcx.features().adt_const_params {
enter_wf_checking_ctxt(tcx, hir_ty.span, param.def_id, |wfcx| {
wfcx.register_bound(
ObligationCause::new(
hir_ty.span,
param.def_id,
ObligationCauseCode::ConstParam(ty),
),
wfcx.param_env,
ty,
tcx.require_lang_item(LangItem::ConstParamTy, Some(hir_ty.span)),
);
Ok(())
})
Expand All @@ -958,14 +970,29 @@ fn check_param_wf(tcx: TyCtxt<'_>, param: &hir::GenericParam<'_>) -> Result<(),
diag.note("the only supported types are integers, `bool` and `char`");

let cause = ObligationCause::misc(hir_ty.span, param.def_id);
let adt_const_params_feature_string =
" more complex and user defined types".to_string();
let may_suggest_feature = match type_allowed_to_implement_const_param_ty(
tcx,
tcx.param_env(param.def_id),
ty,
LangItem::ConstParamTy,
cause,
) {
// Can never implement `ConstParamTy`, don't suggest anything.
Err(ConstParamTyImplementationError::NotAnAdtOrBuiltinAllowed) => false,
Err(
ConstParamTyImplementationError::NotAnAdtOrBuiltinAllowed
| ConstParamTyImplementationError::InvalidInnerTyOfBuiltinTy(..),
) => None,
Err(ConstParamTyImplementationError::UnsizedConstParamsFeatureRequired) => {
Some(vec![
(adt_const_params_feature_string, sym::adt_const_params),
(
" references to implement the `ConstParamTy` trait".into(),
sym::unsized_const_params,
),
])
}
// May be able to implement `ConstParamTy`. Only emit the feature help
// if the type is local, since the user may be able to fix the local type.
Err(ConstParamTyImplementationError::InfrigingFields(..)) => {
Expand All @@ -985,20 +1012,16 @@ fn check_param_wf(tcx: TyCtxt<'_>, param: &hir::GenericParam<'_>) -> Result<(),
}
}

ty_is_local(ty)
ty_is_local(ty).then_some(vec![(
adt_const_params_feature_string,
sym::adt_const_params,
)])
}
// Implments `ConstParamTy`, suggest adding the feature to enable.
Ok(..) => true,
Ok(..) => Some(vec![(adt_const_params_feature_string, sym::adt_const_params)]),
};
if may_suggest_feature {
tcx.disabled_nightly_features(
&mut diag,
Some(param.hir_id),
[(
" more complex and user defined types".to_string(),
sym::adt_const_params,
)],
);
if let Some(features) = may_suggest_feature {
tcx.disabled_nightly_features(&mut diag, Some(param.hir_id), features);
}

Err(diag.emit())
Expand Down
61 changes: 48 additions & 13 deletions compiler/rustc_hir_analysis/src/coherence/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,13 @@ pub(super) fn check_trait<'tcx>(
let checker = Checker { tcx, trait_def_id, impl_def_id, impl_header };
let mut res = checker.check(lang_items.drop_trait(), visit_implementation_of_drop);
res = res.and(checker.check(lang_items.copy_trait(), visit_implementation_of_copy));
res = res.and(
checker.check(lang_items.const_param_ty_trait(), visit_implementation_of_const_param_ty),
);
res = res.and(checker.check(lang_items.const_param_ty_trait(), |checker| {
visit_implementation_of_const_param_ty(checker, LangItem::ConstParamTy)
}));
res = res.and(checker.check(lang_items.unsized_const_param_ty_trait(), |checker| {
visit_implementation_of_const_param_ty(checker, LangItem::UnsizedConstParamTy)
}));

res = res.and(
checker.check(lang_items.coerce_unsized_trait(), visit_implementation_of_coerce_unsized),
);
Expand Down Expand Up @@ -103,7 +107,13 @@ fn visit_implementation_of_copy(checker: &Checker<'_>) -> Result<(), ErrorGuaran
Ok(()) => Ok(()),
Err(CopyImplementationError::InfringingFields(fields)) => {
let span = tcx.hir().expect_item(impl_did).expect_impl().self_ty.span;
Err(infringing_fields_error(tcx, fields, LangItem::Copy, impl_did, span))
Err(infringing_fields_error(
tcx,
fields.into_iter().map(|(field, ty, reason)| (tcx.def_span(field.did), ty, reason)),
LangItem::Copy,
impl_did,
span,
))
}
Err(CopyImplementationError::NotAnAdt) => {
let span = tcx.hir().expect_item(impl_did).expect_impl().self_ty.span;
Expand All @@ -116,7 +126,12 @@ fn visit_implementation_of_copy(checker: &Checker<'_>) -> Result<(), ErrorGuaran
}
}

fn visit_implementation_of_const_param_ty(checker: &Checker<'_>) -> Result<(), ErrorGuaranteed> {
fn visit_implementation_of_const_param_ty(
checker: &Checker<'_>,
kind: LangItem,
) -> Result<(), ErrorGuaranteed> {
assert!(matches!(kind, LangItem::ConstParamTy | LangItem::UnsizedConstParamTy));

let tcx = checker.tcx;
let header = checker.impl_header;
let impl_did = checker.impl_def_id;
Expand All @@ -125,21 +140,41 @@ fn visit_implementation_of_const_param_ty(checker: &Checker<'_>) -> Result<(), E

let param_env = tcx.param_env(impl_did);

if let ty::ImplPolarity::Negative = header.polarity {
if let ty::ImplPolarity::Negative | ty::ImplPolarity::Reservation = header.polarity {
return Ok(());
}

let cause = traits::ObligationCause::misc(DUMMY_SP, impl_did);
match type_allowed_to_implement_const_param_ty(tcx, param_env, self_type, cause) {
match type_allowed_to_implement_const_param_ty(tcx, param_env, self_type, kind, cause) {
Ok(()) => Ok(()),
Err(ConstParamTyImplementationError::InfrigingFields(fields)) => {
let span = tcx.hir().expect_item(impl_did).expect_impl().self_ty.span;
Err(infringing_fields_error(tcx, fields, LangItem::ConstParamTy, impl_did, span))
Err(infringing_fields_error(
tcx,
fields.into_iter().map(|(field, ty, reason)| (tcx.def_span(field.did), ty, reason)),
LangItem::ConstParamTy,
impl_did,
span,
))
}
Err(ConstParamTyImplementationError::NotAnAdtOrBuiltinAllowed) => {
let span = tcx.hir().expect_item(impl_did).expect_impl().self_ty.span;
Err(tcx.dcx().emit_err(errors::ConstParamTyImplOnNonAdt { span }))
}
Err(ConstParamTyImplementationError::InvalidInnerTyOfBuiltinTy(infringing_tys)) => {
let span = tcx.hir().expect_item(impl_did).expect_impl().self_ty.span;
Err(infringing_fields_error(
tcx,
infringing_tys.into_iter().map(|(ty, reason)| (span, ty, reason)),
LangItem::ConstParamTy,
impl_did,
span,
))
}
Err(ConstParamTyImplementationError::UnsizedConstParamsFeatureRequired) => {
let span = tcx.hir().expect_item(impl_did).expect_impl().self_ty.span;
Err(tcx.dcx().emit_err(errors::ConstParamTyImplOnUnsized { span }))
}
}
}

Expand Down Expand Up @@ -501,9 +536,9 @@ pub fn coerce_unsized_info<'tcx>(
Ok(CoerceUnsizedInfo { custom_kind: kind })
}

fn infringing_fields_error(
tcx: TyCtxt<'_>,
fields: Vec<(&ty::FieldDef, Ty<'_>, InfringingFieldsReason<'_>)>,
fn infringing_fields_error<'tcx>(
tcx: TyCtxt<'tcx>,
infringing_tys: impl Iterator<Item = (Span, Ty<'tcx>, InfringingFieldsReason<'tcx>)>,
lang_item: LangItem,
impl_did: LocalDefId,
impl_span: Span,
Expand All @@ -521,13 +556,13 @@ fn infringing_fields_error(

let mut label_spans = Vec::new();

for (field, ty, reason) in fields {
for (span, ty, reason) in infringing_tys {
// Only report an error once per type.
if !seen_tys.insert(ty) {
continue;
}

label_spans.push(tcx.def_span(field.did));
label_spans.push(span);

match reason {
InfringingFieldsReason::Fulfill(fulfillment_errors) => {
Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_hir_analysis/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,14 @@ pub struct CopyImplOnNonAdt {
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(hir_analysis_const_param_ty_impl_on_unsized)]
pub struct ConstParamTyImplOnUnsized {
#[primary_span]
#[label]
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(hir_analysis_const_param_ty_impl_on_non_adt)]
pub struct ConstParamTyImplOnNonAdt {
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ symbols! {
CoerceUnsized,
Command,
ConstParamTy,
ConstParamTy_,
Context,
Continue,
Copy,
Expand Down Expand Up @@ -336,6 +337,7 @@ symbols! {
TyKind,
Unknown,
Unsize,
UnsizedConstParamTy,
Upvars,
Vec,
VecDeque,
Expand Down Expand Up @@ -2001,6 +2003,8 @@ symbols! {
unsafe_no_drop_flag,
unsafe_pin_internals,
unsize,
unsized_const_param_ty,
unsized_const_params,
unsized_fn_params,
unsized_locals,
unsized_tuple_coercion,
Expand Down
Loading

0 comments on commit 9629b90

Please sign in to comment.