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.

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

Split out lit_to_const_for_patterns #116251

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Split out bad lit_to_const_for_patterns
  • Loading branch information
compiler-errors committed Sep 28, 2023
commit 4c05757f1c54dc9d7c241dbacd6a0be3fa79c035
4 changes: 4 additions & 0 deletions compiler/rustc_middle/src/query/erase.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,10 @@ impl EraseType for ty::Binder<'_, &'_ ty::List<Ty<'_>>> {
type Result = [u8; size_of::<ty::Binder<'static, &'static ty::List<Ty<'static>>>>()];
}

impl EraseType for Option<ty::Const<'_>> {
type Result = [u8; size_of::<Option<ty::Const<'static>>>()];
}

impl<T0, T1> EraseType for (&'_ T0, &'_ T1) {
type Result = [u8; size_of::<(&'static (), &'static ())>()];
}
Expand Down
12 changes: 10 additions & 2 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1111,13 +1111,21 @@ rustc_queries! {
desc { "getting a &core::panic::Location referring to a span" }
}

// FIXME get rid of this with valtrees
query lit_to_const(
key: LitToConstInput<'tcx>
) -> Result<ty::Const<'tcx>, LitToConstError> {
) -> Option<ty::Const<'tcx>> {
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 query could alternatively just return an (option of) valtree to make it clear what it's doing

desc { "converting literal to const" }
}

// FIXME: We could get rid of this if we got rid of float patterns, for
// example, but we would also need to make sure that other things like
// const type mismatches are handled elsewhere.
query lit_to_const_for_patterns(
key: LitToConstInput<'tcx>
) -> Result<ty::Const<'tcx>, LitToConstError> {
desc { "converting literal to pattern const" }
}

query check_match(key: LocalDefId) -> Result<(), rustc_errors::ErrorGuaranteed> {
desc { |tcx| "match-checking `{}`", tcx.def_path_str(key) }
cache_on_disk_if { true }
Expand Down
15 changes: 4 additions & 11 deletions compiler/rustc_middle/src/ty/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,17 +194,10 @@ impl<'tcx> Const<'tcx> {
};

if let Some(lit_input) = lit_input {
// If an error occurred, ignore that it's a literal and leave reporting the error up to
// mir.
match tcx.at(expr.span).lit_to_const(lit_input) {
Ok(c) => return Some(c),
Err(e) => {
tcx.sess.delay_span_bug(
expr.span,
format!("Const::from_anon_const: couldn't lit_to_const {e:?}"),
);
}
}
// If we failed to validate the type of the lit (which we sometimes
// need to actually build the valtree), then return `None`, which
// will make it fall back to using an unevaluated const.
return tcx.at(expr.span).lit_to_const(lit_input);
}

match expr.kind {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir_build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ fluent_messages! { "../messages.ftl" }
pub fn provide(providers: &mut Providers) {
providers.check_match = thir::pattern::check_match;
providers.lit_to_const = thir::constant::lit_to_const;
providers.lit_to_const_for_patterns = thir::constant::lit_to_const_for_patterns;
providers.mir_built = build::mir_built;
providers.closure_saved_names_of_captured_variables =
build::closure_saved_names_of_captured_variables;
Expand Down
60 changes: 60 additions & 0 deletions compiler/rustc_mir_build/src/thir/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,66 @@ use crate::build::parse_float_into_scalar;
pub(crate) fn lit_to_const<'tcx>(
Copy link
Member Author

Choose a reason for hiding this comment

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

naming: we could call this lit_to_const_eager or something, to help make it clear that it's trying to eagerly convert lits when possible, but will hold off from doing so if not possible (i.e. we don't have the right type info).

tcx: TyCtxt<'tcx>,
lit_input: LitToConstInput<'tcx>,
) -> Option<ty::Const<'tcx>> {
let LitToConstInput { lit, ty, neg } = lit_input;

let valtree = match (lit, &ty.kind()) {
(ast::LitKind::Str(s, _), ty::Ref(_, inner_ty, _)) if inner_ty.is_str() => {
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 function could ignore the types in the litkinds that don't actually matter?

let str_bytes = s.as_str().as_bytes();
ty::ValTree::from_raw_bytes(tcx, str_bytes)
}
(ast::LitKind::ByteStr(data, _), ty::Ref(_, inner_ty, _))
if matches!(inner_ty.kind(), ty::Slice(_)) =>
{
let bytes = data as &[u8];
ty::ValTree::from_raw_bytes(tcx, bytes)
}
(ast::LitKind::ByteStr(data, _), ty::Ref(_, inner_ty, _)) if inner_ty.is_array() => {
let bytes = data as &[u8];
ty::ValTree::from_raw_bytes(tcx, bytes)
}
(ast::LitKind::Byte(n), ty::Uint(ty::UintTy::U8)) => {
ty::ValTree::from_scalar_int((*n).into())
}
(ast::LitKind::CStr(data, _), ty::Ref(_, inner_ty, _)) if matches!(inner_ty.kind(), ty::Adt(def, _) if Some(def.did()) == tcx.lang_items().c_str()) =>
{
let bytes = data as &[u8];
ty::ValTree::from_raw_bytes(tcx, bytes)
}
(ast::LitKind::Int(n, _), ty::Uint(_)) | (ast::LitKind::Int(n, _), ty::Int(_)) => {
let n = if neg { (*n as i128).overflowing_neg().0 as u128 } else { *n };
let param_ty = ParamEnv::reveal_all().and(ty);
let width = tcx
.layout_of(param_ty)
.unwrap_or_else(|_| bug!("should always be able to compute the layout of a scalar"))
.size;
trace!("trunc {} with size {} and shift {}", n, width.bits(), 128 - width.bits());
let result = width.truncate(n);
trace!("trunc result: {}", result);

let scalar_int = ScalarInt::try_from_uint(result, width)
.unwrap_or_else(|| bug!("expected to create ScalarInt from uint {:?}", result));

ty::ValTree::from_scalar_int(scalar_int)
}
(ast::LitKind::Bool(b), ty::Bool) => ty::ValTree::from_scalar_int((*b).into()),
(ast::LitKind::Char(c), ty::Char) => ty::ValTree::from_scalar_int((*c).into()),
// If there's a type mismatch, it may be a legitimate type mismatch, or
// it might be an unnormalized type. Return `None`, which falls back
// to an unevaluated const, and the error will be caught (or we will see
// that it's ok) elsewhere.
_ => return None,
};

Some(ty::Const::new_value(tcx, valtree, ty))
}

