Skip to content

Commit 6cd977e

Browse files
committed
Prototype code to migrate from #[structural_match] attribute to Structural trait.
As far as I can tell this preserves previous observable stable behavior, *including* the ICE from rust-lang#61188. (In other words, further work is necessary to reap benefits of switching to a trait.)
1 parent cfb6d84 commit 6cd977e

File tree

8 files changed

+169
-18
lines changed

8 files changed

+169
-18
lines changed

src/libcore/marker.rs

+26
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,32 @@ pub trait Unsize<T: ?Sized> {
126126
// Empty.
127127
}
128128

129+
/// Types that can be used in constants within pattern matches.
130+
///
131+
/// Any type that derives `PartialEq` automatically implements this trait.
132+
///
133+
/// If a `const` item contains some type that does not implement this trait,
134+
/// then that type either (1.) does not implement `PartialEq` (which means the
135+
/// constant will not provide that comparison method, which code generation
136+
/// assumes is available), or (2.) it implements *its own* version of
137+
/// `PartialEq` (which we assume does not conform to a structural-equality
138+
/// comparison).
139+
///
140+
/// In either of the two scenarios above, we reject usage of such a constant in
141+
/// a pattern match.
142+
///
143+
/// See also the [structural match RFC][RFC1445], and [issue 63438][] which
144+
/// motivated migrating from attribute-based design to this `Structural` trait.
145+
///
146+
/// [RFC1445]: https://github.com/rust-lang/rfcs/blob/master/text/1445-restrict-constants-in-patterns.md
147+
/// [issue 63438]: https://github.com/rust-lang/rust/issues/63438
148+
#[cfg(not(bootstrap))]
149+
#[unstable(feature = "structural_match", issue = "31434")]
150+
#[lang = "structural_match"]
151+
pub trait Structural {
152+
// Empty.
153+
}
154+
129155
/// Types whose values can be duplicated simply by copying bits.
130156
///
131157
/// By default, variable bindings have 'move semantics.' In other

src/librustc/middle/lang_items.rs

