From f43eb91d239f79e3930b0e9697f7cb9ec9e4aa5f Mon Sep 17 00:00:00 2001
From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com>
Date: Sun, 9 Jan 2022 11:07:29 +0100
Subject: [PATCH 1/3] Add semver guidelines for changing the `repr` of
structs/enums to reference
The current semver guidelines do not mention `repr` at all. Changing the `repr` of a struct/enum will break code, especially unsafe code relying on layout. These breaking changes will rarely manifest themselves as compiler errors, which makes them all the more dangerous.
The new guidelines mention removing a well-defined repr from a struct or enum to be a breaking change.
---
src/doc/src/reference/semver.md | 92 +++++++++++++++++++++++++++++++++
1 file changed, 92 insertions(+)
diff --git a/src/doc/src/reference/semver.md b/src/doc/src/reference/semver.md
index 0404efdcfb2..1bfe9b96520 100644
--- a/src/doc/src/reference/semver.md
+++ b/src/doc/src/reference/semver.md
@@ -62,11 +62,13 @@ considered incompatible.
* Structs
* [Major: adding a private struct field when all current fields are public](#struct-add-private-field-when-public)
* [Major: adding a public field when no private field exists](#struct-add-public-field-when-no-private)
+ * [Major: change a from well-defined repr to another when all fields are public](#struct-change-repr-when-public)
* [Minor: adding or removing private fields when at least one already exists](#struct-private-fields-with-private)
* [Minor: going from a tuple struct with all private fields (with at least one field) to a normal struct, or vice versa](#struct-tuple-normal-with-private)
* Enums
* [Major: adding new enum variants (without `non_exhaustive`)](#enum-variant-new)
* [Major: adding new fields to an enum variant](#enum-fields-new)
+ * [Major: removing an integer repr](#enum-remove-integer-repr)
* Traits
* [Major: adding a non-defaulted trait item](#trait-new-item-no-default)
* [Major: any change to trait item signatures](#trait-item-signature)
@@ -273,6 +275,56 @@ Mitigation strategies:
a struct to prevent users from using struct literal syntax, and instead
provide a constructor method and/or [Default] implementation.
+
+### Major: change a from well-defined repr to another when all fields are public
+
+When a struct has a well-defined repr (`transparent`, `C`) and only public fields, this will break unsafe code relying on
+that repr.
+
+Adding a well-defined repr to a `repr(Rust)` struct is *not* a breaking change.
+
+```rust,ignore
+// MAJOR CHANGE
+
+///////////////////////////////////////////////////////////
+// Before
+#[repr(C)]
+pub struct Foo {
+ pub f1: u8,
+ pub f2: u16,
+ pub f3: u8,
+}
+
+///////////////////////////////////////////////////////////
+// After
+pub struct Foo {
+ pub f1: u8,
+ pub f2: u16,
+ pub f3: u8,
+}
+
+///////////////////////////////////////////////////////////
+// Example usage that will break.
+fn main() {
+ let foo = Foo {
+ f1: 1,
+ f2: 1000,
+ f3: 5,
+ };
+
+ let foo_ptr = &foo as *const Foo as *const u8;
+
+ // this is now unsound because of the change
+ // SAFETY: Foo is repr(C), so we are guaranteed that there will be `3` at this offset (u8, 8 pad, u16)
+ let f2 = unsafe { foo_ptr.offset(4).read() };
+
+ println!("{}", f2);
+}
+```
+
+Mitigation strategies:
+* Only add a `repr` to structs with public fields if you don't plan to change it again.
+
### Minor: adding or removing private fields when at least one already exists
@@ -453,6 +505,46 @@ Mitigation strategies:
Variant1(Foo)
}
```
+
+
+### Major: removing an integer repr
+
+It is a breaking change to remove an integer repr (like `#[repr(u8)]`) from an enum.
+
+```rust,ignore
+// MAJOR CHANGE
+
+///////////////////////////////////////////////////////////
+// Before
+#[repr(i32)]
+pub enum Number {
+ One = 1,
+ Two = 2,
+ Three = 3,
+}
+
+///////////////////////////////////////////////////////////
+// After
+pub enum Number {
+ One = 1,
+ Two = 2,
+ Three = 3,
+}
+
+///////////////////////////////////////////////////////////
+// Example usage that will break.
+fn main() {
+ let num_three = Number::Three;
+
+ // SAFETY: `Number` is `#[repr(i32)]`
+ let three: i32 = unsafe { std::mem::transmute(num_three) }; // Error: cannot transmute between types of different sizes
+
+ println!("{}", three)
+}
+```
+
+Mitigation strategies:
+* Only add `repr` to an enum if you are sure you'll never need to remove it
### Major: adding a non-defaulted trait item
From d3dac5ffd15c526e2381a68c07cf70a4e320b09f Mon Sep 17 00:00:00 2001
From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com>
Date: Sun, 9 Jan 2022 11:36:13 +0100
Subject: [PATCH 2/3] Fix rustfmt errors
---
src/doc/src/reference/semver.md | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/src/doc/src/reference/semver.md b/src/doc/src/reference/semver.md
index 1bfe9b96520..d6bdc5be10c 100644
--- a/src/doc/src/reference/semver.md
+++ b/src/doc/src/reference/semver.md
@@ -305,20 +305,22 @@ pub struct Foo {
///////////////////////////////////////////////////////////
// Example usage that will break.
+use updated_crate::Foo;
+
fn main() {
let foo = Foo {
f1: 1,
f2: 1000,
f3: 5,
};
-
+
let foo_ptr = &foo as *const Foo as *const u8;
-
+
// this is now unsound because of the change
// SAFETY: Foo is repr(C), so we are guaranteed that there will be `3` at this offset (u8, 8 pad, u16)
- let f2 = unsafe { foo_ptr.offset(4).read() };
-
- println!("{}", f2);
+ let f3 = unsafe { foo_ptr.offset(4).read() };
+
+ assert_eq!(5, f3);
}
```
@@ -534,12 +536,12 @@ pub enum Number {
///////////////////////////////////////////////////////////
// Example usage that will break.
fn main() {
- let num_three = Number::Three;
-
+ let num_three = updated_crate::Number::Three;
+
// SAFETY: `Number` is `#[repr(i32)]`
let three: i32 = unsafe { std::mem::transmute(num_three) }; // Error: cannot transmute between types of different sizes
-
- println!("{}", three)
+
+ assert_eq!(3, three)
}
```
From eaf7720056bd1108f7fc4cae4db98d6c70880e4f Mon Sep 17 00:00:00 2001
From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com>
Date: Fri, 14 Jan 2022 17:32:41 +0100
Subject: [PATCH 3/3] add `run-fail` to repr change struct
---
src/doc/src/reference/semver.md | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/doc/src/reference/semver.md b/src/doc/src/reference/semver.md
index d6bdc5be10c..652a1b6133c 100644
--- a/src/doc/src/reference/semver.md
+++ b/src/doc/src/reference/semver.md
@@ -283,7 +283,7 @@ that repr.
Adding a well-defined repr to a `repr(Rust)` struct is *not* a breaking change.
-```rust,ignore
+```rust,ignore,run-fail
// MAJOR CHANGE
///////////////////////////////////////////////////////////