Omit more unnecessary fields from record diffs

This commit is contained in:
Richard Feldman 2023-01-10 01:22:49 -05:00
parent 148119c6b4
commit dc8eb81bb2
No known key found for this signature in database
GPG key ID: F1F21AA5B1D9E43B
2 changed files with 216 additions and 1 deletions

View file

@ -301,6 +301,10 @@ impl AbilitySet {
pub fn is_empty(&self) -> bool {
self.0.is_empty()
}
pub fn len(&self) -> usize {
self.0.len()
}
}
impl FromIterator<Symbol> for AbilitySet {

View file

@ -3022,8 +3022,13 @@ fn diff_record<'b>(
for (k1, v1) in fields1.into_iter() {
match fields2.remove(&k1) {
Some(v2) if v2 != v1 => {
Some(v2) if should_show_field_diff(&v1, &v2) => {
eprintln!(
"\nsame field, diff type: {:?} ==>\n\n\t{:?}\n\n!=\n\n\t{:?}",
&k1, &v1, &v2
);
// The field names are the same but the types are different
// (or at least should be rendered as different)
same_fields_different_types.insert(k1, (v1, v2));
}
Some(_) => {
@ -3150,6 +3155,212 @@ fn diff_record<'b>(
}
}
/// This is a helper for should_show_field_diff - see its doc comment for details.
fn should_show_diff(t1: &ErrorType, t2: &ErrorType) -> bool {
use ErrorType::*;
match (t1, t2) {
(Type(sym1, types1), Type(sym2, types2)) => {
if sym1 != sym2 || types1.len() != types2.len() {
return true;
}
types1
.iter()
.zip(types2.iter())
.any(|(t1, t2)| should_show_diff(t1, t2))
}
(Infinite, Infinite) | (Error, Error) => false,
(FlexVar(v1), FlexVar(v2)) => v1 != v2,
(RigidVar(v1), RigidVar(v2)) => v1 != v2,
(FlexAbleVar(v1, set1), FlexAbleVar(v2, set2)) => {
if v1 != v2 || set1.len() != set2.len() {
return true;
}
set1.sorted_iter()
.zip(set2.sorted_iter())
.any(|(t1, t2)| t1 != t2)
}
(RigidAbleVar(v1, set1), RigidAbleVar(v2, set2)) => {
if v1 != v2 || set1.len() != set2.len() {
return true;
}
set1.sorted_iter()
.zip(set2.sorted_iter())
.any(|(t1, t2)| t1 != t2)
}
(Record(fields1, ext1), Record(fields2, ext2)) => {
if fields1.len() != fields1.len() || ext1 != ext2 {
return true;
}
fields1
.iter()
.zip(fields2.iter())
.any(|((name1, f1), (name2, f2))| name1 != name2 || should_show_field_diff(f1, f2))
}
(TagUnion(tags1, ext1, _polarity1), TagUnion(tags2, ext2, _polarity2)) => {
// If two tag unions differ only in polarity, don't show that as a diff;
// polarity is invisible to the reader!
if tags1.len() != tags2.len() || ext1 != ext2 {
return true;
}
tags1
.iter()
.zip(tags2.iter())
.any(|((name1, payload1), (name2, payload2))| {
if name1 != name2 || payload1.len() != payload2.len() {
return true;
}
payload1
.iter()
.zip(payload2.iter())
.any(|(p1, p2)| should_show_diff(p1, p2))
})
}
(
RecursiveTagUnion(rec1, tags1, ext1, _polarity1),
RecursiveTagUnion(rec2, tags2, ext2, _polarity2),
) => {
// If two tag unions differ only in polarity, don't show that as a diff;
// polarity is invisible to the reader!
if tags1.len() != tags2.len() || ext1 != ext2 || should_show_diff(rec1, rec2) {
return true;
}
tags1
.iter()
.zip(tags2.iter())
.any(|((name1, payload1), (name2, payload2))| {
if name1 != name2 || payload1.len() != payload2.len() {
return true;
}
payload1
.iter()
.zip(payload2.iter())
.any(|(p1, p2)| should_show_diff(p1, p2))
})
}
(Function(params1, ret1, l1), Function(params2, ret2, l2)) => {
if params1.len() != params2.len()
|| should_show_diff(ret1, ret2)
|| should_show_diff(l1, l2)
{
return true;
}
params1
.iter()
.zip(params2.iter())
.any(|(p1, p2)| should_show_diff(p1, p2))
}
(Alias(sym1, params1, t1, kind1), Alias(sym2, params2, t2, kind2)) => {
if sym1 != sym2
|| kind1 != kind2
|| params1.len() != params2.len()
|| should_show_diff(t1, t2)
{
return true;
}
params1
.iter()
.zip(params2.iter())
.any(|(p1, p2)| should_show_diff(p1, p2))
}
(Range(types1), Range(types2)) => {
if types1.len() != types2.len() {
return true;
}
types1
.iter()
.zip(types2.iter())
.any(|(t1, t2)| should_show_diff(t1, t2))
}
(Alias(_sym, _params, aliased, AliasKind::Structural), other)
| (other, Alias(_sym, _params, aliased, AliasKind::Structural)) => {
// Check to see if we should show the diff after unwrapping the alias
should_show_diff(aliased, other)
}
(Alias(_, _, _, AliasKind::Opaque), _)
| (_, Alias(_, _, _, AliasKind::Opaque))
| (Infinite, _)
| (_, Infinite)
| (Error, _)
| (_, Error)
| (Type(_, _), _)
| (_, Type(_, _))
| (FlexVar(_), _)
| (_, FlexVar(_))
| (RigidVar(_), _)
| (_, RigidVar(_))
| (FlexAbleVar(_, _), _)
| (_, FlexAbleVar(_, _))
| (RigidAbleVar(_, _), _)
| (_, RigidAbleVar(_, _))
| (Record(_, _), _)
| (_, Record(_, _))
| (TagUnion(_, _, _), _)
| (_, TagUnion(_, _, _))
| (RecursiveTagUnion(_, _, _, _), _)
| (_, RecursiveTagUnion(_, _, _, _))
| (Function(_, _, _), _)
| (_, Function(_, _, _)) => true,
}
}
/// If these are equivalent, we shouldn't bother showing them in a diff.
/// (For example, if one is Required and the other is Demanded, showing
/// them in a diff will be unhelpful; they'll both be rendered using : and will look
/// exactly the same to the reader!)
fn should_show_field_diff(
field1: &RecordField<ErrorType>,
field2: &RecordField<ErrorType>,
) -> bool {
use RecordField::*;
match (field1, field2) {
// If they're both the same, they don't need a diff
(Demanded(t1), Demanded(t2))
| (Required(t1), Required(t2))
| (RigidRequired(t1), RigidRequired(t2))
| (RigidOptional(t1), RigidOptional(t2))
| (Optional(t1), Optional(t2))
// Demanded and Required don't need a diff
| (Demanded(t1), Required(t2))
| (Required(t1), Demanded(t2))
// Demanded and RigidRequired don't need a diff
| (Demanded(t1), RigidRequired(t2))
| (RigidRequired(t1), Demanded(t2))
=> should_show_diff(t1, t2),
// Everything else needs a diff
(Demanded(_), Optional(_))
| (Demanded(_), RigidOptional(_))
| (Required(_), RigidRequired(_))
| (Required(_), Optional(_))
| (Optional(_), Demanded(_))
| (Optional(_), RigidRequired(_))
| (Optional(_), RigidOptional(_))
| (Optional(_), Required(_))
| (RigidRequired(_), Required(_))
| (RigidRequired(_), Optional(_))
| (RigidRequired(_), RigidOptional(_))
| (Required(_), RigidOptional(_))
| (RigidOptional(_), Demanded(_))
| (RigidOptional(_), Required(_))
| (RigidOptional(_), Optional(_))
| (RigidOptional(_), RigidRequired(_)) => true,
}
}
fn same_tag_name_overlap_diff<'b>(
alloc: &'b RocDocAllocator<'b>,
field: TagName,