/// The old "lit_to_const", which assumes that the type passed in `lit_input`
/// is normalized, and will error out if that is not true. This should only
/// be used in pattern building code (and ideally we'd get rid of this altogether).
pub(crate) fn lit_to_const_for_patterns<'tcx>(
tcx: TyCtxt<'tcx>,
lit_input: LitToConstInput<'tcx>,
) -> Result<ty::Const<'tcx>, LitToConstError> {
let LitToConstInput { lit, ty, neg } = lit_input;

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_mir_build/src/thir/pattern/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,7 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
_ => None,
};
if let Some(lit_input) = lit_input {
match tcx.at(expr.span).lit_to_const(lit_input) {
match tcx.at(expr.span).lit_to_const_for_patterns(lit_input) {
Ok(c) => return self.const_to_pat(Const::Ty(c), id, span, None).kind,
// If an error occurred, ignore that it's a literal
// and leave reporting the error up to const eval of
Expand Down Expand Up @@ -676,7 +676,7 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {

let lit_input =
LitToConstInput { lit: &lit.node, ty: self.typeck_results.expr_ty(expr), neg };
match self.tcx.at(expr.span).lit_to_const(lit_input) {
match self.tcx.at(expr.span).lit_to_const_for_patterns(lit_input) {
Ok(constant) => {
self.const_to_pat(Const::Ty(constant), expr.hir_id, lit.span, None).kind
}
Expand Down
6 changes: 5 additions & 1 deletion compiler/rustc_ty_utils/src/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,11 @@ fn recurse_build<'tcx>(
}
&ExprKind::Literal { lit, neg } => {
let sp = node.span;
match tcx.at(sp).lit_to_const(LitToConstInput { lit: &lit.node, ty: node.ty, neg }) {
match tcx.at(sp).lit_to_const_for_patterns(LitToConstInput {
Copy link
Member

Choose a reason for hiding this comment

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

this is definitely not a pattern xd we'd end up here for things like the 1 in foo::<{ N + 1 }>();

lit: &lit.node,
ty: node.ty,
neg,
}) {
Ok(c) => c,
Err(LitToConstError::Reported(guar)) => ty::Const::new_error(tcx, guar, node.ty),
Err(LitToConstError::TypeError) => {
Expand Down
8 changes: 4 additions & 4 deletions tests/ui/const-generics/projection-as-arg-const.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// This is currently not possible to use projections as const generics.
// More information about this available here:
// https://github.com/rust-lang/rust/pull/104443#discussion_r1029375633
// build-pass

Copy link
Member

Choose a reason for hiding this comment

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

imo this should be a revision, we want to keep asserting that aliases dont work as the type of const params without adt_const_param i think

#![feature(adt_const_params)]
//~^ WARN the feature `adt_const_params` is incomplete

pub trait Identity {
type Identity;
Expand All @@ -11,7 +12,6 @@ impl<T> Identity for T {
}

pub fn foo<const X: <i32 as Identity>::Identity>() {
//~^ ERROR
assert!(X == 12);
}

Expand Down
14 changes: 7 additions & 7 deletions tests/ui/const-generics/projection-as-arg-const.stderr
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
error: `<i32 as Identity>::Identity` is forbidden as the type of a const generic parameter
--> $DIR/projection-as-arg-const.rs:13:21
warning: the feature `adt_const_params` is incomplete and may not be safe to use and/or cause compiler crashes
--> $DIR/projection-as-arg-const.rs:3:12
|
LL | pub fn foo<const X: <i32 as Identity>::Identity>() {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
LL | #![feature(adt_const_params)]
| ^^^^^^^^^^^^^^^^
|
= note: the only supported types are integers, `bool` and `char`
= help: more complex types are supported with `#![feature(adt_const_params)]`
= note: see issue #95174 <https://github.com/rust-lang/rust/issues/95174> for more information
= note: `#[warn(incomplete_features)]` on by default

error: aborting due to previous error
warning: 1 warning emitted

Loading