Skip to content

Forbid non-structural_match types in const generics #65627

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

Merged
Merged
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
127 changes: 125 additions & 2 deletions src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
@@ -4,7 +4,7 @@ pub use self::Variance::*;
pub use self::AssocItemContainer::*;
pub use self::BorrowKind::*;
pub use self::IntVarValue::*;
pub use self::fold::TypeFoldable;
pub use self::fold::{TypeFoldable, TypeVisitor};

use crate::hir::{map as hir_map, GlobMap, TraitMap};
use crate::hir::Node;
@@ -50,7 +50,7 @@ use syntax::symbol::{kw, sym, Symbol, InternedString};
use syntax_pos::Span;

use smallvec;
use rustc_data_structures::fx::FxIndexMap;
use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
use rustc_data_structures::stable_hasher::{StableHasher, HashStable};
use rustc_index::vec::{Idx, IndexVec};

@@ -3393,6 +3393,129 @@ fn asyncness(tcx: TyCtxt<'_>, def_id: DefId) -> hir::IsAsync {
fn_like.asyncness()
}

pub enum NonStructuralMatchTy<'tcx> {
Adt(&'tcx AdtDef),
Param,
}

/// This method traverses the structure of `ty`, trying to find an
/// instance of an ADT (i.e. struct or enum) that was declared without
/// the `#[structural_match]` attribute, or a generic type parameter
/// (which cannot be determined to be `structural_match`).
///
/// The "structure of a type" includes all components that would be
/// considered when doing a pattern match on a constant of that
/// type.
///
/// * This means this method descends into fields of structs/enums,
/// and also descends into the inner type `T` of `&T` and `&mut T`
///
/// * The traversal doesn't dereference unsafe pointers (`*const T`,
/// `*mut T`), and it does not visit the type arguments of an
/// instantiated generic like `PhantomData<T>`.
///
/// The reason we do this search is Rust currently require all ADTs
/// reachable from a constant's type to be annotated with
/// `#[structural_match]`, an attribute which essentially says that
/// the implementation of `PartialEq::eq` behaves *equivalently* to a
/// comparison against the unfolded structure.
///
/// For more background on why Rust has this requirement, and issues
/// that arose when the requirement was not enforced completely, see
/// Rust RFC 1445, rust-lang/rust#61188, and rust-lang/rust#62307.
pub fn search_for_structural_match_violation<'tcx>(
tcx: TyCtxt<'tcx>,
ty: Ty<'tcx>,
) -> Option<NonStructuralMatchTy<'tcx>> {
let mut search = Search { tcx, found: None, seen: FxHashSet::default() };
ty.visit_with(&mut search);
return search.found;

struct Search<'tcx> {
tcx: TyCtxt<'tcx>,

// Records the first ADT or type parameter we find without `#[structural_match`.
found: Option<NonStructuralMatchTy<'tcx>>,

// Tracks ADTs previously encountered during search, so that
// we will not recurse on them again.
seen: FxHashSet<hir::def_id::DefId>,
}

