Skip to content

Commit

Permalink
Merge pull request #288 from JohnTitor/audit-ignore
Browse files Browse the repository at this point in the history
Audit `ignore` annotations
  • Loading branch information
ehuss authored Jul 1, 2021
2 parents 5de61f9 + 5e78961 commit 5023ba0
Show file tree
Hide file tree
Showing 33 changed files with 128 additions and 32 deletions.
1 change: 1 addition & 0 deletions src/aliasing.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ and `output` overlap, such as `compute(&x, &mut x)`.

With that input, we could get this execution:

<!-- ignore: expanded code -->
```rust,ignore
// input == output == 0xabad1dea
// *input == *output == 20
Expand Down
10 changes: 10 additions & 0 deletions src/arc-mutex/arc-base.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ We'll first need a way to construct an `Arc<T>`.
This is pretty simple, as we just need to box the `ArcInner<T>` and get a
`NonNull<T>` pointer to it.

<!-- ignore: simplified code -->
```rust,ignore
impl<T> Arc<T> {
pub fn new(data: T) -> Arc<T> {
Expand Down Expand Up @@ -41,6 +42,7 @@ This is okay because:
if it is the only `Arc` referencing that data (which only happens in `Drop`)
* We use atomics for the shared mutable reference counting

<!-- ignore: simplified code -->
```rust,ignore
unsafe impl<T: Sync + Send> Send for Arc<T> {}
unsafe impl<T: Sync + Send> Sync for Arc<T> {}
Expand All @@ -61,6 +63,8 @@ as `Rc` is not thread-safe.
To dereference the `NonNull<T>` pointer into a `&T`, we can call
`NonNull::as_ref`. This is unsafe, unlike the typical `as_ref` function, so we
must call it like this:

<!-- ignore: simplified code -->
```rust,ignore
unsafe { self.ptr.as_ref() }
```
Expand All @@ -79,11 +83,15 @@ to the data inside?
What we need now is an implementation of `Deref`.

We'll need to import the trait:

<!-- ignore: simplified code -->
```rust,ignore
use std::ops::Deref;
```

And here's the implementation:

<!-- ignore: simplified code -->
```rust,ignore
impl<T> Deref for Arc<T> {
type Target = T;
Expand All @@ -101,6 +109,8 @@ Pretty simple, eh? This simply dereferences the `NonNull` pointer to the
## Code

Here's all the code from this section:

<!-- ignore: simplified code -->
```rust,ignore
use std::ops::Deref;
Expand Down
8 changes: 7 additions & 1 deletion src/arc-mutex/arc-clone.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@ Basically, we need to:

First, we need to get access to the `ArcInner`:

<!-- ignore: simplified code -->
```rust,ignore
let inner = unsafe { self.ptr.as_ref() };
```

We can update the atomic reference count as follows:

<!-- ignore: simplified code -->
```rust,ignore
let old_rc = inner.rc.fetch_add(1, Ordering::???);
```
Expand All @@ -30,13 +32,14 @@ ordering, see [the section on atomics](../atomics.md).

Thus, the code becomes this:

<!-- ignore: simplified code -->
```rust,ignore
let old_rc = inner.rc.fetch_add(1, Ordering::Relaxed);
```

We'll need to add another import to use `Ordering`:

```rust,ignore
```rust
use std::sync::atomic::Ordering;
```

Expand All @@ -61,6 +64,7 @@ machines) incrementing the reference count at once. This is what we'll do.

It's pretty simple to implement this behavior:

<!-- ignore: simplified code -->
```rust,ignore
if old_rc >= isize::MAX as usize {
std::process::abort();
Expand All @@ -69,6 +73,7 @@ if old_rc >= isize::MAX as usize {

Then, we need to return a new instance of the `Arc`:

<!-- ignore: simplified code -->
```rust,ignore
Self {
ptr: self.ptr,
Expand All @@ -78,6 +83,7 @@ Self {

Now, let's wrap this all up inside the `Clone` implementation:

<!-- ignore: simplified code -->
```rust,ignore
use std::sync::atomic::Ordering;
Expand Down
14 changes: 7 additions & 7 deletions src/arc-mutex/arc-drop.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ Basically, we need to:

First, we'll need to get access to the `ArcInner`:

<!-- ignore: simplified code -->
```rust,ignore
let inner = unsafe { self.ptr.as_ref() };
```
Expand All @@ -24,6 +25,7 @@ also return if the returned value from `fetch_sub` (the value of the reference
count before decrementing it) is not equal to `1` (which happens when we are not
the last reference to the data).

<!-- ignore: simplified code -->
```rust,ignore
if inner.rc.fetch_sub(1, Ordering::Relaxed) != 1 {
return;
Expand Down Expand Up @@ -63,20 +65,17 @@ implementation of `Arc`][3]:

To do this, we do the following:

```rust,ignore
atomic::fence(Ordering::Acquire);
```

We'll need to import `std::sync::atomic` itself:

```rust,ignore
```rust
# use std::sync::atomic::Ordering;
use std::sync::atomic;
atomic::fence(Ordering::Acquire);
```

Finally, we can drop the data itself. We use `Box::from_raw` to drop the boxed
`ArcInner<T>` and its data. This takes a `*mut T` and not a `NonNull<T>`, so we
must convert using `NonNull::as_ptr`.

<!-- ignore: simplified code -->
```rust,ignore
unsafe { Box::from_raw(self.ptr.as_ptr()); }
```
Expand All @@ -86,6 +85,7 @@ pointer is valid.

Now, let's wrap this all up inside the `Drop` implementation:

<!-- ignore: simplified code -->
```rust,ignore
impl<T> Drop for Arc<T> {
fn drop(&mut self) {
Expand Down
10 changes: 5 additions & 5 deletions src/arc-mutex/arc-layout.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ same allocation.

Naively, it would look something like this:

```rust,ignore
```rust
use std::sync::atomic;

pub struct Arc<T> {
Expand All @@ -31,7 +31,7 @@ pub struct Arc<T> {

pub struct ArcInner<T> {
rc: atomic::AtomicUsize,
data: T
data: T,
}
```

Expand All @@ -56,18 +56,18 @@ ownership of a value of `ArcInner<T>` (which itself contains some `T`).

With these changes we get our final structure:

```rust,ignore
```rust
use std::marker::PhantomData;
use std::ptr::NonNull;
use std::sync::atomic::AtomicUsize;

pub struct Arc<T> {
ptr: NonNull<ArcInner<T>>,
phantom: PhantomData<ArcInner<T>>
phantom: PhantomData<ArcInner<T>>,
}

pub struct ArcInner<T> {
rc: AtomicUsize,
data: T
data: T,
}
```
6 changes: 4 additions & 2 deletions src/atomics.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,18 @@ exactly what we said but, you know, fast. Wouldn't that be great?
Compilers fundamentally want to be able to do all sorts of complicated
transformations to reduce data dependencies and eliminate dead code. In
particular, they may radically change the actual order of events, or make events
never occur! If we write something like
never occur! If we write something like:

<!-- ignore: simplified code -->
```rust,ignore
x = 1;
y = 3;
x = 2;
```

The compiler may conclude that it would be best if your program did
The compiler may conclude that it would be best if your program did:

<!-- ignore: simplified code -->
```rust,ignore
x = 2;
y = 3;
Expand Down
1 change: 1 addition & 0 deletions src/destructors.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
What the language *does* provide is full-blown automatic destructors through the
`Drop` trait, which provides the following method:

<!-- ignore: function header -->
```rust,ignore
fn drop(&mut self);
```
Expand Down
12 changes: 9 additions & 3 deletions src/dropck.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,15 @@ when we talked about `'a: 'b`, it was ok for `'a` to live _exactly_ as long as
gets dropped at the same time as another, right? This is why we used the
following desugaring of `let` statements:

<!-- ignore: simplified code -->
```rust,ignore
let x;
let y;
```

desugaring to:

<!-- ignore: desugared code -->
```rust,ignore
{
let x;
Expand All @@ -29,6 +33,7 @@ definition. There are some more details about order of drop in [RFC 1857][rfc185

Let's do this:

<!-- ignore: simplified code -->
```rust,ignore
let tuple = (vec![], vec![]);
```
Expand Down Expand Up @@ -259,7 +264,8 @@ lifetime `'b` and that the only uses of `T` will be moves or drops, but omit
the attribute from `'a` and `U`, because we do access data with that lifetime
and that type:

```rust,ignore
```rust
#![feature(dropck_eyepatch)]
use std::fmt::Display;

struct Inspector<'a, 'b, T, U: Display>(&'a u8, &'b u8, T, U);
Expand All @@ -283,7 +289,7 @@ other avenues for such indirect access.)

Here is an example of invoking a callback:

```rust,ignore
```rust
struct Inspector<T>(T, &'static str, Box<for <'r> fn(&'r T) -> String>);

impl<T> Drop for Inspector<T> {
Expand All @@ -297,7 +303,7 @@ impl<T> Drop for Inspector<T> {

Here is an example of a trait method call:

```rust,ignore
```rust
use std::fmt;

struct Inspector<T: fmt::Display>(T, &'static str);
Expand Down
3 changes: 2 additions & 1 deletion src/exception-safety.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ needs to be careful and consider exception safety.
`Vec::push_all` is a temporary hack to get extending a Vec by a slice reliably
efficient without specialization. Here's a simple implementation:

<!-- ignore: simplified code -->
```rust,ignore
impl<T: Clone> Vec<T> {
fn push_all(&mut self, to_push: &[T]) {
Expand Down Expand Up @@ -75,7 +76,6 @@ bubble_up(heap, index):
while index != 0 && heap[index] < heap[parent(index)]:
heap.swap(index, parent(index))
index = parent(index)
```

A literal transcription of this code to Rust is totally fine, but has an annoying
Expand Down Expand Up @@ -147,6 +147,7 @@ way to do this is to store the algorithm's state in a separate struct with a
destructor for the "finally" logic. Whether we panic or not, that destructor
will run and clean up after us.

<!-- ignore: simplified code -->
```rust,ignore
struct Hole<'a, T: 'a> {
data: &'a mut [T],
Expand Down
2 changes: 1 addition & 1 deletion src/exotic-sizes.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ other is still UB).

The following *could* also compile:

```rust,ignore
```rust,compile_fail
enum Void {}
let res: Result<u32, Void> = Ok(0);
Expand Down
Loading

0 comments on commit 5023ba0

Please sign in to comment.