Skip to content

Commit f49d878

Browse files
borsbors
committed
Auto merge of #4066 - rust-lang:hash, r=Manishearth
Properly hash enums While I wrote this I was saved by a clippy lint... I accidentally fetched the discriminant of a reference to an enum and not of an enum. changelog: reduce hash collisions during clippy-internal hashing
2 parents 82b2dfb + 5dea5d4 commit f49d878

File tree

2 files changed

+21
-101
lines changed

2 files changed

+21
-101
lines changed

‎clippy_lints/src/consts.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ impl Hash for Constant {
8181
where
8282
H: Hasher,
8383
{
84+
std::mem::discriminant(self).hash(state);
8485
match *self {
8586
Constant::Str(ref s) => {
8687
s.hash(state);

‎clippy_lints/src/utils/hir_utils.rs‎

Lines changed: 20 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -389,84 +389,65 @@ impl<'a, 'tcx: 'a> SpanlessHash<'a, 'tcx> {
389389

390390
#[allow(clippy::many_single_char_names, clippy::too_many_lines)]
391391
pub fn hash_expr(&mut self, e: &Expr) {
392-
if let Some(e) = constant_simple(self.cx, self.tables, e) {
392+
let simple_const = constant_simple(self.cx, self.tables, e);
393+
394+
// const hashing may result in the same hash as some unrelated node, so add a sort of
395+
// discriminant depending on which path we're choosing next
396+
simple_const.is_some().hash(&mut self.s);
397+
398+
if let Some(e) = simple_const {
393399
return e.hash(&mut self.s);
394400
}
395401

402+
std::mem::discriminant(&e.node).hash(&mut self.s);
403+
396404
match e.node {
397405
ExprKind::AddrOf(m, ref e) => {
398-
let c: fn(_, _) -> _ = ExprKind::AddrOf;
399-
c.hash(&mut self.s);
400406
m.hash(&mut self.s);
401407
self.hash_expr(e);
402408
},
403409
ExprKind::Continue(i) => {
404-
let c: fn(_) -> _ = ExprKind::Continue;
405-
c.hash(&mut self.s);
406410
if let Some(i) = i.label {
407411
self.hash_name(i.ident.name);
408412
}
409413
},
410-
ExprKind::Yield(ref e) => {
411-
let c: fn(_) -> _ = ExprKind::Yield;
412-
c.hash(&mut self.s);
413-
self.hash_expr(e);
414-
},
415414
ExprKind::Assign(ref l, ref r) => {
416-
let c: fn(_, _) -> _ = ExprKind::Assign;
417-
c.hash(&mut self.s);
418415
self.hash_expr(l);
419416
self.hash_expr(r);
420417
},
421418
ExprKind::AssignOp(ref o, ref l, ref r) => {
422-
let c: fn(_, _, _) -> _ = ExprKind::AssignOp;
423-
c.hash(&mut self.s);
424419
o.hash(&mut self.s);
425420
self.hash_expr(l);
426421
self.hash_expr(r);
427422
},
428423
ExprKind::Block(ref b, _) => {
429-
let c: fn(_, _) -> _ = ExprKind::Block;
430-
c.hash(&mut self.s);
431424
self.hash_block(b);
432425
},
433426
ExprKind::Binary(op, ref l, ref r) => {
434-
let c: fn(_, _, _) -> _ = ExprKind::Binary;
435-
c.hash(&mut self.s);
436427
op.node.hash(&mut self.s);
437428
self.hash_expr(l);
438429
self.hash_expr(r);
439430
},
440431
ExprKind::Break(i, ref j) => {
441-
let c: fn(_, _) -> _ = ExprKind::Break;
442-
c.hash(&mut self.s);
443432
if let Some(i) = i.label {
444433
self.hash_name(i.ident.name);
445434
}
446435
if let Some(ref j) = *j {
447436
self.hash_expr(&*j);
448437
}
449438
},
450-
ExprKind::Box(ref e) => {
451-
let c: fn(_) -> _ = ExprKind::Box;
452-
c.hash(&mut self.s);
439+
ExprKind::Box(ref e) | ExprKind::DropTemps(ref e) | ExprKind::Yield(ref e) => {
453440
self.hash_expr(e);
454441
},
455442
ExprKind::Call(ref fun, ref args) => {
456-
let c: fn(_, _) -> _ = ExprKind::Call;
457-
c.hash(&mut self.s);
458443
self.hash_expr(fun);
459444
self.hash_exprs(args);
460445
},
461-
ExprKind::Cast(ref e, ref _ty) => {
462-
let c: fn(_, _) -> _ = ExprKind::Cast;
463-
c.hash(&mut self.s);
446+
ExprKind::Cast(ref e, ref _ty) | ExprKind::Type(ref e, ref _ty) => {
464447
self.hash_expr(e);
465448
// TODO: _ty
466449
},
467450
ExprKind::Closure(cap, _, eid, _, _) => {
468-
let c: fn(_, _, _, _, _) -> _ = ExprKind::Closure;
469-
c.hash(&mut self.s);
470451
match cap {
471452
CaptureClause::CaptureByValue => 0,
472453
CaptureClause::CaptureByRef => 1,
@@ -475,37 +456,24 @@ impl<'a, 'tcx: 'a> SpanlessHash<'a, 'tcx> {
475456
self.hash_expr(&self.cx.tcx.hir().body(eid).value);
476457
},
477458
ExprKind::Field(ref e, ref f) => {
478-
let c: fn(_, _) -> _ = ExprKind::Field;
479-
c.hash(&mut self.s);
480459
self.hash_expr(e);
481460
self.hash_name(f.name);
482461
},
483462
ExprKind::Index(ref a, ref i) => {
484-
let c: fn(_, _) -> _ = ExprKind::Index;
485-
c.hash(&mut self.s);
486463
self.hash_expr(a);
487464
self.hash_expr(i);
488465
},
489-
ExprKind::InlineAsm(..) => {
490-
let c: fn(_, _, _) -> _ = ExprKind::InlineAsm;
491-
c.hash(&mut self.s);
492-
},
466+
ExprKind::InlineAsm(..) | ExprKind::Err => {},
493467
ExprKind::Lit(ref l) => {
494-
let c: fn(_) -> _ = ExprKind::Lit;
495-
c.hash(&mut self.s);
496468
l.hash(&mut self.s);
497469
},
498470
ExprKind::Loop(ref b, ref i, _) => {
499-
let c: fn(_, _, _) -> _ = ExprKind::Loop;
500-
c.hash(&mut self.s);
501471
self.hash_block(b);
502472
if let Some(i) = *i {
503473
self.hash_name(i.ident.name);
504474
}
505475
},
506476
ExprKind::Match(ref e, ref arms, ref s) => {
507-
let c: fn(_, _, _) -> _ = ExprKind::Match;
508-
c.hash(&mut self.s);
509477
self.hash_expr(e);
510478

511479
for arm in arms {
@@ -519,36 +487,25 @@ impl<'a, 'tcx: 'a> SpanlessHash<'a, 'tcx> {
519487
s.hash(&mut self.s);
520488
},
521489
ExprKind::MethodCall(ref path, ref _tys, ref args) => {
522-
let c: fn(_, _, _) -> _ = ExprKind::MethodCall;
523-
c.hash(&mut self.s);
524490
self.hash_name(path.ident.name);
525491
self.hash_exprs(args);
526492
},
527493
ExprKind::Repeat(ref e, ref l_id) => {
528-
let c: fn(_, _) -> _ = ExprKind::Repeat;
529-
c.hash(&mut self.s);
530494
self.hash_expr(e);
531495
let full_table = self.tables;
532496
self.tables = self.cx.tcx.body_tables(l_id.body);
533497
self.hash_expr(&self.cx.tcx.hir().body(l_id.body).value);
534498
self.tables = full_table;
535499
},
536500
ExprKind::Ret(ref e) => {
537-
let c: fn(_) -> _ = ExprKind::Ret;
538-
c.hash(&mut self.s);
539501
if let Some(ref e) = *e {
540502
self.hash_expr(e);
541503
}
542504
},
543505
ExprKind::Path(ref qpath) => {
544-
let c: fn(_) -> _ = ExprKind::Path;
545-
c.hash(&mut self.s);
546506
self.hash_qpath(qpath);
547507
},
548508
ExprKind::Struct(ref path, ref fields, ref expr) => {
549-
let c: fn(_, _, _) -> _ = ExprKind::Struct;
550-
c.hash(&mut self.s);
551-
552509
self.hash_qpath(path);
553510

554511
for f in fields {
@@ -560,46 +517,20 @@ impl<'a, 'tcx: 'a> SpanlessHash<'a, 'tcx> {
560517
self.hash_expr(e);
561518
}
562519
},
563-
ExprKind::Tup(ref tup) => {
564-
let c: fn(_) -> _ = ExprKind::Tup;
565-
c.hash(&mut self.s);
566-
self.hash_exprs(tup);
567-
},
568-
ExprKind::Type(ref e, ref _ty) => {
569-
let c: fn(_, _) -> _ = ExprKind::Type;
570-
c.hash(&mut self.s);
571-
self.hash_expr(e);
572-
// TODO: _ty
520+
ExprKind::Tup(ref v) | ExprKind::Array(ref v) => {
521+
self.hash_exprs(v);
573522
},
574523
ExprKind::Unary(lop, ref le) => {
575-
let c: fn(_, _) -> _ = ExprKind::Unary;
576-
c.hash(&mut self.s);
577-
578524
lop.hash(&mut self.s);
579525
self.hash_expr(le);
580526
},
581-
ExprKind::Array(ref v) => {
582-
let c: fn(_) -> _ = ExprKind::Array;
583-
c.hash(&mut self.s);
584-
585-
self.hash_exprs(v);
586-
},
587527
ExprKind::While(ref cond, ref b, l) => {
588-
let c: fn(_, _, _) -> _ = ExprKind::While;
589-
c.hash(&mut self.s);
590-
591528
self.hash_expr(cond);
592529
self.hash_block(b);
593530
if let Some(l) = l {
594531
self.hash_name(l.ident.name);
595532
}
596533
},
597-
ExprKind::Err => {},
598-
ExprKind::DropTemps(ref e) => {
599-
let c: fn(_) -> _ = ExprKind::DropTemps;
600-
c.hash(&mut self.s);
601-
self.hash_expr(e);
602-
},
603534
}
604535
}
605536

@@ -633,26 +564,16 @@ impl<'a, 'tcx: 'a> SpanlessHash<'a, 'tcx> {
633564
}
634565

635566
pub fn hash_stmt(&mut self, b: &Stmt) {
636-
match b.node {
637-
StmtKind::Local(ref local) => {
638-
let c: fn(_) -> _ = StmtKind::Local;
639-
c.hash(&mut self.s);
567+
std::mem::discriminant(&b.node).hash(&mut self.s);
568+
569+
match &b.node {
570+
StmtKind::Local(local) => {
640571
if let Some(ref init) = local.init {
641572
self.hash_expr(init);
642573
}
643574
},
644-
StmtKind::Item(..) => {
645-
let c: fn(_) -> _ = StmtKind::Item;
646-
c.hash(&mut self.s);
647-
},
648-
StmtKind::Expr(ref expr) => {
649-
let c: fn(_) -> _ = StmtKind::Expr;
650-
c.hash(&mut self.s);
651-
self.hash_expr(expr);
652-
},
653-
StmtKind::Semi(ref expr) => {
654-
let c: fn(_) -> _ = StmtKind::Semi;
655-
c.hash(&mut self.s);
575+
StmtKind::Item(..) => {},
576+
StmtKind::Expr(expr) | StmtKind::Semi(expr) => {
656577
self.hash_expr(expr);
657578
},
658579
}
@@ -661,8 +582,6 @@ impl<'a, 'tcx: 'a> SpanlessHash<'a, 'tcx> {
661582
pub fn hash_guard(&mut self, g: &Guard) {
662583
match g {
663584
Guard::If(ref expr) => {
664-
let c: fn(_) -> _ = Guard::If;
665-
c.hash(&mut self.s);
666585
self.hash_expr(expr);
667586
},
668587
}

0 commit comments

Comments
 (0)