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

Test validity of pattern types #136145

Merged
merged 3 commits into from
Feb 3, 2025
Merged
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
11 changes: 11 additions & 0 deletions compiler/rustc_const_eval/src/interpret/validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1240,6 +1240,17 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValueVisitor<'tcx, M> for ValidityVisitor<'rt,
self.visit_field(val, 0, &self.ecx.project_index(val, 0)?)?;
}
}
ty::Pat(base, pat) => {
// First check that the base type is valid
self.visit_value(&val.transmute(self.ecx.layout_of(*base)?, self.ecx)?)?;
Copy link
Member

Choose a reason for hiding this comment

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

Is there a field projection for this? Or is transmute the way this is meant to be done?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no field projection. Using a field like representation is what we want to get rid of with pattern types to avoid all the pains we have with the rustc_scalar_layout attributes. We could add something to the validation path to show that we're validating the base type, but there's no projection for it, just like niche optimized enums have no projection for the discriminant really

// When you extend this match, make sure to also add tests to
// tests/ui/type/pattern_types/validity.rs((
match **pat {
// Range patterns are precisely reflected into `valid_range` and thus
// handled fully by `visit_scalar` (called below).
ty::PatternKind::Range { .. } => {},
Copy link
Member

@RalfJung RalfJung Feb 2, 2025

Choose a reason for hiding this comment

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

Seems worth asserting that layout.backend_repr is indeed Scalar... skipping the recursion here is a bit dubious, we don't do this for anything else.

Copy link
Member

Choose a reason for hiding this comment

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

In particular, I think this means that e.g. if a u32 range pattern type is uninitialized, or a char range pattern type is not a valid char, we skip the nice error that try_visit_primitive would generate, and instead show a more generic error in visit_scalar.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm tried recursing, but it doesn't actually do anything better

Copy link
Member

Choose a reason for hiding this comment

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

Can you add char cases to pattern_types/validity.rs, one case where the character is valid but outside the range and one case where it's not even a valid char?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That did it, yea

Copy link
Member

Choose a reason for hiding this comment

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

So that test does show a different error with the extra call to visit_value? Great :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes

}
}
_ => {
// default handler
try_validation!(
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_hir_analysis/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,9 @@ hir_analysis_inherent_ty_outside_relevant = cannot define inherent `impl` for a
.help = consider moving this inherent impl into the crate defining the type if possible
.span_help = alternatively add `#[rustc_allow_incoherent_impl]` to the relevant impl items

hir_analysis_invalid_base_type = `{$ty}` is not a valid base type for range patterns
.note = range patterns only support integers

hir_analysis_invalid_generic_receiver_ty = invalid generic `self` parameter type: `{$receiver_ty}`
.note = type of `self` must not be a method generic parameter type

Expand Down Expand Up @@ -438,7 +441,6 @@ hir_analysis_pattern_type_wild_pat = wildcard patterns are not permitted for pat
.label = this type is the same as the inner type without a pattern
hir_analysis_placeholder_not_allowed_item_signatures = the placeholder `_` is not allowed within types on item signatures for {$kind}
.label = not allowed in type signatures

hir_analysis_precise_capture_self_alias = `Self` can't be captured in `use<...>` precise captures list, since it is an alias
.label = `Self` is not a generic argument, but an alias to the type of the {$what}

Expand Down
12 changes: 12 additions & 0 deletions compiler/rustc_hir_analysis/src/errors/pattern_types.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use rustc_macros::Diagnostic;
use rustc_middle::ty::Ty;
use rustc_span::Span;

#[derive(Diagnostic)]
Expand All @@ -7,3 +8,14 @@ pub(crate) struct WildPatTy {
#[primary_span]
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(hir_analysis_invalid_base_type)]
pub(crate) struct InvalidBaseType<'tcx> {
pub ty: Ty<'tcx>,
#[primary_span]
pub ty_span: Span,
pub pat: &'static str,
#[note]
pub pat_span: Span,
}
15 changes: 14 additions & 1 deletion compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ use tracing::{debug, instrument};

use crate::bounds::Bounds;
use crate::check::check_abi_fn_ptr;
use crate::errors::{AmbiguousLifetimeBound, BadReturnTypeNotation, WildPatTy};
use crate::errors::{AmbiguousLifetimeBound, BadReturnTypeNotation, InvalidBaseType, WildPatTy};
use crate::hir_ty_lowering::errors::{GenericsArgsErrExtend, prohibit_assoc_item_constraint};
use crate::hir_ty_lowering::generics::{check_generic_arg_count, lower_generic_args};
use crate::middle::resolve_bound_vars as rbv;
Expand Down Expand Up @@ -2432,13 +2432,26 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
self.ty_infer(None, hir_ty.span)
}
hir::TyKind::Pat(ty, pat) => {
let ty_span = ty.span;
let ty = self.lower_ty(ty);
let pat_ty = match pat.kind {
hir::PatKind::Wild => {
let err = self.dcx().emit_err(WildPatTy { span: pat.span });
Ty::new_error(tcx, err)
}
hir::PatKind::Range(start, end, include_end) => {
let ty = match ty.kind() {
ty::Int(_) | ty::Uint(_) | ty::Char => ty,
_ => Ty::new_error(
tcx,
self.dcx().emit_err(InvalidBaseType {
ty,
pat: "range",
ty_span,
pat_span: pat.span,
}),
),
};
let expr_to_const = |expr: &'tcx hir::PatExpr<'tcx>| -> ty::Const<'tcx> {
let (c, c_ty) = match expr.kind {
hir::PatExprKind::Lit { lit, negated } => {
Expand Down
12 changes: 12 additions & 0 deletions tests/ui/type/pattern_types/chars.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
//! Check that chars can be used in ranges
//@ check-pass

#![feature(pattern_types)]
#![feature(pattern_type_macro)]

use std::pat::pattern_type;

const LOWERCASE: pattern_type!(char is 'a'..='z') = unsafe { std::mem::transmute('b') };

fn main() {}
26 changes: 26 additions & 0 deletions tests/ui/type/pattern_types/nested.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
//! Check that pattern types can only have specific base types
#![feature(pattern_types)]
#![feature(pattern_type_macro)]

use std::pat::pattern_type;

// Undoing an inner pattern type's restrictions should either be forbidden,
// or still validate correctly.
const BAD_NESTING: pattern_type!(pattern_type!(u32 is 1..) is 0..) = todo!();
//~^ ERROR: not a valid base type for range patterns

// We want to get the most narrowest version that a pattern could be
const BAD_NESTING2: pattern_type!(pattern_type!(i32 is 1..) is ..=-1) = todo!();
//~^ ERROR: not a valid base type for range patterns

const BAD_NESTING3: pattern_type!(pattern_type!(i32 is 1..) is ..0) = todo!();
//~^ ERROR: not a valid base type for range patterns

const BAD_NESTING4: pattern_type!(() is ..0) = todo!();
//~^ ERROR: not a valid base type for range patterns

const BAD_NESTING5: pattern_type!(f32 is 1.0 .. 2.0) = todo!();
//~^ ERROR: not a valid base type for range patterns

fn main() {}
62 changes: 62 additions & 0 deletions tests/ui/type/pattern_types/nested.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
error: `(u32) is 1..=` is not a valid base type for range patterns
--> $DIR/nested.rs:10:34
|
LL | const BAD_NESTING: pattern_type!(pattern_type!(u32 is 1..) is 0..) = todo!();
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: range patterns only support integers
--> $DIR/nested.rs:10:63
|
LL | const BAD_NESTING: pattern_type!(pattern_type!(u32 is 1..) is 0..) = todo!();
| ^^^

error: `(i32) is 1..=` is not a valid base type for range patterns
--> $DIR/nested.rs:14:35
|
LL | const BAD_NESTING2: pattern_type!(pattern_type!(i32 is 1..) is ..=-1) = todo!();
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: range patterns only support integers
--> $DIR/nested.rs:14:64
|
LL | const BAD_NESTING2: pattern_type!(pattern_type!(i32 is 1..) is ..=-1) = todo!();
| ^^^^^

error: `(i32) is 1..=` is not a valid base type for range patterns
--> $DIR/nested.rs:17:35
|
LL | const BAD_NESTING3: pattern_type!(pattern_type!(i32 is 1..) is ..0) = todo!();
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: range patterns only support integers
--> $DIR/nested.rs:17:64
|
LL | const BAD_NESTING3: pattern_type!(pattern_type!(i32 is 1..) is ..0) = todo!();
| ^^^

error: `()` is not a valid base type for range patterns
--> $DIR/nested.rs:20:35
|
LL | const BAD_NESTING4: pattern_type!(() is ..0) = todo!();
| ^^
|
note: range patterns only support integers
--> $DIR/nested.rs:20:41
|
LL | const BAD_NESTING4: pattern_type!(() is ..0) = todo!();
| ^^^

error: `f32` is not a valid base type for range patterns
--> $DIR/nested.rs:23:35
|
LL | const BAD_NESTING5: pattern_type!(f32 is 1.0 .. 2.0) = todo!();
| ^^^
|
note: range patterns only support integers
--> $DIR/nested.rs:23:42
|
LL | const BAD_NESTING5: pattern_type!(f32 is 1.0 .. 2.0) = todo!();
| ^^^^^^^^^^

error: aborting due to 5 previous errors

20 changes: 20 additions & 0 deletions tests/ui/type/pattern_types/pattern_type_mismatch.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
//! Check that pattern types patterns must be of the type of the base type

//@ known-bug: unknown
//@ failure-status: 101
//@ normalize-stderr: "note: .*\n\n" -> ""
//@ normalize-stderr: "thread 'rustc' panicked.*\n" -> ""
//@ normalize-stderr: "(error: internal compiler error: [^:]+):\d+:\d+: " -> "$1:LL:CC: "
//@ normalize-stderr: "(delayed at compiler/rustc_mir_transform/src/lib.rs:)\d+:\d+" -> "$1:LL:CC"
//@ rustc-env:RUST_BACKTRACE=0

#![feature(pattern_types)]
#![feature(pattern_type_macro)]

use std::pat::pattern_type;

const BAD_NESTING4: pattern_type!(u8 is 'a'..='a') = todo!();

const BAD_NESTING5: pattern_type!(char is 1..=1) = todo!();

fn main() {}
31 changes: 31 additions & 0 deletions tests/ui/type/pattern_types/pattern_type_mismatch.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
error: internal compiler error: ty::ConstKind::Error constructed but no error reported
|
= error: internal compiler error: ty::ConstKind::Error constructed but no error reported
|
= note: delayed at compiler/rustc_mir_build/src/thir/constant.rs:72:21 - disabled backtrace
= error: internal compiler error: mir_const_qualif: MIR had errors
--> $DIR/pattern_type_mismatch.rs:16:1
|
LL | const BAD_NESTING4: pattern_type!(u8 is 'a'..='a') = todo!();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: delayed at compiler/rustc_mir_transform/src/lib.rs::LL:CC - disabled backtrace
--> $DIR/pattern_type_mismatch.rs:16:1
|
LL | const BAD_NESTING4: pattern_type!(u8 is 'a'..='a') = todo!();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: internal compiler error: mir_const_qualif: MIR had errors
--> $DIR/pattern_type_mismatch.rs:18:1
|
LL | const BAD_NESTING5: pattern_type!(char is 1..=1) = todo!();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: delayed at compiler/rustc_mir_transform/src/lib.rs::LL:CC - disabled backtrace
--> $DIR/pattern_type_mismatch.rs:18:1
|
LL | const BAD_NESTING5: pattern_type!(char is 1..=1) = todo!();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

query stack during panic:
end of query stack
14 changes: 14 additions & 0 deletions tests/ui/type/pattern_types/reverse_range.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
//! Check that the range start must be smaller than the range end
//@ known-bug: unknown
//@ failure-status: 101
//@ normalize-stderr: "note: .*\n\n" -> ""
//@ normalize-stderr: "thread 'rustc' panicked.*\n" -> ""
//@ normalize-stderr: "(error: internal compiler error: [^:]+):\d+:\d+: " -> "$1:LL:CC: "
//@ rustc-env:RUST_BACKTRACE=0

#![feature(pattern_types)]
#![feature(pattern_type_macro)]

use std::pat::pattern_type;

const NONE: pattern_type!(u8 is 1..0) = unsafe { std::mem::transmute(3_u8) };
17 changes: 17 additions & 0 deletions tests/ui/type/pattern_types/reverse_range.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
error[E0601]: `main` function not found in crate `reverse_range`
--> $DIR/reverse_range.rs:14:78
|
LL | const NONE: pattern_type!(u8 is 1..0) = unsafe { std::mem::transmute(3_u8) };
| ^ consider adding a `main` function to `$DIR/reverse_range.rs`


assertion failed: end <= max_value
error: the compiler unexpectedly panicked. this is a bug.

query stack during panic:
#0 [eval_to_allocation_raw] const-evaluating + checking `NONE`
#1 [eval_to_const_value_raw] simplifying constant for the type system `NONE`
... and 1 other queries... use `env RUST_BACKTRACE=1` to see the full query stack
error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0601`.
37 changes: 37 additions & 0 deletions tests/ui/type/pattern_types/validity.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
//! Check that pattern types have their validity checked

#![feature(pattern_types)]
#![feature(pattern_type_macro)]

use std::pat::pattern_type;

const BAD: pattern_type!(u32 is 1..) = unsafe { std::mem::transmute(0) };
//~^ ERROR: it is undefined behavior to use this value

const BAD_UNINIT: pattern_type!(u32 is 1..) =
//~^ ERROR: evaluation of constant value failed
unsafe { std::mem::transmute(std::mem::MaybeUninit::<u32>::uninit()) };

const BAD_PTR: pattern_type!(usize is 1..) = unsafe { std::mem::transmute(&42) };
//~^ ERROR: evaluation of constant value failed

const BAD_AGGREGATE: (pattern_type!(u32 is 1..), u32) = (unsafe { std::mem::transmute(0) }, 0);
//~^ ERROR: it is undefined behavior to use this value

struct Foo(Bar);
struct Bar(pattern_type!(u32 is 1..));

const BAD_FOO: Foo = Foo(Bar(unsafe { std::mem::transmute(0) }));
//~^ ERROR: it is undefined behavior to use this value

const CHAR_UNINIT: pattern_type!(char is 'A'..'Z') =
//~^ ERROR: evaluation of constant value failed
unsafe { std::mem::transmute(std::mem::MaybeUninit::<u32>::uninit()) };

const CHAR_OOB_PAT: pattern_type!(char is 'A'..'Z') = unsafe { std::mem::transmute('a') };
//~^ ERROR: it is undefined behavior to use this value

const CHAR_OOB: pattern_type!(char is 'A'..'Z') = unsafe { std::mem::transmute(u32::MAX) };
//~^ ERROR: it is undefined behavior to use this value

fn main() {}
79 changes: 79 additions & 0 deletions tests/ui/type/pattern_types/validity.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
error[E0080]: it is undefined behavior to use this value
--> $DIR/validity.rs:8:1
|
LL | const BAD: pattern_type!(u32 is 1..) = unsafe { std::mem::transmute(0) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered 0, but expected something greater or equal to 1
|
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
= note: the raw bytes of the constant (size: 4, align: 4) {
00 00 00 00 │ ....
}

error[E0080]: evaluation of constant value failed
--> $DIR/validity.rs:11:1
|
LL | const BAD_UNINIT: pattern_type!(u32 is 1..) =
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ using uninitialized data, but this operation requires initialized memory

error[E0080]: evaluation of constant value failed
--> $DIR/validity.rs:15:1
|
LL | const BAD_PTR: pattern_type!(usize is 1..) = unsafe { std::mem::transmute(&42) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unable to turn pointer into integer
|
= help: this code performed an operation that depends on the underlying bytes representing a pointer
= help: the absolute address of a pointer is not known at compile-time, so such operations are not supported

error[E0080]: it is undefined behavior to use this value
--> $DIR/validity.rs:18:1
|
LL | const BAD_AGGREGATE: (pattern_type!(u32 is 1..), u32) = (unsafe { std::mem::transmute(0) }, 0);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .0: encountered 0, but expected something greater or equal to 1
|
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
= note: the raw bytes of the constant (size: 8, align: 4) {
00 00 00 00 00 00 00 00 │ ........
}

error[E0080]: it is undefined behavior to use this value
--> $DIR/validity.rs:24:1
|
LL | const BAD_FOO: Foo = Foo(Bar(unsafe { std::mem::transmute(0) }));
| ^^^^^^^^^^^^^^^^^^ constructing invalid value at .0.0: encountered 0, but expected something greater or equal to 1
|
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
= note: the raw bytes of the constant (size: 4, align: 4) {
00 00 00 00 │ ....
}

error[E0080]: evaluation of constant value failed
--> $DIR/validity.rs:27:1
|
LL | const CHAR_UNINIT: pattern_type!(char is 'A'..'Z') =
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ using uninitialized data, but this operation requires initialized memory

error[E0080]: it is undefined behavior to use this value
--> $DIR/validity.rs:31:1
|
LL | const CHAR_OOB_PAT: pattern_type!(char is 'A'..'Z') = unsafe { std::mem::transmute('a') };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered 97, but expected something in the range 65..=89
|
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
= note: the raw bytes of the constant (size: 4, align: 4) {
61 00 00 00 │ a...
}

error[E0080]: it is undefined behavior to use this value
--> $DIR/validity.rs:34:1
|
LL | const CHAR_OOB: pattern_type!(char is 'A'..'Z') = unsafe { std::mem::transmute(u32::MAX) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered 0xffffffff, but expected a valid unicode scalar value (in `0..=0x10FFFF` but not in `0xD800..=0xDFFF`)
|
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
= note: the raw bytes of the constant (size: 4, align: 4) {
ff ff ff ff │ ....
}

error: aborting due to 8 previous errors

For more information about this error, try `rustc --explain E0080`.
Loading