Skip to content

Name trait methods should return Cow<'static, str> #1392

@carlocorradini

Description

@carlocorradini

Name trait methods should return Cow<'static, str>.

I came across the Name trait and found it extremely useful, especially when printing debug information or handling encode/decode errors.

What bothers me is that the provided methods, full_name and type_url, always return a newly allocated String, even though their values are mostly, if not always, the same.

  • Cow<'static, str>
    Always return a clone-on-write around str.
    This provides maximum flexibility for both us and the user, as it allows choosing whether the value is borrowed from a string literal (&'static str) or owned as a newly allocated String:

    pub trait Name: Message {
        const NAME: &'static str;
    
        const PACKAGE: &'static str;
    
        fn full_name() -> Cow<'static, str> {
            if Self::PACKAGE.is_empty() {
                Cow::Borrowed(Self::NAME)
            } else {
                Cow::Owned(format!("{}.{}", Self::PACKAGE, Self::NAME))
            }
        }
    
        fn type_url() -> Cow<'static, str> {
            Cow::Owned(format!("/{}", Self::full_name()))
        }
    }

    However, since we control prost-build and it is almost always used, we can take advantage of this by having the generated code always return a borrowed value from a hardcoded string literal (this is already how it is implemented):

    impl Name for MyMessage {
        const NAME: &'static str = "MyMessage";
    
        const PACKAGE: &'static str = "hello.world.v1";
    
        fn full_name() -> Cow<'static, str> {
            Cow::Borrowed("hello.world.v1.MyMessage")
        }
    
        fn type_url() -> Cow<'static, str> {
            Cow::Borrowed("/hello.world.v1.MyMessage")
        }
    }

    I understand this is a breaking change, but using Cow is extremely helpful for avoiding unnecessary allocations. Moreover, since Cow implements Deref and other useful traits, the migration should be mostly seamless (take this with a grain of salt 🧂).

  • &'static str
    A dream...
    Always return a string literal.
    The concatenation of NAME and PACKAGE constants can be done using the const_format crate or an ad-hoc solution.
    Unfortunately, this solution cannot be used at the moment due to the following error: “can't use Self from outer item a const is a separate item from the item that contains it” and other const trait features not been stabilized yet.

    pub trait Name: Message {
        const NAME: &'static str;
    
        const PACKAGE: &'static str;
    
        fn full_name() -> &'static str {
            if Self::PACKAGE.is_empty() {
                Self::NAME
            } else {
                const_format::formatcp!("{}.{}", Self::PACKAGE, Self::NAME)
            }
        }
    
        fn type_url() -> &'static str {
            const_format::formatcp!("/{}", Self::full_name())
        }
    }

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions