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.

Sign up for GitHub

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

Jump to bottom

FR: Procedural macro to derive ContentHash for structs and enums #3054

Closed
emesterhazy opened this issue Feb 14, 2024 · 0 comments · Fixed by #3106
Closed

FR: Procedural macro to derive ContentHash for structs and enums #3054

emesterhazy opened this issue Feb 14, 2024 · 0 comments · Fixed by #3106

Comments

@emesterhazy
Copy link
Contributor

emesterhazy commented Feb 14, 2024 •
edited
Loading

I've already implemented this feature, I'm just creating a bug to collect all of the commits in one place.

Related issues:

PRs, in order:
@martinvonz martinvonz mentioned this issue Feb 15, 2024
Closed
emesterhazy added a commit that referenced this issue Feb 15, 2024
@emesterhazy
Replace uses of content_hash! with #[derive(ContentHash)]
9e54a31 
This is a pure refactor with no behavior changes.

#3054
@emesterhazy emesterhazy mentioned this issue Feb 15, 2024
Merged
emesterhazy added a commit that referenced this issue Feb 15, 2024
@emesterhazy
Replace uses of content_hash! with #[derive(ContentHash)]
3ccd1c9 
This is a pure refactor with no behavior changes.

#3054
emesterhazy added a commit that referenced this issue Feb 16, 2024
@emesterhazy
Implement a procedural macro to derive the ContentHash trait for structs
c6c5858 
This is a no-op in terms of function, but provides a nicer way to derive the
ContentHash trait for structs using the `#[derive(ContentHash)]` syntax used
for other traits such as `Debug`.

This commit only adds the macro. A subsequent commit will replace uses of
`content_hash!{}` with `#[derive(ContentHash)]`.

The new macro generates nice error messages, just like the old macro, although
they are slightly different.

Before:
```
error[E0277]: the trait bound `NotImplemented: content_hash::ContentHash` is not satisfied
   --> lib/src/content_hash.rs:242:30
    |
242 |             struct Broken{t: NotImplemented}
    |                              ^^^^^^^^^^^^^^ the trait `content_hash::ContentHash` is not implemented for `NotImplemented`
    |
    = help: the following other types implement trait `content_hash::ContentHash`:
              bool
              i32
              i64
              u8
              std::collections::HashMap<K, V>
              BTreeMap<K, V>
              std::collections::HashSet<K>
              content_hash::tests::test_struct_sanity::Foo
            and 38 others
```

After:
```
error[E0277]: the trait bound `NotImplemented: content_hash::ContentHash` is not satisfied
   --> lib/src/content_hash.rs:247:13
    |
247 |             t: NotImplemented,
    |             ^ the trait `content_hash::ContentHash` is not implemented for `NotImplemented`
    |
    = help: the following other types implement trait `content_hash::ContentHash`:
              bool
              i32
              i64
              u8
              std::collections::HashMap<K, V>
              BTreeMap<K, V>
              std::collections::HashSet<K>
              content_hash::tests::test_struct_sanity::Foo
            and 38 others

For more information about this error, try `rustc --explain E0277`.
error: could not compile `jj-lib` (lib test) due to 2 previous errors
```

This commit does two things to make proc macros re-exported by jj_lib useable
by deps:

1. jj_lib needs to be able refer to itself as `jj_lib` which it does
   by adding an `extern crate self as jj_lib` declaration.

2. jj_lib::content_hash needs to re-export the `digest::Update` type so that
   users of jj_lib can use the `#[derive(ContentHash)]` proc macro without
   directly depending on the digest crate. This is done by re-exporting it
   as `DigestUpdate`.


#3054
emesterhazy added a commit that referenced this issue Feb 16, 2024
@emesterhazy
Add support for generics to #[derive(ContentHash)]
1e7f62c 
#3054
emesterhazy added a commit that referenced this issue Feb 16, 2024
@emesterhazy
Add support for deriving ContentHash for Enums
c4e1eea 
Here's an example of what the derived output looks like for an enum:

