Skip to content

Commit c8b8aef

Browse files
ibabushkinibabushkin
committed
The bug in question was caused by incorrect traversal of the `rustc::ty::Generics` datastructure, causing index-out-of-bound errors.
1 parent f02043c commit c8b8aef

File tree

6 files changed

+385
-19
lines changed

6 files changed

+385
-19
lines changed

‎src/changes.rs

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -512,15 +512,19 @@ all method invocations using the method syntax become invalid."
512512
}
513513
TraitItemAdded {
514514
defaulted: true, ..
515-
} => "Adding a new defaulted trait item is a breaking change in some specific
515+
} => {
516+
"Adding a new defaulted trait item is a breaking change in some specific
516517
situations: The new trait item could cause a name clash with traits
517518
defined in user code. Because this is a rather special case, this change
518-
is classified as \"technically breaking\".",
519+
is classified as \"technically breaking\"."
520+
}
519521
TraitItemAdded {
520522
sealed_trait: true, ..
521-
} => "Adding a new trait item is a non-breaking change, when user code can't
523+
} => {
524+
"Adding a new trait item is a non-breaking change, when user code can't
522525
provide implementations of the trait, i.e. if the trait is sealed by
523-
inheriting from an unnamable (crate-local) item.",
526+
inheriting from an unnamable (crate-local) item."
527+
}
524528
TraitItemAdded { .. } =>
525529
// neither defaulted or sealed
526530
{
@@ -546,22 +550,30 @@ type or lifetime not fulfilling the bound are rendered invalid."
546550
}
547551
BoundsLoosened {
548552
trait_def: true, ..
549-
} => "Loosening the bounds of a lifetime or type parameter in a trait
553+
} => {
554+
"Loosening the bounds of a lifetime or type parameter in a trait
550555
definition is a breaking change, because the assumption in user code
551556
that the bound in question hold is violated, potentially invalidating
552-
trait implementation or usage.",
557+
trait implementation or usage."
558+
}
553559
BoundsLoosened {
554560
trait_def: false, ..
555-
} => "Loosening the bounds of a lifetime or type parameter in a non-trait
561+
} => {
562+
"Loosening the bounds of a lifetime or type parameter in a non-trait
556563
definition is a non-breaking change, because all old references to the
557-
item would remain valid.",
558-
TraitImplTightened => "Effectively removing a trait implementation for a (possibly
564+
item would remain valid."
565+
}
566+
TraitImplTightened => {
567+
"Effectively removing a trait implementation for a (possibly
559568
parametrized) type is a breaking change, as all old references to trait
560-
methods on the type become invalid.",
561-
TraitImplLoosened => "Effectively adding a trait implementation for a (possibly
569+
methods on the type become invalid."
570+
}
571+
TraitImplLoosened => {
572+
"Effectively adding a trait implementation for a (possibly
562573
parametrized) type is a breaking change in some specific situations,
563574
as name clashes with other trait implementations in user code can be
564-
caused.",
575+
caused."
576+
}
565577
AssociatedItemAdded => {
566578
"Adding a new item to an inherent impl is a breaking change in some
567579
specific situations, for example if this causes name clashes with a trait

‎src/traverse.rs

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -718,10 +718,26 @@ fn diff_generics(
718718
let old_count = old_gen.own_counts();
719719
let new_count = new_gen.own_counts();
720720

721+
let self_add = if old_gen.has_self && new_gen.has_self {
722+
1
723+
} else if !old_gen.has_self && !new_gen.has_self {
724+
0
725+
} else {
726+
unreachable!()
727+
};
728+
729+
// TODO: we might need to track the number of parameters in the parent.
730+
731+
debug!("old_gen: {:?}, new_gen: {:?}", old_gen, new_gen);
732+
debug!(
733+
"old_count.lifetimes: {:?}, new_count.lifetimes: {:?}",
734+
old_count.lifetimes, new_count.lifetimes
735+
);
736+
721737
for i in 0..max(old_count.lifetimes, new_count.lifetimes) {
722738
match (
723-
get_region_from_params(old_gen, i),
724-
get_region_from_params(new_gen, i),
739+
get_region_from_params(old_gen, i + self_add),
740+
get_region_from_params(new_gen, i + self_add),
725741
) {
726742
(Some(old_region), Some(new_region)) => {
727743
// type aliases don't have inferred variance, so we have to ignore that.
@@ -743,11 +759,15 @@ fn diff_generics(
743759
}
744760
}
745761

746-
for i in 0..max(old_count.types, new_count.types) {
747-
match (
748-
get_type_from_params(old_gen, old_count.lifetimes + i),
749-
get_type_from_params(new_gen, new_count.lifetimes + i),
750-
) {
762+
for i in 0 .. max(old_count.types, new_count.types) {
763+
let pair = if i == 0 && self_add == 1 {
764+
(get_type_from_params(old_gen, 0), get_type_from_params(new_gen, 0))
765+
} else {
766+
(get_type_from_params(old_gen, old_count.lifetimes + i),
767+
get_type_from_params(new_gen, new_count.lifetimes + i))
768+
};
769+
770+
match pair {
751771
(Some(old_type), Some(new_type)) => {
752772
// type aliases don't have inferred variance, so we have to ignore that.
753773
if let (Some(old_var), Some(new_var)) = (

‎tests/full.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,7 @@ mod full {
176176
// the libc API on windows did *not* change between these versions
177177
full_test!(libc0, "libc", "0.2.28", "0.2.31", cfg!(windows));
178178
full_test!(libc1, "libc", "0.2.47", "0.2.48", true);
179+
full_test!(rmpv, "rmpv", "0.4.0", "0.4.1", false);
179180
// full_test!(mozjs, "mozjs", "0.2.0", "0.3.0");
180181
// full_test!(rand, "rand", "0.3.10", "0.3.16");
181182
// full_test!(serde_pre, "serde", "0.7.0", "1.0.0");

‎tests/full_cases/rmpv-0.4.0-0.4.1.linux

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
version bump: 0.4.0 -> (breaking) -> 0.4.1
2+
error: breaking changes in `<old::decode::Error as std::convert::From<rmp::decode::MarkerReadError>>`
3+
--> rmpv-0.4.0/src/decode/mod.rs:60:1
4+
|
5+
60 | / impl From<MarkerReadError> for Error {
6+
61 | | fn from(err: MarkerReadError) -> Error {
7+
62 | | Error::InvalidMarkerRead(err.0)
8+
63 | | }
9+
64 | | }
10+
| |_^
11+
|
12+
= warning: trait impl specialized or removed (breaking)
13+
14+
error: breaking changes in `<old::decode::Error as std::convert::From<rmp::decode::ValueReadError>>`
15+
--> rmpv-0.4.0/src/decode/mod.rs:66:1
16+
|
17+
66 | / impl From<ValueReadError> for Error {
18+
67 | | fn from(err: ValueReadError) -> Error {
19+
68 | | match err {
20+
69 | | ValueReadError::InvalidMarkerRead(err) => Error::InvalidMarkerRead(err),
21+
... |
22+
75 | | }
23+
76 | | }
24+
| |_^
25+
|
26+
= warning: trait impl specialized or removed (breaking)
27+
28+
error: breaking changes in `write_value`
29+
--> rmpv-0.4.1/src/encode/value.rs:15:1
30+
|
31+
15 | / pub fn write_value<W>(wr: &mut W, val: &Value) -> Result<(), Error>
32+
16 | | where W: Write
33+
17 | | {
34+
18 | | match *val {
35+
... |
36+
69 | | Ok(())
37+
70 | | }
38+
| |_^
39+
|
40+
= warning: type error: expected enum `old::encode::Error`, found enum `new::encode::Error` (breaking)
41+
42+
error: breaking changes in `write_value_ref`
43+
--> rmpv-0.4.1/src/encode/value_ref.rs:27:1
44+
|
45+
27 | / pub fn write_value_ref<W>(wr: &mut W, val: &ValueRef) -> Result<(), Error>
46+
28 | | where W: Write
47+
29 | | {
48+
30 | | match *val {
49+
... |
50+
81 | | Ok(())
51+
82 | | }
52+
| |_^
53+
|
54+
= warning: type error: expected enum `old::encode::Error`, found enum `new::encode::Error` (breaking)
55+
56+
warning: technically breaking changes in `as_ref`
57+
--> rmpv-0.4.1/src/lib.rs:253:5
58+
|
59+
253 | / pub fn as_ref(&self) -> Utf8StringRef {
60+
254 | | match self.s {
61+
255 | | Ok(ref s) => Utf8StringRef { s: Ok(s.as_str()) },
62+
256 | | Err((ref buf, err)) => Utf8StringRef { s: Err((&buf[..], err)) },
63+
257 | | }
64+
258 | | }
65+
| |_____^
66+
|
67+
= note: added item in inherent impl (technically breaking)
68+
69+
warning: technically breaking changes in `as_ref`
70+
--> rmpv-0.4.1/src/lib.rs:448:5
71+
|
72+
448 | / pub fn as_ref(&self) -> ValueRef {
73+
449 | | match self {
74+
450 | | &Value::Nil => ValueRef::Nil,
75+
451 | | &Value::Boolean(val) => ValueRef::Boolean(val),
76+
... |
77+
464 | | }
78+
465 | | }
79+
| |_____^
80+
|
81+
= note: added item in inherent impl (technically breaking)
82+
83+
warning: technically breaking changes in `<new::decode::Error as std::convert::From<rmp::decode::MarkerReadError>>`
84+
--> rmpv-0.4.1/src/decode/mod.rs:60:1
85+
|
86+
60 | / impl From<MarkerReadError> for Error {
87+
61 | | fn from(err: MarkerReadError) -> Error {
88+
62 | | Error::InvalidMarkerRead(err.0)
89+
63 | | }
90+
64 | | }
91+
| |_^
92+
|
93+
= note: trait impl generalized or newly added (technically breaking)
94+
95+
warning: technically breaking changes in `<new::decode::Error as std::convert::From<rmp::decode::ValueReadError>>`
96+
--> rmpv-0.4.1/src/decode/mod.rs:66:1
97+
|
98+
66 | / impl From<ValueReadError> for Error {
99+
67 | | fn from(err: ValueReadError) -> Error {
100+
68 | | match err {
101+
69 | | ValueReadError::InvalidMarkerRead(err) => Error::InvalidMarkerRead(err),
102+
... |
103+
75 | | }
104+
76 | | }
105+
| |_^
106+
|
107+
= note: trait impl generalized or newly added (technically breaking)
108+
109+
error: aborting due to 4 previous errors
110+
111+
error: rustc-semverver errored

‎tests/full_cases/rmpv-0.4.0-0.4.1.osx

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
version bump: 0.4.0 -> (breaking) -> 0.4.1
2+
error: breaking changes in `<old::decode::Error as std::convert::From<rmp::decode::MarkerReadError>>`
3+
--> rmpv-0.4.0/src/decode/mod.rs:60:1
4+
|
5+
60 | / impl From<MarkerReadError> for Error {
6+
61 | | fn from(err: MarkerReadError) -> Error {
7+
62 | | Error::InvalidMarkerRead(err.0)
8+
63 | | }
9+
64 | | }
10+
| |_^
11+
|
12+
= warning: trait impl specialized or removed (breaking)
13+
14+
error: breaking changes in `<old::decode::Error as std::convert::From<rmp::decode::ValueReadError>>`
15+
--> rmpv-0.4.0/src/decode/mod.rs:66:1
16+
|
17+
66 | / impl From<ValueReadError> for Error {
18+
67 | | fn from(err: ValueReadError) -> Error {
19+
68 | | match err {
20+
69 | | ValueReadError::InvalidMarkerRead(err) => Error::InvalidMarkerRead(err),
21+
... |
22+
75 | | }
23+
76 | | }
24+
| |_^
25+
|
26+
= warning: trait impl specialized or removed (breaking)
27+
28+
error: breaking changes in `write_value`
29+
--> rmpv-0.4.1/src/encode/value.rs:15:1
30+
|
31+
15 | / pub fn write_value<W>(wr: &mut W, val: &Value) -> Result<(), Error>
32+
16 | | where W: Write
33+
17 | | {
34+
18 | | match *val {
35+
... |
36+
69 | | Ok(())
37+
70 | | }
38+
| |_^
39+
|
40+
= warning: type error: expected enum `old::encode::Error`, found enum `new::encode::Error` (breaking)
41+
42+
error: breaking changes in `write_value_ref`
43+
--> rmpv-0.4.1/src/encode/value_ref.rs:27:1
44+
|
45+
27 | / pub fn write_value_ref<W>(wr: &mut W, val: &ValueRef) -> Result<(), Error>
46+
28 | | where W: Write
47+
29 | | {
48+
30 | | match *val {
49+
... |
50+
81 | | Ok(())
51+
82 | | }
52+
| |_^
53+
|
54+
= warning: type error: expected enum `old::encode::Error`, found enum `new::encode::Error` (breaking)
55+
56+
warning: technically breaking changes in `as_ref`
57+
--> rmpv-0.4.1/src/lib.rs:253:5
58+
|
59+
253 | / pub fn as_ref(&self) -> Utf8StringRef {
60+
254 | | match self.s {
61+
255 | | Ok(ref s) => Utf8StringRef { s: Ok(s.as_str()) },
62+
256 | | Err((ref buf, err)) => Utf8StringRef { s: Err((&buf[..], err)) },
63+
257 | | }
64+
258 | | }
65+
| |_____^
66+
|
67+
= note: added item in inherent impl (technically breaking)
68+
69+
warning: technically breaking changes in `as_ref`
70+
--> rmpv-0.4.1/src/lib.rs:448:5
71+
|
72+
448 | / pub fn as_ref(&self) -> ValueRef {
73+
449 | | match self {
74+
450 | | &Value::Nil => ValueRef::Nil,
75+
451 | | &Value::Boolean(val) => ValueRef::Boolean(val),
76+
... |
77+
464 | | }
78+
465 | | }
79+
| |_____^
80+
|
81+
= note: added item in inherent impl (technically breaking)
82+
83+
warning: technically breaking changes in `<new::decode::Error as std::convert::From<rmp::decode::MarkerReadError>>`
84+
--> rmpv-0.4.1/src/decode/mod.rs:60:1
85+
|
86+
60 | / impl From<MarkerReadError> for Error {
87+
61 | | fn from(err: MarkerReadError) -> Error {
88+
62 | | Error::InvalidMarkerRead(err.0)
89+
63 | | }
90+
64 | | }
91+
| |_^
92+
|
93+
= note: trait impl generalized or newly added (technically breaking)
94+
95+
warning: technically breaking changes in `<new::decode::Error as std::convert::From<rmp::decode::ValueReadError>>`
96+
--> rmpv-0.4.1/src/decode/mod.rs:66:1
97+
|
98+
66 | / impl From<ValueReadError> for Error {
99+
67 | | fn from(err: ValueReadError) -> Error {
100+
68 | | match err {
101+
69 | | ValueReadError::InvalidMarkerRead(err) => Error::InvalidMarkerRead(err),
102+
... |
103+
75 | | }
104+
76 | | }
105+
| |_^
106+
|
107+
= note: trait impl generalized or newly added (technically breaking)
108+
109+
error: aborting due to 4 previous errors
110+
111+
error: rustc-semverver errored

0 commit comments

Comments
 (0)