Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Allow projections to be used as const generic #104443

Closed
wants to merge 4 commits into from
Closed

Allow projections to be used as const generic #104443

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
291bc0d
Add normalization for projection types in const generics
GuillaumeGomez Nov 15, 2022
eed7738
Add test for projection in const generics
GuillaumeGomez Nov 15, 2022
9587fd7
Apply normalization for all projection for const generics
GuillaumeGomez Nov 15, 2022
00d9f41
Put normalization outside of lit_to_const query
GuillaumeGomez Nov 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
227 changes: 115 additions & 112 deletions compiler/rustc_hir_analysis/src/check/wfcheck.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ fn check_well_formed(tcx: TyCtxt<'_>, def_id: hir::OwnerId) {

if let Some(generics) = node.generics() {
for param in generics.params {
check_param_wf(tcx, param)
check_param_wf(tcx, param, def_id.def_id)
}
}
}
Expand Down Expand Up @@ -846,133 +846,136 @@ fn check_impl_item(tcx: TyCtxt<'_>, impl_item: &hir::ImplItem<'_>) {
check_associated_item(tcx, impl_item.owner_id.def_id, span, method_sig);
}

fn check_param_wf(tcx: TyCtxt<'_>, param: &hir::GenericParam<'_>) {
fn check_param_wf(tcx: TyCtxt<'_>, param: &hir::GenericParam<'_>, owner_id: LocalDefId) {
match param.kind {
// We currently only check wf of const params here.
hir::GenericParamKind::Lifetime { .. } | hir::GenericParamKind::Type { .. } => (),

// Const parameters are well formed if their type is structural match.
hir::GenericParamKind::Const { ty: hir_ty, default: _ } => {
let ty = tcx.type_of(param.def_id);
enter_wf_checking_ctxt(tcx, param.span, owner_id, |wfcx| {
let ty = tcx.type_of(param.def_id);
let ty = wfcx.normalize(hir_ty.span, None, ty);

if tcx.features().adt_const_params {
if let Some(non_structural_match_ty) =
traits::search_for_adt_const_param_violation(param.span, tcx, ty)
{
// We use the same error code in both branches, because this is really the same
// issue: we just special-case the message for type parameters to make it
// clearer.
match non_structural_match_ty.kind() {
ty::Param(_) => {
// Const parameters may not have type parameters as their types,
// because we cannot be sure that the type parameter derives `PartialEq`
// and `Eq` (just implementing them is not enough for `structural_match`).
struct_span_err!(
tcx.sess,
hir_ty.span,
E0741,
"`{ty}` is not guaranteed to `#[derive(PartialEq, Eq)]`, so may not be \
used as the type of a const parameter",
)
.span_label(
hir_ty.span,
format!("`{ty}` may not derive both `PartialEq` and `Eq`"),
)
.note(
"it is not currently possible to use a type parameter as the type of a \
const parameter",
)
.emit();
}
ty::Float(_) => {
struct_span_err!(
tcx.sess,
hir_ty.span,
E0741,
"`{ty}` is forbidden as the type of a const generic parameter",
)
.note("floats do not derive `Eq` or `Ord`, which are required for const parameters")
.emit();
}
ty::FnPtr(_) => {
struct_span_err!(
tcx.sess,
hir_ty.span,
E0741,
"using function pointers as const generic parameters is forbidden",
)
.emit();
}
ty::RawPtr(_) => {
struct_span_err!(
tcx.sess,
hir_ty.span,
E0741,
"using raw pointers as const generic parameters is forbidden",
)
.emit();
}
_ => {
let mut diag = struct_span_err!(
tcx.sess,
hir_ty.span,
E0741,
"`{}` must be annotated with `#[derive(PartialEq, Eq)]` to be used as \
the type of a const parameter",
non_structural_match_ty,
);

if tcx.features().adt_const_params {
if let Some(non_structural_match_ty) =
traits::search_for_adt_const_param_violation(param.span, tcx, ty)
{
// We use the same error code in both branches, because this is really the same
// issue: we just special-case the message for type parameters to make it
// clearer.
match non_structural_match_ty.kind() {
ty::Param(_) => {
// Const parameters may not have type parameters as their types,
// because we cannot be sure that the type parameter derives `PartialEq`
// and `Eq` (just implementing them is not enough for `structural_match`).
struct_span_err!(
tcx.sess,
hir_ty.span,
E0741,
"`{ty}` is not guaranteed to `#[derive(PartialEq, Eq)]`, so may not be \
used as the type of a const parameter",
)
.span_label(
hir_ty.span,
format!("`{ty}` may not derive both `PartialEq` and `Eq`"),
)
.note(
"it is not currently possible to use a type parameter as the type of a \
const parameter",
)
.emit();
}
ty::Float(_) => {
struct_span_err!(
tcx.sess,
hir_ty.span,
E0741,
"`{ty}` is forbidden as the type of a const generic parameter",
)
.note("floats do not derive `Eq` or `Ord`, which are required for const parameters")
.emit();
if ty == non_structural_match_ty {
diag.span_label(
hir_ty.span,
format!("`{ty}` doesn't derive both `PartialEq` and `Eq`"),
);
}

diag.emit();
}
}
ty::FnPtr(_) => {
struct_span_err!(
tcx.sess,
hir_ty.span,
E0741,
"using function pointers as const generic parameters is forbidden",
)
.emit();
}
} else {
let err_ty_str;
let mut is_ptr = true;

let err = match ty.kind() {
ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) | ty::Error(_) => None,
ty::FnPtr(_) => Some("function pointers"),
ty::RawPtr(_) => Some("raw pointers"),
_ => {
is_ptr = false;
err_ty_str = format!("`{ty}`");
Some(err_ty_str.as_str())
}
ty::RawPtr(_) => {
struct_span_err!(
tcx.sess,
};

if let Some(unsupported_type) = err {
if is_ptr {
tcx.sess.span_err(
hir_ty.span,
E0741,
"using raw pointers as const generic parameters is forbidden",
)
.emit();
}
_ => {
let mut diag = struct_span_err!(
tcx.sess,
&format!(
"using {unsupported_type} as const generic parameters is forbidden",
),
);
} else {
let mut err = tcx.sess.struct_span_err(
hir_ty.span,
E0741,
"`{}` must be annotated with `#[derive(PartialEq, Eq)]` to be used as \
the type of a const parameter",
non_structural_match_ty,
&format!(
"{unsupported_type} is forbidden as the type of a const generic parameter",
),
);
err.note("the only supported types are integers, `bool` and `char`");
if tcx.sess.is_nightly_build() {
err.help(
"more complex types are supported with `#![feature(adt_const_params)]`",
);

if ty == non_structural_match_ty {
diag.span_label(
hir_ty.span,
format!("`{ty}` doesn't derive both `PartialEq` and `Eq`"),
);
}

diag.emit();
err.emit();
}
}
}
} else {
let err_ty_str;
let mut is_ptr = true;

let err = match ty.kind() {
ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) | ty::Error(_) => None,
ty::FnPtr(_) => Some("function pointers"),
ty::RawPtr(_) => Some("raw pointers"),
_ => {
is_ptr = false;
err_ty_str = format!("`{ty}`");
Some(err_ty_str.as_str())
}
};

if let Some(unsupported_type) = err {
if is_ptr {
tcx.sess.span_err(
hir_ty.span,
&format!(
"using {unsupported_type} as const generic parameters is forbidden",
),
);
} else {
let mut err = tcx.sess.struct_span_err(
hir_ty.span,
&format!(
"{unsupported_type} is forbidden as the type of a const generic parameter",
),
);
err.note("the only supported types are integers, `bool` and `char`");
if tcx.sess.is_nightly_build() {
err.help(
"more complex types are supported with `#![feature(adt_const_params)]`",
);
}
err.emit();
}
}
}
});
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_middle/src/ty/consts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ impl<'tcx> Const<'tcx> {

let ty = tcx.type_of(def.def_id_for_type_of());
oli-obk marked this conversation as resolved.
Show resolved Hide resolved

let param_env = tcx.param_env(def.did);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This normalization is causing a lot of cycle error. If I nest it into a if TypeVisitable::has_projections(&ty) {, it reduces greatly the number of such errors but it's not a good solution imo.

Do you have any idea what's going on here? Why would normalization would create cycle errors? (in code like src/test/ui/variance/variance-associated-consts.rs)

Copy link
Contributor

@lcnr lcnr Nov 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah yeah ofc that causes issues 😅

having a const parameter in the where clauses now relies on the param_env query for normalization to get the ty::Const but we need the ty::Const as part of the output of the param_env query.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any idea how to fix it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not really 😅 even changing the type of type level constants to not inherit their parents generics won't work because normalization depends on impl headers which can also contain ty::Const. Normalizing when creating ty::Const is too early 🤔

If we were to delay normalization of the type of consts to a later point this may mostly work. tbh that makes me think that we shouldn't land this change and should keep forbidding projections in the type of type system constants. Sorry for not noticing this issue earlier 😅 especially as we've had to deal with pretty much the same issue for generic_const_exprs as well.

// We check that the `ty` is well formed in `wfcheck::check_param_wf` so
// this should always succeed.
let ty = tcx.normalize_erasing_regions(param_env, ty);

match Self::try_eval_lit_or_param(tcx, ty, expr) {
Some(v) => v,
None => tcx.mk_const(
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_mir_build/src/thir/constant.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use rustc_ast as ast;
use rustc_middle::mir::interpret::{LitToConstError, LitToConstInput};
use rustc_middle::ty::visit::TypeVisitable;
use rustc_middle::ty::{self, ParamEnv, ScalarInt, TyCtxt};
use rustc_span::DUMMY_SP;

Expand All @@ -8,6 +9,7 @@ pub(crate) fn lit_to_const<'tcx>(
lit_input: LitToConstInput<'tcx>,
) -> Result<ty::Const<'tcx>, LitToConstError> {
let LitToConstInput { lit, ty, neg } = lit_input;
assert!(!TypeVisitable::has_projections(&ty));

let trunc = |n| {
let param_ty = ParamEnv::reveal_all().and(ty);
Expand Down
21 changes: 12 additions & 9 deletions compiler/rustc_ty_utils/src/consts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use rustc_hir::def_id::LocalDefId;
use rustc_index::vec::IndexVec;
use rustc_middle::mir::interpret::{LitToConstError, LitToConstInput};
use rustc_middle::ty::abstract_const::{CastKind, Node, NodeId};
use rustc_middle::ty::{self, TyCtxt, TypeVisitable};
use rustc_middle::ty::{self, ParamEnv, TyCtxt, TypeVisitable};
use rustc_middle::{mir, thir};
use rustc_span::Span;
use rustc_target::abi::VariantIdx;
Expand Down Expand Up @@ -222,6 +222,9 @@ impl<'a, 'tcx> AbstractConstBuilder<'a, 'tcx> {
fn recurse_build(&mut self, node: thir::ExprId) -> Result<NodeId, ErrorGuaranteed> {
use thir::ExprKind;
let node = &self.body.exprs[node];
let param_env = ParamEnv::reveal_all().with_reveal_all_normalized(self.tcx);
let node_ty = self.tcx.try_normalize_erasing_regions(param_env, node.ty).unwrap_or(node.ty);
Comment on lines +225 to +226
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both reveal_all and normalize_erasing_regions are almost always the wrong thing. Unless you know what it means to use it, you can be sure it's wrong to use it.

The fact that an unnormalized type ends up in a node.ty is the problem, find out where it comes from and normalize there.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I marked this as resolved mistakenly. Should still check where the node gets its type and normalize there


Ok(match &node.kind {
// I dont know if handling of these 3 is correct
&ExprKind::Scope { value, .. } => self.recurse_build(value)?,
Expand All @@ -231,12 +234,12 @@ impl<'a, 'tcx> AbstractConstBuilder<'a, 'tcx> {
let sp = node.span;
let constant = match self.tcx.at(sp).lit_to_const(LitToConstInput {
lit: &lit.node,
ty: node.ty,
ty: node_ty,
neg,
}) {
Ok(c) => c,
Err(LitToConstError::Reported(guar)) => {
self.tcx.const_error_with_guaranteed(node.ty, guar)
self.tcx.const_error_with_guaranteed(node_ty, guar)
}
Err(LitToConstError::TypeError) => {
bug!("encountered type error in lit_to_const")
Expand All @@ -247,23 +250,23 @@ impl<'a, 'tcx> AbstractConstBuilder<'a, 'tcx> {
}
&ExprKind::NonHirLiteral { lit, user_ty: _ } => {
let val = ty::ValTree::from_scalar_int(lit);
self.nodes.push(Node::Leaf(ty::Const::from_value(self.tcx, val, node.ty)))
self.nodes.push(Node::Leaf(ty::Const::from_value(self.tcx, val, node_ty)))
}
&ExprKind::ZstLiteral { user_ty: _ } => {
let val = ty::ValTree::zst();
self.nodes.push(Node::Leaf(ty::Const::from_value(self.tcx, val, node.ty)))
self.nodes.push(Node::Leaf(ty::Const::from_value(self.tcx, val, node_ty)))
}
&ExprKind::NamedConst { def_id, substs, user_ty: _ } => {
let uneval =
ty::UnevaluatedConst::new(ty::WithOptConstParam::unknown(def_id), substs);

let constant = self.tcx.mk_const(ty::ConstKind::Unevaluated(uneval), node.ty);
let constant = self.tcx.mk_const(ty::ConstKind::Unevaluated(uneval), node_ty);

self.nodes.push(Node::Leaf(constant))
}

ExprKind::ConstParam { param, .. } => {
let const_param = self.tcx.mk_const(ty::ConstKind::Param(*param), node.ty);
let const_param = self.tcx.mk_const(ty::ConstKind::Param(*param), node_ty);
self.nodes.push(Node::Leaf(const_param))
}

Expand Down Expand Up @@ -308,11 +311,11 @@ impl<'a, 'tcx> AbstractConstBuilder<'a, 'tcx> {
// This is important so that `N as usize as usize` doesnt unify with `N as usize`. (untested)
&ExprKind::Use { source } => {
let arg = self.recurse_build(source)?;
self.nodes.push(Node::Cast(CastKind::Use, arg, node.ty))
self.nodes.push(Node::Cast(CastKind::Use, arg, node_ty))
}
&ExprKind::Cast { source } => {
let arg = self.recurse_build(source)?;
self.nodes.push(Node::Cast(CastKind::As, arg, node.ty))
self.nodes.push(Node::Cast(CastKind::As, arg, node_ty))
}
ExprKind::Borrow { arg, .. } => {
let arg_node = &self.body.exprs[*arg];
Expand Down
17 changes: 17 additions & 0 deletions src/test/ui/const-generics/projection-as-arg-const.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// run-pass

pub trait Identity {
type Identity;
}

impl<T> Identity for T {
type Identity = Self;
}

pub fn foo<const X: <i32 as Identity>::Identity>() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tbh I am not sure whether i want to insta stabilize projections in const params or keep them as part of the adt_const_params feature 🤔 needs a lang FCP if insta stable

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you prefer! If a FCP is needed, I'll let someone from your team handle this part.

assert!(X == 12);
}

fn main() {
foo::<12>();
}