Skip to content

Commit 93f7fe3

Browse files
committed
Auto merge of #24270 - pnkfelix:use-disr-val-for-derive-ord, r=brson
Use `discriminant_value` intrinsic for `derive(PartialOrd)` [breaking-change] This is a [breaking-change] because it can change the result of comparison operators when enum discriminants have been explicitly assigned. Notably in a case like: ```rust #[derive(PartialOrd)] enum E { A = 2, B = 1} ``` Under the old deriving, `A < B` held, because `A` came before `B` in the order of declaration. But now we use the ordering according to the provided values, and thus `A > B`. (However, this change is very unlikely to break much, if any, code, since the orderings themselves should all remain well-defined, total, etc.) Fix #15523
2 parents c897ac0 + 05aaad1 commit 93f7fe3

File tree

10 files changed

+264
-33
lines changed

10 files changed

+264
-33
lines changed

src/libcore/intrinsics.rs

+6
Original file line numberDiff line numberDiff line change
@@ -569,4 +569,10 @@ extern "rust-intrinsic" {
569569
pub fn overflowing_sub<T>(a: T, b: T) -> T;
570570
/// Returns (a * b) mod 2^N, where N is the width of N in bits.
571571
pub fn overflowing_mul<T>(a: T, b: T) -> T;
572+
573+
/// Returns the value of the discriminant for the variant in 'v',
574+
/// cast to a `u64`; if `T` has no discriminant, returns 0.
575+
// SNAP 5520801
576+
#[cfg(not(stage0))]
577+
pub fn discriminant_value<T>(v: &T) -> u64;
572578
}

src/libcore/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ mod tuple;
157157

158158
#[doc(hidden)]
159159
mod core {
160+
pub use intrinsics;
160161
pub use panicking;
161162
pub use fmt;
162163
pub use clone;

src/librustc_trans/trans/intrinsic.rs

+12
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use llvm;
1414
use llvm::{SequentiallyConsistent, Acquire, Release, AtomicXchg, ValueRef, TypeKind};
1515
use middle::subst;
1616
use middle::subst::FnSpace;
17+
use trans::adt;
1718
use trans::base::*;
1819
use trans::build::*;
1920
use trans::callee;
@@ -683,6 +684,17 @@ pub fn trans_intrinsic_call<'a, 'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>,
683684
}
684685
}
685686

687+
(_, "discriminant_value") => {
688+
let val_ty = substs.types.get(FnSpace, 0);
689+
match val_ty.sty {
690+
ty::ty_enum(..) => {
691+
let repr = adt::represent_type(ccx, *val_ty);
692+
adt::trans_get_discr(bcx, &*repr, llargs[0], Some(llret_ty))
693+
}
694+
_ => C_null(llret_ty)
695+
}
696+
}
697+
686698
// This requires that atomic intrinsics follow a specific naming pattern:
687699
// "atomic_<operation>[_<ordering>]", and no ordering means SeqCst
688700
(_, name) if name.starts_with("atomic_") => {

src/librustc_typeck/check/mod.rs

+6
Original file line numberDiff line numberDiff line change
@@ -5073,6 +5073,12 @@ pub fn check_intrinsic_type(ccx: &CrateCtxt, it: &ast::ForeignItem) {
50735073

50745074
"assume" => (0, vec![tcx.types.bool], ty::mk_nil(tcx)),
50755075

5076+
"discriminant_value" => (1, vec![
5077+
ty::mk_imm_rptr(tcx,
5078+
tcx.mk_region(ty::ReLateBound(ty::DebruijnIndex::new(1),
5079+
ty::BrAnon(0))),
5080+
param(ccx, 0))], tcx.types.u64),
5081+
50765082
ref other => {
50775083
span_err!(tcx.sess, it.span, E0093,
50785084
"unrecognized intrinsic function: `{}`", *other);

src/libsyntax/ext/deriving/generic/mod.rs

+63-29
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,7 @@ use ext::base::ExtCtxt;
201201
use ext::build::AstBuilder;
202202
use codemap::{self, DUMMY_SP};
203203
use codemap::Span;
204+
use diagnostic::SpanHandler;
204205
use fold::MoveMap;
205206
use owned_slice::OwnedSlice;
206207
use parse::token::InternedString;
@@ -391,6 +392,7 @@ impl<'a> TraitDef<'a> {
391392
ast::ItemEnum(ref enum_def, ref generics) => {
392393
self.expand_enum_def(cx,
393394
enum_def,
395+
&item.attrs[..],
394396
item.ident,
395397
generics)
396398
}
@@ -653,6 +655,7 @@ impl<'a> TraitDef<'a> {
653655
fn expand_enum_def(&self,
654656
cx: &mut ExtCtxt,
655657
enum_def: &EnumDef,
658+
type_attrs: &[ast::Attribute],
656659
type_ident: Ident,
657660
generics: &Generics) -> P<ast::Item> {
658661
let mut field_tys = Vec::new();
@@ -687,6 +690,7 @@ impl<'a> TraitDef<'a> {
687690
method_def.expand_enum_method_body(cx,
688691
self,
689692
enum_def,
693+
type_attrs,
690694
type_ident,
691695
self_args,
692696
&nonself_args[..])
@@ -706,13 +710,30 @@ impl<'a> TraitDef<'a> {
706710
}
707711
}
708712

709-
fn variant_to_pat(cx: &mut ExtCtxt, sp: Span, enum_ident: ast::Ident, variant: &ast::Variant)
710-
-> P<ast::Pat> {
711-
let path = cx.path(sp, vec![enum_ident, variant.node.name]);
712-
cx.pat(sp, match variant.node.kind {
713-
ast::TupleVariantKind(..) => ast::PatEnum(path, None),
714-
ast::StructVariantKind(..) => ast::PatStruct(path, Vec::new(), true),
715-
})
713+
fn find_repr_type_name(diagnostic: &SpanHandler,
714+
type_attrs: &[ast::Attribute]) -> &'static str {
715+
let mut repr_type_name = "i32";
716+
for a in type_attrs {
717+
for r in &attr::find_repr_attrs(diagnostic, a) {
718+
repr_type_name = match *r {
719+
attr::ReprAny | attr::ReprPacked => continue,
720+
attr::ReprExtern => "i32",
721+
722+
attr::ReprInt(_, attr::SignedInt(ast::TyIs)) => "isize",
723+
attr::ReprInt(_, attr::SignedInt(ast::TyI8)) => "i8",
724+
attr::ReprInt(_, attr::SignedInt(ast::TyI16)) => "i16",
725+
attr::ReprInt(_, attr::SignedInt(ast::TyI32)) => "i32",
726+
attr::ReprInt(_, attr::SignedInt(ast::TyI64)) => "i64",
727+
728+
attr::ReprInt(_, attr::UnsignedInt(ast::TyUs)) => "usize",
729+
attr::ReprInt(_, attr::UnsignedInt(ast::TyU8)) => "u8",
730+
attr::ReprInt(_, attr::UnsignedInt(ast::TyU16)) => "u16",
731+
attr::ReprInt(_, attr::UnsignedInt(ast::TyU32)) => "u32",
732+
attr::ReprInt(_, attr::UnsignedInt(ast::TyU64)) => "u64",
733+
}
734+
}
735+
}
736+
repr_type_name
716737
}
717738

718739
impl<'a> MethodDef<'a> {
@@ -983,12 +1004,13 @@ impl<'a> MethodDef<'a> {
9831004
cx: &mut ExtCtxt,
9841005
trait_: &TraitDef,
9851006
enum_def: &EnumDef,
1007+
type_attrs: &[ast::Attribute],
9861008
type_ident: Ident,
9871009
self_args: Vec<P<Expr>>,
9881010
nonself_args: &[P<Expr>])
9891011
-> P<Expr> {
9901012
self.build_enum_match_tuple(
991-
cx, trait_, enum_def, type_ident, self_args, nonself_args)
1013+
cx, trait_, enum_def, type_attrs, type_ident, self_args, nonself_args)
9921014
}
9931015

9941016

@@ -1022,6 +1044,7 @@ impl<'a> MethodDef<'a> {
10221044
cx: &mut ExtCtxt,
10231045
trait_: &TraitDef,
10241046
enum_def: &EnumDef,
1047+
type_attrs: &[ast::Attribute],
10251048
type_ident: Ident,
10261049
self_args: Vec<P<Expr>>,
10271050
nonself_args: &[P<Expr>]) -> P<Expr> {
@@ -1044,8 +1067,8 @@ impl<'a> MethodDef<'a> {
10441067
.collect::<Vec<ast::Ident>>();
10451068

10461069
// The `vi_idents` will be bound, solely in the catch-all, to
1047-
// a series of let statements mapping each self_arg to a usize
1048-
// corresponding to its variant index.
1070+
// a series of let statements mapping each self_arg to an int
1071+
// value corresponding to its discriminant.
10491072
let vi_idents: Vec<ast::Ident> = self_arg_names.iter()
10501073
.map(|name| { let vi_suffix = format!("{}_vi", &name[..]);
10511074
cx.ident_of(&vi_suffix[..]) })
@@ -1160,33 +1183,44 @@ impl<'a> MethodDef<'a> {
11601183
// unreachable-pattern error.
11611184
//
11621185
if variants.len() > 1 && self_args.len() > 1 {
1163-
let arms: Vec<ast::Arm> = variants.iter().enumerate()
1164-
.map(|(index, variant)| {
1165-
let pat = variant_to_pat(cx, sp, type_ident, &**variant);
1166-
let lit = ast::LitInt(index as u64, ast::UnsignedIntLit(ast::TyUs));
1167-
cx.arm(sp, vec![pat], cx.expr_lit(sp, lit))
1168-
}).collect();
1169-
11701186
// Build a series of let statements mapping each self_arg
1171-
// to a usize corresponding to its variant index.
1187+
// to its discriminant value. If this is a C-style enum
1188+
// with a specific repr type, then casts the values to
1189+
// that type. Otherwise casts to `i32` (the default repr
1190+
// type).
1191+
//
11721192
// i.e. for `enum E<T> { A, B(1), C(T, T) }`, and a deriving
11731193
// with three Self args, builds three statements:
11741194
//
11751195
// ```
1176-
// let __self0_vi = match self {
1177-
// A => 0, B(..) => 1, C(..) => 2
1178-
// };
1179-
// let __self1_vi = match __arg1 {
1180-
// A => 0, B(..) => 1, C(..) => 2
1181-
// };
1182-
// let __self2_vi = match __arg2 {
1183-
// A => 0, B(..) => 1, C(..) => 2
1184-
// };
1196+
// let __self0_vi = unsafe {
1197+
// std::intrinsics::discriminant_value(&self) } as i32;
1198+
// let __self1_vi = unsafe {
1199+
// std::intrinsics::discriminant_value(&__arg1) } as i32;
1200+
// let __self2_vi = unsafe {
1201+
// std::intrinsics::discriminant_value(&__arg2) } as i32;
11851202
// ```
11861203
let mut index_let_stmts: Vec<P<ast::Stmt>> = Vec::new();
1204+
1205+
let target_type_name =
1206+
find_repr_type_name(&cx.parse_sess.span_diagnostic, type_attrs);
1207+
11871208
for (&ident, self_arg) in vi_idents.iter().zip(self_args.iter()) {
1188-
let variant_idx = cx.expr_match(sp, self_arg.clone(), arms.clone());
1189-
let let_stmt = cx.stmt_let(sp, false, ident, variant_idx);
1209+
let path = vec![cx.ident_of_std("core"),
1210+
cx.ident_of("intrinsics"),
1211+
cx.ident_of("discriminant_value")];
1212+
let call = cx.expr_call_global(
1213+
sp, path, vec![cx.expr_addr_of(sp, self_arg.clone())]);
1214+
let variant_value = cx.expr_block(P(ast::Block {
1215+
stmts: vec![],
1216+
expr: Some(call),
1217+
id: ast::DUMMY_NODE_ID,
1218+
rules: ast::UnsafeBlock(ast::CompilerGenerated),
1219+
span: sp }));
1220+
1221+
let target_ty = cx.ty_ident(sp, cx.ident_of(target_type_name));
1222+
let variant_disr = cx.expr_cast(sp, variant_value, target_ty);
1223+
let let_stmt = cx.stmt_let(sp, false, ident, variant_disr);
11901224
index_let_stmts.push(let_stmt);
11911225
}
11921226

+77
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
#![feature(core)]
12+
13+
extern crate core;
14+
use core::intrinsics::discriminant_value;
15+
16+
enum CLike1 {
17+
A,
18+
B,
19+
C,
20+
D
21+
}
22+
23+
enum CLike2 {
24+
A = 5,
25+
B = 2,
26+
C = 19,
27+
D
28+
}
29+
30+
#[repr(i8)]
31+
enum CLike3 {
32+
A = 5,
33+
B,
34+
C = -1,
35+
D
36+
}
37+
38+
enum ADT {
39+
First(u32, u32),
40+
Second(u64)
41+
}
42+
43+
enum NullablePointer {
44+
Something(&'static u32),
45+
Nothing
46+
}
47+
48+
static CONST : u32 = 0xBEEF;
49+
50+
pub fn main() {
51+
unsafe {
52+
53+
assert_eq!(discriminant_value(&CLike1::A), 0);
54+
assert_eq!(discriminant_value(&CLike1::B), 1);
55+
assert_eq!(discriminant_value(&CLike1::C), 2);
56+
assert_eq!(discriminant_value(&CLike1::D), 3);
57+
58+
assert_eq!(discriminant_value(&CLike2::A), 5);
59+
assert_eq!(discriminant_value(&CLike2::B), 2);
60+
assert_eq!(discriminant_value(&CLike2::C), 19);
61+
assert_eq!(discriminant_value(&CLike2::D), 20);
62+
63+
assert_eq!(discriminant_value(&CLike3::A), 5);
64+
assert_eq!(discriminant_value(&CLike3::B), 6);
65+
assert_eq!(discriminant_value(&CLike3::C), -1_i8 as u64);
66+
assert_eq!(discriminant_value(&CLike3::D), 0);
67+
68+
assert_eq!(discriminant_value(&ADT::First(0,0)), 0);
69+
assert_eq!(discriminant_value(&ADT::Second(5)), 1);
70+
71+
assert_eq!(discriminant_value(&NullablePointer::Nothing), 1);
72+
assert_eq!(discriminant_value(&NullablePointer::Something(&CONST)), 0);
73+
74+
assert_eq!(discriminant_value(&10), 0);
75+
assert_eq!(discriminant_value(&"test"), 0);
76+
}
77+
}

src/test/run-pass/issue-15523-big.rs

+48
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// Issue 15523: derive(PartialOrd) should use the provided
12+
// discriminant values for the derived ordering.
13+
//
14+
// This test is checking corner cases that arise when you have
15+
// 64-bit values in the variants.
16+
17+
#[derive(PartialEq, PartialOrd)]
18+
#[repr(u64)]
19+
enum Eu64 {
20+
Pos2 = 2,
21+
PosMax = !0,
22+
Pos1 = 1,
23+
}
24+
25+
#[derive(PartialEq, PartialOrd)]
26+
#[repr(i64)]
27+
enum Ei64 {
28+
Pos2 = 2,
29+
Neg1 = -1,
30+
NegMin = 1 << 63,
31+
PosMax = !(1 << 63),
32+
Pos1 = 1,
33+
}
34+
35+
fn main() {
36+
assert!(Eu64::Pos2 > Eu64::Pos1);
37+
assert!(Eu64::Pos2 < Eu64::PosMax);
38+
assert!(Eu64::Pos1 < Eu64::PosMax);
39+
40+
41+
assert!(Ei64::Pos2 > Ei64::Pos1);
42+
assert!(Ei64::Pos2 > Ei64::Neg1);
43+
assert!(Ei64::Pos1 > Ei64::Neg1);
44+
assert!(Ei64::Pos2 > Ei64::NegMin);
45+
assert!(Ei64::Pos1 > Ei64::NegMin);
46+
assert!(Ei64::Pos2 < Ei64::PosMax);
47+
assert!(Ei64::Pos1 < Ei64::PosMax);
48+
}

0 commit comments

Comments
 (0)