impl<'tcx> TypeVisitor<'tcx> for Search<'tcx> {
fn visit_ty(&mut self, ty: Ty<'tcx>) -> bool {
debug!("Search visiting ty: {:?}", ty);

let (adt_def, substs) = match ty.kind {
ty::Adt(adt_def, substs) => (adt_def, substs),
ty::Param(_) => {
self.found = Some(NonStructuralMatchTy::Param);
return true; // Stop visiting.
}
ty::RawPtr(..) => {
// `#[structural_match]` ignores substructure of
// `*const _`/`*mut _`, so skip super_visit_with
//
// (But still tell caller to continue search.)
return false;
}
ty::FnDef(..) | ty::FnPtr(..) => {
// types of formals and return in `fn(_) -> _` are also irrelevant
//
// (But still tell caller to continue search.)
return false;
}
ty::Array(_, n) if n.try_eval_usize(self.tcx, ty::ParamEnv::reveal_all()) == Some(0)
=> {
// rust-lang/rust#62336: ignore type of contents
// for empty array.
return false;
}
_ => {
ty.super_visit_with(self);
return false;
}
};

if !self.tcx.has_attr(adt_def.did, sym::structural_match) {
self.found = Some(NonStructuralMatchTy::Adt(&adt_def));
debug!("Search found adt_def: {:?}", adt_def);
return true; // Stop visiting.
}

if !self.seen.insert(adt_def.did) {
debug!("Search already seen adt_def: {:?}", adt_def);
// let caller continue its search
return false;
}

// `#[structural_match]` does not care about the
// instantiation of the generics in an ADT (it
// instead looks directly at its fields outside
// this match), so we skip super_visit_with.
//
// (Must not recur on substs for `PhantomData<T>` cf
// rust-lang/rust#55028 and rust-lang/rust#55837; but also
// want to skip substs when only uses of generic are
// behind unsafe pointers `*const T`/`*mut T`.)

// even though we skip super_visit_with, we must recur on
// fields of ADT.
let tcx = self.tcx;
for field_ty in adt_def.all_fields().map(|field| field.ty(tcx, substs)) {
if field_ty.visit_with(self) {
// found an ADT without `#[structural_match]`; halt visiting!
assert!(self.found.is_some());
return true;
}
}

// Even though we do not want to recur on substs, we do
// want our caller to continue its own search.
false
}
}
}

pub fn provide(providers: &mut ty::query::Providers<'_>) {
context::provide(providers);
7 changes: 3 additions & 4 deletions src/librustc/ty/relate.rs
Original file line number Diff line number Diff line change
@@ -557,10 +557,9 @@ pub fn super_relate_consts<R: TypeRelation<'tcx>>(
x.val
};

