Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Derive Copy and Clone HeaderSlice and HeaderWithLength #92

Merged
merged 1 commit into from
Jun 17, 2024

Conversation

SimonSapin
Copy link
Contributor

We (apollo-compiler) have an AST where most nodes are wrapped in, approximately:

struct Node<T>(triomphe::Arc<(SourceSpan, T)>);

Arc allows subtrees to be shared without deep cloning, and Arc::make_mut is used extensively for mutability with clone-on-write semantics.

Now I would like to also support Node<str> and rely on Arc::from_header_and_str for the unsafe bits. This requires using HeaderSlice which with triomphe v0.1.12 breaks Arc::make_mut because it does not implement Clone. This PR fixes that.

Clone cannot work for T: !Sized which is the main use case for HeaderSlice but that’s ok, derive automatically insert the appropriate where bound. This is still useful in order to have the same Node<T> use HeaderSlice to support both T: Sized and T: !Sized. (The names of HeaderSlice and its slice field are weird when T: Sized but that weirdness is contained in the module defining struct Node.)

While I don’t have a use for this today this PR also derives Copy, and gives HeaderWithLength the same treatment.

We (apollo-compiler) have an AST where most nodes are wrapped in,
approximately:

```rust
struct Node<T>(triomphe::Arc<(SourceSpan, T)>);
```

`Arc` allows subtrees to be shared without deep cloning,
and `Arc::make_mut` is used extensively for mutability
with clone-on-write semantics.

Now I would like to also support `Node<str>` and rely on
`Arc::from_header_and_str` for the unsafe bits.
This requires using `HeaderSlice` which with triomphe v0.1.12
breaks `Arc::make_mut` because it does not implement `Clone`.
This PR fixes that.

`Clone` cannot work for `T: !Sized`
which is the main use case for `HeaderSlice` but that’s ok,
`derive` automatically insert the appropriate `where` bound.
This is still useful in order to have the same `Node<T>` use `HeaderSlice`
to support both `T: Sized` and `T: !Sized`.
(The names of `HeaderSlice` and its `slice` field are weird when `T: Sized`
but that weirdness is contained in the module defining `struct Node`.)

While I don’t have a use for this today this PR also derives `Copy`,
and gives `HeaderWithLength` the same treatment.
@SimonSapin
Copy link
Contributor Author

SimonSapin commented Jun 17, 2024

If you’re happy with this change, it’d be helpful to also have it on crates.io. Thanks!

@Manishearth Manishearth merged commit 2686349 into Manishearth:master Jun 17, 2024
6 checks passed
@SimonSapin SimonSapin deleted the dst_and_make_mut branch June 18, 2024 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants