Skip to content

Expr walking structural match #66120

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

Closed
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
2 changes: 1 addition & 1 deletion src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ declare_lint! {
declare_lint! {
pub INDIRECT_STRUCTURAL_MATCH,
// defaulting to allow until rust-lang/rust#62614 is fixed.
Allow,
Warn,
"pattern with const indirectly referencing non-`#[structural_match]` type",
@future_incompatible = FutureIncompatibleInfo {
reference: "issue #62411 <https://github.com/rust-lang/rust/issues/62411>",
Expand Down
4 changes: 3 additions & 1 deletion src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,10 @@ pub use self::context::{

pub use self::instance::{Instance, InstanceDef};

pub use self::structural_match::search_for_structural_match_violation;
pub use self::structural_match::search_const_rhs_for_structural_match_violation;
pub use self::structural_match::search_type_for_structural_match_violation;
pub use self::structural_match::type_marked_structural;
pub use self::structural_match::report_structural_match_violation;
pub use self::structural_match::NonStructuralMatchTy;

pub use self::trait_def::TraitDef;
Expand Down
319 changes: 298 additions & 21 deletions src/librustc/ty/structural_match.rs

Large diffs are not rendered by default.

132 changes: 64 additions & 68 deletions src/librustc_mir/hair/pattern/const_to_pat.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::const_eval::const_variant_index;

use rustc::hir;
use rustc::lint;
use rustc::hir::def_id::DefId;
use rustc::mir::Field;
use rustc::infer::InferCtxt;
use rustc::traits::{ObligationCause, PredicateObligation};
Expand All @@ -20,18 +20,22 @@ impl<'a, 'tcx> PatCtxt<'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 structural equality).
///
/// For literals, pass `None` as the `opt_const_def_id`; for a const
/// identifier, pass its `DefId`.
pub(super) fn const_to_pat(
&self,
cv: &'tcx ty::Const<'tcx>,
opt_const_def_id: Option<DefId>,
id: hir::HirId,
span: Span,
) -> Pat<'tcx> {
debug!("const_to_pat: cv={:#?} id={:?}", cv, id);
debug!("const_to_pat: cv.ty={:?} span={:?}", cv.ty, span);
debug!("const_def_to_pat: cv={:#?} const_def_id: {:?} id={:?}", cv, opt_const_def_id, id);
debug!("const_def_to_pat: cv.ty={:?} span={:?}", cv.ty, span);

self.tcx.infer_ctxt().enter(|infcx| {
let mut convert = ConstToPat::new(self, id, span, infcx);
convert.to_pat(cv)
convert.to_pat(cv, opt_const_def_id)
})
}
}
Expand Down Expand Up @@ -67,85 +71,77 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> {

fn tcx(&self) -> TyCtxt<'tcx> { self.infcx.tcx }

fn search_for_structural_match_violation(&self,
ty: Ty<'tcx>)
-> Option<ty::NonStructuralMatchTy<'tcx>>
fn search_const_def_for_structural_match_violation(&self, const_def_id: DefId)
{
ty::search_for_structural_match_violation(self.id, self.span, self.tcx(), ty)
assert!(const_def_id.is_local());
self.tcx().infer_ctxt().enter(|infcx| {
ty::search_const_rhs_for_structural_match_violation(
&infcx, self.param_env, const_def_id, self.id, self.span);
});
}

fn search_ty_for_structural_match_violation(&self, ty: Ty<'tcx>)
{
let structural = ty::search_type_for_structural_match_violation(
self.id, self.span, self.tcx(), ty);
debug!("search_ty_for_structural_match_violation ty: {:?} returned: {:?}", ty, structural);
if let Some(non_sm_ty) = structural {

// double-check there even *is* a semantic `PartialEq` 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.)
//
// Note: To fix rust-lang/rust#65466, one could lift this check
// *before* any structural-match checking, and unconditionally error
// if `PartialEq` is not implemented. However, that breaks stable
// code at the moment, because types like `for <'a> fn(&'a ())` do
// not *yet* implement `PartialEq`. So for now we leave this here.
let warn_instead_of_hard_error: 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(self.span, self.id),
partial_eq_trait_id,
0,
ty,
&[]);
// FIXME: should this call a `predicate_must_hold` variant instead?
self.infcx.predicate_may_hold(&obligation)
};

debug!("call report_structural_match_violation non_sm_ty: {:?} id: {:?} warn: {:?}",
non_sm_ty, self.id, warn_instead_of_hard_error);
ty::report_structural_match_violation(
self.tcx(), non_sm_ty, self.id, self.span, warn_instead_of_hard_error);
}
}

fn type_marked_structural(&self, ty: Ty<'tcx>) -> bool {
ty::type_marked_structural(self.id, self.span, &self.infcx, ty)
}

fn to_pat(&mut self, cv: &'tcx ty::Const<'tcx>) -> Pat<'tcx> {
// This method is just a wrapper handling a validity check; the heavy lifting is
// performed by the recursive `recur` 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

fn to_pat(&mut self,
cv: &'tcx ty::Const<'tcx>,
opt_const_def_id: Option<DefId>)
-> Pat<'tcx>
{
let inlined_const_as_pat = self.recur(cv);

if self.include_lint_checks && !self.saw_const_match_error.get() {
// If we were able to successfully convert the const to some pat,
// double-check that all types in the const implement `Structural`.

let structural = self.search_for_structural_match_violation(cv.ty);
debug!("search_for_structural_match_violation cv.ty: {:?} returned: {:?}",
cv.ty, structural);
if let Some(non_sm_ty) = structural {
let adt_def = match non_sm_ty {
ty::NonStructuralMatchTy::Adt(adt_def) => adt_def,
ty::NonStructuralMatchTy::Param =>
bug!("use of constant whose type is a parameter inside a pattern"),
};
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,
);

// double-check there even *is* a semantic `PartialEq` 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.)
//
// Note: To fix rust-lang/rust#65466, one could lift this check
// *before* any structural-match checking, and unconditionally error
// if `PartialEq` is not implemented. However, that breaks stable
// code at the moment, because types like `for <'a> fn(&'a ())` do
// not *yet* implement `PartialEq`. So for now we leave this here.
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(self.span, self.id),
partial_eq_trait_id,
0,
cv.ty,
&[]);
// FIXME: should this call a `predicate_must_hold` variant instead?
self.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(self.span, &msg);
} else {
self.tcx().lint_hir(lint::builtin::INDIRECT_STRUCTURAL_MATCH,
self.id,
self.span,
&msg);
match opt_const_def_id {
Some(const_def_id) if const_def_id.is_local() => {
self.search_const_def_for_structural_match_violation(const_def_id);
}
_ => {
self.search_ty_for_structural_match_violation(cv.ty);
}
}
}

inlined_const_as_pat
}

Expand Down
7 changes: 4 additions & 3 deletions src/librustc_mir/hair/pattern/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -863,7 +863,8 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
};
match self.tcx.at(span).const_eval(self.param_env.and(cid)) {
Ok(value) => {
let pattern = self.const_to_pat(value, id, span);
let const_def_id = Some(instance.def_id());
let pattern = self.const_to_pat(value, const_def_id, id, span);
if !is_associated_const {
return pattern;
}
Expand Down Expand Up @@ -930,7 +931,7 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
let ty = self.tables.expr_ty(expr);
match lit_to_const(&lit.node, self.tcx, ty, false) {
Ok(val) => {
*self.const_to_pat(val, expr.hir_id, lit.span).kind
*self.const_to_pat(val, None, expr.hir_id, lit.span).kind
},
Err(LitToConstError::UnparseableFloat) => {
self.errors.push(PatternError::FloatBug);
Expand All @@ -948,7 +949,7 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
};
match lit_to_const(&lit.node, self.tcx, ty, true) {
Ok(val) => {
*self.const_to_pat(val, expr.hir_id, lit.span).kind
*self.const_to_pat(val, None, expr.hir_id, lit.span).kind
},
Err(LitToConstError::UnparseableFloat) => {
self.errors.push(PatternError::FloatBug);
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1532,7 +1532,7 @@ pub fn checked_type_of(tcx: TyCtxt<'_>, def_id: DefId, fail: bool) -> Option<Ty<
);
};
}
if ty::search_for_structural_match_violation(
if ty::search_type_for_structural_match_violation(
param.hir_id, param.span, tcx, ty).is_some()
{
struct_span_err!(
Expand Down
66 changes: 66 additions & 0 deletions src/test/ui/consts/const_in_pattern/accept_structural.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// run-pass

#![warn(indirect_structural_match)]

// This test is checking our logic for structural match checking by enumerating
// the different kinds of const expressions. This test is collecting cases where
// we have accepted the const expression as a pattern in the past and wish to
// continue doing so.
//
// Even if a non-structural-match type is part of an expression in a const's
// definition, that does not necessarily disqualify the const from being a match
// pattern: in principle, we just need the types involved in the final value to
// be structurally matchable.

// See also RFC 1445

#![feature(type_ascription)]

#[derive(Copy, Clone, Debug)]
struct NoPartialEq(u32);

#[derive(Copy, Clone, Debug)]
struct NoDerive(u32);

// This impl makes `NoDerive` irreflexive.
impl PartialEq for NoDerive { fn eq(&self, _: &Self) -> bool { false } }
impl Eq for NoDerive { }

type OND = Option<NoDerive>;

fn main() {
const FIELD1: u32 = NoPartialEq(1).0;
match 1 { FIELD1 => dbg!(FIELD1), _ => panic!("whoops"), };
const FIELD2: u32 = NoDerive(1).0;
match 1 { FIELD2 => dbg!(FIELD2), _ => panic!("whoops"), };

enum CLike { One = 1, #[allow(dead_code)] Two = 2, }
const ONE_CAST: u32 = CLike::One as u32;
match 1 { ONE_CAST => dbg!(ONE_CAST), _ => panic!("whoops"), };

const NO_DERIVE_NONE: OND = None;
const INDIRECT: OND = NO_DERIVE_NONE;
match None { INDIRECT => dbg!(INDIRECT), _ => panic!("whoops"), };

const TUPLE: (OND, OND) = (None, None);
match (None, None) { TUPLE => dbg!(TUPLE), _ => panic!("whoops"), };

const TYPE: OND = None: OND;
match None { TYPE => dbg!(TYPE), _ => panic!("whoops"), };

const ARRAY: [OND; 2] = [None, None];
match [None; 2] { ARRAY => dbg!(ARRAY), _ => panic!("whoops"), };

const REPEAT: [OND; 2] = [None; 2];
match [None, None] { REPEAT => dbg!(REPEAT), _ => panic!("whoops"), };

trait Trait: Sized { const ASSOC: Option<Self>; }
impl Trait for NoDerive { const ASSOC: Option<NoDerive> = None; }
match None { NoDerive::ASSOC => dbg!(NoDerive::ASSOC), _ => panic!("whoops"), };

const BLOCK: OND = { NoDerive(10); None };
match None { BLOCK => dbg!(BLOCK), _ => panic!("whoops"), };

const ADDR_OF: &OND = &None;
match &None { ADDR_OF => dbg!(ADDR_OF), _ => panic!("whoops"), };
}
32 changes: 32 additions & 0 deletions src/test/ui/consts/const_in_pattern/reject_non_partial_eq.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// This test is illustrating the difference between how failing to derive
// `PartialEq` is handled compared to failing to implement it at all.

// See also RFC 1445

#[derive(PartialEq, Eq)]
struct Structural(u32);

struct NoPartialEq(u32);

struct NoDerive(u32);

// This impl makes NoDerive irreflexive.
impl PartialEq for NoDerive { fn eq(&self, _: &Self) -> bool { false } }

impl Eq for NoDerive { }

const NO_DERIVE_NONE: Option<NoDerive> = None;
const NO_PARTIAL_EQ_NONE: Option<NoPartialEq> = None;

fn main() {
match None {
NO_DERIVE_NONE => println!("NO_DERIVE_NONE"),
_ => panic!("whoops"),
}

match None {
NO_PARTIAL_EQ_NONE => println!("NO_PARTIAL_EQ_NONE"),
//~^ ERROR must be annotated with `#[derive(PartialEq, Eq)]`
_ => panic!("whoops"),
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
error: to use a constant of type `NoPartialEq` in a pattern, `NoPartialEq` must be annotated with `#[derive(PartialEq, Eq)]`
--> $DIR/reject_non_partial_eq.rs:28:9
|
LL | NO_PARTIAL_EQ_NONE => println!("NO_PARTIAL_EQ_NONE"),
| ^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

Loading