Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Rewrite with future-compat lint for indirect pattern omitting
`#[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
commit b56080162bc45e1cfd561b0a21e50b827c9df679
2 changes: 2 additions & 0 deletions src/librustc_mir/hair/pattern/check_match.rs
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Indentation is weird here, should be:

fn search_for_adt_without_structural_match<'tcx>(
    tcx: TyCtxt<'tcx>,
    ty: Ty<'tcx>,
) -> Option<&'tcx AdtDef> {
    ...

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