Code Monkey home page Code Monkey logo

Comments (8)

seanmonstar avatar seanmonstar commented on July 29, 2024

I imagine a from_static could just be made a const fn, like done for headers.

from http.

WhyNotHugo avatar WhyNotHugo commented on July 29, 2024

I tried making it const fn. I also had to make functions that it calls const fn and replace some ? with match {...}.

Finally, I reached AllocatedExtension::new, where I'm not sure how to continue:

diff --git a/src/method.rs b/src/method.rs
index b7b3b35..a795208 100644
--- a/src/method.rs
+++ b/src/method.rs
@@ -97,7 +96,7 @@ impl Method {
     pub const TRACE: Method = Method(Trace);
 
     /// Converts a slice of bytes to an HTTP method.
-    pub fn from_bytes(src: &[u8]) -> Result<Method, InvalidMethod> {
+    pub const fn from_bytes(src: &[u8]) -> Result<Method, InvalidMethod> {
         match src.len() {
             0 => Err(InvalidMethod::new()),
             3 => match src {
@@ -128,7 +127,10 @@ impl Method {
                 if src.len() < InlineExtension::MAX {
                     Method::extension_inline(src)
                 } else {
-                    let allocated = AllocatedExtension::new(src)?;
+                    let allocated = match AllocatedExtension::new(src) {
+                        Ok(a) => a,
+                        Err(e) => return Err(e),
+                    };
 
                     Ok(Method(ExtensionAllocated(allocated)))
                 }
@@ -136,8 +138,11 @@ impl Method {
         }
     }
 
-    fn extension_inline(src: &[u8]) -> Result<Method, InvalidMethod> {
-        let inline = InlineExtension::new(src)?;
+    const fn extension_inline(src: &[u8]) -> Result<Method, InvalidMethod> {
+        let inline = match InlineExtension::new(src) {
+            Ok(i) => i,
+            Err(err) => return Err(err),
+        };
 
         Ok(Method(ExtensionInline(inline)))
     }
@@ -288,7 +293,7 @@ impl FromStr for Method {
 }
 
 impl InvalidMethod {
-    fn new() -> InvalidMethod {
+    const fn new() -> InvalidMethod {
         InvalidMethod { _priv: () }
     }
 }
@@ -325,10 +330,12 @@ mod extension {
         // Method::from_bytes() assumes this is at least 7
         pub const MAX: usize = 15;
 
-        pub fn new(src: &[u8]) -> Result<InlineExtension, InvalidMethod> {
-            let mut data: [u8; InlineExtension::MAX] = Default::default();
+        pub const fn new(src: &[u8]) -> Result<InlineExtension, InvalidMethod> {
+            let mut data: [u8; InlineExtension::MAX] = [0; 15];
 
-            write_checked(src, &mut data)?;
+            if let Err(err) = write_checked(src, &mut data) {
+                return Err(err);
+            };
 
             // Invariant: write_checked ensures that the first src.len() bytes
             // of data are valid UTF-8.
@@ -339,15 +346,17 @@ mod extension {
             let InlineExtension(ref data, len) = self;
             // Safety: the invariant of InlineExtension ensures that the first
             // len bytes of data contain valid UTF-8.
-            unsafe {str::from_utf8_unchecked(&data[..*len as usize])}
+            unsafe { str::from_utf8_unchecked(&data[..*len as usize]) }
         }
     }
 
     impl AllocatedExtension {
-        pub fn new(src: &[u8]) -> Result<AllocatedExtension, InvalidMethod> {
+        pub const fn new(src: &[u8]) -> Result<AllocatedExtension, InvalidMethod> {
             let mut data: Vec<u8> = vec![0; src.len()];
 
-            write_checked(src, &mut data)?;
+            if let Err(err) = write_checked(src, &mut data) {
+                return Err(err);
+            };
 
             // Invariant: data is exactly src.len() long and write_checked
             // ensures that the first src.len() bytes of data are valid UTF-8.
error[E0277]: the trait bound `Vec<u8>: DerefMut` is not satisfied
   --> src/method.rs:357:50
    |
357 |             if let Err(err) = write_checked(src, &mut data) {
    |                                                  ^^^^^^^^^ the trait `~const DerefMut` is not implemented for `Vec<u8>`
    |
note: the trait `DerefMut` is implemented for `Vec<u8>`, but that implementation is not `const`
   --> src/method.rs:357:50
    |
357 |             if let Err(err) = write_checked(src, &mut data) {
    |                                                  ^^^^^^^^^
``

from http.

WhyNotHugo avatar WhyNotHugo commented on July 29, 2024

Okay, my previous approach doesn't seem to be feasible.

HeaderValue uses the Bytes type under the hood, which might be a good choice here too. It has an overhead of 4usize per instance. Given that methods are usually 3-8 characters, the overhead sounds like a lot, but in the const use case, they'll usually point to the same 'static str, so at least there wouldn't be heap allocations.

Current Method has two variants: one that holds the data inline, and another that holds a heap allocated string.

I see two possible options:

  • Use Bytes type for Method and merge those two variants. This should be quite efficient for const instances.
  • Create a new const constructor for method that can only create methods up to InlineExtension::MAX in length.

from http.

WhyNotHugo avatar WhyNotHugo commented on July 29, 2024

@seanmonstar Any feedback on the above comment? I want to give this a shot, but both approaches are non-trivial so I'd rather hear from you before sinking time into the wrong one.

from http.

seanmonstar avatar seanmonstar commented on July 29, 2024

I think keeping Method small is worthwhile. And with custom methods being so uncommon, I think it makes sense to double box the weird ones, so we only pay a single pointer in the struct.

from http.

WhyNotHugo avatar WhyNotHugo commented on July 29, 2024

And with custom methods being so uncommon

I'm not using custom methods; I'm using standardized methods like PROPFIND and REPORT, which are part of rfc4791 (webdav).

Whether they're uncommon depends on your field of work. Personally I find TRACE to be much more niche (I've never seen it used in the wild).

I think it makes sense to double box the weird ones, so we only pay a single pointer in the struct.

You mean the Method::ExtensionAllocated variant, right? I'll give this a try, I'm pretty sure that I have all the required bits in sight.

from http.

WhyNotHugo avatar WhyNotHugo commented on July 29, 2024

Darn, I missed that const cannot be used in a const fn. Or rather, it's still unstable:

rust-lang/rust#92521

from http.

WhyNotHugo avatar WhyNotHugo commented on July 29, 2024

I guess I'll try the InlineExtension variant then.

from http.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.