```rust
pub enum TreeValue {
    File { id: FileId, executable: bool },
    Symlink(SymlinkId),
    Tree(TreeId),
    GitSubmodule(CommitId),
    Conflict(ConflictId),
}
#[automatically_derived]
impl ::jj_lib::content_hash::ContentHash for TreeValue {
    fn hash(&self, state: &mut impl digest::Update) {
        match self {
            Self::File { id, executable } => {
                state.update(&0u32.to_le_bytes());
                ::jj_lib::content_hash::ContentHash::hash(id, state);
                ::jj_lib::content_hash::ContentHash::hash(executable, state);
            }
            Self::Symlink(field_0) => {
                state.update(&1u32.to_le_bytes());
                ::jj_lib::content_hash::ContentHash::hash(field_0, state);
            }
            Self::Tree(field_0) => {
                state.update(&2u32.to_le_bytes());
                ::jj_lib::content_hash::ContentHash::hash(field_0, state);
            }
            Self::GitSubmodule(field_0) => {
                state.update(&3u32.to_le_bytes());
                ::jj_lib::content_hash::ContentHash::hash(field_0, state);
            }
            Self::Conflict(field_0) => {
                state.update(&4u32.to_le_bytes());
                ::jj_lib::content_hash::ContentHash::hash(field_0, state);
            }
        }
    }
}
```


#3054
emesterhazy added a commit that referenced this issue Feb 16, 2024
@emesterhazy
Replace uses of content_hash! with #[derive(ContentHash)]
0cb8cd1 
This is a pure refactor with no behavior changes.

#3054
emesterhazy added a commit that referenced this issue Feb 16, 2024
@emesterhazy
Replace uses of content_hash! with #[derive(ContentHash)]
e88f5e0 
This is a pure refactor with no behavior changes.

#3054
emesterhazy added a commit that referenced this issue Feb 16, 2024
@emesterhazy
Replace uses of content_hash! with #[derive(ContentHash)]
f5c3fab 
This is a pure refactor with no behavior changes.

#3054
emesterhazy added a commit that referenced this issue Feb 16, 2024
@emesterhazy
Replace uses of content_hash! with #[derive(ContentHash)]
5398539 
This is a pure refactor with no behavior changes.

#3054
emesterhazy added a commit that referenced this issue Feb 17, 2024
@emesterhazy
Replace uses of content_hash! with #[derive(ContentHash)]
4cc3955 
This is a pure refactor with no behavior changes.

#3054
emesterhazy added a commit that referenced this issue Feb 20, 2024
@emesterhazy
Implement a procedural macro to derive the ContentHash trait for structs
c77a476 
This is a no-op in terms of function, but provides a nicer way to derive the
ContentHash trait for structs using the `#[derive(ContentHash)]` syntax used
for other traits such as `Debug`.

This commit only adds the macro. A subsequent commit will replace uses of
`content_hash!{}` with `#[derive(ContentHash)]`.

The new macro generates nice error messages, just like the old macro:

```
error[E0277]: the trait bound `NotImplemented: content_hash::ContentHash` is not satisfied
   --> lib/src/content_hash.rs:265:16
    |
265 |             z: NotImplemented,
    |                ^^^^^^^^^^^^^^ the trait `content_hash::ContentHash` is not implemented for `NotImplemented`
    |
    = help: the following other types implement trait `content_hash::ContentHash`:
              bool
              i32
              i64
              u8
              u32
              u64
              std::collections::HashMap<K, V>
              BTreeMap<K, V>
            and 38 others
```

This commit does two things to make proc macros re-exported by jj_lib useable
by deps:

1. jj_lib needs to be able refer to itself as `jj_lib` which it does
   by adding an `extern crate self as jj_lib` declaration.

2. jj_lib::content_hash needs to re-export the `digest::Update` type so that
   users of jj_lib can use the `#[derive(ContentHash)]` proc macro without
   directly depending on the digest crate. This is done by re-exporting it
   as `DigestUpdate`.


#3054
emesterhazy added a commit that referenced this issue Feb 20, 2024
@emesterhazy
Replace uses of content_hash! with #[derive(ContentHash)]
135c1cc 
This is a pure refactor with no behavior changes.

#3054
emesterhazy added a commit that referenced this issue Feb 20, 2024
@emesterhazy
Implement a procedural macro to derive the ContentHash trait for structs
965d6ce 
This is a no-op in terms of function, but provides a nicer way to derive the
ContentHash trait for structs using the `#[derive(ContentHash)]` syntax used
for other traits such as `Debug`.

This commit only adds the macro. A subsequent commit will replace uses of
`content_hash!{}` with `#[derive(ContentHash)]`.

The new macro generates nice error messages, just like the old macro:

```
error[E0277]: the trait bound `NotImplemented: content_hash::ContentHash` is not satisfied
   --> lib/src/content_hash.rs:265:16
    |
265 |             z: NotImplemented,
    |                ^^^^^^^^^^^^^^ the trait `content_hash::ContentHash` is not implemented for `NotImplemented`
    |
    = help: the following other types implement trait `content_hash::ContentHash`:
              bool
              i32
              i64
              u8
              u32
              u64
              std::collections::HashMap<K, V>
              BTreeMap<K, V>
            and 38 others
```