+1
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,7 @@ language_item_table! {
297297

298298
SizedTraitLangItem, "sized", sized_trait, Target::Trait;
299299
UnsizeTraitLangItem, "unsize", unsize_trait, Target::Trait;
300+
StructuralTraitLangItem, "structural_match", structural_trait, Target::Trait;
300301
CopyTraitLangItem, "copy", copy_trait, Target::Trait;
301302
CloneTraitLangItem, "clone", clone_trait, Target::Trait;
302303
SyncTraitLangItem, "sync", sync_trait, Target::Trait;

src/librustc/traits/error_reporting.rs

+3
Original file line numberDiff line numberDiff line change
@@ -1903,6 +1903,9 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
19031903
ObligationCauseCode::ConstSized => {
19041904
err.note("constant expressions must have a statically known size");
19051905
}
1906+
ObligationCauseCode::ConstPatternStructural => {
1907+
err.note("constants used for pattern-matching must derive `PartialEq` and `Eq`");
1908+
}
19061909
ObligationCauseCode::SharedStatic => {
19071910
err.note("shared static variables must have a type that implements `Sync`");
19081911
}

src/librustc/traits/mod.rs

+3
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,9 @@ pub enum ObligationCauseCode<'tcx> {
236236
/// Computing common supertype in the pattern guard for the arms of a match expression
237237
MatchExpressionArmPattern { span: Span, ty: Ty<'tcx> },
238238

239+
/// Constants in patterns must have `Structural` type.
240+
ConstPatternStructural,
241+
239242
/// Computing common supertype in an if expression
240243
IfExpression(Box<IfExpressionCause>),
241244

src/librustc/traits/structural_impls.rs

+1
Original file line numberDiff line numberDiff line change
@@ -493,6 +493,7 @@ impl<'a, 'tcx> Lift<'tcx> for traits::ObligationCauseCode<'a> {
493493
super::RepeatVec => Some(super::RepeatVec),
494494
super::FieldSized { adt_kind, last } => Some(super::FieldSized { adt_kind, last }),
495495
super::ConstSized => Some(super::ConstSized),
496+
super::ConstPatternStructural => Some(super::ConstPatternStructural),
496497
super::SharedStatic => Some(super::SharedStatic),
497498
super::BuiltinDerivedObligation(ref cause) => {
498499
tcx.lift(cause).map(super::BuiltinDerivedObligation)

src/librustc_mir/hair/pattern/mod.rs

+39-16
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use rustc::lint;
1414
use rustc::mir::{Field, BorrowKind, Mutability};
1515
use rustc::mir::{UserTypeProjection};
1616
use rustc::mir::interpret::{GlobalId, ConstValue, get_slice_bytes, sign_extend};
17+
use rustc::traits::{self, ConstPatternStructural, TraitEngine};
1718
use rustc::traits::{ObligationCause, PredicateObligation};
1819
use rustc::ty::{self, Region, TyCtxt, AdtDef, Ty, UserType, DefIdTree};
1920
use rustc::ty::{CanonicalUserType, CanonicalUserTypeAnnotation, CanonicalUserTypeAnnotations};
@@ -1003,7 +1004,9 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
10031004
if self.include_lint_checks && !saw_error {
10041005
// If we were able to successfully convert the const to some pat, double-check
10051006
// that the type of the const obeys `#[structural_match]` constraint.
1006-
if let Some(adt_def) = search_for_adt_without_structural_match(self.tcx, cv.ty) {
1007+
if let Some(adt_def) =
1008+
search_for_adt_without_structural_match(self.tcx, cv.ty, id, span)
1009+
{
10071010

10081011
let path = self.tcx.def_path_str(adt_def.did);
10091012
let msg = format!(
@@ -1173,8 +1176,8 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
11731176
}
11741177

11751178
/// This method traverses the structure of `ty`, trying to find an
1176-
/// instance of an ADT (i.e. struct or enum) that was declared without
1177-
/// the `#[structural_match]` attribute.
1179+
/// instance of an ADT (i.e. struct or enum) that does not implement
1180+
/// the `Structural` trait.
11781181
///
11791182
/// The "structure of a type" includes all components that would be
11801183
/// considered when doing a pattern match on a constant of that
@@ -1188,31 +1191,35 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
11881191
/// instantiated generic like `PhantomData<T>`.
11891192
///
11901193
/// The reason we do this search is Rust currently require all ADT's
1191-
/// reachable from a constant's type to be annotated with
1192-
/// `#[structural_match]`, an attribute which essentially says that
1193-
/// the implementation of `PartialEq::eq` behaves *equivalently* to a
1194-
/// comparison against the unfolded structure.
1194+
/// reachable from a constant's type to implement `Structural`, a
1195+
/// trait which essentially says that the implementation of
1196+
/// `PartialEq::eq` behaves *equivalently* to a comparison against
1197+
/// the unfolded structure.
11951198
///
11961199
/// For more background on why Rust has this requirement, and issues
11971200
/// that arose when the requirement was not enforced completely, see
11981201
/// Rust RFC 1445, rust-lang/rust#61188, and rust-lang/rust#62307.
11991202
fn search_for_adt_without_structural_match<'tcx>(tcx: TyCtxt<'tcx>,
1200-
ty: Ty<'tcx>)
1203+
ty: Ty<'tcx>,
1204+
id: hir::HirId,
1205+
span: Span)
12011206
-> Option<&'tcx AdtDef>
12021207
{
12031208
// Import here (not mod level), because `TypeFoldable::fold_with`
12041209
// conflicts with `PatternFoldable::fold_with`
12051210
use crate::rustc::ty::fold::TypeVisitor;
12061211
use crate::rustc::ty::TypeFoldable;
12071212

1208-
let mut search = Search { tcx, found: None, seen: FxHashSet::default() };
1213+
let mut search = Search { tcx, id, span, found: None, seen: FxHashSet::default() };
12091214
ty.visit_with(&mut search);
12101215
return search.found;
12111216

12121217
struct Search<'tcx> {
12131218
tcx: TyCtxt<'tcx>,
1219+
id: hir::HirId,
1220+
span: Span,
12141221

1215-
// records the first ADT we find without `#[structural_match`
1222+
// records the first ADT we find that does not implement `Structural`.
12161223
found: Option<&'tcx AdtDef>,
12171224

12181225
// tracks ADT's previously encountered during search, so that
@@ -1251,18 +1258,34 @@ fn search_for_adt_without_structural_match<'tcx>(tcx: TyCtxt<'tcx>,
12511258
}
12521259
};
12531260

1254-
if !self.tcx.has_attr(adt_def.did, sym::structural_match) {
1255-
self.found = Some(&adt_def);
1256-
debug!("Search found adt_def: {:?}", adt_def);
1257-
return true // Halt visiting!
1258-
}
1259-
12601261
if self.seen.contains(adt_def) {
12611262
debug!("Search already seen adt_def: {:?}", adt_def);
12621263
// let caller continue its search
12631264
return false;
12641265
}
12651266

1267+
let non_structural = self.tcx.infer_ctxt().enter(|infcx| {
1268+
let cause = ObligationCause::new(self.span, self.id, ConstPatternStructural);
1269+
let mut fulfillment_cx = traits::FulfillmentContext::new();
1270+
let structural_def_id = self.tcx.lang_items().structural_trait().unwrap();
1271+
fulfillment_cx.register_bound(
1272+
&infcx, ty::ParamEnv::empty(), ty, structural_def_id, cause);
1273+
if let Err(_err) = fulfillment_cx.select_all_or_error(&infcx) {
1274+
// initial prototype: don't report any trait fulfillment error here.
1275+
//
1276+
// infcx.report_fulfillment_errors(&err, None, false);
1277+
true
1278+
} else {
1279+
false
1280+
}
1281+
});
1282+
1283+
if non_structural {
1284+
debug!("Search found ty: {:?}", ty);
1285+
self.found = Some(&adt_def);
1286+
return true // Halt visiting!
1287+
}
1288+
12661289
self.seen.insert(adt_def);
12671290

12681291
// `#[structural_match]` does not care about the

src/libsyntax_ext/deriving/cmp/partial_eq.rs

+93-2
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@ use crate::deriving::{path_local, path_std};
22
use crate::deriving::generic::*;
33
use crate::deriving::generic::ty::*;
44

5-
use syntax::ast::{BinOpKind, Expr, MetaItem};
5+
use syntax::ast::{self, BinOpKind, Expr, ItemKind, MetaItem};
66
use syntax::ext::base::{Annotatable, ExtCtxt, SpecialDerives};
77
use syntax::ptr::P;
88
use syntax::symbol::sym;
9-
use syntax_pos::Span;
9+
use syntax_pos::{self, Span};
1010

1111
pub fn expand_deriving_partial_eq(cx: &mut ExtCtxt<'_>,
1212
span: Span,
@@ -81,6 +81,8 @@ pub fn expand_deriving_partial_eq(cx: &mut ExtCtxt<'_>,
8181
} }
8282
}
8383

84+
inject_impl_of_structural_trait(cx, span, item, push);
85+
8486
// avoid defining `ne` if we can
8587
// c-like enums, enums without any fields and structs without fields
8688
// can safely define only `eq`.
@@ -102,3 +104,92 @@ pub fn expand_deriving_partial_eq(cx: &mut ExtCtxt<'_>,
102104
};
103105
trait_def.expand(cx, mitem, item, push)
104106
}
107+
108+
// Injects `impl<...> Structural for ItemType<...> { }`. In particular,
109+
// does *not* add `where T: Structural` for parameters `T` in `...`.
110+
// (That's the main reason we cannot use TraitDef here.)
111+
fn inject_impl_of_structural_trait(cx: &mut ExtCtxt<'_>,
112+
span: Span,
113+
item: &Annotatable,
114+
push: &mut dyn FnMut(Annotatable)) {
115+
116+
// set the expn ID so we can use the unstable trait.
117+
let span = span.fresh_expansion(syntax_pos::ExpnData::allow_unstable(
118+
syntax_pos::ExpnKind::Macro(syntax_pos::MacroKind::Derive, sym::PartialEq),
119+
span,
120+
cx.parse_sess.edition,
121+
[sym::structural_match][..].into()));
122+
123+
let item = match *item {
124+
Annotatable::Item(ref item) => item,
125+
_ => {
126+
// Non-Item derive is an error, but it should have been
127+
// set earlier; see
128+
// libsyntax/ext/expand.rs:MacroExpander::expand()
129+
return;
130+
}
131+
};
132+
133+
let generics = match item.kind {
134+
ItemKind::Struct(_, ref generics) => generics,
135+
ItemKind::Enum(_, ref generics) => generics,
136+
// Do not inject `impl Structural for Union`. (`PartialEq` does not
137+
// support unions, so we will see error downstream.)
138+
ItemKind::Union(..) => return,
139+
_ => unreachable!(),
140+
};
141+
142+
// Create generics param list for where clauses and impl headers
143+
let mut generics = generics.clone();
144+
145+
// Create the type of `self`.
146+
//
147+
// in addition, remove defaults from type params (impls cannot have them).
148+
let self_params: Vec<_> = generics.params.iter_mut().map(|param| match &mut param.kind {
149+
ast::GenericParamKind::Lifetime => {
150+
ast::GenericArg::Lifetime(cx.lifetime(span, param.ident))
151+
}
152+
ast::GenericParamKind::Type { default } => {
153+
*default = None;
154+
ast::GenericArg::Type(cx.ty_ident(span, param.ident))
155+
}
156+
ast::GenericParamKind::Const { ty: _ } => {
157+
ast::GenericArg::Const(cx.const_ident(span, param.ident))
158+
}
159+
}).collect();
160+
161+
let type_ident = item.ident;
162+
163+
let structural_path = path_std!(cx, marker::Structural);
164+
let trait_ref = cx.trait_ref(structural_path.to_path(cx, span, type_ident, &generics));
165+
let self_type = cx.ty_path(cx.path_all(span, false, vec![type_ident], self_params));
166+
167+
// It would be nice to also encode constraint `where Self: Eq` (by adding it
168+
// onto `generics` cloned above). Unfortunately, that strategy runs afoul of
169+
// rust-lang/rust#48214. So we perform that additional check in the compiler
170+
// itself, instead of encoding it here.
171+
172+
// Keep the lint and stability attributes of the original item, to control
173+
// how the generated implementation is linted.
174+
let mut attrs = Vec::new();
175+
attrs.extend(item.attrs
176+
.iter()
177+
.filter(|a| {
178+
[sym::allow, sym::warn, sym::deny, sym::forbid, sym::stable, sym::unstable]
179+
.contains(&a.name_or_empty())
180+
})
181+
.cloned());
182+
183+
let newitem = cx.item(span,
184+
ast::Ident::invalid(),
185+
attrs,
186+
ItemKind::Impl(ast::Unsafety::Normal,
187+
ast::ImplPolarity::Positive,
188+
ast::Defaultness::Final,
189+
generics,
190+
Some(trait_ref),
191+
self_type,
192+
Vec::new()));
193+
194+
push(Annotatable::Item(newitem));
195+
}

src/test/ui/rfc1445/feature-gate.rs

+3
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,6 @@ fn main() { //[with_gate]~ ERROR compilation successful
2525
_ => { }
2626
}
2727
}
28+
29+
#[cfg(with_gate)]
30+
impl std::marker::Structural for Foo { }

0 commit comments

Comments
 (0)