Skip to content

Commit

Permalink
Rewrite with future-compat lint for indirect pattern omitting
Browse files Browse the repository at this point in the history
`#[structural_match]`.

Outline of changes:

 * Recur as deeply as necessary when searching for `#[structural_match]`.

 * `#[structural_match]`: handle case of `const A: & &Wrap(NoDerive)`
   by including the fields of an ADT during traversal of input
   type. (We continue to not traverse the substs of an ADT, though, so
   that we continue to handle `PhantomData<NoDerive>` and `*NoDerive`
   properly.)

 * Refactored code to use `match` instead of `if let`. This ends up
   *with less* right-ward drift by moving the handling of the main
   *`ty::Adt` case *outside* the match.

 * Using lint (rather than hard error) mmeans we need to check that
   type is `PartialEq` to avoid ICE'ing the compiler in scneario where
   MIR codegen dispatches to `PartialEq::eq`. Added said check, and
   fatal error in that case.
  • Loading branch information
pnkfelix committed Jul 8, 2019
1 parent 36777f1 commit b560801
Show file tree
Hide file tree
Showing 2 changed files with 185 additions and 6 deletions.
2 changes: 2 additions & 0 deletions src/librustc_mir/hair/pattern/check_match.rs
Expand Up @@ -170,6 +170,7 @@ impl<'a, 'tcx> MatchVisitor<'a, 'tcx> {
let mut patcx = PatternContext::new(self.tcx,
self.param_env.and(self.identity_substs),
self.tables);
patcx.include_lint_checks();
let pattern = expand_pattern(cx, patcx.lower_pattern(&pat));
if !patcx.errors.is_empty() {
patcx.report_inlining_errors(pat.span);
Expand Down Expand Up @@ -266,6 +267,7 @@ impl<'a, 'tcx> MatchVisitor<'a, 'tcx> {
let mut patcx = PatternContext::new(self.tcx,
self.param_env.and(self.identity_substs),
self.tables);
patcx.include_lint_checks();
let pattern = patcx.lower_pattern(pat);
let pattern_ty = pattern.ty;
let pats: Matrix<'_, '_> = vec![smallvec![
Expand Down
189 changes: 183 additions & 6 deletions src/librustc_mir/hair/pattern/mod.rs
Expand Up @@ -10,9 +10,11 @@ use crate::const_eval::const_variant_index;
use crate::hair::util::UserAnnotatedTyHelpers;
use crate::hair::constant::*;

use rustc::lint;
use rustc::mir::{Field, BorrowKind, Mutability};
use rustc::mir::{UserTypeProjection};
use rustc::mir::interpret::{GlobalId, ConstValue, sign_extend, AllocId, Pointer};
use rustc::traits::{ObligationCause, PredicateObligation};
use rustc::ty::{self, Region, TyCtxt, AdtDef, Ty, UserType, DefIdTree};
use rustc::ty::{CanonicalUserType, CanonicalUserTypeAnnotation, CanonicalUserTypeAnnotations};
use rustc::ty::subst::{SubstsRef, Kind};
Expand All @@ -22,6 +24,7 @@ use rustc::hir::def::{CtorOf, Res, DefKind, CtorKind};
use rustc::hir::pat_util::EnumerateAndAdjustIterator;

use rustc_data_structures::indexed_vec::Idx;
use rustc_data_structures::fx::FxHashSet;

use std::cmp::Ordering;
use std::fmt;
Expand Down Expand Up @@ -332,6 +335,7 @@ pub struct PatternContext<'a, 'tcx> {
pub tables: &'a ty::TypeckTables<'tcx>,
pub substs: SubstsRef<'tcx>,
pub errors: Vec<PatternError>,
include_lint_checks: bool,
}

impl<'a, 'tcx> Pattern<'tcx> {
Expand Down Expand Up @@ -363,10 +367,16 @@ impl<'a, 'tcx> PatternContext<'a, 'tcx> {
param_env: param_env_and_substs.param_env,
tables,
substs: param_env_and_substs.value,
errors: vec![]
errors: vec![],
include_lint_checks: false,
}
}

pub fn include_lint_checks(&mut self) -> &mut Self {
self.include_lint_checks = true;
self
}

pub fn lower_pattern(&mut self, pat: &'tcx hir::Pat) -> Pattern<'tcx> {
// When implicit dereferences have been inserted in this pattern, the unadjusted lowered
// pattern has the type that results *after* dereferencing. For example, in this code:
Expand Down Expand Up @@ -942,23 +952,94 @@ impl<'a, 'tcx> PatternContext<'a, 'tcx> {

/// Converts an evaluated constant to a pattern (if possible).
/// This means aggregate values (like structs and enums) are converted
/// to a pattern that matches the value (as if you'd compared via equality).
/// to a pattern that matches the value (as if you'd compared via structural equality).
fn const_to_pat(
&self,
instance: ty::Instance<'tcx>,
cv: &'tcx ty::Const<'tcx>,
id: hir::HirId,
span: Span,
) -> Pattern<'tcx> {
// This method is just a warpper handling a validity check; the heavy lifting is
// performed by the recursive const_to_pat_inner method, which is not meant to be
// invoked except by this method.
//
// once indirect_structural_match is a full fledged error, this
// level of indirection can be eliminated

debug!("const_to_pat: cv={:#?} id={:?}", cv, id);
let adt_subpattern = |i, variant_opt| {
debug!("const_to_pat: cv.ty={:?} span={:?}", cv.ty, span);

let mut saw_error = false;
let inlined_const_as_pat = self.const_to_pat_inner(instance, cv, id, span, &mut saw_error);

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,
);

// before issuing lint, double-check there even *is* a
// semantic PartialEq for us to dispatch to.
//
// (If there isn't, then we can safely issue a hard
// error, because that's never worked, due to compiler
// using PartialEq::eq in this scenario in the past.)

let ty_is_partial_eq: bool = {
let partial_eq_trait_id = self.tcx.lang_items().eq_trait().unwrap();
let obligation: PredicateObligation<'_> =
self.tcx.predicate_for_trait_def(self.param_env,
ObligationCause::misc(span, id),
partial_eq_trait_id,
0,
cv.ty,
&[]);
self.tcx
.infer_ctxt()
.enter(|infcx| infcx.predicate_may_hold(&obligation))
};

if !ty_is_partial_eq {
// span_fatal avoids ICE from resolution of non-existent method (rare case).
self.tcx.sess.span_fatal(span, &msg);
} else {
self.tcx.lint_hir(lint::builtin::INDIRECT_STRUCTURAL_MATCH, id, span, &msg);
}
}
}

inlined_const_as_pat
}

/// Recursive helper for `const_to_pat`; invoke that (instead of calling this directly).
fn const_to_pat_inner(
&self,
instance: ty::Instance<'tcx>,
cv: &'tcx ty::Const<'tcx>,
id: hir::HirId,
span: Span,
// This tracks if we signal some hard error for a given const
// value, so that we will not subsequently issue an irrelevant
// lint for the same const value.
saw_const_match_error: &mut bool,
) -> Pattern<'tcx> {

let mut adt_subpattern = |i, variant_opt| {
let field = Field::new(i);
let val = crate::const_eval::const_field(
self.tcx, self.param_env, variant_opt, field, cv
);
self.const_to_pat(instance, val, id, span)
self.const_to_pat_inner(instance, val, id, span, saw_const_match_error)
};
let adt_subpatterns = |n, variant_opt| {
let mut adt_subpatterns = |n, variant_opt| {
(0..n).map(|i| {
let field = Field::new(i);
FieldPattern {
Expand All @@ -967,7 +1048,8 @@ impl<'a, 'tcx> PatternContext<'a, 'tcx> {
}
}).collect::<Vec<_>>()
};
debug!("const_to_pat: cv.ty={:?} span={:?}", cv.ty, span);


let kind = match cv.ty.sty {
ty::Float(_) => {
self.tcx.lint_hir(
Expand All @@ -982,9 +1064,11 @@ impl<'a, 'tcx> PatternContext<'a, 'tcx> {
}
ty::Adt(adt_def, _) if adt_def.is_union() => {
// Matching on union fields is unsafe, we can't hide it in constants
*saw_const_match_error = true;
self.tcx.sess.span_err(span, "cannot use unions in constant patterns");
PatternKind::Wild
}
// keep old code until future-compat upgraded to errors.
ty::Adt(adt_def, _) if !self.tcx.has_attr(adt_def.did, sym::structural_match) => {
let path = self.tcx.def_path_str(adt_def.did);
let msg = format!(
Expand All @@ -993,9 +1077,11 @@ impl<'a, 'tcx> PatternContext<'a, 'tcx> {
path,
path,
);
*saw_const_match_error = true;
self.tcx.sess.span_err(span, &msg);
PatternKind::Wild
}
// keep old code until future-compat upgraded to errors.
ty::Ref(_, ty::TyS { sty: ty::Adt(adt_def, _), .. }, _)
if !self.tcx.has_attr(adt_def.did, sym::structural_match) => {
// HACK(estebank): Side-step ICE #53708, but anything other than erroring here
Expand All @@ -1007,6 +1093,7 @@ impl<'a, 'tcx> PatternContext<'a, 'tcx> {
path,
path,
);
*saw_const_match_error = true;
self.tcx.sess.span_err(span, &msg);
PatternKind::Wild
}
Expand Down Expand Up @@ -1058,6 +1145,96 @@ impl<'a, 'tcx> PatternContext<'a, 'tcx> {
}
}

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<&'tcx AdtDef>,
}

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.sty {
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::Array(_, n) if n.assert_usize(self.tcx) == 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.contains(adt_def) {
debug!("Search already seen adt_def: {:?}", adt_def);
// let caller continue its search
return false;
}

self.seen.insert(adt_def);

// `#[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 PatternContext<'_, 'tcx> {
fn tcx(&self) -> TyCtxt<'tcx> {
self.tcx
Expand Down

0 comments on commit b560801

Please sign in to comment.