This commit does two things to make proc macros re-exported by jj_lib useable
by deps:

1. jj_lib needs to be able refer to itself as `jj_lib` which it does
   by adding an `extern crate self as jj_lib` declaration.

2. jj_lib::content_hash needs to re-export the `digest::Update` type so that
   users of jj_lib can use the `#[derive(ContentHash)]` proc macro without
   directly depending on the digest crate. This is done by re-exporting it
   as `DigestUpdate`.


#3054
emesterhazy added a commit that referenced this issue Feb 20, 2024
@emesterhazy
Add support for generics to #[derive(ContentHash)]
ef0befd 
#3054
emesterhazy added a commit that referenced this issue Feb 20, 2024
@emesterhazy
Add support for generics to #[derive(ContentHash)]
8e1a6c7 
#3054
emesterhazy added a commit that referenced this issue Feb 20, 2024
@emesterhazy
Add support for deriving ContentHash for Enums
b73131c 
Here's an example of what the derived output looks like for an enum:

```rust
pub enum TreeValue {
    File { id: FileId, executable: bool },
    Symlink(SymlinkId),
    Tree(TreeId),
    GitSubmodule(CommitId),
    Conflict(ConflictId),
}
#[automatically_derived]
impl ::jj_lib::content_hash::ContentHash for TreeValue {
    fn hash(&self, state: &mut impl digest::Update) {
        match self {
            Self::File { id, executable } => {
                state.update(&0u32.to_le_bytes());
                ::jj_lib::content_hash::ContentHash::hash(id, state);
                ::jj_lib::content_hash::ContentHash::hash(executable, state);
            }
            Self::Symlink(field_0) => {
                state.update(&1u32.to_le_bytes());
                ::jj_lib::content_hash::ContentHash::hash(field_0, state);
            }
            Self::Tree(field_0) => {
                state.update(&2u32.to_le_bytes());
                ::jj_lib::content_hash::ContentHash::hash(field_0, state);
            }
            Self::GitSubmodule(field_0) => {
                state.update(&3u32.to_le_bytes());
                ::jj_lib::content_hash::ContentHash::hash(field_0, state);
            }
            Self::Conflict(field_0) => {
                state.update(&4u32.to_le_bytes());
                ::jj_lib::content_hash::ContentHash::hash(field_0, state);
            }
        }
    }
}
```


#3054
emesterhazy added a commit that referenced this issue Feb 20, 2024
@emesterhazy
Add support for deriving ContentHash for Enums
966a550 
Here's an example of what the derived output looks like for an enum:

```rust
pub enum TreeValue {
    File { id: FileId, executable: bool },
    Symlink(SymlinkId),
    Tree(TreeId),
    GitSubmodule(CommitId),
    Conflict(ConflictId),
}
#[automatically_derived]
impl ::jj_lib::content_hash::ContentHash for TreeValue {
    fn hash(&self, state: &mut impl digest::Update) {
        match self {
            Self::File { id, executable } => {
                state.update(&0u32.to_le_bytes());
                ::jj_lib::content_hash::ContentHash::hash(id, state);
                ::jj_lib::content_hash::ContentHash::hash(executable, state);
            }
            Self::Symlink(field_0) => {
                state.update(&1u32.to_le_bytes());
                ::jj_lib::content_hash::ContentHash::hash(field_0, state);
            }
            Self::Tree(field_0) => {
                state.update(&2u32.to_le_bytes());
                ::jj_lib::content_hash::ContentHash::hash(field_0, state);
            }
            Self::GitSubmodule(field_0) => {
                state.update(&3u32.to_le_bytes());
                ::jj_lib::content_hash::ContentHash::hash(field_0, state);
            }
            Self::Conflict(field_0) => {
                state.update(&4u32.to_le_bytes());
                ::jj_lib::content_hash::ContentHash::hash(field_0, state);
            }
        }
    }
}
```


#3054
emesterhazy added a commit that referenced this issue Feb 20, 2024
@emesterhazy
Replace uses of content_hash! with #[derive(ContentHash)]
f466f08 
This is a pure refactor with no behavior changes.

#3054
emesterhazy added a commit that referenced this issue Feb 20, 2024
@emesterhazy
Replace uses of content_hash! with #[derive(ContentHash)]
e8f324f 
This is a pure refactor with no behavior changes.

#3054
@emesterhazy emesterhazy mentioned this issue Feb 20, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Output better error messages when deriving ContentHash for an enum fails jj-vcs/jj
1 participant
@emesterhazy