Skip to content

deriv(Hash) for single-variant enum should not hash discriminant #42709

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

Merged
merged 1 commit into from
Jun 28, 2017
Merged
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/libsyntax_ext/deriving/clone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ fn cs_clone(name: &str,
all_fields = af;
vdata = vdata_;
}
EnumMatching(_, variant, ref af) => {
EnumMatching(.., variant, ref af) => {
ctor_path = cx.path(trait_span, vec![substr.type_ident, variant.node.name]);
all_fields = af;
vdata = &variant.node.data;
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax_ext/deriving/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ fn show_substructure(cx: &mut ExtCtxt, span: Span, substr: &Substructure) -> P<E
// based on the "shape".
let (ident, is_struct) = match *substr.fields {
Struct(vdata, _) => (substr.type_ident, vdata.is_struct()),
EnumMatching(_, v, _) => (v.node.name, v.node.data.is_struct()),
EnumMatching(_, _, v, _) => (v.node.name, v.node.data.is_struct()),
EnumNonMatchingCollapsed(..) |
StaticStruct(..) |
StaticEnum(..) => cx.span_bug(span, "nonsensical .fields in `#[derive(Debug)]`"),
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax_ext/deriving/encodable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ fn encodable_substructure(cx: &mut ExtCtxt,
blk])
}

EnumMatching(idx, variant, ref fields) => {
EnumMatching(idx, _, variant, ref fields) => {
// We're not generating an AST that the borrow checker is expecting,
// so we need to generate a unique local variable to take the
// mutable loan out on, otherwise we get conflicts which don't
Expand Down
9 changes: 5 additions & 4 deletions src/libsyntax_ext/deriving/generic/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -305,10 +305,10 @@ pub enum StaticFields {
/// A summary of the possible sets of fields.
pub enum SubstructureFields<'a> {
Struct(&'a ast::VariantData, Vec<FieldInfo<'a>>),
/// Matching variants of the enum: variant index, ast::Variant,
/// Matching variants of the enum: variant index, variant count, ast::Variant,
/// fields: the field name is only non-`None` in the case of a struct
/// variant.
EnumMatching(usize, &'a ast::Variant, Vec<FieldInfo<'a>>),
EnumMatching(usize, usize, &'a ast::Variant, Vec<FieldInfo<'a>>),

/// Non-matching variants of the enum, but with all state hidden from
/// the consequent code. The first component holds `Ident`s for all of
Expand Down Expand Up @@ -1250,7 +1250,7 @@ impl<'a> MethodDef<'a> {
// expressions for referencing every field of every
// Self arg, assuming all are instances of VariantK.
// Build up code associated with such a case.
let substructure = EnumMatching(index, variant, field_tuples);
let substructure = EnumMatching(index, variants.len(), variant, field_tuples);
let arm_expr = self.call_substructure_method(cx,
trait_,
type_ident,
Expand All @@ -1267,12 +1267,13 @@ impl<'a> MethodDef<'a> {
// We need a default case that handles the fieldless variants.
// The index and actual variant aren't meaningful in this case,
// so just use whatever
let substructure = EnumMatching(0, variants.len(), v, Vec::new());
Some(self.call_substructure_method(cx,
trait_,
type_ident,
&self_args[..],
nonself_args,
&EnumMatching(0, v, Vec::new())))
&substructure))
}
_ if variants.len() > 1 && self_args.len() > 1 => {
// Since we know that all the arguments will match if we reach
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax_ext/deriving/hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ fn hash_substructure(cx: &mut ExtCtxt, trait_span: Span, substr: &Substructure)
let mut stmts = Vec::new();

let fields = match *substr.fields {
Struct(_, ref fs) => fs,
Struct(_, ref fs) | EnumMatching(_, 1, .., ref fs) => fs,
EnumMatching(.., ref fs) => {
let variant_value = deriving::call_intrinsic(cx,
trait_span,
Expand Down
13 changes: 11 additions & 2 deletions src/test/run-pass/deriving-hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ impl<'a> Hasher for FakeHasher<'a> {
}
}

fn fake_hash(v: &mut Vec<u8>, e: E) {
e.hash(&mut FakeHasher(v));
fn fake_hash<A: Hash>(v: &mut Vec<u8>, a: A) {
a.hash(&mut FakeHasher(v));
}

fn main() {
Expand All @@ -69,4 +69,13 @@ fn main() {
fake_hash(&mut va, E::A);
fake_hash(&mut vb, E::B);
assert!(va != vb);

// issue #39137: single variant enum hash should not hash discriminant
#[derive(Hash)]
enum SingleVariantEnum {
A(u8),
}
let mut v = vec![];
fake_hash(&mut v, SingleVariantEnum::A(17));
assert_eq!(vec![17], v);
}