// Currently, the values that can be unified are those that
// implement both `PartialEq` and `Eq`, corresponding to
// `structural_match` types.
// FIXME(const_generics): check for `structural_match` synthetic attribute.
// Currently, the values that can be unified are primitive types,
// and those that derive both `PartialEq` and `Eq`, corresponding
// to `structural_match` types.
let new_const_val = match (eagerly_eval(a), eagerly_eval(b)) {
(ConstValue::Infer(_), _) | (_, ConstValue::Infer(_)) => {
// The caller should handle these cases!
144 changes: 15 additions & 129 deletions src/librustc_mir/hair/pattern/mod.rs
Original file line number Diff line number Diff line change
@@ -25,7 +25,6 @@ use rustc::hir::pat_util::EnumerateAndAdjustIterator;
use rustc::hir::ptr::P;

use rustc_index::vec::Idx;
use rustc_data_structures::fx::FxHashSet;

use std::cmp::Ordering;
use std::fmt;
@@ -1000,15 +999,21 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
if self.include_lint_checks && !saw_error {
// If we were able to successfully convert the const to some pat, double-check
// that the type of the const obeys `#[structural_match]` constraint.
if let Some(adt_def) = search_for_adt_without_structural_match(self.tcx, cv.ty) {

let path = self.tcx.def_path_str(adt_def.did);
let msg = format!(
"to use a constant of type `{}` in a pattern, \
`{}` must be annotated with `#[derive(PartialEq, Eq)]`",
path,
path,
);
if let Some(non_sm_ty) = ty::search_for_structural_match_violation(self.tcx, cv.ty) {
let msg = match non_sm_ty {
ty::NonStructuralMatchTy::Adt(adt_def) => {
let path = self.tcx.def_path_str(adt_def.did);
format!(
"to use a constant of type `{}` in a pattern, \
`{}` must be annotated with `#[derive(PartialEq, Eq)]`",
path,
path,
)
}
ty::NonStructuralMatchTy::Param => {
bug!("use of constant whose type is a parameter inside a pattern");
}
};

// before issuing lint, double-check there even *is* a
// semantic PartialEq for us to dispatch to.
@@ -1169,125 +1174,6 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
}
}

/// This method traverses the structure of `ty`, trying to find an
/// instance of an ADT (i.e. struct or enum) that was declared without
/// the `#[structural_match]` attribute.
///
/// The "structure of a type" includes all components that would be
/// considered when doing a pattern match on a constant of that
/// type.
///
/// * This means this method descends into fields of structs/enums,
/// and also descends into the inner type `T` of `&T` and `&mut T`
///
/// * The traversal doesn't dereference unsafe pointers (`*const T`,
/// `*mut T`), and it does not visit the type arguments of an
/// instantiated generic like `PhantomData<T>`.
///
/// The reason we do this search is Rust currently require all ADT's
/// reachable from a constant's type to be annotated with
/// `#[structural_match]`, an attribute which essentially says that
/// the implementation of `PartialEq::eq` behaves *equivalently* to a
/// comparison against the unfolded structure.
///
/// For more background on why Rust has this requirement, and issues
/// that arose when the requirement was not enforced completely, see
/// Rust RFC 1445, rust-lang/rust#61188, and rust-lang/rust#62307.
fn search_for_adt_without_structural_match<'tcx>(tcx: TyCtxt<'tcx>,
ty: Ty<'tcx>)
-> Option<&'tcx AdtDef>
{
// Import here (not mod level), because `TypeFoldable::fold_with`
// conflicts with `PatternFoldable::fold_with`
use crate::rustc::ty::fold::TypeVisitor;
use crate::rustc::ty::TypeFoldable;

let mut search = Search { tcx, found: None, seen: FxHashSet::default() };
ty.visit_with(&mut search);
return search.found;

struct Search<'tcx> {
tcx: TyCtxt<'tcx>,

// records the first ADT we find without `#[structural_match`
found: Option<&'tcx AdtDef>,

// tracks ADT's previously encountered during search, so that
// we will not recur on them again.
seen: FxHashSet<hir::def_id::DefId>,
}

impl<'tcx> TypeVisitor<'tcx> for Search<'tcx> {
fn visit_ty(&mut self, ty: Ty<'tcx>) -> bool {
debug!("Search visiting ty: {:?}", ty);

let (adt_def, substs) = match ty.kind {
ty::Adt(adt_def, substs) => (adt_def, substs),
ty::RawPtr(..) => {
// `#[structural_match]` ignores substructure of
// `*const _`/`*mut _`, so skip super_visit_with
//
// (But still tell caller to continue search.)
return false;
}
ty::FnDef(..) | ty::FnPtr(..) => {
// types of formals and return in `fn(_) -> _` are also irrelevant
//
// (But still tell caller to continue search.)
return false;
}
ty::Array(_, n) if n.try_eval_usize(self.tcx, ty::ParamEnv::reveal_all()) == Some(0)
=> {
// rust-lang/rust#62336: ignore type of contents
// for empty array.
return false;
}
_ => {
ty.super_visit_with(self);
return false;
}
};

if !self.tcx.has_attr(adt_def.did, sym::structural_match) {
self.found = Some(&adt_def);
debug!("Search found adt_def: {:?}", adt_def);
return true // Halt visiting!
}

if !self.seen.insert(adt_def.did) {
debug!("Search already seen adt_def: {:?}", adt_def);
// let caller continue its search
return false;
}

// `#[structural_match]` does not care about the
// instantiation of the generics in an ADT (it
// instead looks directly at its fields outside
// this match), so we skip super_visit_with.
//
// (Must not recur on substs for `PhantomData<T>` cf
// rust-lang/rust#55028 and rust-lang/rust#55837; but also
// want to skip substs when only uses of generic are
// behind unsafe pointers `*const T`/`*mut T`.)

// even though we skip super_visit_with, we must recur on
// fields of ADT.
let tcx = self.tcx;
for field_ty in adt_def.all_fields().map(|field| field.ty(tcx, substs)) {
if field_ty.visit_with(self) {
// found an ADT without `#[structural_match]`; halt visiting!
assert!(self.found.is_some());
return true;
}
}

// Even though we do not want to recur on substs, we do
// want our caller to continue its own search.
false
}
}
}

impl UserAnnotatedTyHelpers<'tcx> for PatCtxt<'_, 'tcx> {
fn tcx(&self) -> TyCtxt<'tcx> {
self.tcx
10 changes: 0 additions & 10 deletions src/librustc_resolve/diagnostics.rs
Original file line number Diff line number Diff line change
@@ -367,16 +367,6 @@ impl<'a> Resolver<'a> {
span, "`Self` in type parameter default".to_string());
err
}
ResolutionError::ConstParamDependentOnTypeParam => {
let mut err = struct_span_err!(
self.session,
span,
E0671,
"const parameters cannot depend on type parameters"
);
err.span_label(span, format!("const parameter depends on type parameter"));
err
}
}
}

