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

Restore 1.60 MSRV and fix build #125

Merged
merged 2 commits into from
Jun 24, 2024

Conversation

tarcieri
Copy link
Contributor

Seems some changes I force pushed to #123 didn't wind up getting merged. In that PR, I noted that pub const fn new was MSRV breaking and got rid of the const but that didn't end up in develop.

I also encountered a build failure on develop since the legacy black_box function wasn't gated on the core_hint_black_box feature and thus clashed with core::hint::black_box when it was imported:

   Compiling subtle v2.6.0 (/Users/tarcieri/src/subtle)
error[E0255]: the name `black_box` is defined multiple times
   --> src/lib.rs:223:1
    |
100 | use core::hint::black_box;
    |     --------------------- previous import of the value `black_box` here
...
223 | fn black_box<T: Copy>(input: T) -> T {
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `black_box` redefined here
    |
    = note: `black_box` must be defined only once in the value namespace of this module
help: you can use `as` to change the binding name of the import
    |
100 | use core::hint::black_box as other_black_box;
    |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

warning: unused import: `core::hint::black_box`
   --> src/lib.rs:100:5
    |
100 | use core::hint::black_box;
    |     ^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `#[warn(unused_imports)]` on by default

This is a breaking change since we shipped a const fn for BlackBox::new already, so I'd suggest releasing this as 2.6.1 and yanking 2.6.0 for being unintentionally MSRV breaking.

Seems some changes I force pushed to dalek-cryptography#123 didn't wind up getting merged.
In that PR, I noted that `pub const fn new` was MSRV breaking and got
rid of the `const` but that didn't end up in `develop`.

I also encountered a build failure on `develop` since the legacy
`black_box` function wasn't gated on the `core_hint_black_box` feature
and thus clashed with `core::hint::black_box` when it was imported:

```
   Compiling subtle v2.6.0 (/Users/tarcieri/src/subtle)
error[E0255]: the name `black_box` is defined multiple times
   --> src/lib.rs:223:1
    |
100 | use core::hint::black_box;
    |     --------------------- previous import of the value `black_box` here
...
223 | fn black_box<T: Copy>(input: T) -> T {
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `black_box` redefined here
    |
    = note: `black_box` must be defined only once in the value namespace of this module
help: you can use `as` to change the binding name of the import
    |
100 | use core::hint::black_box as other_black_box;
    |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

warning: unused import: `core::hint::black_box`
   --> src/lib.rs:100:5
    |
100 | use core::hint::black_box;
    |     ^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `#[warn(unused_imports)]` on by default
```

This is a breaking change since we shipped a `const fn` for
`BlackBox::new` already, so I'd suggest releasing this as 2.6.1 and
yanking 2.6.0 for being unintentionally MSRV breaking.
@isislovecruft isislovecruft merged commit f1f8e53 into dalek-cryptography:develop Jun 24, 2024
4 checks passed
@tarcieri tarcieri deleted the restore-1.60-msrv branch June 24, 2024 18:31
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.

2 participants