Permit algorithms to be used in associated consts

[rozbb]

All I did was change a ton of algorithms' pub statics to pub const. This should not affect runtime performance or memory usage.

The reason I think this is necessary is the following use case that I cannot currently get to work: (playground link here)

struct Algorithm {
    details: u8,
}

static MyAlgorithm: Algorithm = Algorithm {
    details: 100,
};

trait SomeFunctionality {
    const assoc_alg: &'static Algorithm;
}

struct Implementor;

impl SomeFunctionality for Implementor {
    const assoc_alg: &'static Algorithm = &MyAlgorithm;
}

The error the compiler gives me is

error[E0013]: constants cannot refer to statics, use a constant instead
  --> src/main.rs:16:43
   |
16 |     const assoc_alg: &'static Algorithm = &MyAlgorithm;
   |                                           ^^^^^^^^^^^^

From RFC 1440, my guess at the reason consts can't refer to statics is because statics can be types that have destructors, but consts cannot. Anyways, the problem is solved entirely by changing the static MyAlgorithm line to const MyAlgorithm.

Please let me know if you have any issues with this. Thank you for your work!

[coveralls]

Coverage remained the same at 93.894% when pulling a4233e5 on rozbb:master into dd5f7fe on briansmith:master.

[coveralls]

Coverage remained the same at 93.894% when pulling a4233e5 on rozbb:master into dd5f7fe on briansmith:master.

[coveralls]

Coverage remained the same at 93.894% when pulling a4233e5 on rozbb:master into dd5f7fe on briansmith:master.

[coveralls]

Coverage remained the same at 93.894% when pulling a4233e5 on rozbb:master into dd5f7fe on briansmith:master.

[coveralls]

Coverage remained the same at 93.894% when pulling a4233e5 on rozbb:master into dd5f7fe on briansmith:master.

[coveralls]

Coverage remained the same at 93.894% when pulling a4233e5 on rozbb:master into dd5f7fe on briansmith:master.

[rozbb]

Bump. This is a tiny feature change that I just happen to need for my project. I don't believe it has the ability to break existing code (since statics can refer to consts, but not vice-versa), but I can't publish on Cargo if I continue to use my own fork as a dependency.

[briansmith]

My understanding, which may be wrong, is that using const instead of static will cause a new copy of the object to be instantiated into every crate or module that references it. Because of (my fear of) that, I used static. So it would be good to see some evidence that I'm wrong; i.e. when these values are referenced from many crates, all the crates will share a single copy.

In your crate, do you really need to reference these as associated consts? Maybe your trait could instead have a method that returned a (static) reference to the algorithm instead.

[rozbb]

Ah, this is a fair point. So the only way to assign things to a static variables is by reference, i.e., let foo: Foo = GLOBALVAL is illegal, where static GLOBALVAL: Foo = .... Instead you have to do let foo: &'static Foo = &GLOBAL. If we change this to constant, you get the same behavior: let foo: &'static Foo = &GLOBALVAL where const GLOBALVAL: Foo = .... The only thing that changing things to const changes is giving devs the ability to copy these globals, as opposed to just being able to reference them.

So existing code, which only uses &'static refs anyway, will not have any new copies introduced by these changes. Future code, though, could theoretically make copies of the given consts.

However, if it does concern you that a user might accidentally make a copy of a global const, you can simply hide the const definition and only expose a &'static ref to it. Here's an example

pub const Blah: &'static Foo = &_Blah;
const _Blah: Foo = Foo {
    a: 10,
    b: 11,
    c: 12,
};

pub struct Foo {
    a: u8,
    b: u16,
    c: u32,
}

fn do_something(f: &Foo) {
    println!("nope");
}

fn main() {
    do_something(Blah)
}

Using this method seems to be the best of both worlds, and it doesn't seem too cumbersome. Though there is a chance that this would change your API, since the constants you expose would be references instead of statics.

Let me know what you think. This change might not be worth it at this point, but I think as a first-principles option, the "private const, public &'static" method is the best of both worlds.

[briansmith]

What I'd like to do:

1. Make this private static and rename: static ECDSA_P256_SHA256_FIXED_VALUE: Algorithm = Algorithm { ... }.
2. Expose a new pub const: pub const ECDSA_P256_SHA256_FIXED: &'static signature::Algorithm = &ECDSA_P256_SHA256_FIXED_VALUE.
3. Remove the pub use suite_b::ecdsa::Algorithm as EcdsaVerificationAlgorithm.

In other words, I am more interested in this change if it allows us to stop exporting EcdsaVerificationAlgorithm and related types. Does this work?

[briansmith]

Ah, nope, it doesn't work, for the same reason that you are filing this PR in the first place. It does work if the _VALUE part is const instead of static though. So, that's another advantage to doing this.

[rozbb]

Yup, you can have a const item be a &'static ref to another const item. So it'd be possible to expose only &'static things instead of proper Algorithms. The downside, though, is that the types of all these top-level definitions would change, and you'd need a major version bump.

[briansmith]

The major version bump is not an issue. That's going to happen RSN for other reasons anyway.

[rozbb]

Bibbity bump. Do you think this will make it into 0.15? This is the only Rust crypto library that provides all the primitives necessary to implement MLS, and the only workaround for the lack of this feature is to maintain my own fork of ring. Let me know if there's anything I can do.

[rozbb]

Are you still interested in including this in ring? I can re-write all the changes to be compatible with 0.16

[rozbb]

Update: This PR may be unnecessary. Will decide this week.

[briansmith]

I have to admit I didn't (don't) quite understand why this is so important. Now that your project is open source, I'll look at it this weekend and see what the deal is.

[briansmith]

Do you already have a version of your project that is based on a (modified) 0.16?

[briansmith]

Do you already have a version of your project that is based on a (modified) 0.16?

I submitted a PR to your project to try to resolve this without changes to ring: rozbb/molasses#2.

I'm not opposed to making a change to ring but I'm having trouble understanding why it's so important to do so since the current code seems to work well.

[rozbb]

I think you're right. Thank you for spending so much time on this!

I actually do have a local copy that's working with 0.16.5. Changing all my consts to statics and making my interfaces Sync fixes the issue.

Closing