Skip to content

Replace HashFactory boilerplate with derive macros - #76

Open
npajkovsky wants to merge 1 commit into
bcgit:release/0.1.3alphafrom
npajkovsky:proc-macro-hash
Open

Replace HashFactory boilerplate with derive macros#76
npajkovsky wants to merge 1 commit into
bcgit:release/0.1.3alphafrom
npajkovsky:proc-macro-hash

Conversation

@npajkovsky

Copy link
Copy Markdown
Collaborator

HashFactory hand-wrote an 8-arm match for each of the 10 Hash trait methods plus the string-name lookup in AlgorithmFactory::new, so adding an algorithm meant touching a dozen places. Add a bouncycastle-factory-macros crate providing two derives that generate all of it:

  • Hash - the Hash impl, forwarding every method to the
    variant currently held.
  • AlgorithmFactory - Default and AlgorithmFactory (new, default_128_bit,
    default_256_bit), driven by a #[factory(name = ...)]
    helper attribute on each variant.

Adding an algorithm is now one variant plus one attribute. hash_factory.rs drops from 254 to 79 lines with no behaviour change; existing tests and doctests cover the generated code.

Each derive is named after the trait it implements - derives live in the macro namespace and traits in the type namespace, so they don't collide (same convention as serde::Serialize).

Also marks HashFactory #[non_exhaustive] so future variants are additive rather than breaking for downstream matches

Fixes: #66

HashFactory hand-wrote an 8-arm match for each of the 10 Hash trait
methods plus the string-name lookup in AlgorithmFactory::new, so adding
an algorithm meant touching a dozen places. Add a bouncycastle-factory-macros
crate providing two derives that generate all of it:

  * Hash             - the Hash impl, forwarding every method to the
                       variant currently held.
  * AlgorithmFactory - Default and AlgorithmFactory (new, default_128_bit,
                       default_256_bit), driven by a #[factory(name = ...)]
                       helper attribute on each variant.

Adding an algorithm is now one variant plus one attribute. hash_factory.rs
drops from 254 to 79 lines with no behaviour change; existing tests and
doctests cover the generated code.

Each derive is named after the trait it implements - derives live in the
macro namespace and traits in the type namespace, so they don't collide
(same convention as serde::Serialize).

Also marks HashFactory #[non_exhaustive] so future variants are additive
rather than breaking for downstream matches

Fixes: bcgit#66
Signed-off-by: Nikola Pajkovsky <nikolap@openssl.org>
@npajkovsky
npajkovsky requested a review from ounsworth August 13, 2026 12:53
@npajkovsky npajkovsky self-assigned this Aug 13, 2026
@ounsworth

ounsworth commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

I didn't know about the #[non_exhaustive] attribute. That's great. Regardless of whether we macro the factories or not, we should apply that, and I will especially be adding #[non_exhaustive] to all the error type enums.

I particularly like that it doesn't prevent you, inside the crate, from writing exhaustive matches
for example:

        match self {
            Self::SHAKE128(h) => h.hash_xof_out(data, output),
            Self::SHAKE256(h) => h.hash_xof_out(data, output),
        }

which will break and force you to add a new branch if you add a new type to the struct, but any caller outside the library would be required to add the _ = branch if they tried to do the same thing. Brilliant! Thanks for pointing that out.

I created: #79

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.

3 participants