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

Forbid non-structural_match types in const generics #65627

Merged
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
127 changes: 125 additions & 2 deletions src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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};

Expand Down Expand Up @@ -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);
Expand Down
7 changes: 3 additions & 4 deletions src/librustc/ty/relate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!
Expand Down
144 changes: 15 additions & 129 deletions src/librustc_mir/hair/pattern/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
10 changes: 0 additions & 10 deletions src/librustc_resolve/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}

Expand Down
7 changes: 4 additions & 3 deletions src/librustc_resolve/error_codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
```
Expand Down
Loading