Add Method::from_static

[WhyNotHugo]

Allows creating constant Method instances, e.g.:

const PROPFIND: Method = Method::from_static(b"PROPFIND");

Fixes: https://github.com/hyperium/http/issues/587

[WhyNotHugo]

Oh, I just realised that CI failed due to panic! not being available on the MSRV. I'll try refactoring that.

[seanmonstar]

We could probably update the MSRV, depending on what the version required is and how old it is.

[WhyNotHugo]

Rust 1.57.0, from 2021-12-02

[WhyNotHugo]

For tokio, the current MSRV is 1.56.0.

[WhyNotHugo]

I've updated the PR to panic without panic!. I used ([] as [u8; 0])[0];, which is already used as a const panic! hack in a couple of other places in the codebase.

We can remove this hack (both this new instance and existing ones) once the MSRV is bumped to 1.57.0 or greater.

[lperlaki]

I was wondering if we could add a variant StaticRef(&'static [u8]) to the Method Inner enum instead of panicking on longer than 15 byte methods?

[WhyNotHugo]

I don't know of any methods that are longer than 15 bytes, so I'm not sure that it's worth the effort.

[dblsaiko]

What's this stuck on?

[lperlaki]

I would also like to see this being merged as it helps with implementing a webdav server base on hyper

I don't know of any methods that are longer than 15 bytes, so I'm not sure that it's worth the effort.

recently I stubbled over the BASELINE-CONTROL webdav method wich is longe than 15 bytes (16)

I was wondering if we could add a variant StaticRef(&'static [u8]) to the Method Inner enum instead of panicking on longer than 15 byte methods?

figured out that this wouldn't work with the current implementation since the equality also checks the enum variant wich is required that Method can support pattern matching

[orium]

@seanmonstar Anything that needs to be done to land this? Happy to help if there's anything to do.

[WhyNotHugo]

I was wondering if we could add a variant StaticRef(&'static [u8]) to the Method Inner enum instead of panicking on longer than 15 byte methods?

I explored this a bit further.

Adding a new enum variant won't increase the total size of Inner. The associated data is a single reference, so it's also not bigger than the largest of the existing variants.

I've implemented your suggestions. This remove the conditional (compile-time) panic if len > 15.

[WhyNotHugo]

At this point, waiting for feedback from the core devs.

[lperlaki]

I found the Problem with ExtensionStatic is that

const BASELINE_CONTROL: Method = Method::from_static(„BASELINE-CONTROL“);

let m = Method::from_bytes(„BASELINE-CONTROL“);

let is_baseline = match m {
    BASELINE_CONTROL => true,
    _ => false,
};

will not match.

[WhyNotHugo]

I found the Problem with ExtensionStatic is that

const BASELINE_CONTROL: Method = Method::from_static(„BASELINE-CONTROL“);

let m = Method::from_bytes(„BASELINE-CONTROL“);

let is_baseline = match m {
    BASELINE_CONTROL => true,
    _ => false,
};

will not match.

I'll add a test for this case and figure out what's wrong.

[WhyNotHugo]

The derived implementation of PartialEq will consider Inner::ExtensionAllocated to always be different from Inner::ExtensionStatic, regardless of the value of their associated data.

Possible solutions:

• Re-implement PartialEq por Inner. This is likely a lot of very verbose code, and the derived implementation has optimisations which I suspect the core devs will want to keep around.

• Drop ExtensionInline(InlineExtension), which means that common short methods will always be behind a pointer dereference. Not a terrible performance drop, but given how much all the other variants are optimised, it seems non-ideal.

• Drop ExtensionStatic(StaticExtension) entirely, and limit from_static to a maximum of 15 bytes, as originally implemented. This is probably the least ideal approach.

[lperlaki]

• Re-implement PartialEq por Inner. This is likely a lot of very verbose code, and the derived implementation has optimisations which I suspect the core devs will want to keep around.

Not deriving PartialEq will also not implement StructuralPartialEq which is needed if we want to keep the match functionality

• Drop ExtensionInline(InlineExtension), which means that common short methods will always be behind a pointer dereference. Not a terrible performance drop, but given how much all the other variants are optimised, it seems non-ideal.

will not fix the PartialEq implementation between Static and Allocated variant if not by hand implemented and then we have the first problem again

• Drop ExtensionStatic(StaticExtension) entirely, and limit from_static to a maximum of 15 bytes, as originally implemented. This is probably the least ideal approach.

I think this is the only viable solution if we want to keep the pattern matching behaviour

[WhyNotHugo]

I believe that you are correct in all points.

[WhyNotHugo]

Updated based on the latest discussion. I've reflected a short summary of the decision taken in the commit message.