7 changes: 4 additions & 3 deletions src/librustc_resolve/error_codes.rs
Original file line number Diff line number Diff line change
@@ -1880,13 +1880,14 @@ fn main() {
"##,

E0671: r##"
#### Note: this error code is no longer emitted by the compiler.

Const parameters cannot depend on type parameters.
The following is therefore invalid:
```compile_fail,E0671
```compile_fail,E0741
#![feature(const_generics)]

fn const_id<T, const N: T>() -> T { // error: const parameter
// depends on type parameter
fn const_id<T, const N: T>() -> T { // error
N
}
```
23 changes: 1 addition & 22 deletions src/librustc_resolve/late.rs
Original file line number Diff line number Diff line change
@@ -111,9 +111,6 @@ crate enum RibKind<'a> {
/// from the default of a type parameter because they're not declared
/// before said type parameter. Also see the `visit_generics` override.
ForwardTyParamBanRibKind,

/// We forbid the use of type parameters as the types of const parameters.
TyParamAsConstParamTy,
}

impl RibKind<'_> {
@@ -128,8 +125,7 @@ impl RibKind<'_> {
| MacroDefinition(_) => false,
AssocItemRibKind
| ItemRibKind(_)
| ForwardTyParamBanRibKind
| TyParamAsConstParamTy => true,
| ForwardTyParamBanRibKind => true,
}
}
}
@@ -483,18 +479,6 @@ impl<'a, 'tcx> Visitor<'tcx> for LateResolutionVisitor<'a, '_> {
default_ban_rib.bindings.insert(Ident::with_dummy_span(kw::SelfUpper), Res::Err);
}

// We also ban access to type parameters for use as the types of const parameters.
let mut const_ty_param_ban_rib = Rib::new(TyParamAsConstParamTy);
const_ty_param_ban_rib.bindings.extend(generics.params.iter()
.filter(|param| {
if let GenericParamKind::Type { .. } = param.kind {
true
} else {
false
}
})
.map(|param| (Ident::with_dummy_span(param.ident.name), Res::Err)));

for param in &generics.params {
match param.kind {
GenericParamKind::Lifetime { .. } => self.visit_generic_param(param),
@@ -513,15 +497,10 @@ impl<'a, 'tcx> Visitor<'tcx> for LateResolutionVisitor<'a, '_> {
default_ban_rib.bindings.remove(&Ident::with_dummy_span(param.ident.name));
}
GenericParamKind::Const { ref ty } => {
self.ribs[TypeNS].push(const_ty_param_ban_rib);

for bound in &param.bounds {
self.visit_param_bound(bound);
}

self.visit_ty(ty);

const_ty_param_ban_rib = self.ribs[TypeNS].pop().unwrap();
}
}
}
15 changes: 2 additions & 13 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
@@ -215,8 +215,6 @@ enum ResolutionError<'a> {
ForwardDeclaredTyParam, // FIXME(const_generics:defaults)
/// Error E0735: type parameters with a default cannot use `Self`
SelfInTyParamDefault,
/// Error E0671: const parameter cannot depend on type parameter.
ConstParamDependentOnTypeParam,
}

// A minimal representation of a path segment. We use this in resolve because
@@ -2169,15 +2167,6 @@ impl<'a> Resolver<'a> {
return Res::Err;
}

// An invalid use of a type parameter as the type of a const parameter.
if let TyParamAsConstParamTy = all_ribs[rib_index].kind {
if record_used {
self.report_error(span, ResolutionError::ConstParamDependentOnTypeParam);
}
assert_eq!(res, Res::Err);
return Res::Err;
}

match res {
Res::Local(_) => {
use ResolutionError::*;
@@ -2186,7 +2175,7 @@ impl<'a> Resolver<'a> {
for rib in ribs {
match rib.kind {
NormalRibKind | ModuleRibKind(..) | MacroDefinition(..) |
ForwardTyParamBanRibKind | TyParamAsConstParamTy => {
ForwardTyParamBanRibKind => {
// Nothing to do. Continue.
}
ItemRibKind(_) | FnItemRibKind | AssocItemRibKind => {
@@ -2220,7 +2209,7 @@ impl<'a> Resolver<'a> {
let has_generic_params = match rib.kind {
NormalRibKind | AssocItemRibKind |
ModuleRibKind(..) | MacroDefinition(..) | ForwardTyParamBanRibKind |
ConstantItemRibKind | TyParamAsConstParamTy => {
ConstantItemRibKind => {
// Nothing to do. Continue.
continue;
}
11 changes: 11 additions & 0 deletions src/librustc_typeck/collect.rs
Original file line number Diff line number Diff line change
@@ -1532,6 +1532,17 @@ pub fn checked_type_of(tcx: TyCtxt<'_>, def_id: DefId, fail: bool) -> Option<Ty<
);
};
}
if ty::search_for_structural_match_violation(tcx, ty).is_some() {
struct_span_err!(
tcx.sess,
hir_ty.span,
E0741,
"the types of const generic parameters must derive `PartialEq` and `Eq`",
).span_label(
hir_ty.span,
format!("`{}` doesn't derive both `PartialEq` and `Eq`", ty),
).emit();
}
ty
}
x => {
24 changes: 24 additions & 0 deletions src/librustc_typeck/error_codes.rs
Original file line number Diff line number Diff line change
@@ -4978,6 +4978,30 @@ the future, [RFC 2091] prohibits their implementation without a follow-up RFC.
[RFC 2091]: https://github.com/rust-lang/rfcs/blob/master/text/2091-inline-semantic.md
"##,

E0741: r##"
Only `structural_match` types (that is, types that derive `PartialEq` and `Eq`)
may be used as the types of const generic parameters.
```compile_fail,E0741
#![feature(const_generics)]
struct A;
struct B<const X: A>; // error!
```
To fix this example, we derive `PartialEq` and `Eq`.
```
#![feature(const_generics)]
#[derive(PartialEq, Eq)]
struct A;
struct B<const X: A>; // ok!
```
"##,

;
// E0035, merged into E0087/E0089
// E0036, merged into E0087/E0089
1 change: 1 addition & 0 deletions src/test/rustdoc/const-generics/const-impl.rs
Original file line number Diff line number Diff line change
@@ -4,6 +4,7 @@

#![crate_name = "foo"]

#[derive(PartialEq, Eq)]
pub enum Order {
Sorted,
Unsorted,
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::marker::PhantomData;

struct B<T, const N: T>(PhantomData<[T; N]>); //~ ERROR const generics are unstable
//~^ ERROR const parameters cannot depend on type parameters
//~^ ERROR the types of const generic parameters must derive `PartialEq` and `Eq`

fn main() {}
Original file line number Diff line number Diff line change
@@ -1,9 +1,3 @@
error[E0671]: const parameters cannot depend on type parameters
--> $DIR/const-param-type-depends-on-type-param-ungated.rs:3:22
|
LL | struct B<T, const N: T>(PhantomData<[T; N]>);
| ^ const parameter depends on type parameter

error[E0658]: const generics are unstable
--> $DIR/const-param-type-depends-on-type-param-ungated.rs:3:19
|
@@ -13,7 +7,13 @@ LL | struct B<T, const N: T>(PhantomData<[T; N]>);
= note: for more information, see https://github.com/rust-lang/rust/issues/44580
= help: add `#![feature(const_generics)]` to the crate attributes to enable

error[E0741]: the types of const generic parameters must derive `PartialEq` and `Eq`
--> $DIR/const-param-type-depends-on-type-param-ungated.rs:3:22
|
LL | struct B<T, const N: T>(PhantomData<[T; N]>);
| ^ `T` doesn't derive both `PartialEq` and `Eq`

error: aborting due to 2 previous errors

Some errors have detailed explanations: E0658, E0671.
Some errors have detailed explanations: E0658, E0741.
For more information about an error, try `rustc --explain E0658`.
Original file line number Diff line number Diff line change
@@ -7,7 +7,6 @@
// details.

pub struct Dependent<T, const X: T>([(); X]);
//~^ ERROR const parameters cannot depend on type parameters
//~^^ ERROR parameter `T` is never used
//~^ ERROR the types of const generic parameters must derive `PartialEq` and `Eq`

fn main() {}
Original file line number Diff line number Diff line change
@@ -1,9 +1,3 @@
error[E0671]: const parameters cannot depend on type parameters
--> $DIR/const-param-type-depends-on-type-param.rs:9:34
|
LL | pub struct Dependent<T, const X: T>([(); X]);
| ^ const parameter depends on type parameter

warning: the feature `const_generics` is incomplete and may cause the compiler to crash
--> $DIR/const-param-type-depends-on-type-param.rs:1:12
|
@@ -12,15 +6,12 @@ LL | #![feature(const_generics)]
|
= note: `#[warn(incomplete_features)]` on by default

error[E0392]: parameter `T` is never used
--> $DIR/const-param-type-depends-on-type-param.rs:9:22
error[E0741]: the types of const generic parameters must derive `PartialEq` and `Eq`
--> $DIR/const-param-type-depends-on-type-param.rs:9:34
|
LL | pub struct Dependent<T, const X: T>([(); X]);
| ^ unused parameter
|
= help: consider removing `T`, referring to it in a field, or using a marker such as `std::marker::PhantomData`
| ^ `T` doesn't derive both `PartialEq` and `Eq`

error: aborting due to 2 previous errors
error: aborting due to previous error

Some errors have detailed explanations: E0392, E0671.
For more information about an error, try `rustc --explain E0392`.
For more information about this error, try `rustc --explain E0741`.
13 changes: 13 additions & 0 deletions src/test/ui/const-generics/forbid-non-structural_match-types.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#![feature(const_generics)]
//~^ WARN the feature `const_generics` is incomplete and may cause the compiler to crash

#[derive(PartialEq, Eq)]
struct A;

struct B<const X: A>; // ok

struct C;

struct D<const X: C>; //~ ERROR the types of const generic parameters must derive

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
warning: the feature `const_generics` is incomplete and may cause the compiler to crash
--> $DIR/forbid-non-structural_match-types.rs:1:12
|
LL | #![feature(const_generics)]
| ^^^^^^^^^^^^^^
|
= note: `#[warn(incomplete_features)]` on by default

error[E0741]: the types of const generic parameters must derive `PartialEq` and `Eq`
--> $DIR/forbid-non-structural_match-types.rs:11:19
|
LL | struct D<const X: C>;
| ^ `C` doesn't derive both `PartialEq` and `Eq`

error: aborting due to previous error

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